From 9295c9df6d82b849b5fb31e0a542ca42bc5dde89 Mon Sep 17 00:00:00 2001 From: Ender Tunc Date: Thu, 16 Mar 2023 11:57:07 +0100 Subject: [PATCH 1/4] update configuration settings and integrate new API calls to gitlab flow --- .../scalasteward/core/application/Cli.scala | 47 ++++++- .../core/application/Config.scala | 4 +- .../core/forge/gitlab/GitLabApiAlg.scala | 121 ++++++++++++++---- .../scalasteward/core/forge/gitlab/Url.scala | 10 ++ 4 files changed, 153 insertions(+), 29 deletions(-) diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala index 3bbccaa37d..a5186ca132 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala @@ -23,6 +23,7 @@ import com.monovore.decline.Opts.{flag, option, options} import com.monovore.decline._ import org.http4s.Uri import org.http4s.syntax.literals._ +import org.scalasteward.core.application.Cli.gitlabMergeRequestApprovalsConfig import org.scalasteward.core.application.Config._ import org.scalasteward.core.data.Resolver import org.scalasteward.core.forge.ForgeType @@ -31,7 +32,9 @@ import org.scalasteward.core.forge.github.GitHubApp import org.scalasteward.core.git.Author import org.scalasteward.core.util.Nel import org.scalasteward.core.util.dateTime.renderFiniteDuration + import scala.concurrent.duration._ +import scala.util.{Failure, Success, Try} object Cli { final case class EnvVar(name: String, value: String) @@ -44,6 +47,21 @@ object Cli { val processTimeout = "process-timeout" } + implicit val mergeRequestApprovalsConfigArgument: Argument[MergeRequestApprovalsConfig] = + Argument.from("approvals_rule_name=required_approvals") { s => + s.split(":").toList match { + case approvalRuleName :: requiredApprovalsAsString :: Nil => + Try(requiredApprovalsAsString.trim.toInt) match { + case Failure(_) => + s"[$requiredApprovalsAsString] is not a valid Integer".invalidNel + case Success(requiredApprovals) => + new MergeRequestApprovalsConfig(approvalRuleName.trim, requiredApprovals).validNel + } + case _ => + s"The value is expected in the following format: APPROVALS_RULE_NAME:REQUIRED_APPROVALS.".invalidNel + } + } + implicit val envVarArgument: Argument[EnvVar] = Argument.from("name=value") { s => s.trim.split('=').toList match { @@ -276,7 +294,7 @@ object Cli { private val gitlabRequiredReviewers: Opts[Option[Int]] = option[Int]( - "gitlab-required-reviewers", + "gitlabRequiredReviewers", "When set, the number of required reviewers for a merge request will be set to this number (non-negative integer). Is only used in the context of gitlab-merge-when-pipeline-succeeds being enabled, and requires that the configured access token have the appropriate privileges. Also requires a Gitlab Premium subscription." ).validate("Required reviewers must be non-negative")(_ >= 0).orNone @@ -286,10 +304,31 @@ object Cli { "Flag indicating if a merge request should remove the source branch when merging." ).orFalse - private val gitLabCfg: Opts[GitLabCfg] = - (gitlabMergeWhenPipelineSucceeds, gitlabRequiredReviewers, gitlabRemoveSourceBranch).mapN( - GitLabCfg.apply + private val gitlabMergeRequestApprovalsConfig: Opts[Option[Nel[MergeRequestApprovalsConfig]]] = + options[MergeRequestApprovalsConfig]( + "merge-request-level-approval-rule", + s"Additional repo config file $multiple" ) + // ToDo better message + .validate("")(_.forall(_.requiredApproves >= 0) == true) + .orNone + + private val gitlabReviewersAndApprovalsConfig + : Opts[Option[Either[Int, Nel[MergeRequestApprovalsConfig]]]] = + ((gitlabRequiredReviewers, gitlabMergeRequestApprovalsConfig).tupled.mapValidated { + case (None, None) => None.validNel + case (None, Some(gitlabMergeRequestApprovalsConfig)) => + Some(gitlabMergeRequestApprovalsConfig.asRight[Int]).validNel + case (Some(requiredReviewers), None) => Some(Left(requiredReviewers)).validNel + case (Some(_), Some(_)) => + s"You can't use both --gitlabRequiredReviewers and --merge-request-level-approval-rule at the same time".invalidNel + }) + + private val gitLabCfg: Opts[GitLabCfg] = + (gitlabMergeWhenPipelineSucceeds, gitlabReviewersAndApprovalsConfig, gitlabRemoveSourceBranch) + .mapN( + GitLabCfg.apply + ) private val githubAppId: Opts[Long] = option[Long]( diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala index 2ad3116a02..dab3539637 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala @@ -156,9 +156,11 @@ object Config { final case class GitHubCfg( ) extends ForgeSpecificCfg + final case class MergeRequestApprovalsConfig(approvalRuleName: String, requiredApproves: Int) + final case class GitLabCfg( mergeWhenPipelineSucceeds: Boolean, - requiredReviewers: Option[Int], + requiredReviewers: Option[Either[Int, Nel[MergeRequestApprovalsConfig]]], removeSourceBranch: Boolean ) extends ForgeSpecificCfg diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala index 8ec4d09efc..2c5cd20d11 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala @@ -23,13 +23,18 @@ import io.circe._ import io.circe.generic.semiauto._ import io.circe.syntax._ import org.http4s.{Request, Status, Uri} -import org.scalasteward.core.application.Config.{ForgeCfg, GitLabCfg} +import org.scalasteward.core.application.Config.{ForgeCfg, GitLabCfg, MergeRequestApprovalsConfig} import org.scalasteward.core.data.Repo import org.scalasteward.core.forge.ForgeApiAlg import org.scalasteward.core.forge.data._ import org.scalasteward.core.git.{Branch, Sha1} import org.scalasteward.core.util.uri.uriDecoder -import org.scalasteward.core.util.{intellijThisImportIsUsed, HttpJsonClient, UnexpectedResponse} +import org.scalasteward.core.util.{ + intellijThisImportIsUsed, + HttpJsonClient, + Nel, + UnexpectedResponse +} import org.typelevel.log4cats.Logger import scala.concurrent.duration.{Duration, DurationInt} @@ -48,6 +53,8 @@ final private[gitlab] case class MergeRequestPayload( target_branch: Branch ) +final private[gitlab] case class UpdateMergeRequestLevelApprovalRulePayload(approvals_required: Int) + private[gitlab] object MergeRequestPayload { def apply( id: String, @@ -87,6 +94,11 @@ final private[gitlab] case class MergeRequestApprovalsOut( approvalsRequired: Int ) +final private[gitlab] case class MergeRequestLevelApprovalRuleOut( + id: Int, + name: String +) + final private[gitlab] case class CommitId(id: Sha1) { val commitOut: CommitOut = CommitOut(id) } @@ -102,6 +114,8 @@ private[gitlab] object GitLabJsonCodec { intellijThisImportIsUsed(uriDecoder) implicit val forkPayloadEncoder: Encoder[ForkPayload] = deriveEncoder + implicit val updateMergeRequestLevelApprovalRulePayloadEncoder + : Encoder[UpdateMergeRequestLevelApprovalRulePayload] = deriveEncoder implicit val userOutDecoder: Decoder[UserOut] = Decoder.instance { _.downField("username").as[String].map(UserOut(_)) } @@ -140,6 +154,14 @@ private[gitlab] object GitLabJsonCodec { } yield MergeRequestApprovalsOut(requiredReviewers) } + implicit val mergeRequestLevelApprovalRuleOutDecoder: Decoder[MergeRequestLevelApprovalRuleOut] = + Decoder.instance { c => + for { + id <- c.downField("id").as[Int] + name <- c.downField("string").as[String] + } yield MergeRequestLevelApprovalRuleOut(id, name) + } + implicit val projectIdDecoder: Decoder[ProjectId] = deriveDecoder implicit val mergeRequestPayloadEncoder: Encoder[MergeRequestPayload] = deriveEncoder[MergeRequestPayload].mapJson(_.dropNullValues) @@ -240,7 +262,13 @@ final class GitLabApiAlg[F[_]: Parallel]( for { mr <- mergeRequest mrWithStatus <- waitForMergeRequestStatus(mr.iid) - _ <- maybeSetReviewers(repo, mrWithStatus) + _ <- gitLabCfg.requiredReviewers match { + case Some(Right(approvalRules)) => + setApprovalRules(repo, mrWithStatus, approvalRules) + case Some(Left(requiredReviewers)) => + setReviewers(repo, mrWithStatus, requiredReviewers) + case None => F.unit + } mergedUponSuccess <- mergePipelineUponSuccess(repo, mrWithStatus) } yield mergedUponSuccess } @@ -270,29 +298,74 @@ final class GitLabApiAlg[F[_]: Parallel]( case mr => logger.info(s"Unable to automatically merge ${mr.webUrl}").map(_ => mr) } + import cats.implicits._ - private def maybeSetReviewers(repo: Repo, mrOut: MergeRequestOut): F[MergeRequestOut] = - gitLabCfg.requiredReviewers match { - case Some(requiredReviewers) => - for { - _ <- logger.info( - s"Setting number of required reviewers on ${mrOut.webUrl} to $requiredReviewers" + private def setReviewers( + repo: Repo, + mrOut: MergeRequestOut, + requiredReviewers: Int + ): F[MergeRequestOut] = + for { + _ <- logger.info( + s"Setting number of required reviewers on ${mrOut.webUrl} to $requiredReviewers" + ) + _ <- + client + .put[MergeRequestApprovalsOut]( + url.requiredApprovals(repo, mrOut.iid, requiredReviewers), + modify(repo) ) - _ <- - client - .put[MergeRequestApprovalsOut]( - url.requiredApprovals(repo, mrOut.iid, requiredReviewers), - modify(repo) - ) - .map(_ => ()) - .recoverWith { case UnexpectedResponse(_, _, _, status, body) => - logger - .warn(s"Unexpected response setting required reviewers: $status: $body") - .as(()) - } - } yield mrOut - case None => F.pure(mrOut) - } + .map(_ => ()) + .recoverWith { case UnexpectedResponse(_, _, _, status, body) => + logger + .warn(s"Unexpected response setting required reviewers: $status: $body") + .as(()) + } + } yield mrOut + + private def setApprovalRules( + repo: Repo, + mrOut: MergeRequestOut, + approvalsConfig: Nel[MergeRequestApprovalsConfig] + ): F[MergeRequestOut] = + for { + _ <- logger.info( + s"Adjusting merge request approvals rules on ${mrOut.webUrl} with following config: $approvalsConfig" + ) + activeApprovalRules <- + client + .get[List[MergeRequestLevelApprovalRuleOut]]( + url.listMergeRequestLevelApprovalRules(repo, mrOut.iid), + modify(repo) + ) + .recoverWith { case UnexpectedResponse(_, _, _, status, body) => + // ToDo better log + logger + .warn(s"Unexpected response setting required reviewers: $status: $body") + .as(List.empty) + } + approvalRuleNamesFromConfig = approvalsConfig.map(_.approvalRuleName) + approvalRulesToUpdate = activeApprovalRules.intersect(approvalRuleNamesFromConfig.toList) + _ <- + approvalRulesToUpdate.map { mergeRequestApprovalConfig => + client + .putWithBody[Unit, UpdateMergeRequestLevelApprovalRulePayload]( + url.updateMergeRequestLevelApprovalRule( + repo, + mrOut.iid, + mergeRequestApprovalConfig.id + ), + UpdateMergeRequestLevelApprovalRulePayload(mergeRequestApprovalConfig.id), + modify(repo) + ) + .recoverWith { case UnexpectedResponse(_, _, _, status, body) => + // ToDo better log + logger + .warn(s"Unexpected response setting required reviewers: $status: $body") + .as(List.empty) + } + }.sequence + } yield mrOut private def getUsernameToUserIdsMapping(repo: Repo, usernames: Set[String]): F[Map[String, Int]] = usernames.toList diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala index 6f12fb90ae..b6e0203597 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala @@ -17,6 +17,7 @@ package org.scalasteward.core.forge.gitlab import org.http4s.Uri +import org.scalasteward.core.application.Config.MergeRequestApprovalsConfig import org.scalasteward.core.data.Repo import org.scalasteward.core.forge.data.PullRequestNumber import org.scalasteward.core.git.Branch @@ -47,6 +48,15 @@ class Url(apiHost: Uri) { (existingMergeRequest(repo, number) / "approvals") .withQueryParam("approvals_required", approvalsRequired) + def listMergeRequestLevelApprovalRules(repo: Repo, number: PullRequestNumber): Uri = + existingMergeRequest(repo, number) / "approval_rules" + + def updateMergeRequestLevelApprovalRule( + repo: Repo, + number: PullRequestNumber, + approvalRuleId: Int + ): Uri = existingMergeRequest(repo, number) / "approval_rules" / approvalRuleId + def listMergeRequests(repo: Repo, source: String, target: String): Uri = mergeRequest(repo) .withQueryParam("source_branch", source) From 11dac1c606d93c7ccf8fc58b083265087501bc27 Mon Sep 17 00:00:00 2001 From: Ender Tunc Date: Sat, 18 Mar 2023 22:00:47 +0100 Subject: [PATCH 2/4] finalize the implementation and add tests --- .../scalasteward/core/application/Cli.scala | 20 +-- .../core/application/Config.scala | 4 +- .../core/forge/gitlab/GitLabApiAlg.scala | 65 ++++---- .../scalasteward/core/forge/gitlab/Url.scala | 1 - .../core/application/CliTest.scala | 70 ++++++++- .../core/forge/gitlab/GitLabAlgTest.scala | 73 +++++++++ .../core/forge/gitlab/GitLabApiAlgTest.scala | 146 +++++++++++++++++- 7 files changed, 328 insertions(+), 51 deletions(-) create mode 100644 modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabAlgTest.scala diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala index a5186ca132..6c175c9959 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala @@ -23,7 +23,6 @@ import com.monovore.decline.Opts.{flag, option, options} import com.monovore.decline._ import org.http4s.Uri import org.http4s.syntax.literals._ -import org.scalasteward.core.application.Cli.gitlabMergeRequestApprovalsConfig import org.scalasteward.core.application.Config._ import org.scalasteward.core.data.Resolver import org.scalasteward.core.forge.ForgeType @@ -47,7 +46,7 @@ object Cli { val processTimeout = "process-timeout" } - implicit val mergeRequestApprovalsConfigArgument: Argument[MergeRequestApprovalsConfig] = + implicit val mergeRequestApprovalsConfigArgument: Argument[MergeRequestApprovalRulesCfg] = Argument.from("approvals_rule_name=required_approvals") { s => s.split(":").toList match { case approvalRuleName :: requiredApprovalsAsString :: Nil => @@ -55,7 +54,7 @@ object Cli { case Failure(_) => s"[$requiredApprovalsAsString] is not a valid Integer".invalidNel case Success(requiredApprovals) => - new MergeRequestApprovalsConfig(approvalRuleName.trim, requiredApprovals).validNel + MergeRequestApprovalRulesCfg(approvalRuleName.trim, requiredApprovals).validNel } case _ => s"The value is expected in the following format: APPROVALS_RULE_NAME:REQUIRED_APPROVALS.".invalidNel @@ -294,7 +293,7 @@ object Cli { private val gitlabRequiredReviewers: Opts[Option[Int]] = option[Int]( - "gitlabRequiredReviewers", + "gitlab-required-reviewers", "When set, the number of required reviewers for a merge request will be set to this number (non-negative integer). Is only used in the context of gitlab-merge-when-pipeline-succeeds being enabled, and requires that the configured access token have the appropriate privileges. Also requires a Gitlab Premium subscription." ).validate("Required reviewers must be non-negative")(_ >= 0).orNone @@ -304,24 +303,25 @@ object Cli { "Flag indicating if a merge request should remove the source branch when merging." ).orFalse - private val gitlabMergeRequestApprovalsConfig: Opts[Option[Nel[MergeRequestApprovalsConfig]]] = - options[MergeRequestApprovalsConfig]( + private val gitlabMergeRequestApprovalsConfig: Opts[Option[Nel[MergeRequestApprovalRulesCfg]]] = + options[MergeRequestApprovalRulesCfg]( "merge-request-level-approval-rule", s"Additional repo config file $multiple" ) - // ToDo better message - .validate("")(_.forall(_.requiredApproves >= 0) == true) + .validate("Merge request level required approvals must be non-negative")( + _.forall(_.requiredApprovals >= 0) == true + ) .orNone private val gitlabReviewersAndApprovalsConfig - : Opts[Option[Either[Int, Nel[MergeRequestApprovalsConfig]]]] = + : Opts[Option[Either[Int, Nel[MergeRequestApprovalRulesCfg]]]] = ((gitlabRequiredReviewers, gitlabMergeRequestApprovalsConfig).tupled.mapValidated { case (None, None) => None.validNel case (None, Some(gitlabMergeRequestApprovalsConfig)) => Some(gitlabMergeRequestApprovalsConfig.asRight[Int]).validNel case (Some(requiredReviewers), None) => Some(Left(requiredReviewers)).validNel case (Some(_), Some(_)) => - s"You can't use both --gitlabRequiredReviewers and --merge-request-level-approval-rule at the same time".invalidNel + s"You can't use both --gitlab-required-reviewers and --merge-request-level-approval-rule at the same time".invalidNel }) private val gitLabCfg: Opts[GitLabCfg] = diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala index dab3539637..bf5688478b 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala @@ -156,11 +156,11 @@ object Config { final case class GitHubCfg( ) extends ForgeSpecificCfg - final case class MergeRequestApprovalsConfig(approvalRuleName: String, requiredApproves: Int) + final case class MergeRequestApprovalRulesCfg(approvalRuleName: String, requiredApprovals: Int) final case class GitLabCfg( mergeWhenPipelineSucceeds: Boolean, - requiredReviewers: Option[Either[Int, Nel[MergeRequestApprovalsConfig]]], + requiredApprovals: Option[Either[Int, Nel[MergeRequestApprovalRulesCfg]]], removeSourceBranch: Boolean ) extends ForgeSpecificCfg diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala index 2c5cd20d11..b30345897e 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala @@ -23,7 +23,7 @@ import io.circe._ import io.circe.generic.semiauto._ import io.circe.syntax._ import org.http4s.{Request, Status, Uri} -import org.scalasteward.core.application.Config.{ForgeCfg, GitLabCfg, MergeRequestApprovalsConfig} +import org.scalasteward.core.application.Config.{ForgeCfg, GitLabCfg, MergeRequestApprovalRulesCfg} import org.scalasteward.core.data.Repo import org.scalasteward.core.forge.ForgeApiAlg import org.scalasteward.core.forge.data._ @@ -158,7 +158,7 @@ private[gitlab] object GitLabJsonCodec { Decoder.instance { c => for { id <- c.downField("id").as[Int] - name <- c.downField("string").as[String] + name <- c.downField("name").as[String] } yield MergeRequestLevelApprovalRuleOut(id, name) } @@ -262,7 +262,7 @@ final class GitLabApiAlg[F[_]: Parallel]( for { mr <- mergeRequest mrWithStatus <- waitForMergeRequestStatus(mr.iid) - _ <- gitLabCfg.requiredReviewers match { + _ <- gitLabCfg.requiredApprovals match { case Some(Right(approvalRules)) => setApprovalRules(repo, mrWithStatus, approvalRules) case Some(Left(requiredReviewers)) => @@ -326,11 +326,11 @@ final class GitLabApiAlg[F[_]: Parallel]( private def setApprovalRules( repo: Repo, mrOut: MergeRequestOut, - approvalsConfig: Nel[MergeRequestApprovalsConfig] + approvalRulesCfg: Nel[MergeRequestApprovalRulesCfg] ): F[MergeRequestOut] = for { _ <- logger.info( - s"Adjusting merge request approvals rules on ${mrOut.webUrl} with following config: $approvalsConfig" + s"Adjusting merge request approvals rules on ${mrOut.webUrl} with following config: $approvalRulesCfg" ) activeApprovalRules <- client @@ -339,34 +339,47 @@ final class GitLabApiAlg[F[_]: Parallel]( modify(repo) ) .recoverWith { case UnexpectedResponse(_, _, _, status, body) => - // ToDo better log logger - .warn(s"Unexpected response setting required reviewers: $status: $body") + .warn(s"Unexpected response getting merge request approval rules: $status: $body") .as(List.empty) } - approvalRuleNamesFromConfig = approvalsConfig.map(_.approvalRuleName) - approvalRulesToUpdate = activeApprovalRules.intersect(approvalRuleNamesFromConfig.toList) + approvalRulesToUpdate = calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg) _ <- - approvalRulesToUpdate.map { mergeRequestApprovalConfig => - client - .putWithBody[Unit, UpdateMergeRequestLevelApprovalRulePayload]( - url.updateMergeRequestLevelApprovalRule( - repo, - mrOut.iid, - mergeRequestApprovalConfig.id - ), - UpdateMergeRequestLevelApprovalRulePayload(mergeRequestApprovalConfig.id), - modify(repo) - ) - .recoverWith { case UnexpectedResponse(_, _, _, status, body) => - // ToDo better log - logger - .warn(s"Unexpected response setting required reviewers: $status: $body") - .as(List.empty) - } + approvalRulesToUpdate.map { case (approvalRuleCfg, activeRule) => + logger.info( + s"Setting required approval count to ${approvalRuleCfg.requiredApprovals} for merge request approval rule '${approvalRuleCfg.approvalRuleName}' on ${mrOut.webUrl}" + ) >> + client + .putWithBody[ + MergeRequestLevelApprovalRuleOut, + UpdateMergeRequestLevelApprovalRulePayload + ]( + url.updateMergeRequestLevelApprovalRule( + repo, + mrOut.iid, + activeRule.id + ), + UpdateMergeRequestLevelApprovalRulePayload(approvalRuleCfg.requiredApprovals), + modify(repo) + ) + .as(()) + .recoverWith { case UnexpectedResponse(_, _, _, status, body) => + logger + .warn(s"Unexpected response setting required approvals: $status: $body") + } }.sequence } yield mrOut + private[gitlab] def calculateRulesToUpdate( + activeApprovalRules: List[MergeRequestLevelApprovalRuleOut], + approvalRulesCfg: Nel[MergeRequestApprovalRulesCfg] + ): List[(MergeRequestApprovalRulesCfg, MergeRequestLevelApprovalRuleOut)] = + activeApprovalRules.flatMap { activeRule => + approvalRulesCfg + .find(_.approvalRuleName == activeRule.name) + .map(_ -> activeRule) + } + private def getUsernameToUserIdsMapping(repo: Repo, usernames: Set[String]): F[Map[String, Int]] = usernames.toList .parTraverse { username => diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala index b6e0203597..cefaf4078c 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala @@ -17,7 +17,6 @@ package org.scalasteward.core.forge.gitlab import org.http4s.Uri -import org.scalasteward.core.application.Config.MergeRequestApprovalsConfig import org.scalasteward.core.data.Repo import org.scalasteward.core.forge.data.PullRequestNumber import org.scalasteward.core.git.Branch diff --git a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala index 6093cdd3fa..aaedb80fea 100644 --- a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala +++ b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala @@ -6,9 +6,12 @@ import munit.FunSuite import org.http4s.syntax.literals._ import org.scalasteward.core.application.Cli.EnvVar import org.scalasteward.core.application.Cli.ParseResult._ -import org.scalasteward.core.application.Config.StewardUsage +import org.scalasteward.core.application.Config.{MergeRequestApprovalRulesCfg, StewardUsage} import org.scalasteward.core.forge.ForgeType import org.scalasteward.core.forge.github.GitHubApp +import org.scalasteward.core.util.Nel +import cats.syntax.either._ + import scala.concurrent.duration._ class CliTest extends FunSuite { @@ -63,7 +66,7 @@ class CliTest extends FunSuite { assertEquals(obtained.githubApp, Some(GitHubApp(12345678L, File("example_app_key")))) assertEquals(obtained.refreshBackoffPeriod, 1.day) assert(!obtained.gitLabCfg.mergeWhenPipelineSucceeds) - assertEquals(obtained.gitLabCfg.requiredReviewers, None) + assertEquals(obtained.gitLabCfg.requiredApprovals, None) assert(obtained.bitbucketCfg.useDefaultReviewers) assert(!obtained.bitbucketServerCfg.useDefaultReviewers) } @@ -151,7 +154,7 @@ class CliTest extends FunSuite { assert(clue(obtained).startsWith("Usage")) } - test("parseArgs: non-default GitLab arguments") { + test("parseArgs: non-default GitLab arguments and required reviewers") { val params = minimumRequiredParams ++ List( List("--gitlab-merge-when-pipeline-succeeds"), List("--gitlab-required-reviewers", "5") @@ -159,12 +162,60 @@ class CliTest extends FunSuite { val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten) assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds) - assertEquals(obtained.gitLabCfg.requiredReviewers, Some(5)) + assertEquals(obtained.gitLabCfg.requiredApprovals, Some(5.asLeft)) } - test("parseArgs: invalid GitLab required reviewers") { + test("parseArgs: non-default GitLab arguments and merge request level approval rule") { val params = minimumRequiredParams ++ List( List("--gitlab-merge-when-pipeline-succeeds"), + List("--merge-request-level-approval-rule", "All eligible users:0") + ) + val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten) + + assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds) + assertEquals( + obtained.gitLabCfg.requiredApprovals, + Some(Nel.one(MergeRequestApprovalRulesCfg("All eligible users", 0)).asRight) + ) + } + + test("parseArgs: multiple Gitlab merge request level approval rule") { + val params = minimumRequiredParams ++ List( + List("--merge-request-level-approval-rule", "All eligible users:1"), + List("--merge-request-level-approval-rule", "Only Main:2") + ) + val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten) + + assertEquals( + obtained.gitLabCfg.requiredApprovals, + Some( + Nel + .of( + MergeRequestApprovalRulesCfg("All eligible users", 1), + MergeRequestApprovalRulesCfg("Only Main", 2) + ) + .asRight + ) + ) + } + + test("parseArgs: only allow one way to define Gitlab required approvals arguments") { + val params = minimumRequiredParams ++ List( + List("--merge-request-level-approval-rule", "All eligible users:0"), + List("--gitlab-required-reviewers", "5") + ) + val Error(errorMsg) = Cli.parseArgs(params.flatten) + + assert( + clue(errorMsg).startsWith( + "You can't use both --gitlab-required-reviewers and --merge-request-level-approval-rule at the same time" + ) + ) + + } + + test("parseArgs: invalid GitLab required reviewers") { + val params = minimumRequiredParams ++ List( List("--gitlab-required-reviewers", "-3") ) val Error(errorMsg) = Cli.parseArgs(params.flatten) @@ -172,6 +223,15 @@ class CliTest extends FunSuite { assert(clue(errorMsg).startsWith("Required reviewers must be non-negative")) } + test("parseArgs: invalid GitLab merge request level approval rule") { + val params = minimumRequiredParams ++ List( + List("--merge-request-level-approval-rule", "All eligible users:-3") + ) + val Error(errorMsg) = Cli.parseArgs(params.flatten) + + assert(clue(errorMsg).startsWith("Merge request level required approvals must be non-negative")) + } + test("parseArgs: validate-repo-config") { val Success(StewardUsage.ValidateRepoConfig(file)) = Cli.parseArgs( List( diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabAlgTest.scala new file mode 100644 index 0000000000..1ff86842a9 --- /dev/null +++ b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabAlgTest.scala @@ -0,0 +1,73 @@ +package org.scalasteward.core.forge.gitlab + +import munit.CatsEffectSuite +import org.http4s.Request +import org.scalasteward.core.TestInstances.ioLogger +import org.scalasteward.core.application.Config.{GitLabCfg, MergeRequestApprovalRulesCfg} +import org.scalasteward.core.data.Repo +import org.scalasteward.core.forge.ForgeType +import org.scalasteward.core.mock.MockConfig.config +import org.scalasteward.core.mock.MockContext.context.httpJsonClient +import org.scalasteward.core.mock.MockEff +import org.scalasteward.core.util.Nel + +class GitLabAlgTest extends CatsEffectSuite { + + private val gitlabApiAlg = new GitLabApiAlg[MockEff]( + forgeCfg = config.forgeCfg.copy(tpe = ForgeType.GitLab), + gitLabCfg = GitLabCfg( + mergeWhenPipelineSucceeds = false, + requiredApprovals = None, + removeSourceBranch = true + ), + modify = (_: Repo) => (request: Request[MockEff]) => MockEff.pure(request) + ) + + test( + "calculateRulesToUpdate -- ignore active approval rule that doesn't have approval rule configuration" + ) { + val activeApprovalRules = + List( + MergeRequestLevelApprovalRuleOut(name = "A", id = 101), + MergeRequestLevelApprovalRuleOut(name = "B", id = 201) + ) + val approvalRulesCfg = + Nel.one(MergeRequestApprovalRulesCfg(approvalRuleName = "B", requiredApprovals = 1)) + + val result = + gitlabApiAlg.calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg) + val expectedResult = + List( + ( + MergeRequestApprovalRulesCfg(approvalRuleName = "B", requiredApprovals = 1), + MergeRequestLevelApprovalRuleOut(id = 201, name = "B") + ) + ) + + assertEquals(result, expectedResult) + } + + test( + "calculateRulesToUpdate -- ignore approval rule configuration that doesn't have active approval rule" + ) { + val activeApprovalRules = + List(MergeRequestLevelApprovalRuleOut(name = "A", id = 101)) + val approvalRulesCfg = + Nel.of( + MergeRequestApprovalRulesCfg(approvalRuleName = "A", requiredApprovals = 1), + MergeRequestApprovalRulesCfg(approvalRuleName = "B", requiredApprovals = 2) + ) + + val result = + gitlabApiAlg.calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg) + val expectedResult = + List( + ( + MergeRequestApprovalRulesCfg(approvalRuleName = "A", requiredApprovals = 1), + MergeRequestLevelApprovalRuleOut(name = "A", id = 101) + ) + ) + + assertEquals(result, expectedResult) + } +} diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala index b6a224456a..510e36956b 100644 --- a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala +++ b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala @@ -1,5 +1,6 @@ package org.scalasteward.core.forge.gitlab +import cats.syntax.either._ import cats.syntax.semigroupk._ import io.circe.Json import io.circe.literal._ @@ -12,7 +13,7 @@ import org.http4s.headers.Allow import org.http4s.implicits._ import org.scalasteward.core.TestInstances.{dummyRepoCache, ioLogger} import org.scalasteward.core.TestSyntax._ -import org.scalasteward.core.application.Config.GitLabCfg +import org.scalasteward.core.application.Config.{GitLabCfg, MergeRequestApprovalRulesCfg} import org.scalasteward.core.data.{Repo, RepoData, UpdateData} import org.scalasteward.core.forge.data._ import org.scalasteward.core.forge.gitlab.GitLabJsonCodec._ @@ -22,6 +23,7 @@ import org.scalasteward.core.mock.MockConfig.config import org.scalasteward.core.mock.MockContext.context.httpJsonClient import org.scalasteward.core.mock.{MockEff, MockState} import org.scalasteward.core.repoconfig.RepoConfig +import org.scalasteward.core.util.Nel import org.typelevel.ci.CIStringSyntax class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { @@ -103,6 +105,16 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { ) ) + case GET -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "approval_rules" => + Ok(getMrApprovalRules) + + case PUT -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "approval_rules" / "101" => + Ok( + updateMrApprovalRule.deepMerge( + json""" { "id": 101, "approvals_required": 0 } """ + ) + ) + case POST -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "notes" => Ok(json""" { "body": "Superseded by #1234" @@ -124,7 +136,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab), GitLabCfg( mergeWhenPipelineSucceeds = false, - requiredReviewers = None, + requiredApprovals = None, removeSourceBranch = false ), user @@ -134,7 +146,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = false, - requiredReviewers = None, + requiredApprovals = None, removeSourceBranch = false ), user @@ -144,7 +156,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = true, - requiredReviewers = None, + requiredApprovals = None, removeSourceBranch = false ), user @@ -154,7 +166,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = false, - requiredReviewers = None, + requiredApprovals = None, removeSourceBranch = true ), user @@ -164,7 +176,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = true, - requiredReviewers = Some(0), + requiredApprovals = Some(0.asLeft), removeSourceBranch = false ), user @@ -174,7 +186,18 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = true, - requiredReviewers = Some(0), + requiredApprovals = Some(0.asLeft), + removeSourceBranch = false + ), + user + ) + + private val gitlabApiAlgWithApprovalRules = ForgeSelection.forgeApiAlg[MockEff]( + config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), + GitLabCfg( + mergeWhenPipelineSucceeds = true, + requiredApprovals = + Some(Nel.one(MergeRequestApprovalRulesCfg("All eligible users", 0)).asRight), removeSourceBranch = false ), user @@ -363,6 +386,51 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { assertIO(prOut, expected) } + test("createPullRequest -- with approval rules") { + val prOut = gitlabApiAlgWithApprovalRules + .createPullRequest( + Repo("foo", "bar"), + newPRData + ) + .runA(state) + + val expected = PullRequestOut( + uri"https://gitlab.com/foo/bar/merge_requests/150", + PullRequestState.Open, + PullRequestNumber(150), + "title" + ) + + assertIO(prOut, expected) + } + + test("createPullRequest -- no fail upon update approval rule error") { + val localApp = HttpApp[MockEff] { req => + (req: @unchecked) match { + case PUT -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "approval_rules" / "101" => + BadRequest(s"Cannot update merge requests approval rules") + } + } + + val localState = MockState.empty.copy(clientResponses = auth <+> localApp <+> httpApp) + + val prOut = gitlabApiAlgWithApprovalRules + .createPullRequest( + Repo("foo", "bar"), + newPRData + ) + .runA(localState) + + val expected = PullRequestOut( + uri"https://gitlab.com/foo/bar/merge_requests/150", + PullRequestState.Open, + PullRequestNumber(150), + "title" + ) + + assertIO(prOut, expected) + } + test("createPullRequest -- with non-existent user as reviewer") { val prOut = gitlabApiAlgWithAssigneeAndReviewers .createPullRequest( @@ -605,4 +673,68 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { "multiple_approval_rules_available": true } """ + + private val updateMrApprovalRule = + json""" + { + "id": 1021, + "name": "scala-steward", + "rule_type": "regular", + "eligible_approvers": [ + { + "id": 1, + "username": "scala-steward", + "name": "Scala Steward", + "state": "active", + "avatar_url": "https://secure.gravatar.com/avatar/5286ca631fff30960bfc2b337144556f?s=800&d=identicon", + "web_url": "https://gitlab.com/scala-steward" + } + ], + "approvals_required": 0, + "users": [], + "groups": [], + "contains_hidden_groups": false, + "section": null, + "source_rule": { + "approvals_required": 3 + }, + "overridden": true + } + """ + + private val getMrApprovalRules = + json""" + [ + { + "id": 101, + "name": "All eligible users", + "rule_type": "any_approver", + "eligible_approvers": [], + "approvals_required": 2, + "users": [], + "groups": [], + "contains_hidden_groups": false, + "section": null, + "source_rule": { + "approvals_required": 0 + }, + "overridden": true + }, + { + "id": 102, + "name": "scala-steward-test", + "rule_type": "regular", + "eligible_approvers": [], + "approvals_required": 2, + "users": [], + "groups": [], + "contains_hidden_groups": false, + "section": null, + "source_rule": { + "approvals_required": 3 + }, + "overridden": true + } + ] + """ } From c6cde8bf22e5c513e7f88c1ff523f34ff68c33d0 Mon Sep 17 00:00:00 2001 From: Ender Tunc Date: Sun, 19 Mar 2023 13:40:02 +0100 Subject: [PATCH 3/4] run sbt docs/mdoc and add small update in tests for --gitlab-remove-source-branch --- docs/help.md | 4 +++- .../scala/org/scalasteward/core/application/CliTest.scala | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/help.md b/docs/help.md index 78ea0316a9..3b90c02a56 100644 --- a/docs/help.md +++ b/docs/help.md @@ -5,7 +5,7 @@ All command line arguments for the `scala-steward` application. ``` Usage: scala-steward validate-repo-config - scala-steward --workspace --repos-file [--git-author-name ] --git-author-email [--git-author-signing-key ] --git-ask-pass [--sign-commits] [--forge-type ] [--forge-api-host ] --forge-login [--do-not-fork] [--add-labels] [--ignore-opts-files] [--env-var ]... [--process-timeout ] [--whitelist ]... [--read-only ]... [--enable-sandbox | --disable-sandbox] [--max-buffer-size ] [--repo-config ]... [--disable-default-repo-config] [--scalafix-migrations ]... [--disable-default-scalafix-migrations] [--artifact-migrations ]... [--disable-default-artifact-migrations] [--cache-ttl ] [--bitbucket-use-default-reviewers] [--bitbucket-server-use-default-reviewers] [--gitlab-merge-when-pipeline-succeeds] [--gitlab-required-reviewers ] [--gitlab-remove-source-branch] [--azure-repos-organization ] [--github-app-id --github-app-key-file ] [--url-checker-test-url ]... [--default-maven-repo ] [--refresh-backoff-period ] + scala-steward --workspace --repos-file [--git-author-name ] --git-author-email [--git-author-signing-key ] --git-ask-pass [--sign-commits] [--forge-type ] [--forge-api-host ] --forge-login [--do-not-fork] [--add-labels] [--ignore-opts-files] [--env-var ]... [--process-timeout ] [--whitelist ]... [--read-only ]... [--enable-sandbox | --disable-sandbox] [--max-buffer-size ] [--repo-config ]... [--disable-default-repo-config] [--scalafix-migrations ]... [--disable-default-scalafix-migrations] [--artifact-migrations ]... [--disable-default-artifact-migrations] [--cache-ttl ] [--bitbucket-use-default-reviewers] [--bitbucket-server-use-default-reviewers] [--gitlab-merge-when-pipeline-succeeds] [--gitlab-required-reviewers ] [--merge-request-level-approval-rule ]... [--gitlab-remove-source-branch] [--azure-repos-organization ] [--github-app-id --github-app-key-file ] [--url-checker-test-url ]... [--default-maven-repo ] [--refresh-backoff-period ] @@ -80,6 +80,8 @@ Options and flags: Whether to merge a gitlab merge request when the pipeline succeeds --gitlab-required-reviewers When set, the number of required reviewers for a merge request will be set to this number (non-negative integer). Is only used in the context of gitlab-merge-when-pipeline-succeeds being enabled, and requires that the configured access token have the appropriate privileges. Also requires a Gitlab Premium subscription. + --merge-request-level-approval-rule + Additional repo config file (can be used multiple times) --gitlab-remove-source-branch Flag indicating if a merge request should remove the source branch when merging. --azure-repos-organization diff --git a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala index aaedb80fea..125f1208bb 100644 --- a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala +++ b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala @@ -157,22 +157,26 @@ class CliTest extends FunSuite { test("parseArgs: non-default GitLab arguments and required reviewers") { val params = minimumRequiredParams ++ List( List("--gitlab-merge-when-pipeline-succeeds"), + List("--gitlab-remove-source-branch"), List("--gitlab-required-reviewers", "5") ) val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten) assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds) + assert(obtained.gitLabCfg.removeSourceBranch) assertEquals(obtained.gitLabCfg.requiredApprovals, Some(5.asLeft)) } test("parseArgs: non-default GitLab arguments and merge request level approval rule") { val params = minimumRequiredParams ++ List( List("--gitlab-merge-when-pipeline-succeeds"), + List("--gitlab-remove-source-branch"), List("--merge-request-level-approval-rule", "All eligible users:0") ) val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten) assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds) + assert(obtained.gitLabCfg.removeSourceBranch) assertEquals( obtained.gitLabCfg.requiredApprovals, Some(Nel.one(MergeRequestApprovalRulesCfg("All eligible users", 0)).asRight) From 8766c3bf19f089ec68696fdf7ad0b2dfd91912a7 Mon Sep 17 00:00:00 2001 From: Ender Tunc Date: Fri, 31 Mar 2023 16:47:01 +0200 Subject: [PATCH 4/4] add more tests --- .../scalasteward/core/application/Cli.scala | 14 +++++---- .../core/application/CliTest.scala | 28 ++++++++++++++---- .../core/forge/gitlab/GitLabApiAlgTest.scala | 29 ++++++++++++++++++- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala index 6c175c9959..11b821f63f 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala @@ -46,18 +46,22 @@ object Cli { val processTimeout = "process-timeout" } - implicit val mergeRequestApprovalsConfigArgument: Argument[MergeRequestApprovalRulesCfg] = + implicit val mergeRequestApprovalsCfgArgument: Argument[MergeRequestApprovalRulesCfg] = Argument.from("approvals_rule_name=required_approvals") { s => - s.split(":").toList match { + s.trim.split("=").toList match { case approvalRuleName :: requiredApprovalsAsString :: Nil => Try(requiredApprovalsAsString.trim.toInt) match { case Failure(_) => - s"[$requiredApprovalsAsString] is not a valid Integer".invalidNel + Validated.invalidNel(s"[$requiredApprovalsAsString] is not a valid Integer") case Success(requiredApprovals) => - MergeRequestApprovalRulesCfg(approvalRuleName.trim, requiredApprovals).validNel + Validated.valid( + MergeRequestApprovalRulesCfg(approvalRuleName.trim, requiredApprovals) + ) } case _ => - s"The value is expected in the following format: APPROVALS_RULE_NAME:REQUIRED_APPROVALS.".invalidNel + Validated.invalidNel( + "The value is expected in the following format: APPROVALS_RULE_NAME=REQUIRED_APPROVALS" + ) } } diff --git a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala index 125f1208bb..aa5800325d 100644 --- a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala +++ b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala @@ -1,6 +1,7 @@ package org.scalasteward.core.application import better.files.File +import cats.data.Validated import cats.data.Validated.Valid import munit.FunSuite import org.http4s.syntax.literals._ @@ -171,7 +172,7 @@ class CliTest extends FunSuite { val params = minimumRequiredParams ++ List( List("--gitlab-merge-when-pipeline-succeeds"), List("--gitlab-remove-source-branch"), - List("--merge-request-level-approval-rule", "All eligible users:0") + List("--merge-request-level-approval-rule", "All eligible users=0") ) val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten) @@ -185,8 +186,8 @@ class CliTest extends FunSuite { test("parseArgs: multiple Gitlab merge request level approval rule") { val params = minimumRequiredParams ++ List( - List("--merge-request-level-approval-rule", "All eligible users:1"), - List("--merge-request-level-approval-rule", "Only Main:2") + List("--merge-request-level-approval-rule", "All eligible users=1"), + List("--merge-request-level-approval-rule", "Only Main=2") ) val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten) @@ -205,7 +206,7 @@ class CliTest extends FunSuite { test("parseArgs: only allow one way to define Gitlab required approvals arguments") { val params = minimumRequiredParams ++ List( - List("--merge-request-level-approval-rule", "All eligible users:0"), + List("--merge-request-level-approval-rule", "All eligible users=0"), List("--gitlab-required-reviewers", "5") ) val Error(errorMsg) = Cli.parseArgs(params.flatten) @@ -229,7 +230,7 @@ class CliTest extends FunSuite { test("parseArgs: invalid GitLab merge request level approval rule") { val params = minimumRequiredParams ++ List( - List("--merge-request-level-approval-rule", "All eligible users:-3") + List("--merge-request-level-approval-rule", "All eligible users=-3") ) val Error(errorMsg) = Cli.parseArgs(params.flatten) @@ -304,4 +305,21 @@ class CliTest extends FunSuite { ) assert(error.startsWith("Missing value for option: --azure-repos-organization")) } + + test("mergeRequestApprovalsConfigArgument: without equals sign") { + assertEquals( + Cli.mergeRequestApprovalsCfgArgument.read("only-main"), + Validated.invalidNel( + s"The value is expected in the following format: APPROVALS_RULE_NAME=REQUIRED_APPROVALS" + ) + ) + } + + test("mergeRequestApprovalsConfigArgument: non-integer required approvals") { + val nonIntegerRequiredApprovals = "two" + assertEquals( + Cli.mergeRequestApprovalsCfgArgument.read(s"only-main=$nonIntegerRequiredApprovals"), + Validated.invalidNel(s"[$nonIntegerRequiredApprovals] is not a valid Integer") + ) + } } diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala index 510e36956b..906758bbeb 100644 --- a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala +++ b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala @@ -404,11 +404,38 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { assertIO(prOut, expected) } + test("createPullRequest -- no fail upon list approval rules error") { + val localApp = HttpApp[MockEff] { req => + (req: @unchecked) match { + case GET -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "approval_rules" => + BadRequest(s"Cannot get merge request approval rules") + } + } + + val localState = MockState.empty.copy(clientResponses = auth <+> localApp <+> httpApp) + + val prOut = gitlabApiAlgWithApprovalRules + .createPullRequest( + Repo("foo", "bar"), + newPRData + ) + .runA(localState) + + val expected = PullRequestOut( + uri"https://gitlab.com/foo/bar/merge_requests/150", + PullRequestState.Open, + PullRequestNumber(150), + "title" + ) + + assertIO(prOut, expected) + } + test("createPullRequest -- no fail upon update approval rule error") { val localApp = HttpApp[MockEff] { req => (req: @unchecked) match { case PUT -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "approval_rules" / "101" => - BadRequest(s"Cannot update merge requests approval rules") + BadRequest(s"Cannot update merge request approval rule") } }