TS-46357 Collect the tests of a @ParameterizedClass

CHANGELOG.md

@@ -5,6 +5,9 @@ We use [semantic versioning](http://semver.org/):
 - PATCH version when you make backwards compatible bug fixes.
 
 # Next version
+- [fix] _impacted-test-engine_: Tests in a `@ParameterizedClass` are now collected.
+- [fix] _impacted-test-engine_: A test failure inside a nested test container (e.g. an invocation of a `@ParameterizedClass`) is no longer swallowed.
+- [fix] _teamscale-jacoco-agent_, _teamscale-maven-plugin_: A test that was executed more than once (e.g. once per parameter set of a `@ParameterizedClass`) is now reported once in the testwise coverage report, with the coverage of all of its executions merged, their durations summed up and the most severe of their results. Previously each execution overwrote the previous one.
 
 # 38.1.0
 - [feature] _agent_: `git-properties-jar` now also accepts a folder, which is searched for `git.properties` files. Previously, a folder was rejected with a warning and no commit was auto-detected.

agent/src/main/kotlin/com/teamscale/jacoco/agent/convert/Converter.kt

@@ -94,9 +94,7 @@ class Converter
 				arguments.getOutputFile(),
 				arguments.splitAfter, null
 			).use { coverageWriter ->
-				jacocoExecutionDataList.forEach { executionDataFile ->
-					generator.convertAndConsume(executionDataFile, coverageWriter)
-				}
+				generator.convertAndConsumePerTest(jacocoExecutionDataList, coverageWriter)
 			}
 		}
 	}

agent/src/main/kotlin/com/teamscale/jacoco/agent/testimpact/CoverageViaHttpStrategy.kt

