Skip to content

Split CI tests & caching improvements#394

Open
ianrumac wants to merge 5 commits intodevelopfrom
ir/fix/split-tests-ci
Open

Split CI tests & caching improvements#394
ianrumac wants to merge 5 commits intodevelopfrom
ir/fix/split-tests-ci

Conversation

@ianrumac
Copy link
Copy Markdown
Collaborator

@ianrumac ianrumac commented Apr 10, 2026

Changes in this pull request

Checklist

  • All unit tests pass.
  • All UI tests pass.
  • Demo project builds and runs.
  • I added/updated tests or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have run ktlint in the main directory and fixed any issues.
  • I have updated the SDK documentation as well as the online docs.
  • I have reviewed the contributing guide

Greptile Summary

This PR refactors CI into parallel, dedicated jobs (build, unit-tests, 5-shard instrumentation-tests, coverage-report), introduces AVD snapshot caching to speed up emulator startup, and enables Gradle build caching, parallel execution, and configuration cache in gradle.properties. The jacoco-combined.gradle fix correctly adds explicit compile-task dependencies so the coverage-report job can run without re-executing unit tests.

Confidence Score: 5/5

Safe to merge; all findings are P2 style/improvement suggestions that don't block correctness.

The core logic is sound: jobs are correctly separated, artifacts are properly passed between jobs, AVD caching is correctly gated on cache-hit, and the jacoco dependency fix is correct. The findings (configuration cache compatibility, AVD cold-cache race, Firebase coverage not captured, coverage-report gate) are all P2 quality improvements rather than defects in the changed path.

gradle.properties (configuration cache) and .github/workflows/build+test.yml (Firebase + coverage-report gating)

Important Files Changed

Filename Overview
.github/workflows/build+test.yml Significant refactor: splits into build, unit-tests, instrumentation-tests (5-shard matrix), and coverage-report jobs; switches to ubuntu-latest, upgrades action versions, adds AVD caching, and retains Firebase Test Lab in the build job for app-module tests.
.github/workflows/pr-tests.yml New workflow for PRs to develop: mirrors build+test.yml structure with unit-tests, 5-shard instrumentation-tests, and coverage-report jobs; commits badge updates back to PR branch.
.github/workflows/on-release.yml Minor change to release notification workflow; dispatches to paywall-next repository on release creation.
gradle.properties Enables parallel builds, Gradle build caching, configuration cache, and caps workers at 4 to speed up CI; configuration cache compatibility with Groovy build scripts warrants attention.
gradle/jacoco-combined.gradle Adds compileDebugKotlin and compileDebugJavaWithJavac as explicit dependencies of jacocoTestReport so Gradle 8 strict task validation passes when the coverage-report CI job runs without testDebugUnitTest.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    PR[Pull Request] --> BT[build+test.yml\non PR to main]
    PR --> PT[pr-tests.yml\non PR to develop]

    BT --> B[build job\nassemble + Firebase Test Lab]
    BT --> UT1[unit-tests job\ntestDebugUnitTest]
    BT --> IT1[instrumentation-tests job\n5-shard matrix]

    IT1 -->|shard 0-4| S1[connectedDebugAndroidTest\nshardIndex=N/5]
    S1 --> CA1[Upload coverage artifacts\nper shard]

    UT1 --> UC1[Upload unit coverage\nbuild/jacoco/]

    UT1 & IT1 -->|needs| CR1[coverage-report job]
    CR1 --> DL1[Download all artifacts]
    DL1 --> GR1[jacocoTestReport\ncompileDebugSources only]
    GR1 --> UPL1[Upload jacoco report]
    GR1 --> BADGE1[Commit coverage badge]

    PT --> UT2[unit-tests job]
    PT --> IT2[instrumentation-tests job\n5-shard matrix]
    UT2 & IT2 -->|needs| CR2[coverage-report job]
    CR2 --> BADGE2[Commit coverage badge]

    style B fill:#ffe0b2
    style CR1 fill:#e8f5e9
    style CR2 fill:#e8f5e9
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: gradle.properties
Line: 29

Comment:
**Configuration cache compatibility with Groovy build scripts**

`org.gradle.configuration-cache=true` may not be fully compatible with `jacoco-combined.gradle`. That script uses `project.afterEvaluate {}`, accesses `project.buildDir` inside task configuration blocks, and registers tasks dynamically — all patterns that Gradle 8's configuration cache is strict about. If any task in the graph is incompatible the cache is silently invalidated (or Gradle throws a configuration-cache error, depending on the `--configuration-cache-problems` setting). Consider running `./gradlew :superwall:jacocoTestReport --configuration-cache` locally first to confirm there are no problems before this lands.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .github/workflows/build+test.yml
Line: 96-136

Comment:
**AVD cache miss triggers concurrent AVD creation across all 5 shards**

All 5 matrix shards share the same `avd-api33-google_apis-x86_64-pixel_7_pro-v1` key and each independently evaluates `steps.avd-cache.outputs.cache-hit != 'true'`. On a cold cache (first run, or after a manual key bump) all five shards will simultaneously enter the "Create AVD and generate snapshot" step, creating five identical AVDs and racing to write the same cache key. Only one write wins; the other four are wasted work. One common mitigation is a dedicated pre-job that creates the AVD and saves the cache before the matrix fans out, then the matrix shards always see a cache hit.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .github/workflows/build+test.yml
Line: 49-54

Comment:
**Firebase Test Lab tests run in parallel with local emulator instrumentation tests**

The `build` job runs `:app:assembleAndroidTest` and then executes those tests on Firebase Test Lab, while the separate `instrumentation-tests` job runs `:superwall:connectedDebugAndroidTest` on local emulators. These target different modules so they aren't strictly duplicates, but the Firebase run's coverage is never downloaded or merged into the `coverage-report` job. If Firebase tests are meant to be the app-level integration gate, their results (and any coverage) are silently dropped from the combined report. If Firebase is no longer needed now that local sharding covers the same ground, this step could be removed to reduce cost.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .github/workflows/build+test.yml
Line: 203-205

Comment:
**Coverage report blocked if any single shard fails**

`needs: [unit-tests, instrumentation-tests]` with no `if: always()` means the `coverage-report` job (and the badge commit it contains) is skipped entirely when even one of the five instrumentation shards fails. With `fail-fast: false` this could happen often. Consider adding `if: always()` (or `if: needs.unit-tests.result == 'success'`) so a partial coverage report and badge update is still committed even when some shards have failures.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .github/workflows/pr-tests.yml
Line: 157-159

Comment:
**Same coverage-report blocking pattern as `build+test.yml`**

`needs: [unit-tests, instrumentation-tests]` with no `if` guard means badge commits are skipped whenever any shard fails — the same concern described in `build+test.yml`. The pattern appears in both workflows; applying `if: always()` in both places would ensure badges are always updated.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Enable caching, improve avd issues" | Re-trigger Greptile

Greptile also left 5 inline comments on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant