Fix testkit on sbt launching tests defined in other test modules when…

… depending on another test artifact (#2052) This workaround alters the classpath scanning logic to only consider classes defined in the first `test-classes` directory encountered on the classpath to be those defined in 'current module'. It seems that for SBT the heuristic that the current test module classes will be placed in the first `test-classes` dir on the classpath generally holds. However, if it fails, you may be able to fix test running by overriding `_sbtIsClassDefinedInCurrentTestModule` or `_sbtFindCurrentTestModuleClasspathElement` in your tests to suit your needs. Also fix `DistageScalatestTestSuiteRunner#modifyClasspathScan` modifier not being applied (it was never applied, ugh...)
7mind · Jan 17, 2024 · 1f264a4 · 1f264a4
1 parent e666c70
commit 1f264a4
Show file tree

Hide file tree

Showing 6 changed files with 257 additions and 8 deletions.
diff --git a/build.sbt b/build.sbt
@@ -3419,6 +3419,171 @@ lazy val `distage-testkit-scalatest` = project.in(file("distage/distage-testkit-
   )
   .disablePlugins(AssemblyPlugin)
 
+lazy val `distage-testkit-scalatest-sbt-module-filtering-test` = project.in(file("distage/distage-testkit-scalatest-sbt-module-filtering-test"))
+  .dependsOn(
+    `distage-testkit-scalatest` % "test->compile,test"
+  )
+  .settings(
+    libraryDependencies ++= Seq(
+      "org.scala-lang.modules" %% "scala-collection-compat" % V.collection_compat,
+      "org.scalatest" %% "scalatest" % V.scalatest % Test
+    ),
+    libraryDependencies ++= { if (scalaVersion.value.startsWith("2.")) Seq(
+      compilerPlugin("org.typelevel" % "kind-projector" % V.kind_projector cross CrossVersion.full)
+    ) else Seq.empty }
+  )
+  .settings(
+    crossScalaVersions := Seq(
+      "3.2.2",
+      "2.13.12",
+      "2.12.18"
+    ),
+    scalaVersion := crossScalaVersions.value.head,
+    organization := "io.7mind.izumi",
+    Compile / unmanagedSourceDirectories += baseDirectory.value / ".jvm/src/main/scala" ,
+    Compile / unmanagedSourceDirectories ++= (scalaBinaryVersion.value :: CrossVersion.partialVersion(scalaVersion.value).toList.map(_._1))
+      .map(v => baseDirectory.value / s".jvm/src/main/scala-$v").distinct,
+    Compile / unmanagedResourceDirectories += baseDirectory.value / ".jvm/src/main/resources" ,
+    Test / unmanagedSourceDirectories += baseDirectory.value / ".jvm/src/test/scala" ,
+    Test / unmanagedSourceDirectories ++= (scalaBinaryVersion.value :: CrossVersion.partialVersion(scalaVersion.value).toList.map(_._1))
+      .map(v => baseDirectory.value / s".jvm/src/test/scala-$v").distinct,
+    Test / unmanagedResourceDirectories += baseDirectory.value / ".jvm/src/test/resources" ,
+    scalacOptions ++= Seq(
+      s"-Xmacro-settings:product-name=${name.value}",
+      s"-Xmacro-settings:product-version=${version.value}",
+      s"-Xmacro-settings:product-group=${organization.value}",
+      s"-Xmacro-settings:scala-version=${scalaVersion.value}",
+      s"-Xmacro-settings:scala-versions=${crossScalaVersions.value.mkString(":")}"
+    ),
+    Compile / unmanagedSourceDirectories ++= {
+      val version = scalaVersion.value
+      val crossVersions = crossScalaVersions.value
+      import Ordering.Implicits._
+      val ltEqVersions = crossVersions.map(CrossVersion.partialVersion).filter(_ <= CrossVersion.partialVersion(version)).flatten
+      (Compile / unmanagedSourceDirectories).value.flatMap {
+        case dir if dir.getPath.endsWith("scala") => ltEqVersions.map { case (m, n) => file(dir.getPath + s"-$m.$n+") }
+        case _ => Seq.empty
+      }
+    },
+    Test / unmanagedSourceDirectories ++= {
+      val version = scalaVersion.value
+      val crossVersions = crossScalaVersions.value
+      import Ordering.Implicits._
+      val ltEqVersions = crossVersions.map(CrossVersion.partialVersion).filter(_ <= CrossVersion.partialVersion(version)).flatten
+      (Test / unmanagedSourceDirectories).value.flatMap {
+        case dir if dir.getPath.endsWith("scala") => ltEqVersions.map { case (m, n) => file(dir.getPath + s"-$m.$n+") }
+        case _ => Seq.empty
+      }
+    },
+    Test / testOptions += Tests.Argument("-oDF"),
+    scalacOptions ++= { (isSnapshot.value, scalaVersion.value) match {
+      case (_, "2.12.18") => Seq(
+        "-Wconf:any:error",
+        "-release:8",
+        "-explaintypes",
+        "-Xsource:3",
+        "-P:kind-projector:underscore-placeholders",
+        "-Ypartial-unification",
+        if (insideCI.value) "-Wconf:any:error" else "-Wconf:any:warning",
+        "-Wconf:cat=optimizer:warning",
+        "-Wconf:cat=other-match-analysis:error",
+        "-Ybackend-parallelism",
+        math.min(16, math.max(1, sys.runtime.availableProcessors() - 1)).toString,
+        "-Xlint:adapted-args",
+        "-Xlint:by-name-right-associative",
+        "-Xlint:constant",
+        "-Xlint:delayedinit-select",
+        "-Xlint:doc-detached",
+        "-Xlint:inaccessible",
+        "-Xlint:infer-any",
+        "-Xlint:missing-interpolator",
+        "-Xlint:nullary-override",
+        "-Xlint:nullary-unit",
+        "-Xlint:option-implicit",
+        "-Xlint:package-object-classes",
+        "-Xlint:poly-implicit-overload",
+        "-Xlint:private-shadow",
+        "-Xlint:stars-align",
+        "-Xlint:type-parameter-shadow",
+        "-Xlint:unsound-match",
+        "-opt-warnings:_",
+        "-Ywarn-extra-implicit",
+        "-Ywarn-unused:_",
+        "-Ywarn-adapted-args",
+        "-Ywarn-dead-code",
+        "-Ywarn-inaccessible",
+        "-Ywarn-infer-any",
+        "-Ywarn-nullary-override",
+        "-Ywarn-nullary-unit",
+        "-Ywarn-numeric-widen",
+        "-Ywarn-unused-import",
+        "-Ywarn-value-discard",
+        "-Ycache-plugin-class-loader:always",
+        "-Ycache-macro-class-loader:last-modified"
+      )
+      case (_, "2.13.12") => Seq(
+        "-Wconf:any:error",
+        "-release:8",
+        "-explaintypes",
+        "-Xsource:3",
+        "-P:kind-projector:underscore-placeholders",
+        if (insideCI.value) "-Wconf:any:error" else "-Wconf:any:warning",
+        "-Wconf:cat=optimizer:warning",
+        "-Wconf:cat=other-match-analysis:error",
+        "-Vimplicits",
+        "-Vtype-diffs",
+        "-Ybackend-parallelism",
+        math.min(16, math.max(1, sys.runtime.availableProcessors() - 1)).toString,
+        "-Wdead-code",
+        "-Wextra-implicit",
+        "-Wnumeric-widen",
+        "-Woctal-literal",
+        "-Wvalue-discard",
+        "-Wunused:_",
+        "-Wmacros:after",
+        "-Ycache-plugin-class-loader:always",
+        "-Ycache-macro-class-loader:last-modified",
+        "-Wunused:-synthetics"
+      )
+      case (_, "3.2.2") => Seq(
+        "-Yretain-trees",
+        "-language:3.2",
+        "-release:8",
+        "-Ykind-projector:underscores",
+        "-no-indent",
+        "-explain"
+      )
+      case (_, _) => Seq.empty
+    } },
+    scalacOptions -= "-Wconf:any:warning",
+    scalacOptions += "-Wconf:cat=deprecation:warning",
+    scalacOptions += "-Wconf:msg=legacy-binding:silent",
+    scalacOptions += "-Wconf:msg=nowarn:silent",
+    scalacOptions += "-Wconf:msg=parameter.*x\\$4.in.anonymous.function.is.never.used:silent",
+    scalacOptions += "-Wconf:msg=constructor.modifiers.are.assumed.by.synthetic.*method:silent",
+    scalacOptions += "-Wconf:msg=package.object.inheritance:silent",
+    scalacOptions += "-Wconf:cat=lint-eta-sam:silent",
+    Compile / sbt.Keys.doc / scalacOptions -= "-Wconf:any:error",
+    scalacOptions ++= Seq(
+      s"-Xmacro-settings:scalatest-version=${V.scalatest}",
+      s"-Xmacro-settings:is-ci=${insideCI.value}"
+    ),
+    scalacOptions ++= { (isSnapshot.value, scalaVersion.value) match {
+      case (false, "2.12.18") => Seq(
+        "-opt:l:inline",
+        "-opt-inline-from:izumi.**"
+      )
+      case (false, "2.13.12") => Seq(
+        "-opt:l:inline",
+        "-opt-inline-from:izumi.**"
+      )
+      case (_, _) => Seq.empty
+    } },
+    Test / packageDoc / publishArtifact := false,
+    publish / skip := true
+  )
+  .disablePlugins(AssemblyPlugin)
+
 lazy val `logstage-core` = project.in(file("logstage/logstage-core"))
   .dependsOn(
     `fundamentals-bio` % "test->compile;compile->compile",
@@ -4115,6 +4280,7 @@ lazy val `microsite` = project.in(file("doc/microsite"))
     `distage-framework-docker` % "test->compile;compile->compile",
     `distage-testkit-core` % "test->compile;compile->compile",
     `distage-testkit-scalatest` % "test->compile;compile->compile",
+    `distage-testkit-scalatest-sbt-module-filtering-test` % "test->compile;compile->compile",
     `logstage-core` % "test->compile;compile->compile",
     `logstage-rendering-circe` % "test->compile;compile->compile",
     `logstage-adapter-slf4j` % "test->compile;compile->compile",
@@ -4569,7 +4735,8 @@ lazy val `distage` = (project in file(".agg/distage-distage"))
     `distage-framework`,
     `distage-framework-docker`,
     `distage-testkit-core`,
-    `distage-testkit-scalatest`
+    `distage-testkit-scalatest`,
+    `distage-testkit-scalatest-sbt-module-filtering-test`
   )
 
 lazy val `distage-jvm` = (project in file(".agg/distage-distage-jvm"))
@@ -4589,7 +4756,8 @@ lazy val `distage-jvm` = (project in file(".agg/distage-distage-jvm"))
     `distage-framework`,
     `distage-framework-docker`,
     `distage-testkit-core`,
-    `distage-testkit-scalatest`
+    `distage-testkit-scalatest`,
+    `distage-testkit-scalatest-sbt-module-filtering-test`
   )
 
 lazy val `logstage` = (project in file(".agg/logstage-logstage"))

diff --git a/...ng-test/src/test/scala/izumi/distage/testkit/modulefiltering/SbtModuleFilteringTest.scala b/...ng-test/src/test/scala/izumi/distage/testkit/modulefiltering/SbtModuleFilteringTest.scala
@@ -0,0 +1,3 @@
+package izumi.distage.testkit.modulefiltering
+
+final class SbtModuleFilteringTest extends SbtModuleFilteringPoisonPillTest
diff --git a/...tkit-scalatest/src/main/scala/org/scalatest/distage/DistageScalatestTestSuiteRunner.scala b/...tkit-scalatest/src/main/scala/org/scalatest/distage/DistageScalatestTestSuiteRunner.scala
@@ -1,7 +1,7 @@
 package org.scalatest.distage
 
 import _root_.distage.TagK
-import io.github.classgraph.ClassGraph
+import io.github.classgraph.{ClassGraph, ClassInfo}
 import izumi.distage.modules.DefaultModule
 import izumi.distage.testkit.DebugProperties
 import izumi.distage.testkit.model.{DistageTest, SuiteId}
@@ -12,12 +12,14 @@ import izumi.distage.testkit.services.scalatest.dstest.{DistageTestsRegistrySing
 import izumi.distage.testkit.spec.AbstractDistageSpec
 import izumi.fundamentals.platform.console.TrivialLogger
 import izumi.fundamentals.platform.functional.Identity
+import izumi.fundamentals.platform.jvm.IzClasspath
 import org.scalatest.*
 import org.scalatest.exceptions.TestCanceledException
 
 import java.util.concurrent.atomic.AtomicBoolean
 import scala.collection.immutable.TreeSet
 import scala.util.Try
+import scala.util.chaining.scalaUtilChainingOps
 
 trait ScalatestInitWorkaround {
   def awaitTestsLoaded(): Unit
@@ -48,18 +50,23 @@ object ScalatestInitWorkaround {
         val scan = new ClassGraph()
           .enableClassInfo()
           .addClassLoader(classLoader)
+          .pipe(instance.modifyClasspathScan)
           .scan()
         try {
           val suiteClassName = classOf[DistageScalatestTestSuiteRunner[Identity]].getName
-          val testClasses = scan.getSubclasses(suiteClassName).asScala.filterNot(_.isAbstract)
+
+          val allTestClasses = scan.getSubclasses(suiteClassName).asScala.filterNot(_.isAbstract)
+          val onlyTestClassesInCurrentModule = allTestClasses.filter(instance._sbtIsClassDefinedInCurrentTestModule(classLoader))
+
           lazy val debugLogger = TrivialLogger.make[ScalatestInitWorkaroundImpl.type](DebugProperties.`izumi.distage.testkit.debug`.name)
-          testClasses.foreach(
+          onlyTestClassesInCurrentModule.foreach(
             classInfo =>
               Try {
                 debugLogger.log(s"Added scanned class `${classInfo.getName}` to current test run")
                 classInfo.loadClass().getDeclaredConstructor().newInstance()
               }
           )
+
           DistageTestsRegistrySingleton.disableRegistration()
           latch.countDown()
         } finally {
@@ -77,14 +84,51 @@ abstract class DistageScalatestTestSuiteRunner[F[_]](
 ) extends TestSuite
   with AbstractDistageSpec[F] {
 
-  /** Modify test discovery options for SBT test runner only.
+  /**
+    * Modify test discovery options for SBT test runner only.
     * Overriding this with [[withWhitelistJarsOnly]] will slightly boost test start-up speed,
     * but will disable the ability to discover tests that inherit [[izumi.distage.testkit.services.scalatest.dstest.DistageAbstractScalatestSpec]]
     * indirectly through a different library JAR. (this does not affect local sbt modules)
     */
-  protected def modifyClasspathScan: ClassGraph => ClassGraph = identity
+  def modifyClasspathScan: ClassGraph => ClassGraph = identity
   protected final def withWhitelistJarsOnly: ClassGraph => ClassGraph = _.acceptJars("distage-testkit-scalatest*")
 
+  /**
+    * Override this to change the heuristic by which testkit determines that a test class is defined in the current SBT module.
+    *
+    * Affects SBT test runner only.
+    *
+    * By default we assume that classes with classfiles located in the first directory on the classpath
+    * which contains `test-classes` in its pathname are the classes defined in the current SBT test module.
+    *
+    * @see [[_sbtFindCurrentTestModuleClasspathElement]] - override this to change just the method for finding the `test-classes` directory not all the logic
+    */
+  def _sbtIsClassDefinedInCurrentTestModule(classLoader: ClassLoader): ClassInfo => Boolean = {
+    val classpathElems = IzClasspath.safeClasspathSeq(classLoader)
+    _sbtFindCurrentTestModuleClasspathElement(classpathElems) match {
+      case Some(firstTestClassesDir) =>
+        (classInfo: ClassInfo) => {
+          val file = classInfo.getClasspathElementFile
+          file.isDirectory && file.toString == firstTestClassesDir
+        }
+      case None =>
+        import izumi.fundamentals.platform.strings.IzString.*
+        System.err.println(
+          s"""DISTAGE-TESTKIT CRITICAL: Couldn't find a `test-classes` directory on the classpath, disabling fix preventing launch of tests defined in other sbt modules.
+             |Classpath was = ${classpathElems.niceList()}""".stripMargin
+        )
+        _ => true
+    }
+  }
+
+  /**
+    * Override this to change the method for finding the `test-classes` directory for [[_sbtIsClassDefinedInCurrentTestModule]]
+    */
+  protected def _sbtFindCurrentTestModuleClasspathElement(classpathElems: Seq[String]): Option[String] = {
+    val firstTestClassesDir = classpathElems.find(_.contains("test-classes"))
+    firstTestClassesDir
+  }
+
   // initialize status early, so that runner can set it to `true` even before this test is discovered
   // by scalatest, if it was already executed by that time
   private[this] val status: StatefulStatus = DistageTestsRegistrySingleton.registerStatus[F](suiteId)

diff --git a/...c/test/scala/izumi/distage/testkit/modulefiltering/SbtModuleFilteringPoisonPillTest.scala b/...c/test/scala/izumi/distage/testkit/modulefiltering/SbtModuleFilteringPoisonPillTest.scala
@@ -0,0 +1,22 @@
+package izumi.distage.testkit.modulefiltering
+
+import izumi.distage.testkit.scalatest.SpecIdentity
+
+import java.util.concurrent.atomic.AtomicReference
+
+object SbtModuleFilteringPoisonPillTest {
+  val poisonPillTestsLaunched: AtomicReference[Int] = new AtomicReference(0)
+}
+
+open class SbtModuleFilteringPoisonPillTest extends SpecIdentity {
+
+  "SBT test module filtering fix" should {
+
+    "prevent `sbt test` task in `distage-testkit-scalatest-sbt-module-filtering-test` from launching test classes defined in `distage-testkit-scalatest` test scope" in {
+      val testsLaunched = SbtModuleFilteringPoisonPillTest.poisonPillTestsLaunched.updateAndGet(_ + 1)
+      assert(testsLaunched == 1)
+    }
+
+  }
+
+}
diff --git a/project/Deps.sc b/project/Deps.sc
@@ -365,6 +365,7 @@ object Izumi {
       final lazy val framework = ArtifactId("distage-framework")
       final lazy val testkitCore = ArtifactId("distage-testkit-core")
       final lazy val testkitScalatest = ArtifactId("distage-testkit-scalatest")
+      final lazy val testkitScalatestSbtModuleFilteringTest = ArtifactId("distage-testkit-scalatest-sbt-module-filtering-test")
       final lazy val extensionLogstage = ArtifactId("distage-extension-logstage")
     }
 
@@ -635,6 +636,17 @@ object Izumi {
           "libraryDependencySchemes" += """"org.scala-lang.modules" %% "scala-xml" % VersionScheme.Always""".raw
         ),
       ),
+      Artifact(
+        name = Projects.distage.testkitScalatestSbtModuleFilteringTest,
+        libs = Nil,
+        depends = Seq(
+          Projects.distage.testkitScalatest tin Scope.Test.all
+        ),
+        platforms = Targets.jvm3,
+        settings = Seq(
+          "skip" in SettingScope.Raw("publish") := true
+        ),
+      ),
     ),
     pathPrefix = Projects.distage.basePath,
     defaultPlatforms = Targets.cross3,

diff --git a/sbtgen.sc b/sbtgen.sc
@@ -1,5 +1,5 @@
 #!/bin/sh
-cs launch com.lihaoyi:ammonite_2.13.0:1.6.9 --fork -M ammonite.Main -- sbtgen.sc $*
+cs launch com.lihaoyi:ammonite_2.13.12:2.5.11 --fork -M ammonite.Main -- sbtgen.sc $*
 exit
 !#
 import $file.project.Deps, Deps._
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		package izumi.distage.testkit.modulefiltering

		final class SbtModuleFilteringTest extends SbtModuleFilteringPoisonPillTest