@@ -28,8 +28,8 @@ class CoverageViaHttpStrategy(
 		val builder = TestInfoBuilder(test)
 		val dump = controller.dumpAndReset()
 		reportGenerator.updateClassDirCache()
-		reportGenerator.convert(dump)?.let { builder.setCoverage(it) }
-		builder.setExecution(testExecution)
+		reportGenerator.convert(dump)?.let { builder.addCoverage(it) }
+		builder.addExecution(testExecution)
 		val testInfo = builder.build()
 		logger.debug("Generated test info {}", testInfo)
 		return testInfo

gradle/libs.versions.toml

@@ -57,6 +57,7 @@ log4j-core = { module = "org.apache.logging.log4j:log4j-core", version = "2.26.1
 junit-bom = { module = "org.junit:junit-bom", version = "6.1.3" }
 junit-jupiter = { module = "org.junit.jupiter:junit-jupiter" }
 junit-jupiter-params = { module = "org.junit.jupiter:junit-jupiter-params" }
+junit-jupiter-engine = { module = "org.junit.jupiter:junit-jupiter-engine" }
 junit-platform-launcher = { module = "org.junit.platform:junit-platform-launcher" }
 junit-platform-engine = { module = "org.junit.platform:junit-platform-engine" }
 junit-platform-commons = { module = "org.junit.platform:junit-platform-commons" }

impacted-test-engine/build.gradle.kts

@@ -32,6 +32,13 @@ dependencies {
 	compileOnly(libs.junit.platform.engine)
 	compileOnly(libs.junit.platform.commons)
 	testImplementation(libs.junit.platform.engine)
+	testImplementation(libs.junit.platform.launcher)
 	testImplementation(libs.junit.jupiter.params)
+	testImplementation(libs.junit.jupiter.engine)
 	testImplementation(libs.mockito.kotlin)
 }
+
+tasks.test {
+	// The sample tests are discovered explicitly by JupiterClassTemplateTest and must not be run by Gradle itself.
+	exclude("**/test_descriptor/samples/**")
+}

impacted-test-engine/src/main/kotlin/com/teamscale/test_impacted/engine/InternalImpactedTestEngine.kt

@@ -3,6 +3,7 @@ package com.teamscale.test_impacted.engine
 import com.teamscale.test_impacted.commons.LoggerUtils.createLogger
 import com.teamscale.test_impacted.engine.ImpactedTestEngine.Companion.ENGINE_NAME
 import com.teamscale.test_impacted.engine.executor.TestwiseCoverageCollectingExecutionListener
+import com.teamscale.test_impacted.test_descriptor.ClassTemplateRegistry
 import com.teamscale.test_impacted.test_descriptor.TestDescriptorResolverRegistry.getTestDescriptorResolver
 import com.teamscale.test_impacted.test_descriptor.TestDescriptorUtils.getAvailableTests
 import com.teamscale.test_impacted.test_descriptor.TestDescriptorUtils.getTestDescriptorAsString
@@ -30,6 +31,12 @@ internal class InternalImpactedTestEngine(
 	private val teamscaleAgentNotifier = configuration.teamscaleAgentNotifier
 	private val testDataWriter = configuration.testDataWriter
 
+	/**
+	 * The tests of the `@ParameterizedClass`es in the test tree, recorded during discovery because the JUnit platform
+	 * prunes them from the tree before the tests are executed.
+	 */
+	private val classTemplateRegistry = ClassTemplateRegistry()
+
 	/**
 	 * Performs test discovery by aggregating the result of all [TestEngine]s from the [TestEngineRegistry]
 	 * in a single engine [TestDescriptor].
@@ -47,6 +54,7 @@ internal class InternalImpactedTestEngine(
 			)
 
 			engineDescriptor.addChild(delegateEngineDescriptor)
+			classTemplateRegistry.record(delegateEngineDescriptor)
 		}
 
 		LOG.fine {
@@ -64,15 +72,15 @@ internal class InternalImpactedTestEngine(
 	 */
 	fun execute(request: ExecutionRequest) {
 		val rootTestDescriptor = request.rootTestDescriptor
-		val availableTests = getAvailableTests(rootTestDescriptor)
+		val availableTests = getAvailableTests(rootTestDescriptor, classTemplateRegistry)
 
 		LOG.fine {
 			"Starting selection and sorting ${ImpactedTestEngine.ENGINE_ID}:\n${
 				getTestDescriptorAsString(rootTestDescriptor)
 			}"
 		}
 
-		testSorter.selectAndSort(rootTestDescriptor)
+		testSorter.selectAndSort(rootTestDescriptor, availableTests)
 
 		LOG.fine {
 			"Starting execution of request for engine ${ImpactedTestEngine.ENGINE_ID}:\n${
@@ -105,7 +113,8 @@ internal class InternalImpactedTestEngine(
 				TestwiseCoverageCollectingExecutionListener(
 					teamscaleAgentNotifier,
 					testDescriptorResolver,
-					request.engineExecutionListener
+					request.engineExecutionListener,
+					classTemplateRegistry
 				)
 
 			testEngine.execute(

impacted-test-engine/src/main/kotlin/com/teamscale/test_impacted/engine/executor/ITestSorter.kt

@@ -7,6 +7,8 @@ interface ITestSorter {
 	/**
 	 * Removes any tests from the test descriptor that should not be executed and changes the execution order of the
 	 * remaining tests.
+	 *
+	 * @param availableTests The tests contained in the given [testDescriptor], as determined during discovery.
 	 */
-	fun selectAndSort(testDescriptor: TestDescriptor)
+	fun selectAndSort(testDescriptor: TestDescriptor, availableTests: AvailableTests)
 }

impacted-test-engine/src/main/kotlin/com/teamscale/test_impacted/engine/executor/ImpactedTestsSorter.kt

@@ -2,7 +2,6 @@ package com.teamscale.test_impacted.engine.executor
 
 import com.teamscale.client.TestWithClusterId.Companion.fromClusteredTestDetails
 import com.teamscale.test_impacted.engine.ImpactedTestEngine
-import com.teamscale.test_impacted.test_descriptor.TestDescriptorUtils.getAvailableTests
 import org.junit.platform.engine.TestDescriptor
 import java.util.*
 
@@ -12,9 +11,7 @@ import java.util.*
  */
 class ImpactedTestsSorter(private val impactedTestsProvider: ImpactedTestsProvider) : ITestSorter {
 
-	override fun selectAndSort(testDescriptor: TestDescriptor) {
-		val availableTests = getAvailableTests(testDescriptor)
-
+	override fun selectAndSort(testDescriptor: TestDescriptor, availableTests: AvailableTests) {
 		val testClusters = impactedTestsProvider.getImpactedTestsFromTeamscale(
 			availableTests.testList
 				.map { fromClusteredTestDetails(it, impactedTestsProvider.partition) })

impacted-test-engine/src/main/kotlin/com/teamscale/test_impacted/engine/executor/NOPTestSorter.kt

@@ -7,7 +7,7 @@ import org.junit.platform.engine.TestDescriptor
  * Teamscale to select or prioritize tests.
  */
 class NOPTestSorter : ITestSorter {
-	override fun selectAndSort(testDescriptor: TestDescriptor) {
+	override fun selectAndSort(testDescriptor: TestDescriptor, availableTests: AvailableTests) {
 		// Nothing to do
 	}
 }

impacted-test-engine/src/main/kotlin/com/teamscale/test_impacted/engine/executor/TestwiseCoverageCollectingExecutionListener.kt

@@ -3,7 +3,9 @@ package com.teamscale.test_impacted.engine.executor
 import com.teamscale.report.testwise.model.ETestExecutionResult
 import com.teamscale.report.testwise.model.TestExecution
 import com.teamscale.test_impacted.commons.LoggerUtils.createLogger
+import com.teamscale.test_impacted.test_descriptor.ClassTemplateRegistry
 import com.teamscale.test_impacted.test_descriptor.ITestDescriptorResolver
+import com.teamscale.test_impacted.test_descriptor.TestDescriptorUtils.isClassTemplate
 import com.teamscale.test_impacted.test_descriptor.TestDescriptorUtils.isRepresentative
 import org.junit.platform.engine.EngineExecutionListener
 import org.junit.platform.engine.TestDescriptor
@@ -24,11 +26,13 @@ import java.io.StringWriter
  * @param teamscaleAgentNotifier The notifier responsible for signaling test events to the Teamscale JaCoCo agent.
  * @param testDescriptorResolver A resolver interface used to map [TestDescriptor] objects to uniform paths.
  * @param delegateEngineExecutionListener The underlying [EngineExecutionListener] to which events are delegated.
+ * @param classTemplateRegistry The tests of the `@ParameterizedClass`es as recorded during discovery.
  */
 class TestwiseCoverageCollectingExecutionListener(
 	private val teamscaleAgentNotifier: TeamscaleAgentNotifier,
 	private val testDescriptorResolver: ITestDescriptorResolver,
-	private val delegateEngineExecutionListener: EngineExecutionListener
+	private val delegateEngineExecutionListener: EngineExecutionListener,
+	private val classTemplateRegistry: ClassTemplateRegistry
 ) : EngineExecutionListener {
 	companion object {
 		private val LOG = createLogger()
@@ -47,6 +51,14 @@ class TestwiseCoverageCollectingExecutionListener(
 	}
 
 	override fun executionSkipped(testDescriptor: TestDescriptor, reason: String) {
+		if (testDescriptor.isClassTemplate()) {
+			// The tests of a @ParameterizedClass were pruned from the test tree, so report the ones that were
+			// recorded during discovery instead of descending into the now empty descriptor. They are not forwarded
+			// to the delegate listener, which only knows the descriptors that are still part of the tree.
+			testDescriptor.reportSkipped(reason)
+			delegateEngineExecutionListener.executionSkipped(testDescriptor, reason)
+			return
+		}
 		if (!testDescriptor.isRepresentative()) {
 			delegateEngineExecutionListener.executionStarted(testDescriptor)
 			testDescriptor.children.forEach { executionSkipped(it, reason) }
@@ -67,6 +79,18 @@ class TestwiseCoverageCollectingExecutionListener(
 		}
 	}
 
+	/** Records a [ETestExecutionResult.SKIPPED] execution for every test below the given descriptor.  */
+	private fun TestDescriptor.reportSkipped(reason: String) {
+		if (isRepresentative()) {
+			testDescriptorResolver.getUniformPath(this)?.let { testUniformPath ->
+				testExecutions.add(TestExecution(testUniformPath, 0L, ETestExecutionResult.SKIPPED, reason))
+			}
+			return
+		}
+		val tests = if (isClassTemplate()) classTemplateRegistry.testsOf(this) else children
+		tests.forEach { it.reportSkipped(reason) }
+	}
+
 	override fun executionStarted(testDescriptor: TestDescriptor) {
 		if (testDescriptor.isRepresentative()) {
 			testDescriptorResolver.getUniformPath(testDescriptor)?.let { testUniformPath ->
@@ -90,6 +114,10 @@ class TestwiseCoverageCollectingExecutionListener(
 			val testExecutionResults = testResultCache.computeIfAbsent(
 				testDescriptor.parent.get().uniqueId
 			) { mutableListOf() }
+			// Containers may be nested arbitrarily deep below their representative, e.g. a @ParameterizedClass
+			// contains one invocation per parameter set which in turn contains the test methods. Hand the results
+			// collected for this container up to its parent so that they reach the representative.
+			testResultCache.remove(testDescriptor.uniqueId)?.let { testExecutionResults.addAll(it) }
 			testExecutionResults.add(testExecutionResult)
 		}
 
@@ -108,14 +136,16 @@ class TestwiseCoverageCollectingExecutionListener(
 		val message = StringBuilder()
 		var status = TestExecutionResult.Status.SUCCESSFUL
 		testExecutionResults.forEach { executionResult ->
-			if (message.isNotEmpty()) {
-				message.append("\n\n")
-			}
-			message.append(executionResult.throwable.orElse(null).buildStacktrace())
 			// Aggregate status here to most severe status according to SUCCESSFUL < ABORTED < FAILED
 			if (status.ordinal < executionResult.status.ordinal) {
 				status = executionResult.status
 			}
+
+			val stacktrace = executionResult.throwable.orElse(null).buildStacktrace() ?: return@forEach
+			if (message.isNotEmpty()) {
+				message.append("\n\n")
+			}
+			message.append(stacktrace)
 		}
 
 		return buildTestExecution(testUniformPath, duration, status, message.toString())

impacted-test-engine/src/main/kotlin/com/teamscale/test_impacted/test_descriptor/ClassTemplateRegistry.kt

@@ -0,0 +1,34 @@
+package com.teamscale.test_impacted.test_descriptor
+
+import com.teamscale.test_impacted.test_descriptor.TestDescriptorUtils.isClassTemplate
+import org.junit.platform.engine.TestDescriptor
+import org.junit.platform.engine.UniqueId
+
+/**
+ * Remembers the tests of the `@ParameterizedClass`es in the discovered test tree.
+ *
+ * The JUnit platform prunes the tests of a `@ParameterizedClass` out of the test tree right after discovery and only
+ * re-registers them, once per parameter set, while the class is being executed. They are therefore no longer visible
+ * when the engine collects the available tests, which is why we have to record them while they are still there.
+ */
+class ClassTemplateRegistry {
+	private val testsByClassTemplate = mutableMapOf<UniqueId, List<TestDescriptor>>()
+
+	/**
+	 * Records the tests of all `@ParameterizedClass`es below the given descriptor. Must be called during discovery,
+	 * i.e. before the JUnit platform prunes the test tree.
+	 */
+	fun record(testDescriptor: TestDescriptor) {
+		if (testDescriptor.isClassTemplate() && testDescriptor.children.isNotEmpty()) {
+			testsByClassTemplate[testDescriptor.uniqueId] = testDescriptor.children.toList()
+		}
+		testDescriptor.children.forEach { record(it) }
+	}
+
+	/**
+	 * Returns the tests that were recorded for the given `@ParameterizedClass` during discovery, falling back to its
+	 * current children if nothing was recorded for it.
+	 */
+	fun testsOf(classTemplate: TestDescriptor): Collection<TestDescriptor> =
+		testsByClassTemplate[classTemplate.uniqueId] ?: classTemplate.children
+}

impacted-test-engine/src/main/kotlin/com/teamscale/test_impacted/test_descriptor/JUnitClassBasedTestDescriptorResolverBase.kt

@@ -12,9 +12,13 @@ abstract class JUnitClassBasedTestDescriptorResolverBase : ITestDescriptorResolv
 	override fun getUniformPath(descriptor: TestDescriptor): String? =
 		descriptor.getClassName()?.let { className ->
 			val dotName = className.replace(".", "/")
-			"$dotName/${descriptor.legacyReportingName.trim { it <= ' ' }}"
+			"$dotName/${getTestName(descriptor)}"
 		}
 
+	/** Returns the name of the test within its class.  */
+	protected open fun getTestName(descriptor: TestDescriptor): String =
+		descriptor.legacyReportingName.trim { it <= ' ' }
+
 	override fun getClusterId(descriptor: TestDescriptor): String? {
 		val classSegmentName = descriptor.getClassName()
 

impacted-test-engine/src/main/kotlin/com/teamscale/test_impacted/test_descriptor/JUnitJupiterTestDescriptorResolver.kt

@@ -1,15 +1,28 @@
 package com.teamscale.test_impacted.test_descriptor
 
-import com.teamscale.test_impacted.test_descriptor.TestDescriptorUtils.getUniqueIdSegment
+import com.teamscale.test_impacted.test_descriptor.TestDescriptorUtils.isInsideClassTemplate
 import org.junit.platform.engine.TestDescriptor
 
 /** Test default test descriptor resolver for the JUnit jupiter [TestEngine].  */
 class JUnitJupiterTestDescriptorResolver : JUnitClassBasedTestDescriptorResolverBase() {
+	override fun getTestName(descriptor: TestDescriptor): String {
+		val reportingName = super.getTestName(descriptor)
+		if (!descriptor.isInsideClassTemplate()) {
+			return reportingName
+		}
+		// Within a @ParameterizedClass the jupiter engine appends the index of the enclosing invocation(s) to the
+		// reporting name, e.g. "testOne()[1]". All invocations run the same test, so the index must not become part
+		// of the uniform path: whatever the test covered in any of them belongs to that one test.
+		return reportingName.replace(INVOCATION_INDEXES, "")
+	}
+
 	override fun TestDescriptor.getClassName(): String? {
-		val classSegment = getUniqueIdSegment(CLASS_SEGMENT_TYPE).orElse(null) ?: return null
+		val classSegment = uniqueId.segments
+			.firstOrNull { it.type == CLASS_SEGMENT_TYPE || it.type == CLASS_TEMPLATE_SEGMENT_TYPE }
+			?.value ?: return null
 
 		val nestedClassNames = uniqueId.segments
-			.filter { it.type == NESTED_CLASS_SEGMENT_TYPE }
+			.filter { it.type == NESTED_CLASS_SEGMENT_TYPE || it.type == NESTED_CLASS_TEMPLATE_SEGMENT_TYPE }
 			.joinToString("") { "\$${it.value}" }
 
 		return classSegment + nestedClassNames
@@ -19,12 +32,27 @@ class JUnitJupiterTestDescriptorResolver : JUnitClassBasedTestDescriptorResolver
 		get() = "junit-jupiter"
 
 	companion object {
+		/** The invocation indexes that the jupiter engine appends to reporting names within a @ParameterizedClass.  */
+		private val INVOCATION_INDEXES = Regex("(\\[\\d+])+$")
+
 		/** The segment type name that the jupiter engine uses for the class descriptor nodes.  */
 		const val CLASS_SEGMENT_TYPE = "class"
 
 		/** The segment type name that the jupiter engine uses for @Nested inner class descriptor nodes.  */
 		const val NESTED_CLASS_SEGMENT_TYPE = "nested-class"
 
+		/** The segment type name that the jupiter engine uses for top-level @ParameterizedClass descriptor nodes.  */
+		const val CLASS_TEMPLATE_SEGMENT_TYPE = "class-template"
+
+		/** The segment type name that the jupiter engine uses for @Nested @ParameterizedClass descriptor nodes.  */
+		const val NESTED_CLASS_TEMPLATE_SEGMENT_TYPE = "nested-class-template"
+
+		/**
+		 * The segment type name that the jupiter engine uses for the individual invocations of a @ParameterizedClass.
+		 * These are only registered dynamically during test execution.
+		 */
+		const val CLASS_TEMPLATE_INVOCATION_SEGMENT_TYPE = "class-template-invocation"
+
 		/** The segment type name that the jupiter engine uses for the method descriptor nodes.  */
 		const val METHOD_SEGMENT_TYPE = "method"