Merge pull request #2017 from scala-steward-org/topic/clean-up-VCSRepoAlg

Clean up VCSRepoAlg

Author: fthomas

modules/core/src/main/scala/org/scalasteward/core/vcs/VCSRepoAlg.scala

@@ -33,29 +33,25 @@ final class VCSRepoAlg[F[_]](config: Config)(implicit
     F: MonadThrow[F]
 ) {
   def cloneAndSync(repo: Repo, repoOut: RepoOut): F[Unit] =
-    for {
-      _ <-
-        if (config.doNotFork) logger.info(s"Clone ${repo.show}")
-        else logger.info(s"Clone and synchronize ${repo.show}")
-      _ <- clone(repo, repoOut)
-      _ <- syncFork(repo, repoOut)
-      _ <- logger.attemptLogWarn_("Initializing and cloning submodules failed") {
-        gitAlg.initSubmodules(repo)
-      }
-    } yield ()
+    clone(repo, repoOut) >>
+      (if (config.doNotFork) F.unit else syncFork(repo, repoOut)) >>
+      initSubmodules(repo)
 
   private def clone(repo: Repo, repoOut: RepoOut): F[Unit] =
-    for {
-      _ <- gitAlg.clone(repo, withLogin(repoOut.clone_url))
-      _ <- gitAlg.setAuthor(repo, config.gitCfg.gitAuthor)
-    } yield ()
+    logger.info(s"Clone ${repoOut.repo.show}") >>
+      gitAlg.clone(repo, withLogin(repoOut.clone_url)) >>
+      gitAlg.setAuthor(repo, config.gitCfg.gitAuthor)
 
-  private[vcs] def syncFork(repo: Repo, repoOut: RepoOut): F[Unit] =
-    if (config.doNotFork) F.unit
-    else
-      repoOut.parentOrRaise[F].flatMap { parent =>
+  private def syncFork(repo: Repo, repoOut: RepoOut): F[Unit] =
+    repoOut.parentOrRaise[F].flatMap { parent =>
+      logger.info(s"Synchronize with ${parent.repo.show}") >>
         gitAlg.syncFork(repo, withLogin(parent.clone_url), parent.default_branch)
-      }
+    }
+
+  private def initSubmodules(repo: Repo): F[Unit] =
+    logger.attemptLogWarn_("Initializing and cloning submodules failed") {
+      gitAlg.initSubmodules(repo)
+    }
 
   private val withLogin: Uri => Uri =
     util.uri.withUserInfo.set(UserInfo(config.vcsLogin, None))

modules/core/src/test/scala/org/scalasteward/core/vcs/VCSRepoAlgTest.scala

@@ -10,35 +10,37 @@ import org.scalasteward.core.mock.{MockContext, MockEff, MockState}
 import org.scalasteward.core.vcs.data.{Repo, RepoOut, UserOut}
 
 class VCSRepoAlgTest extends FunSuite {
-  val repo: Repo = Repo("fthomas", "datapackage")
-  val repoDir: String = (config.workspace / "fthomas/datapackage").toString
-  val parentRepoOut: RepoOut = RepoOut(
+  private val repo = Repo("fthomas", "datapackage")
+  private val repoDir = (config.workspace / "fthomas/datapackage").toString
+  private val parentRepoOut = RepoOut(
     "datapackage",
     UserOut("fthomas"),
     None,
     uri"https://github.com/fthomas/datapackage",
     Branch("master")
   )
 
-  val forkRepoOut: RepoOut = RepoOut(
+  private val forkRepoOut = RepoOut(
     "datapackage",
-    UserOut("scalasteward"),
+    UserOut("scala-steward"),
     Some(parentRepoOut),
     uri"https://github.com/scala-steward/datapackage",
     Branch("master")
   )
 
-  test("cloneAndSync") {
+  private val parentUrl = s"https://${config.vcsLogin}@github.com/fthomas/datapackage"
+  private val forkUrl = s"https://${config.vcsLogin}@github.com/scala-steward/datapackage"
+
+  test("cloneAndSync: doNotFork = false") {
     val state = vcsRepoAlg.cloneAndSync(repo, forkRepoOut).runS(MockState.empty).unsafeRunSync()
-    val url0 = s"https://${config.vcsLogin}@github.com/fthomas/datapackage"
-    val url1 = s"https://${config.vcsLogin}@github.com/scala-steward/datapackage"
     val expected = MockState.empty.copy(
       trace = Vector(
-        Log("Clone and synchronize fthomas/datapackage"),
-        Cmd(envVars, config.workspace.toString, "git", "clone", url1, repoDir),
+        Log("Clone scala-steward/datapackage"),
+        Cmd(envVars, config.workspace.toString, "git", "clone", forkUrl, repoDir),
         Cmd(envVars, repoDir, "git", "config", "user.email", "bot@example.org"),
         Cmd(envVars, repoDir, "git", "config", "user.name", "Bot Doe"),
-        Cmd(envVars, repoDir, "git", "remote", "add", "upstream", url0),
+        Log("Synchronize with fthomas/datapackage"),
+        Cmd(envVars, repoDir, "git", "remote", "add", "upstream", parentUrl),
         Cmd(envVars, repoDir, "git", "fetch", "--force", "--tags", "upstream", "master"),
         Cmd(envVars, repoDir, "git", "checkout", "-B", "master", "--track", "upstream/master"),
         Cmd(envVars, repoDir, "git", "merge", "upstream/master"),
@@ -49,21 +51,30 @@ class VCSRepoAlgTest extends FunSuite {
     assertEquals(state, expected)
   }
 
-  test("syncFork should throw an exception when doNotFork = false and there is no parent") {
-    val result = vcsRepoAlg
-      .syncFork(repo, parentRepoOut)
+  test("cloneAndSync: doNotFork = true") {
+    val config = MockContext.config.copy(doNotFork = true)
+    val state = new VCSRepoAlg[MockEff](config)
+      .cloneAndSync(repo, parentRepoOut)
       .runS(MockState.empty)
-      .attempt
       .unsafeRunSync()
-    assert(clue(result).isLeft)
+    val expected = MockState.empty.copy(
+      trace = Vector(
+        Log("Clone fthomas/datapackage"),
+        Cmd(envVars, config.workspace.toString, "git", "clone", parentUrl, repoDir),
+        Cmd(envVars, repoDir, "git", "config", "user.email", "bot@example.org"),
+        Cmd(envVars, repoDir, "git", "config", "user.name", "Bot Doe"),
+        Cmd(envVars, repoDir, "git", "submodule", "update", "--init", "--recursive")
+      )
+    )
+    assertEquals(state, expected)
   }
 
-  test("syncFork should do nothing when doNotFork = true") {
-    val config = MockContext.config.copy(doNotFork = true)
-    val state = new VCSRepoAlg[MockEff](config)
-      .syncFork(repo, parentRepoOut)
+  test("cloneAndSync: doNotFork = false, no parent") {
+    val result = vcsRepoAlg
+      .cloneAndSync(repo, parentRepoOut)
       .runS(MockState.empty)
+      .attempt
       .unsafeRunSync()
-    assertEquals(state, MockState.empty)
+    assert(clue(result).isLeft)
   }
 }