Skip to content

Commit

Permalink
Fix jacoco classloader issues when collecting coverage
Browse files Browse the repository at this point in the history
In some cases, when the jacoco runtime is provided via TestKit's
withPluginClasspath, the jacoco agent class ends up in a separate
classloader from the coverage plugin.

We ensure that the coverage plugin is in the same classloader as the
jacoco runtime by passing the coverage plugin jar into the TestKit's
plugin classpath instead of letting the TestKit build resolve it
normally. As a minor side effect, the coverage plugin version no longer
needs to be explicitly specified, and the coverage plugin no longer
needs to be published to the gradle plugin portal.
  • Loading branch information
ogolberg authored Apr 13, 2024
1 parent 5cb3be9 commit e760c6b
Show file tree
Hide file tree
Showing 14 changed files with 114 additions and 71 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ In the test project's `build.gradle.kts`, make sure to apply the coverage plugin

```
plugins {
id("com.toasttab.testkit.coverage") version <<version>>
id("com.toasttab.testkit.coverage")
}
```

Expand Down
21 changes: 0 additions & 21 deletions coverage-plugin/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import com.toasttab.gradle.testkit.shared.configureInstrumentation
import org.gradle.internal.component.external.model.ModuleComponentArtifactIdentifier
import org.gradle.jvm.tasks.Jar

plugins {
`kotlin-conventions`
`plugin-publishing-conventions`
jacoco
}

Expand All @@ -17,19 +12,3 @@ dependencies {
testImplementation(libs.strikt.core)
testImplementation(libs.jacoco.core)
}

configureInstrumentation()

gradlePlugin {
plugins {
create("jacoco") {
id = "com.toasttab.testkit.coverage"
implementationClass = "com.toasttab.gradle.testkit.FlushJacocoPlugin"
description = ProjectInfo.description
displayName = ProjectInfo.name
tags = listOf("jacoco", "testkit")
}
}
}


Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
implementation-class=com.toasttab.gradle.testkit.FlushJacocoPlugin
1 change: 1 addition & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ strikt-core = { module = "io.strikt:strikt-core", version.ref = "strikt" }

[plugins]
gradle-publish = { id = "com.gradle.plugin-publish", version = "1.2.0" }
build-config = { id = "com.github.gmazzo.buildconfig", version = "5.3.5" }
18 changes: 18 additions & 0 deletions integration-tests/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import com.toasttab.gradle.testkit.shared.configureInstrumentation

plugins {
`kotlin-conventions`
jacoco
}

configureInstrumentation(projects.coveragePlugin)

