fix(ci): restore SonarCloud coverage (turbo test-cache replayed empty lcov) - #463
Conversation
… lcov) The Sonar job's persisted turbo cache (added #429) replayed the `test` task instead of running vitest, so no lcov was generated and the merged coverage/lcov.info was empty -> Sonar reported 0.0% (regressed from ~56%). Declare coverage/** as the test task's turbo outputs and move each workspace's vitest reportsDirectory inside its package so a cache hit restores real coverage; repoint the sonarcloud.yml merge step at the in-package lcov paths. Also exclude the generated Zod snapshot from coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Review Complete
Scope: PR #463 — fix(ci): restore SonarCloud coverage (turbo test-cache replayed empty lcov) (thomasluizon/orbit-ui-mobile, fix/sonar-coverage-turbo-cache → main)
Recommendation: APPROVE
| Severity | Count |
|---|---|
| Critical (incl. |
0 |
| High | 0 |
| Medium | 0 |
| Low / Info | 0 |
No prior reviews/threads existed on this PR — this is the first pass, covering the entire diff.
What was reviewed: all 6 changed files, read in full — .github/workflows/sonarcloud.yml, apps/mobile/vitest.config.ts, apps/web/vitest.config.ts, packages/shared/vitest.config.ts, sonar-project.properties, turbo.json. This is a CI/tooling-only fix (no product code touched): it moves each package's vitest reportsDirectory in-package (./coverage instead of ../../coverage/<app>), declares outputs: ["coverage/**"] on turbo's test task so a cache hit can actually restore coverage output, repoints the SonarCloud lcov-merge step's source paths to match, and adds a coverage exclusion for the generated Zod snapshot.
Verification performed: traced the whole chain — confirmed reportsDirectory: './coverage' resolves relative to each vitest config's root (the package dir) and matches the merge step's new apps/web/coverage/lcov.info / apps/mobile/coverage/lcov.info / packages/shared/coverage/lcov.info paths; confirmed the merge script's SF: prefix-rewrite logic is unaffected by the path change; confirmed root .gitignore's unanchored coverage/ pattern still ignores all three new output dirs plus the merged root output (no risk of committing generated coverage); confirmed no other workflow or script still references the old coverage/web / coverage/mobile / coverage/shared paths; confirmed packages/shared/src/types/__generated__/ (the new sonar exclusion) exists; confirmed adding outputs to turbo.json's test task changes its hash, guaranteeing a genuine (non-replayed) cache miss on the first run after merge — so the fix is self-healing, not dependent on a manual --force/cache-bust.
Subagents (gated by diff surface):
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED — apps/web/vitest.config.ts and apps/mobile/vitest.config.ts carry the identical reportsDirectory change; other differences between them are pre-existing and platform-legitimate. |
| i18n-syncer | N/A — no user-facing strings touched. |
| contract-aligner | N/A — no packages/shared/src/types/*/endpoints.ts change; orbit-api untouched. |
| security-reviewer | N/A — no orbit-api code in this diff. |
| design-reviewer | N/A — no apps/* UI files changed. |
Validation: N/A across the board per the CI adaptation — /validate skipped since CI runs Build/Unit Tests/SonarCloud separately, and this PR's own SonarCloud run is the live functional gate for the fix per the PR body.
Deferred: DESIGN.md/AI-slop (#8), Backend hard rules (#13), and FEATURES.md parity (#14) are N/A — none of their gated surfaces are touched. The backward-compat guard (Phase 5) is N/A — no shared-types/DTO hunks in this diff, and its cross-repo half isn't verifiable in CI regardless (orbit-api not checked out). No Critical/High findings existed to trigger the adversarial skeptic pass or a cross-model second opinion — both phases are correctly empty, not skipped.
What's good: root-cause fix (correctly diagnoses turbo's output-capture mechanics rather than reaching for --force); every dependent surface updated consistently in one PR; web/mobile changed in lockstep.
No changes requested — clean to merge.
|



Problem
orbit-ui-mobile SonarCloud Coverage = 0.0% (regressed from ~56%).
Root cause — turbo cache replay
#429 added a persisted
.turbocache to the SonarCloud job. The turbotesttask is cacheable but declared nooutputs, and each vitest wrote coverage outside its package (../../coverage/*), which turbo can't capture. So on a warm cache the Sonar job replaystestinstead of running vitest → no lcov is written → the merge step produces an emptycoverage/lcov.info→ Sonar parses it as 0.0%.Fix (durable, not a
--forceband-aid)turbo.json:testtask gainsoutputs: ["coverage/**"](a cache hit now restores real coverage).apps/web,apps/mobile,packages/sharedvitest configs:reportsDirectorymoved inside each package (./coverage) so turbo can capture/restore it. lcov reporter + instrumented paths unchanged..github/workflows/sonarcloud.yml: merge step repointed to the in-package lcov paths; theSF:repo-root path-rewrite and mergedcoverage/lcov.infooutput are kept (per-package SF paths are workspace-relative and need the rewrite).sonar-project.properties: generated Zod snapshotpackages/shared/src/types/__generated__/**added tosonar.coverage.exclusions.Verification (local)
turbo run test --force -- --coverageexecuted vitest (no replay); all three in-package lcov files non-empty (web 445 / mobile 52 / shared 141 SF-records); the merge logic produced a 638-recordcoverage/lcov.infowith correct repo-rootSF:paths. This PR's own SonarCloud run is the live gate — coverage should jump from 0% to a real value.Note: once 0% is fixed, the real % will read low initially (mobile screens/components are in
sonar.sourcesbut not yet instrumented) — addressed in the ui coverage burn-down.🤖 Generated with Claude Code