rework strategy for running tests

- make all expect tests explicit with sbt-projectmatrix - keep only the 3 latest patch versions as targets (instead of 7) - move away from one command per scala version - speed up test execution by removing forking and limiting sequential run to `integration` suites - speed up windows tests by avoiding compiling upfront scala 3 projects as part of publishLocalTransitive by setting `publishLocal / skip` and by expressing dependency to `publishLocalTransitive` in the task graph instead of a sequence in a command - run docs checks in a separate CI job - run tests for all scala versions on windows
scalacenter · May 16, 2023 · ec8c956 · ec8c956
1 parent 231c272
commit ec8c956
Show file tree

Hide file tree

Showing 7 changed files with 105 additions and 127 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -6,59 +6,33 @@ on:
   pull_request:
 jobs:
   test:
-    name: ${{ matrix.command }}
-    runs-on: ubuntu-latest
     strategy:
       fail-fast: false
       matrix:
-        command:
-          - "ci-212"
-          - "ci-213"
-          - "ci-3"
-    steps:
-      - uses: actions/checkout@v3
-      - uses: coursier/setup-action@v1
-        with:
-          jvm: temurin:8
-      - run: sbt ${{ matrix.command }}
-  jdk11_212:
-    name: JDK11/scala_2.12 tests
-    runs-on: ubuntu-latest
+        os: ["ubuntu"]
+        jvm: ["8", "11", "17"]
+        include:
+          - os: windows
+            jvm: 17
+    name: ${{ matrix.os }} / JDK${{ matrix.jvm }}
+    runs-on: ${{ matrix.os }}-latest
     steps:
       - uses: actions/checkout@v3
       - uses: coursier/setup-action@v1
         with:
-          jvm: temurin:11
-      - run: sbt ci-212
-  jdk11_213:
-    name: JDK11/scala_2.13 tests
+          jvm: temurin:${{ matrix.jvm }}
+      - if: ${{ matrix.os != 'windows' }}
+        run: sbt test
+      - if: ${{ matrix.os == 'windows' }}
+        run: sbt ci-windows
+  docs:
+    name: Compile docs
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
       - uses: coursier/setup-action@v1
-        with:
-          jvm: temurin:11
-      - run: sbt ci-213
-
-  jdk17_213:
-    name: JDK17/scala_2.13 tests
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - uses: coursier/setup-action@v1
-        with:
-          jvm: temurin:17
-      - run: sbt ci-213
-
-  windows_213:
-    name: Windows/scala_2.13 tests
-    runs-on: windows-latest
-    steps:
-      - uses: actions/checkout@v3
-      - uses: coursier/setup-action@v1
-      - run: sbt ci-213-windows
-        shell: bash
-  checks:
+      - run: sbt ci-docs
+  formatting:
     name: Scalafmt and Scalafix
     runs-on: ubuntu-latest
     steps:

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -32,14 +32,20 @@ sbt shell.
 > unit2_13/test
 
 # Integration tests for rules, cli, core. Contains a lot
-# of different test suites, so it's recommended to use testOnly.
+# of different test suites, so it's recommended to use testOnly
+# and/or testQuick.
 > integration2_13/test
 
+# Use testWindows to exclude tests that are not expected to succeed
+# on that OS.
+> unit2_13/testWindows
+> integration2_13/testWindows
+
 # Only run tests for built-in rules, using scalafix-testkit.
-> expect2_13Target2_13/test
+> expect2_13Target2_13_10/test
 
 # Only run ProcedureSyntax unit tests.
