build cache support for reportScoverage tasks #74
Comments
|
Please could you try again using version 2.2.0 of this plugin? I hope I've managed to configure it correctly. Thanks, |
|
Looks like the up-to-date check works: Caching doesn't: Also upgrading from 2.1.0 to 2.2.0 breaks the gradle ScalaStyle plugin: My repositories {
// https://github.com/ngbinh/gradle-scalastyle-plugin/issues/39
maven {
name = "ngbinh"
url = uri("https://dl.bintray.com/ngbinh/maven")
}
// https://plugins.gradle.org/plugin/org.scoverage
maven {
name = "gradle-plugins"
url = uri("https://plugins.gradle.org/m2/")
}
}
dependencies {
"implementation"("org.github.ngbinh.scalastyle:gradle-scalastyle-plugin_2.12:1.0.1")
"implementation"("gradle.plugin.org.scoverage:gradle-scoverage:2.2.0")
}And my root val scoverageVersion = "1.3.1"
val scalaRelease = "2.12.6"
val scalaLibrary = "org.scala-lang:scala-library:$scalaRelease"
val scalaCompiler = "org.scala-lang:scala-compiler:$scalaRelease"
val scalaReflect = "org.scala-lang:scala-reflect:$scalaRelease"
// Don't manipulate versions for gradle internals: https://github.com/typesafehub/zinc/issues/87
val blacklistConfigs = setOf("zinc", "checkstyle", "scoverage", "scalaStyle")
subprojects {
apply<org.gradle.api.plugins.scala.ScalaPlugin>()
apply<org.github.ngbinh.scalastyle.ScalaStylePlugin>()
apply<org.scoverage.ScoveragePlugin>()
configurations.all {
if (!blacklistConfigs.contains(name)) {
resolutionStrategy {
preferProjectModules()
// Enforce consistent third-party package versions across all projects in this repository.
// This should avoid issues where a common module is built against one version of a third-party library,
// then deployed as part of a service to run against a different version.
force(scalaLibrary)
force(scalaCompiler)
force(scalaReflect)
}
}
}
dependencies {
"compile"(scalaLibrary)
"compile"(scalaCompiler)
"compile"(scalaReflect)
"scoverage"("org.scoverage:scalac-scoverage-plugin_2.12:$scoverageVersion")
"scoverage"("org.scoverage:scalac-scoverage-runtime_2.12:$scoverageVersion")
}
}
Running gradle a second time yields the same error as ngbinh/gradle-scalastyle-plugin#38 |
Sorry, I didn't realise they were different things. I think it's a case of just adding an annotation here, so I'll see about getting that done.
Interesting, given the other conversation about this plugin trying not to modify the Gradle classpath. I expect that you should not be adding this build plugin to the implementation configuration? i.e. remove this line |
|
Thanks for looking into this. The line Here's a minimal repro: https://github.com/dsilvasc/repro-gradle-scoverage-74 I think the issue is caused by the gradle-scoverage dependency on Line 47 in 12b196a If I exclude that, then the problem goes away. |
|
Ah, that's odd. The dependency in my build.gradle is essentially an The pom published to maven central doesn't have any dependency: http://search.maven.org/remotecontent?filepath=org/scoverage/gradle-scoverage/2.2.0/gradle-scoverage-2.2.0.pom The pom published to the plugins repo looks entiely different: I'll see if I can find out why tomorrow, but for the moment please use maven central? Thanks, |
|
Thanks, that works too :) |
|
2.3.0 published to the plugin portal with cachable tasks and without dependencies - please would you be able to have a look? Thanks, |
|
@maiflai Sorry for the delay here -- I've upgraded to It just didn't work for testScoverage: |
|
Hi, Compilation writes the scoverage.coverage.xml; it contains the statement identifiers. Test writes a set of measurement files which reference the statement identifiers. It looks like Gradle is complaining that the test task claims to own the data directory, but the scoverage.coverage.xml file was created by something else. I'll see if I can work around it. |
|
Please could you re-test this with the new 2.4.0 release? Thanks, |
|
Please can I close this issue? |
|
Hi Stu, Sorry for missing the comment about 2.4.0. I just tried it and this is what I get: |
|
Note that Gradle is removing the restriction on how many output files can be listed for a cacheable task in version 5.0: The log I included in the previous comment was from Gradle 4.10. |
|
Here's the relevant Gradle test case for 4.10: The test case is gone in 5.0: And here's the commit changing the behavior in Gradle 5: |
|
Not sure if the behavior on Gradle 4.10 is related to this in ScoverageExtension: t.tasks[ScoveragePlugin.TEST_NAME].outputs.upToDateWhen { extension.dataDir.listFiles(measurementFile) }Note also that
|
|
Ah, ok. I might need to take another look at specifying wildcards to match the measurement files; my last attempt with a .* broke on Windows. The task outputs can only be determined after the task has run (and this gets very messy when you try to aggregate coverage across modules). |
|
Regarding the |
|
@wolfs I'm coming late into this, but I believe you're right. I don't see much point in configuring the In general, I don't think there's any need for a cache configuration specific for this plugin. The instrumentation is made via the scala compiler, and it has its own "cache mechanism" (aka incremental compilation). Other than the compiler and the tests, there's only the aggregation and check tasks, but they are incredibly fast in comparison to the other tasks, so I don't see much point in caching them. |
|
I think this was introduced in response to #29 (comment), but it was some time ago. I seem to recall that we have an directory for outputs that is shared between two tasks:
Gradle didn't handle this very well, but things may have changed since then.
I'm not sure I agree here. Personally I like unit tests to skip execution when no inputs have changed. This said, I do often mark tests that execute against external systems to run whenever they are included in the graph. |
I was under the impression that whenever I Still, I'm not sure the additional |
I often run the root
Perhaps - but I think this is a worst-case scenario where a
It always used the be the case that the |
Right, me too.
I don't know about the This is not such a rare case to have tests that do not depend on instrumented classes. One more example would be java modules in a hybrid multi-module project which are covered by a Java coverage tool (like cobertura). In fact, I'm not even sure there's a use-case where the builtin dependency mechanism misses an opportunity to not re-run tests while the reliance on the measurement files does. So in that sense, it seems redundant to have the additional
You mean the instrumented classes or the measurements collected by the tests? That would be weird if it deleted the measurements, but in such a case I believe gradle will decide to re-run the tests anyhow since a new compilation has been made (otherwise how did the compiler plugin delete the files?). |
|
https://github.com/scoverage/scalac-scoverage-plugin/blob/master/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala#L119 shows that the plugin wipes the measurement directory when it recompiles. Looking back I think the problem was that we originally declared the measument directory to be an output of the instrumentation task. This measurement directory then changed when the tests were run, which triggered a recompilation, and so on. The current code declares that only the measurement index is an output of the instrumentation task while measurement files are outputs of the test task. I'm not sure how the build cache can work properly if we do not accurately state the outputs of the tasks? I think the next version of the scalac plugin uses these measurement files for reporting, rather than the statically named XML files. |
Indeed, it seems to delete the measurement files regardless if the source code changed or not. The plugin would normally execute only when the source did change, but one might also explicitly run it via Interestingly enough, trying to execute only the scoverage compilation results in a build failure: This is a bug that may be unrelated to this issue, though. I would argue that the scalac plugin should not delete the measurement files at all, even if the source code did change. It's a confusing side-effect, as noted in scoverage/scalac-scoverage-plugin#67. In comparison, normal Java / Scala compilation never deletes test results. The test tasks should perhaps declare the index file as an additional input, but I believe that would only serve as a sort of a "good practice" rather than actually making any difference. If I'm not mistaken, the scalac plugin either changes both the index file and the instrumented classes (at least one), or doesn't change any of them; i.e, there's no case when one changes and the other doesn't. Given that the classes are already declared as input for the test tasks, declaring the index as an input as well will affect nothing. |
|
Any update, this causes extremely long build times. |
The
reportScoveragetask runs on every./gradlew checkinvocation, even when running it multiple times in sequence without any changes to the source.Would it be possible to make it work with up-to-date checks and the gradle build cache?
The text was updated successfully, but these errors were encountered: