TS-47712 Record the coverage of the JVMs our tests spawn

.github/workflows/actions.yml

@@ -71,7 +71,11 @@ jobs:
           ORG_GRADLE_PROJECT_dockerHubUsername: ${{ secrets.DOCKERHUB_USER }}
           ORG_GRADLE_PROJECT_dockerHubPassword: ${{ secrets.DOCKERHUB_TOKEN }}
       - name: Upload coverage to Teamscale
-        if: always() && github.event_name == 'push'
+        # Fork pull requests get no secrets, so the upload can only run for pushes and for pull requests
+        # from branches in this repository. github.event.pull_request is null on push events.
+        if: >-
+          always() && (github.event_name == 'push'
+            || github.event.pull_request.head.repo.full_name == github.repository)
         uses: cqse/teamscale-upload-action@v2.10.0
         with:
           server: 'https://cqse.teamscale.io'
@@ -81,6 +85,11 @@ jobs:
           partition: 'Coverage'
           format: 'JACOCO'
           message: 'Linux Coverage'
+          # On pull_request, actions/checkout checks out the synthetic refs/pull/N/merge commit and
+          # GitHub sets GITHUB_SHA to it, which is what teamscale-upload's commit auto-detection picks
+          # up. That commit is not reachable from any branch, so Teamscale never fetches it and cannot
+          # resolve the revision. Name the pull request's head commit instead.
+          revision: ${{ github.event.pull_request.head.sha || github.sha }}
           files: '**/jacocoTestReport.xml'
 
   test-windows:
@@ -96,7 +105,11 @@ jobs:
       - name: Build with Gradle
         run: ./gradlew build --max-workers=2
       - name: Upload coverage to Teamscale
-        if: always() && github.event_name == 'push'
+        # Fork pull requests get no secrets, so the upload can only run for pushes and for pull requests
+        # from branches in this repository. github.event.pull_request is null on push events.
+        if: >-
+          always() && (github.event_name == 'push'
+            || github.event.pull_request.head.repo.full_name == github.repository)
         uses: cqse/teamscale-upload-action@v2.10.0
         with:
           server: 'https://cqse.teamscale.io'
@@ -106,4 +119,9 @@ jobs:
           partition: 'Coverage Windows'
           format: 'JACOCO'
           message: 'Coverage Windows'
+          # On pull_request, actions/checkout checks out the synthetic refs/pull/N/merge commit and
+          # GitHub sets GITHUB_SHA to it, which is what teamscale-upload's commit auto-detection picks
+          # up. That commit is not reachable from any branch, so Teamscale never fetches it and cannot
+          # resolve the revision. Name the pull request's head commit instead.
+          revision: ${{ github.event.pull_request.head.sha || github.sha }}
           files: '**/jacocoTestReport.xml'

buildSrc/src/main/kotlin/com.teamscale.spawned-jvm-coverage.gradle.kts