-> expect2_13Target2_13/testOnly -- -z ProcedureSyntax
+> expect2_13Target2_13_10/testOnly -- -z ProcedureSyntax
 ```
 
 [sbt-projectmatrix](https://github.com/sbt/sbt-projectmatrix) is used to

diff --git a/build.sbt b/build.sbt
@@ -1,10 +1,10 @@
 import Dependencies._
+import TargetAxis.TargetProjectMatrix
 import sbt.Keys.scalacOptions
 
 inThisBuild(
   List(
     onLoadMessage := s"Welcome to scalafix ${version.value}",
-    fork := true,
     semanticdbEnabled := true,
     semanticdbVersion := scalametaV,
     scalafixScalaBinaryVersion := "2.13",
@@ -187,7 +187,7 @@ lazy val input = projectMatrix
     coverageEnabled := false
   )
   .defaultAxes(VirtualAxis.jvm)
-  .jvmPlatform(buildScalaVersions)
+  .jvmPlatformFull(buildWithTargetVersions.map(_._2))
   .disablePlugins(ScalafixPlugin)
 
 lazy val output = projectMatrix
@@ -236,6 +236,7 @@ lazy val integration = projectMatrix
   .in(file("scalafix-tests/integration"))
   .settings(
     noPublishAndNoMima,
+    Test / parallelExecution := false,
     libraryDependencies += {
       if (!isScala3.value) {
         coursier
@@ -277,6 +278,9 @@ lazy val integration = projectMatrix
         resolve(output, Compile / sourceDirectory).value
     ),
     Test / test := (Test / test)
+      .dependsOn(cli.projectRefs.map(_ / publishLocalTransitive): _*)
+      .value,
+    Test / testWindows := (Test / testWindows)
       .dependsOn(cli.projectRefs.map(_ / publishLocalTransitive): _*)
       .value
   )
@@ -339,32 +343,14 @@ lazy val expect = projectMatrix
     }
   )
   .defaultAxes(VirtualAxis.jvm)
-  .jvmPlatform(
-    scalaVersions = Seq(scala3),
-    axisValues = Seq(TargetAxis(scala3)),
-    settings = Seq()
-  )
-  .jvmPlatform(
-    scalaVersions = Seq(scala212),
-    axisValues = Seq(TargetAxis(scala3)),
-    settings = Seq()
-  )
-  .jvmPlatform(
-    scalaVersions = Seq(scala213),
-    axisValues = Seq(TargetAxis(scala213)),
-    settings = Seq()
-  )
-  .jvmPlatform(
-    scalaVersions = Seq(scala212),
-    axisValues = Seq(TargetAxis(scala212)),
-    settings = Seq()
-  )
+  .jvmPlatformWithTargets(buildWithTargetVersions)
   .dependsOn(integration)
 
 lazy val docs = projectMatrix
   .in(file("scalafix-docs"))
   .settings(
     noPublishAndNoMima,
+    fork := true,
     run / baseDirectory := (ThisBuild / baseDirectory).value,
     moduleName := "scalafix-docs",
     scalacOptions += "-Wconf:msg='match may not be exhaustive':s", // silence exhaustive pattern matching warning for documentation

diff --git a/project/Dependencies.scala b/project/Dependencies.scala
@@ -12,11 +12,10 @@ object Dependencies {
   val scala3 = "3.2.2"
 
   val buildScalaVersions = Seq(scala212, scala213, scala3)
-  val testTargetScalaVersions = Seq(scala212, scala213, scala3)
-
-  // we support 3 last binary versions of scala212 and scala213
-  val testedPreviousScalaVersions: Map[String, List[String]] =
-    List(scala213, scala212).map(version => version -> previousVersions(version)).toMap
+  val buildWithTargetVersions: Seq[(String, String)] =
+    buildScalaVersions.map(sv => (sv, sv)) ++
+      Seq(scala213, scala212).flatMap(sv => previousVersions(sv).map(prev => (sv, prev))) ++
+      Seq(scala213, scala212).map(sv => (sv, scala3))
 
   val bijectionCoreV = "0.9.7"
   val collectionCompatV = "2.10.0"
@@ -59,12 +58,12 @@ object Dependencies {
   val munit = "org.scalameta" %% "munit" % munitV
   val semanticdbScalacCore = "org.scalameta" % "semanticdb-scalac-core" % scalametaV cross CrossVersion.full
 
-  private def previousVersions(scalaVersion: String): List[String] = {
+  private def previousVersions(scalaVersion: String): Seq[String] = {
     val split = scalaVersion.split('.')
     val binaryVersion = split.take(2).mkString(".")
     val compilerVersion = Try(split.last.toInt).toOption
     val previousPatchVersions =
-      compilerVersion.map(version => List.range(version - 7, version).filter(_ >= 0)).getOrElse(Nil)
+      compilerVersion.map(version => List.range(version - 2, version).filter(_ >= 0)).getOrElse(Nil)
     previousPatchVersions.map(v => s"$binaryVersion.$v")
   }
 }
diff --git a/project/ScalafixBuild.scala b/project/ScalafixBuild.scala
@@ -123,6 +123,9 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
       else scalatest
     }
 
+    lazy val testWindows =
+      taskKey[Unit]("run tests, excluding those incompatible with Windows")
+
     /**
      * Lookup a setting key for the project of the same scala version in the
      * given matrix
@@ -168,36 +171,11 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
         "integration3/test:runMain scalafix.tests.util.SaveExpect" ::
         s
     },
-    commands += Command.command("ci-3") { s =>
-      "unit3/test" ::
-        "integration3/test" ::
-        "expects2_12Target3/test" ::
-        "expects3Target3/test" ::
-        s
-    },
-    commands += Command.command("ci-213") { s =>
-      "unit2_13/test" ::
-        "integration2_13/test" ::
-        "expects2_13Target2_13/test" ::
-        "docs2_13/run" ::
+    commands += Command.command("ci-docs") { s =>
+      "docs2_13/run" :: // reduce risk of errors on deploy-website.yml
         "interfaces/doc" ::
-        testRulesAgainstPreviousScalaVersions(scala213, s)
-    },
-    commands += Command.command("ci-212") { s =>
-      "unit_2_12/test" ::
-        "integration2_12/test" ::
-        "expects2_12Target2_12/test" ::
-        testRulesAgainstPreviousScalaVersions(scala212, s)
-    },
-    commands += Command.command("ci-213-windows") { s =>
-      "publishLocalTransitive" :: // scalafix.tests.interfaces.ScalafixSuite
-        "unit2_13/testOnly -- -l scalafix.internal.tests.utils.SkipWindows" ::
-        "integration2_13/testOnly -- -l scalafix.internal.tests.utils.SkipWindows" ::
-        "expects2_13Target2_13/test" ::
         s
     },
-    // There is flakyness in CliGitDiffTests and CliSemanticTests
-    Test / parallelExecution := false,
     Test / publishArtifact := false,
     licenses := Seq(
       "Apache-2.0" -> url("http://www.apache.org/licenses/LICENSE-2.0")
@@ -232,10 +210,11 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
   )
 
   override def projectSettings: Seq[Def.Setting[_]] = List(
-    // Change working directory to match when `fork := false`.
-    Test / baseDirectory := (ThisBuild / baseDirectory).value,
     // Prevent issues with scalatest serialization
     Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat,
+    Test / testWindows := (Test / testOnly)
+      .toTask(" -- -l scalafix.internal.tests.utils.SkipWindows")
+      .value,
     // avoid "missing dependency" on artifacts with full scala version when bumping scala
     versionPolicyIgnored ++= {
       PreviousScalaVersion.get(scalaVersion.value) match {
@@ -252,6 +231,8 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
     // don't publish scala 3 artifacts for now
     publish / skip := (if ((publish / skip).value) true
                        else scalaBinaryVersion.value == "3"),
+    publishLocal / skip := (if ((publishLocal / skip).value) true
+                            else scalaBinaryVersion.value == "3"),
     versionPolicyIntention := Compatibility.BinaryCompatible,
     scalacOptions ++= compilerOptions.value,
     scalacOptions ++= semanticdbSyntheticsCompilerOption.value,
@@ -306,21 +287,4 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
       }
     }
   )
-
-  private def testRulesAgainstPreviousScalaVersions(
-      scalaVersion: String,
-      state: State
-  ): State = {
-    val projectSuffix = scalaVersion.split('.').take(2).mkString("_")
-    testedPreviousScalaVersions
-      .getOrElse(scalaVersion, Nil)
-      .flatMap { v =>
-        List(
-          s"""set Project("testsInput${projectSuffix}", file(".")) / scalaVersion := "$v"""",
-          s"show testsInput${projectSuffix} / scalaVersion",
-          s"unit${projectSuffix}Target${projectSuffix} / testOnly scalafix.tests.rule.RuleSuite"
-        )
-      }
-      .foldRight(state)(_ :: _)
-  }
 }
diff --git a/project/TargetAxis.scala b/project/TargetAxis.scala
@@ -5,10 +5,8 @@ import sbtprojectmatrix.ProjectMatrixPlugin.autoImport._
 /** Use on ProjectMatrix rows to tag an affinity to a custom scalaVersion */
 case class TargetAxis(scalaVersion: String) extends VirtualAxis.WeakAxis {
 
-  private val scalaBinaryVersion = CrossVersion.binaryScalaVersion(scalaVersion)
-
-  override val idSuffix = s"Target${scalaBinaryVersion.replace('.', '_')}"
-  override val directorySuffix = s"target$scalaBinaryVersion"
+  override val idSuffix = s"Target${scalaVersion.replace('.', '_')}"
+  override val directorySuffix = s"target$scalaVersion"
   override val suffixOrder = VirtualAxis.scalaABIVersion("any").suffixOrder + 1
 }
 
@@ -28,7 +26,7 @@ object TargetAxis {
   ): Def.Initialize[Task[T]] =
     Def.taskDyn {
       val sv = targetScalaVersion(virtualAxes.value).get
-      val project = matrix.finder().apply(sv)
+      val project = exactOrBinaryScalaVersionMatch(matrix, sv)
       Def.task((project / key).value)
     }
 
@@ -43,8 +41,59 @@ object TargetAxis {
   ): Def.Initialize[T] =
     Def.settingDyn {
       val sv = targetScalaVersion(virtualAxes.value).get
-      val project = matrix.finder().apply(sv)
+      val project = exactOrBinaryScalaVersionMatch(matrix, sv)
       Def.setting((project / key).value)
     }
 
+  private def exactOrBinaryScalaVersionMatch(
+      matrix: ProjectMatrix,
+      scalaVersion: String
+  ): Project = {
+    val projectsWithAxisValues = matrix.allProjects().flatMap {
+      case (p, axisValues) => axisValues.map(v => (p, v))
+    }
+
+    projectsWithAxisValues.collectFirst {
+      case (p, VirtualAxis.ScalaVersionAxis(_, value))
+          if value == scalaVersion ||
+            value == CrossVersion.binaryScalaVersion(scalaVersion) =>
+        p
+    }.get
+  }
+
+  implicit class TargetProjectMatrix(projectMatrix: ProjectMatrix) {
+
+    /** Like jvmPlatform but with the full scala version attached */
+    def jvmPlatformFull(scalaVersions: Seq[String]): ProjectMatrix = {
+      scalaVersions.foldLeft(projectMatrix) { (acc, sv) =>
+        acc.customRow(
+          autoScalaLibrary = true,
+          axisValues = Seq(
+            VirtualAxis.jvm,
+            VirtualAxis.scalaVersionAxis(sv, sv)
+          ),
+          process = p => p
+        )
+      }
+    }
+
+    /**
+     * Like jvmPlatform but adding a target axis with the scala version provided
+     * as the second element of the tuple
+     */
+    def jvmPlatformWithTargets(
+        buildWithTargetVersions: Seq[(String, String)]
+    ): ProjectMatrix = {
+      buildWithTargetVersions.foldLeft(projectMatrix) {
+        case (acc, (build, target)) =>
+          acc.jvmPlatform(
+            scalaVersions = Seq(build),
+            axisValues = Seq(TargetAxis(target)),
+            settings = Seq()
+          )
+      }
+    }
+
+  }
+
 }
diff --git a/scalafix-tests/integration/src/test/scala/scalafix/tests/reflect/RuleDecoderSuite.scala b/scalafix-tests/integration/src/test/scala/scalafix/tests/reflect/RuleDecoderSuite.scala
@@ -34,7 +34,7 @@ class RuleDecoderSuite extends AnyFunSuite {
     assert(expectedName == rules.name.value)
   }
 
-  test("relative resolves from custom working directory") {
+  test("relative resolves from custom working directory", SkipWindows) {
     val rules = decoder.read(Conf.Str(s"file:$relpath")).get
     assert(expectedName == rules.name.value)
   }