Merge pull request #965 from cqse/ts/46357_no_container_found

TS-46357 Collect the tests of a `@ParameterizedClass`

Author: Stef2k16

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())