diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 743ac38e0..6774edb30 100644 --- a/.github/workflows/actions.yml +++ b/.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' diff --git a/buildSrc/src/main/kotlin/com.teamscale.spawned-jvm-coverage.gradle.kts b/buildSrc/src/main/kotlin/com.teamscale.spawned-jvm-coverage.gradle.kts new file mode 100644 index 000000000..b74e42606 --- /dev/null +++ b/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() +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.JAVA_RUNTIME)) + attribute(Category.CATEGORY_ATTRIBUTE, objects.named(Category.LIBRARY)) + attribute(LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE, objects.named(LibraryElements.JAR)) + attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling.EXTERNAL)) + attribute( + TargetJvmEnvironment.TARGET_JVM_ENVIRONMENT_ATTRIBUTE, + objects.named(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.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.VERIFICATION)) + attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling.EXTERNAL)) + attribute( + VerificationType.VERIFICATION_TYPE_ATTRIBUTE, + objects.named(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, Serializable { + override fun execute(t: Task) { + file.delete() + } +} diff --git a/common-system-test/src/main/kotlin/com/teamscale/test/commons/ProcessUtils.kt b/common-system-test/src/main/kotlin/com/teamscale/test/commons/ProcessUtils.kt index 649be4514..e1e299e85 100644 --- a/common-system-test/src/main/kotlin/com/teamscale/test/commons/ProcessUtils.kt +++ b/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.withCoverageArgumentIfRequested(): List { + 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) { + 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.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) { + 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. */ diff --git a/system-tests/cucumber-maven-tia/build.gradle.kts b/system-tests/cucumber-maven-tia/build.gradle.kts index 36400e900..be75224bf 100644 --- a/system-tests/cucumber-maven-tia/build.gradle.kts +++ b/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 { diff --git a/system-tests/gradle-cucumber/build.gradle.kts b/system-tests/gradle-cucumber/build.gradle.kts index 4edcc7677..3f11094ec 100644 --- a/system-tests/gradle-cucumber/build.gradle.kts +++ b/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 { diff --git a/system-tests/gradle-multi-module/build.gradle.kts b/system-tests/gradle-multi-module/build.gradle.kts index ac31019c7..9223c3634 100644 --- a/system-tests/gradle-multi-module/build.gradle.kts +++ b/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 { diff --git a/system-tests/maven-external-upload-test/build.gradle.kts b/system-tests/maven-external-upload-test/build.gradle.kts index 508a88b91..1b16d0889 100644 --- a/system-tests/maven-external-upload-test/build.gradle.kts +++ b/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 { diff --git a/system-tests/tia-maven/build.gradle.kts b/system-tests/tia-maven/build.gradle.kts index 95042d297..be75224bf 100644 --- a/system-tests/tia-maven/build.gradle.kts +++ b/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") } - diff --git a/teamscale-gradle-plugin/build.gradle.kts b/teamscale-gradle-plugin/build.gradle.kts index 285b8e9e6..00981093d 100644 --- a/teamscale-gradle-plugin/build.gradle.kts +++ b/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) } diff --git a/teamscale-gradle-plugin/src/test/kotlin/com/teamscale/TeamscalePluginTestBase.kt b/teamscale-gradle-plugin/src/test/kotlin/com/teamscale/TeamscalePluginTestBase.kt index 67f179d2c..41e9a7e12 100644 --- a/teamscale-gradle-plugin/src/test/kotlin/com/teamscale/TeamscalePluginTestBase.kt +++ b/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")