dependencies {
implementation(gradleApi())
testImplementation(libs.junit)
testImplementation(libs.strikt.core)
testImplementation(projects.jacocoReflect)
testImplementation(projects.junit5)
testImplementation(gradleTestKit())
testImplementation(libs.jacoco.core)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright (c) 2024 Toast Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.toasttab.gradle.testkit

import org.gradle.api.Plugin
import org.gradle.api.Project

class TestPlugin : Plugin<Project> {
override fun apply(project: Project) {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
implementation-class=com.toasttab.gradle.testkit.TestPlugin
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

package com.toasttab.gradle.testkit

import com.toasttab.gradle.testkit.jacoco.JacocoRt
import org.gradle.testkit.runner.GradleRunner
import org.jacoco.core.data.ExecutionDataReader
import org.junit.jupiter.api.Test
Expand Down Expand Up @@ -43,6 +42,7 @@ class FlushJacocoPluginIntegrationTest {
plugins {
java
id("com.toasttab.testkit.coverage")
id("com.toasttab.testkit.test")
}
""".trimIndent()
)
Expand All @@ -68,6 +68,6 @@ class FlushJacocoPluginIntegrationTest {
}.read()
}

expectThat(classes).contains(JacocoRt::class.java.name.replace('.', '/'))
expectThat(classes).contains(TestPlugin::class.java.name.replace('.', '/'))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ class TestProjectExtension : ParameterResolver, BeforeAllCallback, AfterTestExec
val classpath = Path(instrumentedProperty).listDirectoryEntries().toMutableList()

System.getProperty("testkit-plugin-external-jars").split(File.pathSeparatorChar).mapTo(classpath, ::Path)
System.getProperty("testkit-coverage-jars").split(File.pathSeparatorChar).mapTo(classpath, ::Path)
classpath.add(Path(System.getProperty("testkit-plugin-jacoco-jar")))

PluginClasspath.Custom(classpath)
Expand Down
2 changes: 1 addition & 1 deletion settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ rootProject.name = "testkit-plugins"
apply(plugin = "net.vivin.gradle-semantic-build-versioning")

include(
":jacoco-reflect", ":junit5", ":testkit-plugin", ":coverage-plugin"
":jacoco-reflect", ":junit5", ":testkit-plugin", ":coverage-plugin", ":integration-tests"
)
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.gradle.api.plugins.JavaPlugin
import org.gradle.api.tasks.PathSensitivity
import org.gradle.api.tasks.testing.Test
import org.gradle.jvm.tasks.Jar
import org.gradle.kotlin.dsl.dependencies
import org.gradle.kotlin.dsl.named
import org.gradle.kotlin.dsl.register
import org.gradle.testing.jacoco.plugins.JacocoPlugin
Expand All @@ -36,43 +37,52 @@ private fun Project.jacocoAgentRuntime() = zipTree(configurations.getAt(JacocoPl
it.name == "jacocoagent.jar"
}.singleFile

fun Project.configureInstrumentation() {
pluginManager.withPlugin("jacoco") {
tasks.register<CopyLocalJarsTask>(COPY_LOCAL_JARS_TASK) {
artifacts = runtimeArtifacts()
fun Project.configureInstrumentation(coverageArtifact: Any) {
tasks.register<CopyLocalJarsTask>(COPY_LOCAL_JARS_TASK) {
artifacts = runtimeArtifacts()

jar = tasks.named<Jar>(JavaPlugin.JAR_TASK_NAME)
jar = tasks.named<Jar>(JavaPlugin.JAR_TASK_NAME)

dir = localJarsDir()
}
dir = localJarsDir()
}

tasks.register<InstrumentWithJacocoOfflineTask>(INSTRUMENT_LOCAL_JARS_TASK) {
dependsOn(COPY_LOCAL_JARS_TASK)

classpath = configurations.getAt(JacocoPlugin.ANT_CONFIGURATION_NAME)

tasks.register<InstrumentWithJacocoOfflineTask>(INSTRUMENT_LOCAL_JARS_TASK) {
dependsOn(COPY_LOCAL_JARS_TASK)
jars = localJarsDir()
dir = instrumentedDir()
}

val coverage = configurations.create("_coverage_plugin_")
dependencies {
add(coverage.name, coverageArtifact)
}

classpath = configurations.getAt(JacocoPlugin.ANT_CONFIGURATION_NAME)
tasks.named<Test>(JavaPlugin.TEST_TASK_NAME) {
dependsOn(INSTRUMENT_LOCAL_JARS_TASK)

jars = localJarsDir()
dir = instrumentedDir()
}
val runtimeArtifacts = runtimeArtifacts()

tasks.named<Test>(JavaPlugin.TEST_TASK_NAME) {
dependsOn(INSTRUMENT_LOCAL_JARS_TASK)
inputs.files(runtimeArtifacts.artifactFiles).withPropertyName("plugin-artifacts").withPathSensitivity(
PathSensitivity.RELATIVE
)

val runtimeArtifacts = runtimeArtifacts()
inputs.files(coverage).withPropertyName("coverage-artifacts").withPathSensitivity(
PathSensitivity.RELATIVE
)

inputs.files(runtimeArtifacts.artifactFiles).withPropertyName("plugin-artifacts").withPathSensitivity(
PathSensitivity.RELATIVE
)
inputs.dir(instrumentedDir()).withPropertyName("instrumented-artifacts")
.withPathSensitivity(PathSensitivity.RELATIVE)
inputs.dir(instrumentedDir()).withPropertyName("instrumented-artifacts")
.withPathSensitivity(PathSensitivity.RELATIVE)

systemProperty("testkit-plugin-instrumented-jars", instrumentedDir().get().asFile.path)
systemProperty("testkit-plugin-external-jars",
runtimeArtifacts.filter(ArtifactResult::isExternalPluginDependency)
.joinToString(separator = File.pathSeparator) {
it.file.path
})
systemProperty("testkit-plugin-jacoco-jar", jacocoAgentRuntime().path)
}
systemProperty("testkit-plugin-instrumented-jars", instrumentedDir().get().asFile.path)
systemProperty("testkit-plugin-external-jars",
runtimeArtifacts.filter(ArtifactResult::isExternalPluginDependency)
.joinToString(separator = File.pathSeparator) {
it.file.path
})
systemProperty("testkit-coverage-jars", coverage.asPath)
systemProperty("testkit-plugin-jacoco-jar", jacocoAgentRuntime().path)
}
}
6 changes: 6 additions & 0 deletions testkit-plugin/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ plugins {
`kotlin-conventions`
`kotlin-dsl`
`plugin-publishing-conventions`
alias(libs.plugins.build.config)
}

dependencies {
Expand All @@ -17,6 +18,11 @@ sourceSets {
}
}

buildConfig {
packageName.set("com.toasttab.gradle.testkit")
buildConfigField("String", "VERSION", "\"$version\"")
}

tasks {
test {
useJUnitPlatform()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,13 @@ class TestkitPlugin @Inject constructor(
}
}

project.configureInstrumentation()

project.tasks.named<Test>("test") {
dependsOn("copyTestProjects")

doFirst(JacocoOutputCleanupTestTaskAction(fs, destfile))

inputs.dir(testProjectDir).withPropertyName("testkit-projects-input").withPathSensitivity(PathSensitivity.RELATIVE)
inputs.dir(testProjectDir).withPropertyName("testkit-projects-input")
.withPathSensitivity(PathSensitivity.RELATIVE)

// declare an additional jacoco output file so that the JUnit JVM and the TestKit JVM
// do not try to write to the same file
Expand All @@ -65,21 +64,25 @@ class TestkitPlugin @Inject constructor(
systemProperty("testkit-projects", "${testProjectDir.get()}")
}

project.pluginManager.withPlugin("jvm-test-suite") {
// add the TestKit jacoco file to outgoing artifacts so that it can be aggregated
project.configurations.getAt("coverageDataElementsForTest").outgoing.artifact(destfile) {
type = ArtifactTypeDefinition.BINARY_DATA_TYPE
builtBy("test")
project.pluginManager.withPlugin("jacoco") {
project.pluginManager.withPlugin("jvm-test-suite") {
// add the TestKit jacoco file to outgoing artifacts so that it can be aggregated
project.configurations.getAt("coverageDataElementsForTest").outgoing.artifact(destfile) {
type = ArtifactTypeDefinition.BINARY_DATA_TYPE
builtBy("test")
}
}
}

project.tasks.named<JacocoReportBase>("jacocoTestReport") {
// add the TestKit jacoco file to the local jacoco report
executionData.from(
project.layout.buildDirectory.dir("jacoco").map {
it.files("test.exec", "testkit.exec")
}
)
project.configureInstrumentation("com.toasttab.gradle.testkit:coverage-plugin:${BuildConfig.VERSION}")

project.tasks.named<JacocoReportBase>("jacocoTestReport") {
// add the TestKit jacoco file to the local jacoco report
executionData.from(
project.layout.buildDirectory.dir("jacoco").map {
it.files("test.exec", "testkit.exec")
}
)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
plugins {
java
jacoco
id("com.toasttab.testkit")
}

Expand Down

0 comments on commit e760c6b

Please sign in to comment.