Harden JaCoCo configuration and limit coverage excludes to generated code - #3941
Open
shobhitagarwal1612 wants to merge 2 commits into
Open
shobhitagarwal1612 wants to merge 2 commits into
shobhitagarwal1612 wants to merge 2 commits into
Conversation
`JacocoReport` skips rather than fails when its execution data is missing, so a wrong or unwritten `.exec` path publishes an empty report instead of breaking the build. Add a `verifyJacocoExecutionData` task that fails when no execution data was produced. It has to be a separate task because a check inside the report task would never run once that task is skipped. Resolve the execution data through a `fileTree` over the known `.exec` locations rather than a single hardcoded path. Where AGP writes unit test coverage depends on whether the `jacoco` plugin is applied before or after AGP: applying it first makes AGP redirect output to `outputs/unit_test_code_coverage` instead of the Gradle JaCoCo plugin's `build/jacoco` default. A file tree only matches files that exist, so it picks up whichever path is actually written and keeps working if that ordering changes. Rename `jacocoLocalDebugUnitTestReport` to `jacocoTestReport`. The task is registered for KMP and Android KMP modules too, where it runs `jvmTest` or `testAndroidHostTest` and has nothing to do with a local debug variant. Move the JaCoCo version into the version catalog alongside the other pinned tool versions.
The exclude list had grown to cover hand-written classes alongside generated ones, which hid real gaps in the coverage report. Measuring a report built with excludes disabled showed six patterns matching only hand-written Kotlin: `migration/*`, `firebase/base/*`, `firebase/schema/*Reference*`, `FirebaseStorageManager*`, `FirestoreDataStore*` and `LocationSharedFlowCallback*`. Drop them so the 240 lines they hid, 61 of which are already covered, are measured like any other source. Replace `**/*Module*` with `**/di/**`. The old pattern matched 159 classes on substring alone and would have hidden any hand-written class merely named "...Module..." anywhere in the tree. Every one of its non-generated matches lives under `di/`, and the generated `..._HiltModule` classes outside it stay excluded via `**/*Hilt*`. `**/di/**` also picks up `di/coroutines`, which the old single-segment matching missed. Cover the generated code that was leaking into the report in the other direction. Mapping every source file under `app/build/generated` against the report found Room's auto-migration implementations being measured, so broaden `**/LocalDatabase_Impl*` to `**/LocalDatabase_*`. Add `**/*_MembersInjector*`, `**/*_GeneratedInjector*`, `**/*_AssistedFactory*` and `**/*_ComponentTreeDeps*` for the remaining Dagger artifacts; those carry no lines, but leaving them in contradicts what this list is for. All ten generated source roots are now fully excluded. Reported coverage for `:app` moves from 78.46% to 77.44%, reflecting previously hidden code rather than any change in what the tests exercise.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3941 +/- ##
============================================
- Coverage 69.99% 69.17% -0.82%
- Complexity 2033 2047 +14
============================================
Files 424 441 +17
Lines 11647 11862 +215
Branches 1522 1543 +21
============================================
+ Hits 8152 8206 +54
- Misses 2723 2879 +156
- Partials 772 777 +5 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent changes to the JaCoCo setup, one per commit.
JacocoReportskips rather than failswhen execution data is missing, publishing an empty report on a green build. Adds a
verifyJacocoExecutionDatatask, separate because a check inside the report taskwould never run once it's skipped.
.execfrom either known path. AGP redirects coverage tooutputs/unit_test_code_coveragewhenjacocois applied before it, otherwise theGradle plugin's
build/jacocodefault wins. AfileTreematches whichever exists.jacocoLocalDebugUnitTestReport→jacocoTestReport. It's registeredfor KMP modules too, where it runs
jvmTest. CI updated.(
migration/*, thefirebase/*ones,LocationSharedFlowCallback*) and werehiding real gaps — dropped.
**/*Module*matched 159 classes on substring alone andwould hide any hand-written
...Module...; replaced with**/di/**. Completed thegenerated side, which was leaking Room's auto-migrations.
Excludes were decided by measuring a report built with excludes disabled, and by mapping
all 424 generated sources under
app/build/generatedagainst the report — not by name.Codecov will show a drop — expected
:app78.46% → 77.44%. No test or production code changed; 215 previously hiddenlines are now measured (+55 covered).
Newly visible:
FirestoreDataStore(0/82),FirebaseStorageManager(7/17),firebase/schema/*Reference(50/110).Verification
@gino-m @andreia-ferreira PTAL?