@@ -0,0 +1,118 @@
+import org.gradle.api.artifacts.component.ProjectComponentIdentifier
+import java.io.Serializable
+
+// Records the coverage of our own production classes inside the JVMs that this project's tests spawn.
+//
+// Those tests do their real work in the Maven and Gradle builds they start as child processes, so the JaCoCo
+// agent that com.teamscale.coverage attaches to the test JVM never sees the classes that matter: the Mojos of
+// the Maven plugin, the tasks of the Gradle plugin and the report generator they call into. This plugin
+// attaches a second, plain JaCoCo agent to those child processes and folds what it records into the project's
+// jacocoTestReport, which is the report that CI uploads.
+
+plugins {
+	id("com.teamscale.coverage")
+}
+
+// The version catalog accessors are not available in precompiled script plugins, cf. com.teamscale.java-convention.
+val catalogs = extensions.getByType<VersionCatalogsExtension>()
+val libs = catalogs.named("libs")
+
+/**
+ * The plain JaCoCo agent that instruments our classes in the spawned JVMs. Not the profiler, cf. the note above.
+ * Deliberately not named jacocoAgent: inside dependencies {} that name binds to the accessor of the JaCoCo
+ * plugin's own configuration of that name rather than to this variable, which would silently leave this one empty.
+ */
+val spawnedJvmJacocoAgent = configurations.dependencyScope("spawnedJvmJacocoAgent")
+val jacocoAgentJar = configurations.resolvable("spawnedJvmJacocoAgentPath") {
+	extendsFrom(spawnedJvmJacocoAgent.get())
+}
+
+/**
+ * The projects whose production classes run inside the spawned JVMs, declared by the consuming build script.
+ * Their class and source directories are resolved through this configuration instead of being read from the
+ * projects directly, which is what keeps the build compatible with project isolation.
+ */
+val spawnedJvmCode = configurations.dependencyScope("spawnedJvmCode")
+val spawnedJvmCodePath = configurations.resolvable("spawnedJvmCodePath") {
+	extendsFrom(spawnedJvmCode.get())
+	// Every project that runs in a spawned JVM is named explicitly, so that the report does not also list the
+	// classes that such a project merely depends on. Those would show up as entirely uncovered, because they
+	// either do not run at all or, like the profiler, only run under their relocated names.
+	isTransitive = false
+	// The attributes of a runtime classpath. Asking for Bundling.EXTERNAL is also what selects the plain
+	// variant of the projects that publish a shaded jar, whose classes we could not map back to the sources.
+	attributes {
+		attribute(Usage.USAGE_ATTRIBUTE, objects.named<Usage>(Usage.JAVA_RUNTIME))
+		attribute(Category.CATEGORY_ATTRIBUTE, objects.named<Category>(Category.LIBRARY))
+		attribute(LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE, objects.named<LibraryElements>(LibraryElements.JAR))
+		attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named<Bundling>(Bundling.EXTERNAL))
+		attribute(
+			TargetJvmEnvironment.TARGET_JVM_ENVIRONMENT_ATTRIBUTE,
+			objects.named<TargetJvmEnvironment>(TargetJvmEnvironment.STANDARD_JVM)
+		)
+	}
+}
+
+dependencies {
+	spawnedJvmJacocoAgent("org.jacoco:org.jacoco.agent:${libs.findVersion("jacoco").get().requiredVersion}:runtime")
+}
+
+/**
+ * The file that every JVM spawned by this project's tests appends its coverage to. One file for all of them is
+ * safe: JaCoCo takes an exclusive lock on the destination file before writing it and reads the resulting
+ * concatenation back as consecutive sessions.
+ */
+val spawnedJvmExecutionData = layout.buildDirectory.file("jacoco/spawnedJvms.exec")
+
+tasks.test {
+	// Resolved eagerly so that the option is a plain string, cf. multiple-agents-test, which attaches a
+	// foreign JaCoCo agent to its own test JVM the same way.
+	val agentJar = jacocoAgentJar.get().singleFile
+	val destination = spawnedJvmExecutionData.get().asFile
+	outputs.file(spawnedJvmExecutionData)
+	// The include pattern matches the VM names of the classes, so it deliberately does not match the
+	// shadow.com.teamscale.* classes of the profiler, which is attached to some of the same JVMs. Both
+	// agents therefore instrument a disjoint set of classes and cannot interfere with each other.
+	// The name of the property is repeated in ProcessUtils, which reads it.
+	systemProperty(
+		"systemTestCoverageAgent",
+		"-javaagent:$agentJar=destfile=$destination,append=true,output=file,dumponexit=true,jmx=false," +
+				"includes=com.teamscale.*"
+	)
+	doFirst("deleteSpawnedJvmCoverage", DeleteFile(destination))
+}
+
+tasks.jacocoTestReport {
+	executionData(spawnedJvmExecutionData)
+	// A system test has no production code of its own, so without these its report stays empty no matter how
+	// much coverage was recorded. Projects that do have production code, like the Gradle plugin, declare
+	// nothing here and keep the class directories that the jacoco plugin derives from their own source set.
+	classDirectories.from(spawnedJvmCodePath.get().incoming.artifactView {
+		componentFilter { it is ProjectComponentIdentifier }
+		attributes.attribute(
+			LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE, objects.named<LibraryElements>(LibraryElements.CLASSES)
+		)
+	}.files)
+	// Only needed for the HTML report; Teamscale maps the XML report onto the sources by package and file
+	// name. Resolved leniently so that a project without a sources variant cannot fail the build over it.
+	sourceDirectories.from(spawnedJvmCodePath.get().incoming.artifactView {
+		withVariantReselection()
+		lenient(true)
+		componentFilter { it is ProjectComponentIdentifier }
+		attributes {
+			attribute(Category.CATEGORY_ATTRIBUTE, objects.named<Category>(Category.VERIFICATION))
+			attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named<Bundling>(Bundling.EXTERNAL))
+			attribute(
+				VerificationType.VERIFICATION_TYPE_ATTRIBUTE,
+				objects.named<VerificationType>(VerificationType.MAIN_SOURCES)
+			)
+		}
+	}.files)
+}
+
+/** Deletes the given file before the task runs, so that a run never reports the coverage of the previous one. */
+class DeleteFile(private val file: File) : Action<Task>, Serializable {
+	override fun execute(t: Task) {
+		file.delete()
+	}
+}

