Rewire Jacoco for Gradle 8/9 #3052
Conversation
The jacoco plugin was applied on the wrong Project object which lead to multiple workarounds: * somehow `apply plugin:` binds to the script default project which is rootProject * so the plugin wasn't applied to currentProject * so it didn't register .jacoco on the Test tasks * so manual `applyTo()` was required * calling `applyTo test` resolved to `rootProject.jacoco.applyTo(currentProject.tasks.test)` (because the currentProject.jacoco wasn't registered!) * which lead to a cross-project configuration leak, and we had the warning: > Task :*:test Resolution of the configuration :jacocoAgent was attempted from a context different than the project context. Have a look at the documentation to understand why this is a problem and how it can be resolved. This behavior has been deprecated. This will fail with an error in Gradle 9.0. For more information, please refer to https://docs.gradle.org/8.2/userguide/viewing_debugging_dependencies.html#sub:resolving-unsafe-configuration-resolution-errors in the Gradle documentation.
Because of the manual applyTo() call the project object was always rootProject which set the wrong destinationFile and tasks overwrote each others' outputs. The workaround to manually split these files is no longer necessary after the previous commit.
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #3052 +/- ##
============================================
- Coverage 85.69% 85.45% -0.25%
- Complexity 2874 2888 +14
============================================
Files 326 329 +3
Lines 8727 8801 +74
Branches 1077 1093 +16
============================================
+ Hits 7479 7521 +42
- Misses 971 992 +21
- Partials 277 288 +11 ☔ View full report in Codecov by Sentry. |
Wow. It was long time ago. In those times, I was much more up-to-date with the Gradle development and recommendations for plugin makers :-D. |
|
Seems like the Android coverage is now working correctly? We do decrease the coverage, but that's because we miss some stuff in Android. I think this PR is ready now, is it not? Thanks again for taking this on! |
|
Yeah, I think this is ready. It's a bit ugly and hacky with the coverage gathering from android modules and exclusions. And where I managed to generate coverage for a dependency of the androidTest module. The I like how the The coverage change is all expected: https://app.codecov.io/gh/mockito/mockito/pull/3052/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mockito It shows that android module is now covered and some of the core mockito classes are also covered more (probably because the coverage files are also aggregated from android module's unit tests). I wouldn't look at it as a decrease, but more as a "more realistic measurement" of the coverage 😅. If you're ok with the resulted code and tricks, happy to merge this now. |
|
Yeah very happy that we now have a proper view of test coverage of our Android code. It also shows we are missing a bit there. I was about to merge this, but then I spotted one last nit: the report includes |
|
Also, with regards to cleaning up the instrumentation and unit tests for Android, I would be happy to merge a PR for that if you fancy picking it up. |
Fixes the
:jacocoAgentwarning listed in #3051, see individual commits to make sense of the "full rewrite". cc @szpak for review because #1229 (comment).Checklist
including project members to get a better picture of the change
commit is meaningful and help the people that will explore a change in 2 years
Fixes #<issue number>in the description if relevantFixes #<issue number>if relevant