Merge pull request #243 from cqse/ts/34273_gradle_partition

TS-34273 Fix Gradle plugin partition evaluation

CHANGELOG.md

@@ -6,6 +6,9 @@ We use [semantic versioning](http://semver.org/):
 
 # Next Release
 
+- [fix] _teamscale-gradle-plugin_: Reports uploaded by `teamscaleReportUpload` ended up in wrong partition
+- [fix] _impacted-test-engine_: Failure when no tests were impacted
+
 # 30.0.1
 - [fix] _report-generator_: Fixed Gradle module metadata which resulted in `Could not find org.jacoco.agent-0.8.8-runtime`
 

common-system-test/src/main/java/com/teamscale/test/commons/ExternalReport.java

@@ -0,0 +1,20 @@
+package com.teamscale.test.commons;
+
+/** Holds a single report that was uploaded to our fake Teamscale server. */
+public class ExternalReport {
+	private final String reportString;
+	private final String partition;
+
+	public ExternalReport(String reportString, String partition) {
+		this.reportString = reportString;
+		this.partition = partition;
+	}
+
+	public String getReportString() {
+		return reportString;
+	}
+
+	public String getPartition() {
+		return partition;
+	}
+}

common-system-test/src/main/java/com/teamscale/test/commons/TeamscaleMockServer.java

@@ -44,7 +44,7 @@ public class TeamscaleMockServer {
 			.adapter(TestwiseCoverageReport.class);
 
 	/** All reports uploaded to this Teamscale instance. */
-	public final List<String> uploadedReports = new ArrayList<>();
+	public final List<ExternalReport> uploadedReports = new ArrayList<>();
 	/** All user agents that were present in the received requests. */
 	public final Set<String> collectedUserAgents = new HashSet<>();
 
@@ -77,7 +77,7 @@ public TeamscaleMockServer(int port, String... impactedTests) throws IOException
 	 * @throws IOException when parsing the report fails.
 	 */
 	public TestwiseCoverageReport parseUploadedTestwiseCoverageReport(int index) throws IOException {
-		return testwiseCoverageReportJsonAdapter.fromJson(uploadedReports.get(index));
+		return testwiseCoverageReportJsonAdapter.fromJson(uploadedReports.get(index).getReportString());
 	}
 
 	private String handleImpactedTests(Request request, Response response) throws IOException {
@@ -93,8 +93,9 @@ private String handleReport(Request request, Response response) throws IOExcepti
 		request.raw().setAttribute("org.eclipse.jetty.multipartConfig", multipartConfigElement);
 
 		Part file = request.raw().getPart("report");
+		String partition = request.queryParams("partition");
 		String reportString = IOUtils.toString(file.getInputStream());
-		uploadedReports.add(reportString);
+		uploadedReports.add(new ExternalReport(reportString, partition));
 		file.delete();
 
 		return "success";
@@ -107,5 +108,4 @@ public void shutdown() {
 		service.stop();
 		service.awaitStop();
 	}
-
 }

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

@@ -98,7 +98,7 @@ private void removeNonImpactedTests(TestDescriptor testDescriptor, Set<? super T
 		for (TestDescriptor descriptor : new ArrayList<>(testDescriptor.getChildren())) {
 			removeNonImpactedTests(descriptor, testDescriptors);
 		}
-		if (testDescriptor.getChildren().isEmpty()) {
+		if (testDescriptor.getChildren().isEmpty() && !testDescriptor.isRoot()) {
 			testDescriptor.removeFromHierarchy();
 		}
 	}

impacted-test-engine/src/test/java/com/teamscale/test_impacted/engine/NoImpactedTestsTest.java

@@ -0,0 +1,53 @@
+package com.teamscale.test_impacted.engine;
+
+import com.teamscale.client.PrioritizableTestCluster;
+import com.teamscale.test_impacted.engine.executor.DummyEngine;
+import com.teamscale.test_impacted.test_descriptor.JUnitJupiterTestDescriptorResolver;
+import org.junit.platform.engine.EngineExecutionListener;
+import org.junit.platform.engine.TestDescriptor;
+import org.junit.platform.engine.TestEngine;
+import org.junit.platform.engine.UniqueId;
+
+import java.util.Collections;
+import java.util.List;
+
+import static com.teamscale.test_impacted.engine.executor.SimpleTestDescriptor.testCase;
+import static com.teamscale.test_impacted.engine.executor.SimpleTestDescriptor.testContainer;
+import static java.util.Collections.singletonList;
+
+/** Test setup where no test is impacted. */
+class NoImpactedTestsTest extends ImpactedTestEngineTestBase {
+
+	private static final String FIRST_TEST_CLASS = "FirstTestClass";
+	private static final String NON_IMPACTED_TEST_CASE_1 = "nonImpactedTestCase1()";
+	/**
+	 * For this test setup we rely on the {@link JUnitJupiterTestDescriptorResolver} for resolving uniform paths and
+	 * cluster ids. Therefore, the engine root is set accordingly.
+	 */
+	private final UniqueId engine1RootId = UniqueId.forEngine("junit-jupiter");
+
+	/** FirstTestClass contains one non-impacted test. */
+	private final UniqueId firstTestClassId = engine1RootId.append(
+			JUnitJupiterTestDescriptorResolver.CLASS_SEGMENT_TYPE, FIRST_TEST_CLASS);
+	private final UniqueId nonImpactedTestCase1Id = firstTestClassId
+			.append(JUnitJupiterTestDescriptorResolver.METHOD_SEGMENT_TYPE, NON_IMPACTED_TEST_CASE_1);
+	private final TestDescriptor nonImpactedTestCase1 = testCase(nonImpactedTestCase1Id);
+	private final TestDescriptor firstTestClass = testContainer(firstTestClassId, nonImpactedTestCase1);
+
+	private final TestDescriptor testEngine1Root = testContainer(engine1RootId, firstTestClass);
+
+	@Override
+	public List<TestEngine> getEngines() {
+		return singletonList(new DummyEngine(testEngine1Root));
+	}
+
+	@Override
+	public List<PrioritizableTestCluster> getImpactedTests() {
+		return Collections.emptyList();
+	}
+
+	@Override
+	public void verifyCallbacks(EngineExecutionListener executionListener) {
+
+	}
+}

system-tests/default-excludes-test/src/test/java/com/teamscale/tia/client/DefaultExcludesSystemTest.java

@@ -28,7 +28,7 @@ public void systemTest() throws Exception {
 		SystemTestUtils.dumpCoverage(AGENT_PORT);
 
 		assertThat(teamscaleMockServer.uploadedReports).hasSize(1);
-		String report = teamscaleMockServer.uploadedReports.get(0);
+		String report = teamscaleMockServer.uploadedReports.get(0).getReportString();
 		assertThat(report).doesNotContain("shadow");
 		assertThat(report).doesNotContain("junit");
 		assertThat(report).doesNotContain("eclipse");

teamscale-gradle-plugin/build.gradle.kts

@@ -39,6 +39,7 @@ dependencies {
     implementation(libs.jgit)
     implementation(libs.retrofit.converter.moshi)
     testImplementation(libs.okio)
+    testImplementation(project(":common-system-test"))
 }
 
 tasks.processResources {

teamscale-gradle-plugin/src/main/kotlin/com/teamscale/Report.kt

@@ -31,13 +31,13 @@ data class Report(
 
     /** The partition to upload the report to. */
     @Input
-    var partition: String,
+    val partition: Property<String>,
 
     /** The commit message shown in Teamscale for the upload. */
     @Input
-    var message: String,
+    val message: Property<String>,
 
     /** Whether the report only contains partial data (subset of tests). Only relevant for TESTWISE_COVERAGE. */
     @Input
-    var partial: Boolean = false
+    val partial: Boolean = false
 )

teamscale-gradle-plugin/src/main/kotlin/com/teamscale/TeamscaleUploadTask.kt

@@ -81,7 +81,7 @@ abstract class TeamscaleUploadTask : DefaultTask() {
     private fun uploadReports(enabledReports: List<Report>) {
         // We want to upload e.g. all JUnit test reports that go to the same partition
         // as one commit, so we group them before uploading them
-        for ((key, reports) in enabledReports.groupBy { Triple(it.format, it.partition, it.message) }) {
+        for ((key, reports) in enabledReports.groupBy { Triple(it.format, it.partition.get(), it.message.get()) }) {
             val (format, partition, message) = key
             val reportFiles = reports.flatMap { it.reportFiles.files }.filter { it.exists() }.distinct()
             logger.info("Uploading ${reportFiles.size} ${format.name} report(s) to partition $partition...")

teamscale-gradle-plugin/src/main/kotlin/com/teamscale/TestImpacted.kt

@@ -189,7 +189,7 @@ open class TestImpacted @Inject constructor(objects: ObjectFactory) : Test() {
             writeEngineProperty("server.userName", serverConfiguration.userName!!)
             writeEngineProperty("server.userAccessToken", serverConfiguration.userAccessToken!!)
         }
-        writeEngineProperty("partition", report.partition)
+        writeEngineProperty("partition", report.partition.get())
         writeEngineProperty("endCommit", endCommit?.toString())
         writeEngineProperty("baseline", baseline?.toString())
         writeEngineProperty("reportDirectory", reportOutputDir.absolutePath)

teamscale-gradle-plugin/src/main/kotlin/com/teamscale/config/ReportConfigurationBase.kt

@@ -40,8 +40,8 @@ abstract class ReportConfigurationBase(private val format: EReportFormat, val pr
             upload = upload,
             format = format,
             reportFiles = getReportFiles(),
-            message = message.get(),
-            partition = partition.get()
+            message = message,
+            partition = partition
         )
     }
 

teamscale-gradle-plugin/src/test/kotlin/com/teamscale/TeamscalePluginTest.kt

@@ -4,15 +4,14 @@ import com.squareup.moshi.Moshi
 import com.teamscale.TestwiseCoverageReportAssert.Companion.assertThat
 import com.teamscale.report.testwise.model.ETestExecutionResult
 import com.teamscale.report.testwise.model.TestwiseCoverageReport
-import okio.BufferedSource
-import okio.buffer
-import okio.source
+import com.teamscale.test.commons.TeamscaleMockServer
 import org.assertj.core.api.Assertions.assertThat
 import org.gradle.testkit.runner.BuildResult
 import org.gradle.testkit.runner.GradleRunner
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.io.TempDir
 import java.io.File
 
 /**
@@ -27,6 +26,28 @@ class TeamscalePluginTest {
 
         /** Set this to true to enable debugging of the impacted tests engine via port 5005. */
         private const val DEBUG_TEST_ENGINE = false
+
+        /**
+         * This port must match what is configured in the test project's build.gradle.
+         */
+        private const val FAKE_TEAMSCALE_PORT = 64000
+
+    }
+
+    private lateinit var teamscaleMockServer: TeamscaleMockServer
+
+    @BeforeEach
+    fun startFakeTeamscaleServer() {
+        teamscaleMockServer = TeamscaleMockServer(
+            FAKE_TEAMSCALE_PORT,
+            "com/example/project/JUnit4Test/systemTest"
+        )
+        teamscaleMockServer.uploadedReports.clear()
+    }
+
+    @AfterEach
+    fun serverShutdown() {
+        teamscaleMockServer.shutdown()
     }
 
     /** The temp dir in which the simulated checkout and test execution will happen. */
@@ -56,39 +77,49 @@ class TeamscalePluginTest {
     }
 
     @Test
-    fun `impacted unit tests produce coverage`() {
+    fun `all unit tests produce coverage`() {
         val build = build(
             true, true,
+            "--continue",
             "clean",
             "unitTest",
             "--impacted",
-            "--run-all-tests"
+            "--run-all-tests",
+            "teamscaleReportUpload"
         )
         assertThat(build.output).contains("FAILURE (21 tests, 14 successes, 1 failures, 6 skipped)")
             .doesNotContain("you did not provide all relevant class files")
         val testwiseCoverageReportFile =
             File(temporaryFolder, "build/reports/testwise-coverage/unitTest/Unit-Tests.json")
         assertThat(testwiseCoverageReportFile).exists()
 
-        val source: BufferedSource = testwiseCoverageReportFile.source().buffer()
-        val testwiseCoverageReport =
-            Moshi.Builder().build().adapter(TestwiseCoverageReport::class.java).fromJson(source)
-        assertThat(testwiseCoverageReport!!)
-            .containsExecutionResult("com/example/project/IgnoredJUnit4Test/systemTest", ETestExecutionResult.SKIPPED)
-            .containsExecutionResult("com/example/project/JUnit4Test/systemTest", ETestExecutionResult.PASSED)
-            .containsExecutionResult(
-                "com/example/project/JUnit5Test/withValueSource(String)",
-                ETestExecutionResult.PASSED
-            )
-            .containsExecutionResult("com/example/project/FailingRepeatedTest/testRepeatedTest()", ETestExecutionResult.FAILURE)
-            .containsExecutionResult("FibonacciTest/test[4]", ETestExecutionResult.PASSED)
-            .containsCoverage(
-                "com/example/project/JUnit4Test/systemTest",
-                "com/example/project/Calculator.java",
-                "13,16,20-22"
-            )
-            // 19 Tests because JUnit 5 parameterized tests are grouped
-            .hasSize(19)
+        assertFullCoverage(testwiseCoverageReportFile.readText())
+
+        assertThat(teamscaleMockServer.uploadedReports).hasSize(1)
+        assertFullCoverage(teamscaleMockServer.uploadedReports[0].reportString)
+        assertThat(teamscaleMockServer.uploadedReports[0].partition).isEqualTo("Unit Tests")
+    }
+
+    @Test
+    fun `only impacted unit tests are executed`() {
+        val build = build(
+            true, false,
+            "--continue",
+            "clean",
+            "unitTest",
+            "--impacted",
+            "teamscaleReportUpload"
+        )
+        assertThat(build.output).contains("SUCCESS (1 tests, 1 successes, 0 failures, 0 skipped)")
+        val testwiseCoverageReportFile =
+            File(temporaryFolder, "build/reports/testwise-coverage/unitTest/Unit-Tests.json")
+        assertThat(testwiseCoverageReportFile).exists()
+
+        assertPartialCoverage(testwiseCoverageReportFile.readText())
+
+        assertThat(teamscaleMockServer.uploadedReports).hasSize(1)
+        assertPartialCoverage(teamscaleMockServer.uploadedReports[0].reportString)
+        assertThat(teamscaleMockServer.uploadedReports[0].partition).isEqualTo("Unit Tests")
     }
 
     @Test
@@ -105,25 +136,8 @@ class TeamscalePluginTest {
             File(temporaryFolder, "build/reports/testwise-coverage/unitTest/Unit-Tests.json")
         assertThat(testwiseCoverageReportFile).exists()
 
-        val source = testwiseCoverageReportFile.source().buffer()
-        val testwiseCoverageReport =
-            Moshi.Builder().build().adapter(TestwiseCoverageReport::class.java).fromJson(source)
-        assertThat(testwiseCoverageReport!!)
-            .containsExecutionResult("com/example/project/IgnoredJUnit4Test/systemTest", ETestExecutionResult.SKIPPED)
-            .containsExecutionResult("com/example/project/JUnit4Test/systemTest", ETestExecutionResult.PASSED)
-            .containsExecutionResult(
-                "com/example/project/JUnit5Test/withValueSource(String)",
-                ETestExecutionResult.PASSED
-            )
-            .containsExecutionResult("com/example/project/FailingRepeatedTest/testRepeatedTest()", ETestExecutionResult.FAILURE)
-            .containsExecutionResult("FibonacciTest/test[4]", ETestExecutionResult.PASSED)
-            .containsCoverage(
-                "com/example/project/JUnit4Test/systemTest",
-                "com/example/project/Calculator.java",
-                "13,16,20-22"
-            )
-            // 19 Tests because JUnit 5 parameterized tests are grouped
-            .hasSize(19)
+        val source = testwiseCoverageReportFile.readText()
+        assertFullCoverage(source)
     }
 
     private fun build(executesTask: Boolean, expectFailure: Boolean, vararg arguments: String): BuildResult {
@@ -163,5 +177,45 @@ class TeamscalePluginTest {
 
         return buildResult
     }
+
+    private fun assertFullCoverage(source: String) {
+        val testwiseCoverageReport =
+            Moshi.Builder().build().adapter(TestwiseCoverageReport::class.java).fromJson(source)
+        assertThat(testwiseCoverageReport!!)
+            .hasPartial(false)
+            .containsExecutionResult("com/example/project/IgnoredJUnit4Test/systemTest", ETestExecutionResult.SKIPPED)
+            .containsExecutionResult("com/example/project/JUnit4Test/systemTest", ETestExecutionResult.PASSED)
+            .containsExecutionResult(
+                "com/example/project/JUnit5Test/withValueSource(String)",
+                ETestExecutionResult.PASSED
+            )
+            .containsExecutionResult(
+                "com/example/project/FailingRepeatedTest/testRepeatedTest()",
+                ETestExecutionResult.FAILURE
+            )
+            .containsExecutionResult("FibonacciTest/test[4]", ETestExecutionResult.PASSED)
+            .containsCoverage(
+                "com/example/project/JUnit4Test/systemTest",
+                "com/example/project/Calculator.java",
+                "13,16,20-22"
+            )
+            // 19 Tests because JUnit 5 parameterized tests are grouped
+            .hasSize(19)
+    }
+
+    private fun assertPartialCoverage(source: String) {
+        val testwiseCoverageReport =
+            Moshi.Builder().build().adapter(TestwiseCoverageReport::class.java).fromJson(source)
+        assertThat(testwiseCoverageReport!!)
+            .hasPartial(true)
+            .containsExecutionResult("com/example/project/JUnit4Test/systemTest", ETestExecutionResult.PASSED)
+            .containsCoverage(
+                "com/example/project/JUnit4Test/systemTest",
+                "com/example/project/Calculator.java",
+                "13,16,20-22"
+            )
+            .hasSize(19)
+            .hasTestsWithCoverage(1)
+    }
 }