Add unit tests for the Firestore data layer - #3942
Draft
shobhitagarwal1612 wants to merge 3 commits into
Draft
shobhitagarwal1612 wants to merge 3 commits into
shobhitagarwal1612 wants to merge 3 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.
Adds unit tests for the firebase data layer, which the narrowed JaCoCo excludes made visible. Recovers 91 of the 222 uncovered lines in the package (58.9% -> 83.6%), taking :app from 77.44% to 78.26%. The three addMutationToBatch implementations are the substantive ones: each rejects unknown mutation types, a branch nothing exercised before. The rest pin down subcollection wiring and the CancellationException handlers that let an aborted sync return empty rather than fail. Adds a shared canceledTask() helper to FirestoreTestUtil for driving those cancellation paths. FirestoreDataStore is only partly covered: its remaining lines chain several Firestore layers deep or call the static Firebase.messaging, so only the applyMutations user-mismatch precondition is tested.
shobhitagarwal1612
requested a deployment
to
e2e-approval
September 15, 2026 16:16 — with
GitHub Actions
Waiting
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3942 +/- ##
============================================
- Coverage 69.99% 69.88% -0.11%
- Complexity 2033 2094 +61
============================================
Files 424 441 +17
Lines 11647 11862 +215
Branches 1522 1543 +21
============================================
+ Hits 8152 8290 +138
- Misses 2723 2794 +71
- Partials 772 778 +6 🚀 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
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.
Adds unit tests for the firebase data layer, which the narrowed JaCoCo excludes in the
preceding PR made visible. No production code changes.
addMutationToBatchon the LOI and submission document references. CREATE/UPDATEmerge, DELETE deletes, and an unknown type throws — the last branch was never
exercised.
JobCollectionReference,SurveyDocumentReferenceandTermsOfServiceDocumentReferenceeach swallowCancellationExceptionso an abortedsync returns empty rather than failing. Now pinned, via a shared
canceledTask()helper in
FirestoreTestUtil.GroundFirestore,SurveyDocumentReferenceandSubmissionCollectionReference.FirebaseStorageManagerpath building and download URL resolution, plusFirestoreUuidGeneratorandCaptureLocationResultConverterround-tripping.11 test files, following the Robolectric + mocked-Firestore pattern already used by
SurveysCollectionReferenceTest.Coverage
data/remote/firebase:app+91 lines. Recovers most of the 1.02pp the excludes change costs.
Not covered
FirestoreDataStorestill has 63 uncovered lines. Its methods chainprovider → FirebaseFirestore → GroundFirestore → survey → lois, andsubscribeToSurveyUpdatescalls the staticFirebase.messaging; mocking that depth assertslittle beyond "the mocks were called". Only
applyMutations' user-mismatch precondition istested, since that one guards a real correctness property.
Verification
@andreia-ferreira PTAL?