Test discovery: Make classpath scanning fallback configurable (#4017)

Kotest will now by default use classpath scanning if and only if one or more non-class selectors are present. The new system property `kotest.framework.discovery.classpath.fallback.enabled=true` can be used to enable classpath scanning if no selectors are present. Also makes Kotest's internal debug logging multi-process-aware, naming log files like "kotest-PID.log", containing the respective process id. Fixes #3973. --------- Co-authored-by: Sam <sam@sksamuel.com>
kotest · Jun 15, 2024 · 0df2299 · 0df2299
1 parent e93f1d7
commit 0df2299
Show file tree

Hide file tree

Showing 14 changed files with 185 additions and 145 deletions.
diff --git a/.gitignore b/.gitignore
@@ -31,9 +31,7 @@ allure-results/
 !gradle/wrapper/gradle-wrapper.jar
 
 allure.log
-kotest.log
-kotest-tests-junit-report.log
-kotest-tests-core.log
+kotest-*.log
 
 local.properties
 .kotest

diff --git a/documentation/docs/framework/config_props.md b/documentation/docs/framework/config_props.md
@@ -84,6 +84,13 @@ object KotestEngineProperties {
     */
    const val invocationTimeout = "kotest.framework.invocation.timeout"
 
+   /**
+    * Use classpath scanning for test discovery if no selectors are present (defaults to "false").
+    * - Do not enable this when using Gradle with `maxParallelForks > 1`. Gradle might inadvertently invoke one
+    *   Kotest instance with an empty class list, resulting in duplicate test runs (#3973).
+    */
+   const val discoveryClasspathFallbackEnabled = "kotest.framework.discovery.classpath.fallback.enabled"
+
    const val disableTestNestedJarScanning = "kotest.framework.disable.test.nested.jar.scanning"
 
    const val concurrentSpecs = "kotest.framework.spec.concurrent"

diff --git a/kotest-common/api/kotest-common.api b/kotest-common/api/kotest-common.api
@@ -258,7 +258,6 @@ public final class io/kotest/mpp/UuidKt {
 }
 
 public final class io/kotest/mpp/WriteLogKt {
-	public static final fun getFile ()Ljava/io/FileWriter;
 	public static final fun writeLog (Lio/kotest/common/TimeMarkCompat;Ljava/lang/Throwable;Lkotlin/jvm/functions/Function0;)V
 }
 

diff --git a/kotest-common/src/jvmMain/kotlin/io/kotest/mpp/writeLog.kt b/kotest-common/src/jvmMain/kotlin/io/kotest/mpp/writeLog.kt
@@ -3,7 +3,20 @@ package io.kotest.mpp
 import io.kotest.common.TimeMarkCompat
 import java.io.FileWriter
 
-val file: FileWriter by lazy { FileWriter(syspropOrEnv("KOTEST_DEBUG_PATH") ?: "kotest.log", false) }
+/**
+ * Kotest debug log file ("kotest-PID.log" by default, with PID representing the runner's process ID).
+ *
+ * The process ID is included because Gradle's `maxParallelForks` > 1 will run multiple processes for Kotest in
+ * parallel.
+ * `KOTEST_DEBUG_PATH` can be used to specify the log file's path, with the string `PID` being replaced
+ * by the respective process ID.
+ */
+private val file: FileWriter by lazy {
+   FileWriter(
+      (syspropOrEnv("KOTEST_DEBUG_PATH") ?: "kotest-PID.log").replace("PID", "${ProcessHandle.current().pid()}"),
+      false
+   )
+}
 
 actual fun writeLog(start: TimeMarkCompat, t: Throwable?, f: () -> String) {
    file.write(start.elapsedNow().inWholeMicroseconds.toString())

diff --git a/...ckhound/src/jvmMain/kotlin/io/kotest/extensions/blockhound/KotestBlockHoundIntegration.kt b/...ckhound/src/jvmMain/kotlin/io/kotest/extensions/blockhound/KotestBlockHoundIntegration.kt
@@ -21,6 +21,10 @@ class KotestBlockHoundIntegration : BlockHoundIntegration {
          allowBlockingCallsInside("io.kotest.runner.junit.platform.SynchronizedEngineExecutionListener", method)
       }
 
+      // Allow blocking calls used when running Kotest in debug mode.
+      allowBlockingCallsInside("io.kotest.mpp.WriteLogKt", "writeLog")
+      allowBlockingCallsInside("kotlin.jvm.internal.Reflection", "renderLambdaToString")
+
       blockingMethodCallback {
          when (BlockHound.effectiveMode) {
             BlockHoundMode.ERROR -> throw BlockingOperationError(it)

diff --git a/kotest-framework/kotest-framework-api/api/kotest-framework-api.api b/kotest-framework/kotest-framework-api/api/kotest-framework-api.api
@@ -897,6 +897,7 @@ public abstract class io/kotest/core/config/AbstractProjectConfig {
 	public fun getCoroutineTestScope ()Ljava/lang/Boolean;
 	public fun getDefaultTestCaseConfig ()Lio/kotest/core/test/config/TestCaseConfig;
 	public fun getDisableTestNestedJarScanning ()Ljava/lang/Boolean;
+	public fun getDiscoveryClasspathFallbackEnabled ()Ljava/lang/Boolean;
 	public fun getDispatcherAffinity ()Ljava/lang/Boolean;
 	public fun getDisplayFullTestPath ()Ljava/lang/Boolean;
 	public fun getDisplaySpecIfNoActiveTests ()Ljava/lang/Boolean;
@@ -924,6 +925,7 @@ public abstract class io/kotest/core/config/AbstractProjectConfig {
 	public fun listeners ()Ljava/util/List;
 	public fun setAllowOutOfOrderCallbacks (Ljava/lang/Boolean;)V
 	public fun setDisableTestNestedJarScanning (Ljava/lang/Boolean;)V
+	public fun setDiscoveryClasspathFallbackEnabled (Ljava/lang/Boolean;)V
 	public fun setDispatcherAffinity (Ljava/lang/Boolean;)V
 	public fun setDisplayFullTestPath (Ljava/lang/Boolean;)V
 	public fun setProjectWideFailFast (Ljava/lang/Boolean;)V
@@ -971,6 +973,7 @@ public final class io/kotest/core/config/Defaults {
 	public static final field defaultInvocationTimeoutMillis J
 	public static final field defaultTimeoutMillis J
 	public static final field disableTestNestedJarScanning Z
+	public static final field discoveryClasspathFallbackEnabled Z
 	public static final field dispatcherAffinity Z
 	public static final field displayFullTestPath Z
 	public static final field displaySpecIfNoActiveTests Z
@@ -1076,6 +1079,7 @@ public final class io/kotest/core/config/ProjectConfiguration {
 	public final fun getCoroutineTestScope ()Z
 	public final fun getDefaultTestConfig ()Lio/kotest/core/test/config/TestCaseConfig;
 	public final fun getDisableTestNestedJarScanning ()Z
+	public final fun getDiscoveryClasspathFallbackEnabled ()Z
 	public final fun getDispatcherAffinity ()Z
 	public final fun getDisplayFullTestPath ()Z
 	public final fun getDisplaySpecIfNoActiveTests ()Z
@@ -1116,6 +1120,7 @@ public final class io/kotest/core/config/ProjectConfiguration {
 	public final fun setCoroutineTestScope (Z)V
 	public final fun setDefaultTestConfig (Lio/kotest/core/test/config/TestCaseConfig;)V
 	public final fun setDisableTestNestedJarScanning (Z)V
+	public final fun setDiscoveryClasspathFallbackEnabled (Z)V
 	public final fun setDispatcherAffinity (Z)V
 	public final fun setDisplayFullTestPath (Z)V
 	public final fun setDisplaySpecIfNoActiveTests (Z)V
@@ -1576,6 +1581,7 @@ public final class io/kotest/core/internal/KotestEngineProperties {
 	public static final field disableJarDiscovery Ljava/lang/String;
 	public static final field disableSourceRef Ljava/lang/String;
 	public static final field disableTestNestedJarScanning Ljava/lang/String;
+	public static final field discoveryClasspathFallbackEnabled Ljava/lang/String;
 	public static final field dumpConfig Ljava/lang/String;
 	public static final field duplicateTestNameMode Ljava/lang/String;
 	public static final field excludeTags Ljava/lang/String;

diff --git a/...kotest-framework-api/src/commonMain/kotlin/io/kotest/core/config/AbstractProjectConfig.kt b/...kotest-framework-api/src/commonMain/kotlin/io/kotest/core/config/AbstractProjectConfig.kt
@@ -272,6 +272,13 @@ abstract class AbstractProjectConfig {
 
    open var allowOutOfOrderCallbacks: Boolean? = null
 
+   /**
+    * Set to true if you wish to enable classpath scanning for test discovery if no selectors are present.
+    *
+    * Note: JVM ONLY
+    */
+   open var discoveryClasspathFallbackEnabled: Boolean? = null
+
    /**
     * Set to false if you wish to allow nested jar scanning for tests.
     *

diff --git a/...st-framework/kotest-framework-api/src/commonMain/kotlin/io/kotest/core/config/Defaults.kt b/...st-framework/kotest-framework-api/src/commonMain/kotlin/io/kotest/core/config/Defaults.kt
@@ -12,6 +12,7 @@ import io.kotest.core.test.config.TestCaseConfig
 object Defaults {
 
    const val threads = 1
+   const val discoveryClasspathFallbackEnabled: Boolean = false
    const val disableTestNestedJarScanning: Boolean = true
 
    val assertionMode: AssertionMode = AssertionMode.None

diff --git a/.../kotest-framework-api/src/commonMain/kotlin/io/kotest/core/config/ProjectConfiguration.kt b/.../kotest-framework-api/src/commonMain/kotlin/io/kotest/core/config/ProjectConfiguration.kt
@@ -339,6 +339,7 @@ class ProjectConfiguration {
     */
    var tagInheritance: Boolean = false
 
+   var discoveryClasspathFallbackEnabled: Boolean = Defaults.discoveryClasspathFallbackEnabled
    var disableTestNestedJarScanning: Boolean = Defaults.disableTestNestedJarScanning
 
    /**

diff --git a/...est-framework-api/src/commonMain/kotlin/io/kotest/core/internal/KotestEngineProperties.kt b/...est-framework-api/src/commonMain/kotlin/io/kotest/core/internal/KotestEngineProperties.kt
@@ -74,6 +74,13 @@ object KotestEngineProperties {
     */
    const val invocationTimeout = "kotest.framework.invocation.timeout"
 
+   /**
+    * Use classpath scanning for test discovery if no selectors are present (defaults to "false").
+    * - Do not enable this when using Gradle with `maxParallelForks > 1`. Gradle might inadvertently invoke one
+    *   Kotest instance with an empty class list, resulting in duplicate test runs (#3973).
+    */
+   const val discoveryClasspathFallbackEnabled = "kotest.framework.discovery.classpath.fallback.enabled"
+
    const val disableTestNestedJarScanning = "kotest.framework.disable.test.nested.jar.scanning"
 
    const val concurrentSpecs = "kotest.framework.spec.concurrent"

diff --git a/.../kotest-framework-discovery/src/jvmMain/kotlin/io/kotest/framework/discovery/Discovery.kt b/.../kotest-framework-discovery/src/jvmMain/kotlin/io/kotest/framework/discovery/Discovery.kt
@@ -10,7 +10,6 @@ import io.kotest.core.internal.KotestEngineProperties
 import io.kotest.core.spec.Spec
 import io.kotest.mpp.log
 import io.kotest.mpp.syspropOrEnv
-import java.lang.management.ManagementFactory
 import java.util.concurrent.ConcurrentHashMap
 import kotlin.reflect.KClass
 
@@ -43,15 +42,12 @@ class Discovery(
 
    private val requests = ConcurrentHashMap<DiscoveryRequest, DiscoveryResult>()
 
-   // the results of a classpath scan, lazily executed and memoized.
-   private val scanResult = lazy { classgraph().scan() }
-
    // filter functions
    //private val isScript: (KClass<*>) -> Boolean = { ScriptTemplateWithArgs::class.java.isAssignableFrom(it.java) }
    private val isSpecSubclassKt: (KClass<*>) -> Boolean = { Spec::class.java.isAssignableFrom(it.java) }
    private val isSpecSubclass: (Class<*>) -> Boolean = { Spec::class.java.isAssignableFrom(it) }
    private val isAbstract: (KClass<*>) -> Boolean = { it.isAbstract }
-   private val fromClassPaths: List<KClass<out Spec>> by lazy { scanUris() }
+   private val cachedSpecsFromClassPaths: List<KClass<out Spec>> by lazy { specsFromClassGraph() }
 
    /**
     * Returns a function that applies all the [DiscoveryFilter]s to a given class.
@@ -72,19 +68,6 @@ class Discovery(
    fun discover(request: DiscoveryRequest): DiscoveryResult =
       requests.getOrPut(request) { doDiscovery(request).getOrElse { DiscoveryResult.error(it) } }
 
-//   /**
-//    * Scans the classpaths for kotlin script files.
-//    */
-//   private fun discoverScripts(): List<KClass<out ScriptTemplateWithArgs>> {
-//      log { "Discovery: Running script scan" }
-//      return scanResult.value
-//         .allClasses
-//         .filter { it.extendsSuperclass(ScriptTemplateWithArgs::class.java.name) }
-//         .map { it.load(false) }
-//         .filter(isScript)
-//         .filterIsInstance<KClass<out ScriptTemplateWithArgs>>()
-//   }
-
    /**
     * Loads a class reference from a [ClassInfo].
     *
@@ -95,98 +78,82 @@ class Discovery(
 
    private fun doDiscovery(request: DiscoveryRequest): Result<DiscoveryResult> = runCatching {
 
-      val specClasses =
-         if (request.onlySelectsSingleClasses()) loadSelectedSpecs(request) else fromClassPaths
-
-      val filtered = specClasses
-         .asSequence()
-         .filter(selectorFn(request.selectors))
-         .filter(filterFn(request.filters))
-         // all classes must subclass one of the spec parents
-         .filter(isSpecSubclassKt)
-         // we don't want abstract classes
-         .filterNot(isAbstract)
-         .toList()
-
-      log { "After filters there are ${filtered.size} spec classes" }
+      log { "[Discovery] Starting spec discovery" }
 
-      log { "[Discovery] Further filtering classes via discovery extensions [$discoveryExtensions]" }
+      if (request.selectors.isEmpty() && !configuration.discoveryClasspathFallbackEnabled) {
+         log { "[Discovery] no specs discovered: no selectors provided and classpath fallback is disabled" }
+         return@runCatching DiscoveryResult(emptyList(), emptyList(), null)
+      }
 
-      val afterExtensions = discoveryExtensions
-         .fold(filtered) { cl, ext -> ext.afterScan(cl) }
-         .sortedBy { it.simpleName }
+      val specsSelected = request.specsFromClassDiscoverySelectorsOnlyOrNull()
+         ?: cachedSpecsFromClassPaths
+            .asSequence()
+            .filter(isSpecSubclassKt)
+            .filterNot(isAbstract)
+            .filter(selectorFn(request.selectors))
+            .toList()
 
-      log { "After discovery extensions there are ${afterExtensions.size} spec classes" }
+      val specsAfterInitialFiltering = specsSelected.filter(filterFn(request.filters))
 
-//      val scriptsEnabled = System.getProperty(KotestEngineProperties.scriptsEnabled) == "true" ||
-//         System.getenv(KotestEngineProperties.scriptsEnabled) == "true"
+      log { "[Discovery] ${specsAfterInitialFiltering.size} specs remain after initial filtering" }
 
-//      val scripts = when {
-//         scriptsEnabled -> discoverScripts()
-//         else -> emptyList()
-//      }
+      val specsAfterExtensionFiltering = discoveryExtensions
+         .fold(specsAfterInitialFiltering) { cl, ext -> ext.afterScan(cl) }
+         .sortedBy { it.simpleName }
 
-      if (scanResult.isInitialized()) runCatching { scanResult.value.close() }
+      log { "[Discovery] ${specsAfterExtensionFiltering.size} specs remain after extension filtering" }
 
-      log { "Discovery result [${afterExtensions.size} specs; scripts]" }
-      DiscoveryResult(afterExtensions, emptyList(), null)
+      DiscoveryResult(specsAfterExtensionFiltering, emptyList(), null)
    }
 
    /**
-    * Returns whether this is a request that selects single classes
-    * only. Used to avoid full classpath scans when not necessary.
+    * Returns the request's [Spec]s if they are completely specified by class selectors, null otherwise.
     */
-   private fun DiscoveryRequest.onlySelectsSingleClasses(): Boolean =
-      selectors.isNotEmpty() &&
-         selectors.all { it is DiscoverySelector.ClassDiscoverySelector }
+   private fun DiscoveryRequest.specsFromClassDiscoverySelectorsOnlyOrNull(): List<KClass<out Spec>>? {
+      if (selectors.isEmpty() || !selectors.all { it is DiscoverySelector.ClassDiscoverySelector })
+         return null
 
-   /**
-    * Returns a list of [Spec] classes from discovery requests that only have
-    * selectors of type [DiscoverySelector.ClassDiscoverySelector].
-    */
-   private fun loadSelectedSpecs(request: DiscoveryRequest): List<KClass<out Spec>> {
-      log { "Discovery: Loading specified classes..." }
       val start = System.currentTimeMillis()
 
       // first filter down to spec instances only, then load the full class
-      val loadedClasses = request
-         .selectors
+      val specs = selectors
          .asSequence()
          .filterIsInstance<DiscoverySelector.ClassDiscoverySelector>()
          .map { Class.forName(it.className, false, this::class.java.classLoader) }
          .filter(isSpecSubclass)
          .map { Class.forName(it.name).kotlin }
          .filterIsInstance<KClass<out Spec>>()
+         .filterNot(isAbstract)
          .toList()
 
-      val duration = System.currentTimeMillis() - start
-      log { "Discovery: Loading of selected classes completed in ${duration}ms" }
-      return loadedClasses
+      log {
+         val duration = System.currentTimeMillis() - start
+         "[Discovery] Collected specs via ${selectors.size} class discovery selectors in ${duration}ms," +
+            " found ${specs.size} specs"
+      }
+
+      return specs
    }
 
    /**
     * Returns a list of [Spec] classes detected using classgraph in the list of
     * locations specified by the uris param.
     */
-   private fun scanUris(): List<KClass<out Spec>> {
-
-      log {
-         val uptime = ManagementFactory.getRuntimeMXBean().uptime
-         "Discovery: Starting test discovery scan... [uptime=$uptime]"
+   private fun specsFromClassGraph(): List<KClass<out Spec>> {
+      val start = System.currentTimeMillis()
+      val specs = classgraph().scan().use { scanResult ->
+         scanResult
+            .getSubclasses(Spec::class.java.name)
+            .map { Class.forName(it.name).kotlin }
+            .filterIsInstance<KClass<out Spec>>()
       }
 
-      val result = scanResult.value
-         .getSubclasses(Spec::class.java.name)
-         .map { Class.forName(it.name).kotlin }
-         .filterIsInstance<KClass<out Spec>>()
-
       log {
-         val start = System.currentTimeMillis()
          val duration = System.currentTimeMillis() - start
-         "Discovery: Test discovery completed in ${duration}ms"
+         "[Discovery] Scanned classgraph for specs in ${duration}ms, found ${specs.size} specs"
       }
 
-      return result
+      return specs
    }
 
    private fun classgraph(): ClassGraph {
@@ -197,7 +164,7 @@ class Discovery(
          .ignoreClassVisibility()
 
       if (configuration.disableTestNestedJarScanning) {
-         log { "Nested jar scanning is disabled" }
+         log { "[Discovery] Nested jar scanning is disabled" }
          cg.disableNestedJarScanning()
          cg.disableModuleScanning()
       }

diff --git a/...ine/src/commonMain/kotlin/io/kotest/engine/config/applyConfigFromAbstractProjectConfig.kt b/...ine/src/commonMain/kotlin/io/kotest/engine/config/applyConfigFromAbstractProjectConfig.kt
@@ -40,6 +40,7 @@ internal fun applyConfigFromProjectConfig(config: AbstractProjectConfig, configu
    config.projectTimeout?.let { configuration.projectTimeout = it }
 
    // discovery
+   config.discoveryClasspathFallbackEnabled?.let { configuration.discoveryClasspathFallbackEnabled = it }
    config.disableTestNestedJarScanning?.let { configuration.disableTestNestedJarScanning = it }
 
    // test names

diff --git a/...work-engine/src/jvmMain/kotlin/io/kotest/engine/config/applyConfigFromSystemProperties.kt b/...work-engine/src/jvmMain/kotlin/io/kotest/engine/config/applyConfigFromSystemProperties.kt
@@ -35,6 +35,7 @@ internal actual fun applyConfigFromSystemProperties(configuration: ProjectConfig
    duplicateTestNameMode()?.let { configuration.duplicateTestNameMode = it }
    projectTimeout()?.let { configuration.projectTimeout = it }
    logLevel(configuration.logLevel).let { configuration.logLevel = it }
+   discoveryClasspathScanningEnabled()?.let { configuration.discoveryClasspathFallbackEnabled = it }
    disableTestNestedJarScanning()?.let { configuration.disableTestNestedJarScanning = it }
 }
 
@@ -56,8 +57,11 @@ internal fun invocationTimeout(): Long? =
 internal fun allowMultilineTestName(): Boolean? =
    sysprop(KotestEngineProperties.allowMultilineTestName)?.let { it.uppercase() == "TRUE" }
 
+internal fun discoveryClasspathScanningEnabled(): Boolean? =
+   sysprop(KotestEngineProperties.discoveryClasspathFallbackEnabled)?.toBoolean()
+
 internal fun disableTestNestedJarScanning(): Boolean? =
-   sysprop(KotestEngineProperties.concurrentSpecs)?.toBoolean()
+   sysprop(KotestEngineProperties.disableTestNestedJarScanning)?.toBoolean()
 
 internal fun concurrentSpecs(): Int? =
    sysprop(KotestEngineProperties.concurrentSpecs)?.toInt()