common-system-test/src/main/kotlin/com/teamscale/test/commons/ProcessUtils.kt

@@ -101,6 +101,7 @@ object ProcessUtils {
 				val exitCode = executeWithoutConcurrencyLimit(
 					builder, input, -1, consoleCharset, stdoutConsumer, stderrConsumer
 				)
+				awaitCoverageOfSingleUseDaemon(commands)
 				return ProcessResult(
 					stdout = stdoutConsumer.content,
 					stderr = stderrConsumer.content,
@@ -117,11 +118,14 @@ object ProcessUtils {
 		 *
 		 * @return ProcessBuilder configured with commands and working directory
 		 */
-		fun build(): ProcessBuilder = ProcessBuilder(commands.withDebuggerArgumentIfRequested()).apply {
-			workingDirectory?.let { directory(it) }
-			environmentVariables?.let { environment().putAll(it) }
-			removeEnvironmentVariables.forEach { environment().remove(it) }
-		}
+		fun build(): ProcessBuilder =
+			ProcessBuilder(commands.withDebuggerArgumentIfRequested().withCoverageArgumentIfRequested()).apply {
+				workingDirectory?.let { directory(it) }
+				// Merged before the caller's variables, so that a test that sets MAVEN_OPTS itself still wins.
+				addCoverageEnvironment(commands)
+				environmentVariables?.let { environment().putAll(it) }
+				removeEnvironmentVariables.forEach { environment().remove(it) }
+			}
 	}
 
 	/**
@@ -148,6 +152,75 @@ object ProcessUtils {
 	private fun String.isJavaExecutable() =
 		substringAfterLast('/').substringAfterLast('\\').removeSuffix(".exe") == "java"
 
+	/**
+	 * The `-javaagent` option that records the coverage of our own classes in the JVMs a system test spawns, set
+	 * by the com.teamscale.spawned-jvm-coverage convention plugin. Absent unless that plugin is applied.
+	 *
+	 * Both `MAVEN_OPTS` and the Gradle command line split the option on whitespace, so a path containing a space
+	 * would produce a broken JVM command line. Recording coverage is a nice to have, the system tests are not,
+	 * so we rather record nothing in that case.
+	 */
+	private val COVERAGE_AGENT_ARGUMENT: String? =
+		System.getProperty("systemTestCoverageAgent")?.takeIf { it.isNotEmpty() && it.none(Char::isWhitespace) }
+
+	/**
+	 * Attaches [COVERAGE_AGENT_ARGUMENT] to a Gradle invocation, whose build logic runs in the daemon rather than
+	 * in the launcher that `GRADLE_OPTS` would reach. `--no-daemon` makes that daemon exit with the build, which
+	 * is when JaCoCo writes what it recorded.
+	 */
+	private fun List<String>.withCoverageArgumentIfRequested(): List<String> {
+		val agent = COVERAGE_AGENT_ARGUMENT?.takeIf { isLauncherFor("gradlew") } ?: return this
+		return this + listOf("-Dorg.gradle.jvmargs=$agent", "--no-daemon")
+	}
+
+	/**
+	 * Attaches [COVERAGE_AGENT_ARGUMENT] to a Maven invocation. The wrapper boots Maven in the JVM it starts, so
+	 * `MAVEN_OPTS` reaches exactly the JVM the Mojos run in and leaves the forked test JVMs alone. Those already
+	 * have the profiler attached, and a second agent in them would only duplicate what the profiler records.
+	 */
+	private fun ProcessBuilder.addCoverageEnvironment(commands: List<String>) {
+		val agent = COVERAGE_AGENT_ARGUMENT?.takeIf { commands.isLauncherFor("mvnw") } ?: return
+		environment().merge("MAVEN_OPTS", agent) { inherited, added -> "$inherited $added" }
+	}
+
+	/**
+	 * Whether this command line invokes the given wrapper script, with or without a path and the Windows file
+	 * extension. On Windows the script is invoked through `cmd /c`, so it is not necessarily the first argument.
+	 */
+	private fun List<String>.isLauncherFor(wrapperScript: String) = any {
+		it.substringAfterLast('/').substringAfterLast('\\')
+			.removeSuffix(".cmd").removeSuffix(".bat") == wrapperScript
+	}
+
+	/**
+	 * Waits until the single-use daemon of a Gradle build we recorded coverage of has written what it recorded.
+	 *
+	 * `--no-daemon` runs the build in a daemon that the launcher forks and that shuts itself down once the build
+	 * finished, so it outlives the process we just waited for. JaCoCo writes its data from a shutdown hook of
+	 * that daemon, which without this would race the jacocoTestReport task and cost us most of the coverage.
+	 * Maven needs none of this: it runs the build in the very JVM that we started.
+	 */
+	private fun awaitCoverageOfSingleUseDaemon(commands: List<String>) {
+		val destination = COVERAGE_AGENT_ARGUMENT
+			?.takeIf { commands.isLauncherFor("gradlew") }
+			?.substringAfter("destfile=")?.substringBefore(',')
+			?.let { File(it) } ?: return
+		var previousLength = -1L
+		repeat(COVERAGE_FLUSH_POLLS) {
+			val length = if (destination.isFile) destination.length() else 0
+			// Two polls in a row that saw the same non-empty file. JaCoCo holds an exclusive lock while it
+			// writes, so a length that stopped growing means the daemon is done rather than merely slow.
+			if (length > 0 && length == previousLength) return
+			previousLength = length
+			Thread.sleep(COVERAGE_FLUSH_POLL_INTERVAL_MILLIS)
+		}
+	}
+
+	private const val COVERAGE_FLUSH_POLL_INTERVAL_MILLIS = 100L
+
+	/** Polls for at most five seconds, which is far more than the daemon has ever needed to shut down. */
+	private const val COVERAGE_FLUSH_POLLS = 50
+
 	/**
 	 * Immutable result of process execution.
 	 */

system-tests/cucumber-maven-tia/build.gradle.kts

@@ -1,7 +1,13 @@
 plugins {
     com.teamscale.`kotlin-convention`
     com.teamscale.`system-test-convention`
-    com.teamscale.coverage
+    com.teamscale.`spawned-jvm-coverage`
+}
+
+dependencies {
+    spawnedJvmCode(project(":teamscale-maven-plugin"))
+    spawnedJvmCode(project(":report-generator"))
+    spawnedJvmCode(project(":teamscale-client"))
 }
 
 tasks.test {

system-tests/gradle-cucumber/build.gradle.kts

@@ -1,7 +1,13 @@
 plugins {
     com.teamscale.`kotlin-convention`
     com.teamscale.`system-test-convention`
-    com.teamscale.coverage
+    com.teamscale.`spawned-jvm-coverage`
+}
+
+dependencies {
+    spawnedJvmCode(project(":teamscale-gradle-plugin"))
+    spawnedJvmCode(project(":report-generator"))
+    spawnedJvmCode(project(":teamscale-client"))
 }
 
 tasks.test {

system-tests/gradle-multi-module/build.gradle.kts

@@ -1,7 +1,13 @@
 plugins {
 	com.teamscale.`kotlin-convention`
 	com.teamscale.`system-test-convention`
-	com.teamscale.coverage
+	com.teamscale.`spawned-jvm-coverage`
+}
+
+dependencies {
+	spawnedJvmCode(project(":teamscale-gradle-plugin"))
+	spawnedJvmCode(project(":report-generator"))
+	spawnedJvmCode(project(":teamscale-client"))
 }
 
 tasks.test {

system-tests/maven-external-upload-test/build.gradle.kts

@@ -1,6 +1,12 @@
 plugins {
 	com.teamscale.`kotlin-convention`
 	com.teamscale.`system-test-convention`
+	com.teamscale.`spawned-jvm-coverage`
+}
+
+dependencies {
+	spawnedJvmCode(project(":teamscale-maven-plugin"))
+	spawnedJvmCode(project(":teamscale-client"))
 }
 
 tasks.test {

system-tests/tia-maven/build.gradle.kts

@@ -1,11 +1,16 @@
 plugins {
     com.teamscale.`kotlin-convention`
     com.teamscale.`system-test-convention`
-    com.teamscale.coverage
+    com.teamscale.`spawned-jvm-coverage`
+}
+
+dependencies {
+    spawnedJvmCode(project(":teamscale-maven-plugin"))
+    spawnedJvmCode(project(":report-generator"))
+    spawnedJvmCode(project(":teamscale-client"))
 }
 
 tasks.test {
     // install dependencies needed by the Maven test project
     dependsOn(":publishToMavenLocal")
 }
-

teamscale-gradle-plugin/build.gradle.kts

@@ -6,7 +6,7 @@ plugins {
 	`java-gradle-plugin`
 	`kotlin-dsl`
 	com.teamscale.`kotlin-convention`
-	com.teamscale.coverage
+	com.teamscale.`spawned-jvm-coverage`
 	com.teamscale.publish
 	alias(libs.plugins.pluginPublish)
 }

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

@@ -17,6 +17,20 @@ import java.lang.management.ManagementFactory
  */
 abstract class TeamscalePluginTestBase {
 
+	companion object {
+		/**
+		 * The `-javaagent` option that records the coverage of this plugin, supplied by the
+		 * com.teamscale.spawned-jvm-coverage convention plugin. Absent unless that plugin is applied.
+		 *
+		 * TestKit runs every build below in a daemon of its own, so without this the plugin would be entirely
+		 * uncovered even though these tests exercise all of it. `GRADLE_OPTS` would only reach the launcher,
+		 * which is why the daemon is asked for the agent through `org.gradle.jvmargs`. The option is dropped
+		 * when it contains whitespace, which would break the command line, cf. ProcessUtils.
+		 */
+		private val coverageAgent: String? =
+			System.getProperty("systemTestCoverageAgent")?.takeIf { it.none(Char::isWhitespace) }
+	}
+
 	/** Teamscale mock server to be used during the tests. */
 	protected lateinit var teamscaleMockServer: TeamscaleMockServer
 
@@ -56,6 +70,7 @@ abstract class TeamscalePluginTestBase {
 		val runner = GradleRunner.create()
 		runner.forwardOutput()
 		runnerArgs.add("--stacktrace")
+		coverageAgent?.let { runnerArgs.add("-Dorg.gradle.jvmargs=$it") }
 
 		if (ManagementFactory.getRuntimeMXBean().inputArguments.toString()
 				.contains("-agentlib:jdwp")