After updating to Gradle 8.7 Jacoco agent no longe...
# community-support
t
After updating to Gradle 8.7 Jacoco agent no longer instruments the plugin
src/main
source sets and
jacocoTestReport
reports 0% coverage. Any ideas why this happens and how to address? I am relying on jacoco-gradle-testkit-plugin which follows the same strategy suggested by @Vampire on Jacoco & Gradle Test Kit with Java thread. During the test execution the file called
gradle.properties
is created under the
GradleRunner.projectDir
.
Copy code
#Generated by pl.droidsonroids.jacoco.testkit
org.gradle.jvmargs="-javaagent\:/Users/user/.gradle/caches/modules-2/files-2.1/org.jacoco/org.jacoco.agent/0.8.11/d5287cced3d0afd0cfa0be7f84773ea811836f21/org.jacoco.agent-0.8.11-runtime.jar\=destfile\=/Users/user/work/myproject/build-logic/android/build/jacoco/test.exec"
I was trying to figure out what might have caused regression between 8.6 and 8.7. So far I suspect [#27328] - Use artifact transforms to instrument TestKit injected classpath. My
build.gradle.kts
Copy code
plugins {
    `kotlin-dsl`
    id("java-gradle-plugin")
    id("groovy-gradle-plugin")
    jacoco
    id("pl.droidsonroids.jacoco.testkit")
}

tasks.test {
    finalizedBy(tasks.jacocoTestReport)
}

tasks.jacocoTestReport {
    reports {
        xml.required.set(true)
        csv.required.set(false)
    }
    dependsOn(tasks.test)
}

tasks.jacocoTestCoverageVerification {
    violationRules {
        rule {
            limit {
                minimum = "0.49".toBigDecimal()
            }
        }
    }
    dependsOn(tasks.jacocoTestReport)
}
Util method to inject
gradle.properties
Copy code
import org.gradle.testkit.runner.GradleRunner
import java.io.File
import java.io.InputStream

object GradleRunnerExt {
    fun GradleRunner.withJaCoCo(): GradleRunner {
        // creates gradle.properties in the project under test with org.gradle.jvmargs="-javaagent:..." 
        javaClass.classLoader.getResourceAsStream("testkit-gradle.properties")
            ?.toFile(File(projectDir, "gradle.properties"))
        return this
    }

    private fun InputStream.toFile(file: File) {
        use { input ->
            file.outputStream().use { input.copyTo(it) }
        }
    }
}
Then in the tests I do call
Copy code
GradleRunner.create().withPluginClasspath()
            .withProjectDir(root)
            .withJaCoCo()
            .withArguments(":app:mergeDebugResources")
            .build()
cc @Anze Sodja
There is ongoing discussion about supporting Jacoc in Teskit. Just referencing it here for the additional context https://github.com/gradle/gradle/issues/1465
a
Not sure if that worked before intentionally or just by a coincidence relying on Gradle internals: we definitely don't have any coverage for that use case. Note that in the linked thread it's mentioned that
withDebug(true)
is required to force Gradle in in-process mode. Is that something you are missing?
👀 1
t
Without the
"pl.droidsonroids.jacoco.testkit"
plugin +
withDebug(true)
Jacoco throws following warning.
Copy code
> Task :android:jacocoTestReport
[ant:jacocoReport] Classes in bundle 'android' do not match with execution data. For report generation the same class files must be used as at runtime.
Gradle 8.6, without
pl.droidsonroids.jacoco.testkit
,
withDebug(false)
• jacoco/test.exec does not include information from the
src/main
Gradle 8.6, with
pl.droidsonroids.jacoco.testkit
,
withDebug(false)
• jacoco/test.exec does include information from the
src/main
Gradle 8.7, with
pl.droidsonroids.jacoco.testkit
,
withDebug(false)
• jacoco/test.exec does not include information from the
src/main
Gradle 8.6 and Gradle 8.7, without
pl.droidsonroids.jacoco.testkit
,
withDebug(true)
• jacoco/test.exec does include information from the
src/main
• throws ‘For report generation the same class files must be used as at runtime.’ Judging by this observations smth happened to the list of classes used to be instrumented during the Gradle Test Kit execution. On Gradle 8.7 the classes from
src/main
are not included into the the
.exec
file and leads to the empty coverage reports.
v
I am relying on jacoco-gradle-testkit-plugin which follows the same strategy suggested by @Vampire on Jacoco & Gradle Test Kit with Java thread.
It's similar, not the same. I started trying to use that plugin, found it to only be working sometimes and only on a very narrow happy-path. Thus I made it properly in my plugin build. Afair it is still like written in that thread.
But I don't know whether it works the same with 8.7 as that specific build is on 7.6.4. But I'd expect it to still work as intended.
t
@Vampire is it similar to what Denes Daniel was doing? • https://github.com/morganstanley/gradle-plugin-application/blob/v2.0.1/build.gradle.kts#L238-L325 3 • https://github.com/morganstanley/gradle-plugin-application/blob/v2.0.1/src/test/java/com/ms/gradle/application/ApplicationPluginFunctionalTest.java#L144-L154 5 I would love to see some sample that implements your solution, if any 🙂 Some parts of the thread does not make too much sense to me. For example, what is
JacocoDumper
? Should I add this to the project that I prepare during the test and pass to
GradleTestkit.withProjectDir(root)
? Or to my source project?
v
Unfortunately the project is closed source, so no, no public example. The dumper makes sure the JaCoCo data is dumped in time before the test is seen as finished and so before the report task tries to read it. Otherwise data may be missing or the file locked as it is then written when the daemon in which the test was executed is closed. I'm not going to analyse what Denes Daniel is doing there, sorry, no time for that. But from a very quick look it also might be similar, but is also not safe.
❤️ 1
The
JacocoDumper
is in the testee's settings script, as described in that thread
And then in the settings script of the projects under test I always add
I have a base class for all the functional tests where such common infrastructure things are done
t
Ok, I will try to replicate it.
v
Feel free to ask here or there if you have problems with it 🙂
t
I will create a sample project and share the link. If you will be kind to review the replication attempt I would appreciate it 🙇
v
If it's pretty minimal and I'll find the time, sure 🙂
❤️ 1
t
@Vampire I’ve created a repo and tried to replicate the instructions discussed https://github.com/tomkoptel/jacoco-gradle-testkit The
develop
uses Gradle 8.6 and does produce 100% coverage. Switching to https://github.com/tomkoptel/jacoco-gradle-testkit/tree/gradle-8.7 and executing
./gradlew :jacocoTestReport
gives 0% coverage. Maybe, I’ve missed smth from your setup. Can you take a look, please?
v
As I said, I did not test with 8.7. So if it works with 8.6 but not with 8.7, I guess I expected wrongly. 😞
t
Does the setup looks right to you 🙂 ? Maybe, I’ve missed smth 🤷
v
You have
finalizedBy(tasks.jacocoTestReport)
twice, and needlessly redo some conventional defaults like configuring the execution data of the jacoco report task. But that's of course off-topic to the question. • you miss to do escape backslashes in the two paths, that breaks it on windows • you add a backslash in front of the two paths, which is just useless as on Linux it will then be
\/
in the properties file and on Windows
\<drive letter>
which both just resolves to the character after the backslash as neither is a special escape sequence • you enclose the jvmargs entry in the properties file with double quotes which my instructions don't, but I'm not sure whether it makes a difference (this cannot replace the backslash escapes iirc if that was the intention) Other than that it looks correct I'd say, so it should probably work on *nix as intended
Note that in the linked thread it's mentioned that
withDebug(true)
is required to force Gradle in in-process mode. Is that something you are missing?
@Anze Sodja With my tactic both should work, but neither does, with the difference that with debug the report complains about different class files while without debug there is no complaint but just 0 coverage.
I also think this is related to the instrumentation changes. And I guess probably with those changes also the work-around that @adam mentioned in this comment got lost: https://github.com/gradle/gradle/issues/13614#issuecomment-658046647
Also interesting. If I disable the instrumentation agent using
org.gradle.internal.instrumentation.agent=false
then it complains about the class mismatch both with and without debug
a
I think I know what is the issue. When there is an agent, we should disable instrumentation or fail, see this comment: AbstractInjectedClasspathInstrumentationStrategy.kt. But with changes we don't do that. Could you open an issue?
v
That's most probably the part the comment I linked to referred to I guess
a
Looks like, thanks for the research. 🙂
👌 1
v
I'm not so sure whether disabling instrumentation really is relevant with the changed instrumentation logic. As I said, if I run with debug and thus in-process, I get "class files do not match" and 0% coverage. If I run without debug and with instrumentation agent, I do not get "class files do not match" and 0% coverage. If I run without debug and without instrumentation agent, I again get "class files do not match" and 0% coverage. If I change the logic to do offline instrumentation, it works fine with and without debug and I get 100% coverage. You just cannot mix testkit tests and non-testkit tests in the same test task, because you need to disable the on-the-fly instrumentation of the JaCoCo agent in the test task itself so that when using
withDebug
you do not get warnings about trying to instrument already instrumented classes.
This is NOT thoroughly tested. Just the happy path with and without debug is known working, so there can well be problems or things that should be made cleaner. But here the patch that makes the MCVE work on the 8.7 branch:
Copy code
diff --git a/build.gradle.kts b/build.gradle.kts
index 8fb0479..ad39200 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -1,8 +1,10 @@
+import org.gradle.plugin.devel.tasks.PluginUnderTestMetadata.IMPLEMENTATION_CLASSPATH_PROP_KEY
+import org.gradle.plugin.devel.tasks.PluginUnderTestMetadata.METADATA_FILE_NAME
+import org.jetbrains.kotlin.konan.file.use
 import java.io.Serializable
 import java.nio.channels.FileChannel
-import java.nio.file.OpenOption
 import java.nio.file.StandardOpenOption
-import java.nio.file.attribute.FileAttribute
+import java.util.Properties

 plugins {
     `kotlin-dsl`
@@ -23,7 +25,6 @@ dependencies {
 }

 tasks.test {
-    finalizedBy(tasks.jacocoTestReport)
     finalizedBy(tasks.jacocoTestReport)
     val testRuns = layout.buildDirectory.dir("testRuns")
     systemProperty("testEnv.workDir", LazyString(testRuns.map { it.asFile.apply { mkdirs() }.absolutePath }))
@@ -43,8 +44,40 @@ tasks.test {
     }
 }

-tasks.jacocoTestReport {
-    executionData(tasks.test.map { test -> test.the<JacocoTaskExtension>().destinationFile })
+jacoco {
+    toolVersion = "0.8.12"
+}
+
+tasks.test {
+    the<JacocoTaskExtension>().excludes = listOf("*")
+}
+
+val jacocoAnt by configurations.existing
+tasks.pluginUnderTestMetadata {
+    actions.clear()
+    doLast {
+        val instrumentedPluginClasspath = temporaryDir.resolve("instrumentedPluginClasspath")
+        instrumentedPluginClasspath.deleteRecursively()
+        ant.withGroovyBuilder {
+            "taskdef"("name" to "instrument",
+                "classname" to "org.jacoco.ant.InstrumentTask",
+                "classpath" to jacocoAnt.get().asPath)
+            "instrument"("destdir" to instrumentedPluginClasspath) {
+                pluginClasspath.asFileTree.addToAntBuilder(ant, "resources")
+            }
+        }
+
+        val properties = Properties();
+        if (!pluginClasspath.isEmpty) {
+            properties.setProperty(
+                IMPLEMENTATION_CLASSPATH_PROP_KEY,
+                instrumentedPluginClasspath.absoluteFile.invariantSeparatorsPath
+            )
+        }
+        outputDirectory.file(METADATA_FILE_NAME).get().asFile.outputStream().use {
+            properties.store(it, null)
+        }
+    }
 }

 tasks.jacocoTestReport {
diff --git a/src/test/kotlin/my/sample/MySamplePluginTest.kt b/src/test/kotlin/my/sample/MySamplePluginTest.kt
index 5e01907..c012ef5 100644
--- a/src/test/kotlin/my/sample/MySamplePluginTest.kt
+++ b/src/test/kotlin/my/sample/MySamplePluginTest.kt
@@ -16,7 +16,7 @@ class MySamplePluginTest {
     }

     private val jacocoAgentJar: String get() = System.getProperty("jacocoAgentJar")!!
-    private val jacocoDestfile: String get() = System.getProperty("jacocoDestfile")!!
+    private val jacocoDestfile: String get() = System.getProperty("jacocoDestfile")!!.replace("""\""", """\\""")

     @Test
     fun `task is executed`() {
@@ -50,11 +50,17 @@ class MySamplePluginTest {
         )
         testProjectDir.newFile("gradle.properties").writeText(
             """
-                org.gradle.jvmargs="-javaagent:\$jacocoAgentJar=destfile=\$jacocoDestfile,append=true,dumponexit=false,jmx=true"
+                systemProp.jacoco-agent.destfile=$jacocoDestfile
+                systemProp.jacoco-agent.append=true
+                systemProp.jacoco-agent.dumponexit=false
+                systemProp.jacoco-agent.jmx=true
             """.trimIndent()
         )
         GradleRunner.create()
             .withPluginClasspath()
+            .run {
+                withPluginClasspath(pluginClasspath + File(jacocoAgentJar))
+            }
             .withProjectDir(testProjectDir.root)
             .withArguments("mySampleTask")
             .build()
With 8.8 one could probably use
pluginClasspath.replace...
instead of replacing the whole task actions.
Thanks for that nice riddle 🙂
t
This is awesome! Thank you @Vampire 🙇 I’ve pushed to the patch with Gradle 8.7 update to the Github repo update and co-referenced on the Gradle portal.
👌 1
@Anze Sodja we have multiple issues logged on the Github side (https://github.com/gradle/gradle/issues?q=is%3Aissue+is%3Aopen+jacoco+testkit) about test kit and Jacoco integration. Where do you think the discussion above should be shared to?
v
If you share a link, make sure to use a Linen link as that is not bound to the 90 day limit
👍 1
a
> Where do you think the discussion above should be shared to? Could you maybe open an issue that behaviour changed with 8.7 and link a reproducer? In the end I will probably look at it, since I last touched that code. But it's nice that it goes through triage process and it gets correctly prioritized. Otherwise I might open an issue myself later this week. For other issues, I can say something internally. Although this area is not a focus, so probably there won't be any better solution right away.
👍 1
t
@Vampire I know you did very basic happy path. I’ve tried to apply the fix discussed in the private kotlin repo. The jacoco fails to instrument code that uses inlined functions. I’ve added the dummy code that causes the issue https://github.com/tomkoptel/jacoco-gradle-testkit/commit/1e17fe9e6bc3377dcc7c5c3e6cb896466bb4e812
Copy code
Error while instrumenting /jacoco-gradle-testkit/build/classes/kotlin/main/my/sample/MySamplePlugin$apply$1$2$invoke$inlined$the$1.class
Is there any ideas how to properly configura Jacoco in case of Kotlin inlined classes?
v
Besides that you should never use
tasks.withType<X> { ... }
which breaks task-configuration avoidance for all tasks of type
X
but always
tasks.withType<X>().configureEach { ... }
, what is the actual error that happened?
Maybe downgrading to Gradle 3.0 helps. 😄 The cause you get is a file not found exception as it searches for
...invoke$inlined...
while the file is called
...invoke$$inlined...
. This was once fixed for https://issues.gradle.org/browse/GRADLE-3511 with commit b6503cb2462ab84ce65ae51761eec49efd3160ae for Gradle 3.0, but either the fix was lost, or it was not fixed for all cases.
So you should create an issue also for that.
🙇 1
I have no idea whether for that a work-around can be found
Ah, you can replicate what
addToAntBuilder
is doing with only using public API and work-arounding the Gradle bug:
Copy code
"instrument"("destdir" to instrumentedPluginClasspath) {
    pluginClasspath.asFileTree.visit {
        "gradleFileResource"(
            "file" to file.absolutePath.replace("$", "$$"),
            "name" to relativePath.pathString.replace("$", "$$")
        )
    }
}
t
yep, it works 😄 I will update the repo.
👌 1
I’ve also had to update the way classpath string generated for
PluginUnderTestMetadata.METADATA_FILE_NAME
https://github.com/tomkoptel/jacoco-gradle-testkit/commit/dccec7b010d1880718ab50c8f1fc4170bb9d881b Without it the project under test was not able to apply plugins used in the main project. On the private repo we use Android Gradle Plugin and it was not applied to the project under test. Not sure, if what I did is a correct way, but it works.
v
I highly doubt it. By putting the original classpath in front of the instrumented one, you most probably do not get the instrumented classes used? I guess the jars land in the
instrumentedClasses
directory and thus are not used or something like that.
t
You are right. I had to swap the order https://github.com/tomkoptel/jacoco-gradle-testkit/commit/9bd6d03c6d3c0a1aa527581d3e52bce8378a8461 I’ve double checked and the coverage numbers back to normal.
v
If you don't need coverage numbers from the jars, then yes.
inputs.property("jacocoAntPath", objects.fileCollection().from(jacocoAnt))
also most probably is not what you want
Most probably more something like
Copy code
diff --git a/build.gradle.kts b/build.gradle.kts
index 709ac89..638be72 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -53,19 +53,17 @@ tasks.test {

 val jacocoAnt by configurations.existing
 tasks.pluginUnderTestMetadata {
-    inputs.property("jacocoAntPath", objects.fileCollection().from(jacocoAnt))
+    inputs.files(jacocoAnt).withPropertyName("jacocoAntPath").withNormalizer(ClasspathNormalizer::class.java)
+    val jacocoAntPath = jacocoAnt.get().asPath
     actions.clear()
     doLast {
-        val collection = inputs.properties["jacocoAntPath"] as FileCollection
-        val classpath = collection.asPath
-
         val instrumentedPluginClasspath = temporaryDir.resolve("instrumentedPluginClasspath")
         instrumentedPluginClasspath.deleteRecursively()
         ant.withGroovyBuilder {
             "taskdef"(
                 "name" to "instrument",
                 "classname" to "org.jacoco.ant.InstrumentTask",
-                "classpath" to classpath
+                "classpath" to jacocoAntPath
             )
             "instrument"("destdir" to instrumentedPluginClasspath) {
                 pluginClasspath.asFileTree.visit {
@@ -119,4 +117,3 @@ class LazyString(private val source: Lazy<String>) : Serializable {

     override fun toString() = source.value
 }
-
t
The reason I was adding
jacocoAntPath
property, so that we can add configuration cache compatible input. I found that it is not possible get
jacocoAntPath
back with
doLast { val collection = inputs.properties["jacocoAntPath"] as FileCollection }
as mentioned in Add ability to get named Task file inputs and outputs #21456. So what I did is following. Which seems to work, but again I have no idea if that is a correct way to consume
jacocoAntPath
(
inputs.properties["jacocoAntPath"]
just returns
null
).
Copy code
inputs.files(jacocoAnt).withPropertyName("jacocoAntPath").withNormalizer(ClasspathNormalizer::class.java)
    actions.clear()
    doLast {
        val classpath = inputs.files.asPath
https://github.com/tomkoptel/jacoco-gradle-testkit/commit/32966b6bc10039ec017f36d9dcf8f253d68a256d
v
Using the inputs to cc-safely transport information is a pretty bad abuse. The inputs define the inputs of your task, that influence the output produced. Storing a value there to later retrieve it by name is totally unnecessary and very strange additionally if you can just use a local variable. The intention was clear from the changes, it's just not a good idea. Defining them as input files with runtime classpath normalization is correct though, hence I "kept" that. But while I didn't test it, I think the patch I just gave you should still be CC safe.
I think this is the better approach and also more resembling the old behavior instead of appending the original classpath:
Copy code
diff --git a/build.gradle.kts b/build.gradle.kts
index 709ac89..22796da 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -18,6 +18,7 @@ jacocoAgentJar.isCanBeResolved = true

 dependencies {
     compileOnly(gradleApi())
+    implementation("commons-io:commons-io:+")
     testImplementation("junit:junit:4.13.2")
     testImplementation(gradleTestKit())
     jacocoAgentJar("org.jacoco:org.jacoco.agent:0.8.12:runtime")
@@ -53,19 +54,17 @@ tasks.test {

 val jacocoAnt by configurations.existing
 tasks.pluginUnderTestMetadata {
-    inputs.property("jacocoAntPath", objects.fileCollection().from(jacocoAnt))
+    inputs.files(jacocoAnt).withPropertyName("jacocoAntPath").withNormalizer(ClasspathNormalizer::class.java)
+    val jacocoAntPath = jacocoAnt.get().asPath
     actions.clear()
     doLast {
-        val collection = inputs.properties["jacocoAntPath"] as FileCollection
-        val classpath = collection.asPath
-
         val instrumentedPluginClasspath = temporaryDir.resolve("instrumentedPluginClasspath")
         instrumentedPluginClasspath.deleteRecursively()
         ant.withGroovyBuilder {
             "taskdef"(
                 "name" to "instrument",
                 "classname" to "org.jacoco.ant.InstrumentTask",
-                "classpath" to classpath
+                "classpath" to jacocoAntPath
             )
             "instrument"("destdir" to instrumentedPluginClasspath) {
                 pluginClasspath.asFileTree.visit {
@@ -79,12 +78,17 @@ tasks.pluginUnderTestMetadata {

         val properties = Properties()
         if (!pluginClasspath.isEmpty) {
-            val originalClasspath = pluginClasspath.joinToString(File.pathSeparator) {
-                it.absoluteFile.invariantSeparatorsPath
-            }
             properties.setProperty(
                 IMPLEMENTATION_CLASSPATH_PROP_KEY,
-                instrumentedPluginClasspath.absoluteFile.invariantSeparatorsPath + File.pathSeparator + originalClasspath
+                listOf(
+                    instrumentedPluginClasspath
+                        .absoluteFile
+                        .invariantSeparatorsPath,
+                    *instrumentedPluginClasspath
+                        .listFiles { dir, name -> name.endsWith(".jar") }!!
+                        .map { it.absoluteFile.invariantSeparatorsPath }
+                        .toTypedArray()
+                ).joinToString(File.pathSeparator)
             )
         }
         outputDirectory.file(PluginUnderTestMetadata.METADATA_FILE_NAME).get().asFile.outputStream().use {
diff --git a/src/main/kotlin/my/sample/MySampleTask.kt b/src/main/kotlin/my/sample/MySampleTask.kt
index 831a7a3..31be2e8 100644
--- a/src/main/kotlin/my/sample/MySampleTask.kt
+++ b/src/main/kotlin/my/sample/MySampleTask.kt
@@ -1,5 +1,6 @@
 package my.sample

+import org.apache.commons.io.IOUtil
 import org.gradle.api.DefaultTask
 import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.provider.Property
@@ -16,6 +17,7 @@ abstract class MySampleTask : DefaultTask() {

     @TaskAction
     fun run() {
+        println(IOUtil::class)
         output.get().asFile.writeText(input.get())
     }
 }
diff --git a/src/test/kotlin/my/sample/MySamplePluginTest.kt b/src/test/kotlin/my/sample/MySamplePluginTest.kt
index 5bc8fb8..d2b76da 100644
--- a/src/test/kotlin/my/sample/MySamplePluginTest.kt
+++ b/src/test/kotlin/my/sample/MySamplePluginTest.kt
@@ -62,7 +62,7 @@ class MySamplePluginTest {
                 withPluginClasspath(pluginClasspath + File(jacocoAgentJar))
             }
             .withProjectDir(testProjectDir.root)
-            .withArguments("mySampleTask")
+            .withArguments("mySampleTask", "-d")
             .build()
         val output = SimpleDateFormat("yyyy-MM-dd").format(Date())
         val outputContents = testProjectDir.root.resolve("build/mySampleTaskOutput.txt").readText()
t
@Anze Sodja I’ve created the issue https://github.com/gradle/gradle/issues/28734
a
Thanks! I've opened one before that, gradle/gradle#28729, but no worries 🙂. I'll close one as a duplicate. I also have a distribution that should fix it: https://services.gradle.org/distributions-snapshots/gradle-8.8-branch-asodja_fix_testkit_jacoco-20240408155641+0000-bin.zip Could you maybe give it a try?
👌 1
139 Views