test(ui): coverage pass 2 — ~87% honest SonarCloud line coverage (web+mobile+shared) (#243) - #509
Conversation
…ss web+mobile+shared (#243) Second coverage burn-down toward the #243 SonarCloud-to-zero launch gate. Adds ~500 intelligent Vitest tests across the three workspaces and mirrors #508's mobile instrument-gap fix onto web. - apps/web: add coverage.include mirroring sonar.sources so files no test imports are counted (closes the same #446/#508 gap on web, which was mobile-only); new hook/component/page tests. 74% -> 84% lines locally. - apps/mobile: hook/store/lib + component/screen tests; unblock keyboard-aware-scroll-view by adding an emit-capable Keyboard + findNodeHandle to the RN test mock. 79% -> 85%. - packages/shared: store/util/validation/query-key branch tests. 94% -> 99% lines. - Extend sonar.coverage.exclusions with genuinely-presentational web files only (route/error/not-found shells, static legal pages, motion/nav glue, the desktop astra-copilot rail chrome, style primitives) — enumerated with rationale, mirroring #508's mobile set. The 12 borderline web files with real logic are tested, not excluded. - Raise the Vitest coverage thresholds in all three configs to ratchet the gains. Estimated combined SonarCloud line coverage ~87% (up from ~79%). All suites green (shared/web/mobile); web verified under --sequence.shuffle; lint + type-check clean. 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
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Complete
Scope: PR #509 (thomasluizon/orbit-ui-mobile, chore/ui-coverage-pass-2 → main) — 105 changed files, all test files/configs/mocks, zero production code changed.
Recommendation: APPROVE
| Severity | Count |
|---|---|
| Critical | 0 |
| High | 0 |
| Medium | 1 |
| Low / Info | 0 |
Code Review: PR #509 — test(ui): coverage pass 2 — ~87% honest SonarCloud line coverage (web+mobile+shared) (#243)
Recommendation: APPROVE
Summary
Test-only PR (105 files: Vitest tests, 3 vitest.config.ts, 2 mobile test-mocks/*, sonar-project.properties, .gitignore — no production files). Mirrors #508's web-side coverage-instrument-gap fix, raises thresholds, adds ~500 tests. Sampled tests (config/mock diffs, celebration-queue.test.ts, use-friends.test.ts, keyboard-aware-scroll-view.test.tsx, use-bulk-actions.test.ts, use-dismiss-guard.test.ts, bottom-tab-bar.test.tsx) are consistently high quality — real edge-case assertions, correct mock seams per each workspace's CLAUDE.md, zero red-flag patterns. New sonar.coverage.exclusions web entries verified byte-consistent with the new apps/web/vitest.config.ts exclude list.
Findings
Critical / High
None.
Medium
Five non-trivial, live web files got zero new coverage while their exact mobile counterparts were tested in this same PR — contradicts root CLAUDE.md's mandatory "every change lands in BOTH apps/web AND apps/mobile in the same task" rule.
| Web file (untested, not excluded) | Mobile mirror (tested this PR) |
|---|---|
apps/web/hooks/use-bulk-actions.ts (103 lines; consumed by 5 live components) |
apps/mobile/__tests__/hooks/use-bulk-actions.test.ts (new, 76 lines) |
apps/web/hooks/use-dismiss-guard.ts (46 lines; same 5 consumers) |
apps/mobile/__tests__/hooks/use-dismiss-guard.test.ts (new, 76 lines) |
apps/web/app/(app)/social/_components/habit-multi-select.tsx (225 lines — search/filter/toggle/cap logic) |
apps/mobile/__tests__/app/social/_components/habit-multi-select.test.tsx (new, 147 lines) |
apps/web/components/habits/habit-form-fields/frequency-type-cards.tsx (229 lines) |
apps/mobile/__tests__/.../frequency-type-cards.test.tsx (new, 160 lines) |
apps/web/components/habits/habit-list/empty-state.tsx + empty-state-message.ts (164 lines) |
apps/mobile/__tests__/components/habit-list/empty-state.test.tsx (new, 157 lines) |
Verified independently (not just via subagent): each web file is live (real imports, not dead code), not present in sonar.coverage.exclusions or apps/web/vitest.config.ts coverage.exclude, and not mentioned in the PR body's "Honest ceiling (left uncovered on purpose)" disclosure (which only names the habit-list.tsx/habit-form-fields.tsx mega-components). Confirmed via git diff --name-only against main that only the mobile test files were added for these five surfaces — no matching web test files exist in the diff. SonarCloud will score these five 0%, undercutting the claimed web coverage figure, and — unlike the disclosed exclusions — this gap isn't acknowledged in the PR body.
Fix: add matching web tests (apps/web/__tests__/hooks/use-bulk-actions.test.ts, .../use-dismiss-guard.test.ts, .../components/social/habit-multi-select.test.tsx, .../components/habits/habit-form-fields/frequency-type-cards.test.tsx, .../components/habits/habit-list/empty-state.test.tsx) using the already-added mobile tests as a template — use-dismiss-guard.ts even delegates to the already-tested shared resolveDismissGuardAction, so its web wrapper test is low-risk and small.
Not blocking on its own — nothing regresses, no security/compat risk, CI gates evidently still pass — but it's a real, actionable parity gap that a plain "mirror file was also changed" check would miss (it's a case of an existing untested file staying untested while its mirror got fresh coverage in the same task).
Low / Info
None (signal gate — no nits reported).
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED at the changed-file-mirror level — superseded on one point by the Medium finding above (existing-untested-file gap, found via independent grep/diff, not a file-mirror miss) |
| i18n-syncer | N/A — no user-facing strings or i18n JSON touched |
| contract-aligner | N/A — no packages/shared/src/types/*/endpoints.ts/orbit-api changes; orbit-api not checked out in this job, cross-repo dimension not verifiable |
| security-reviewer | N/A — no orbit-api code |
| design-reviewer | N/A — no apps/* UI/production files changed, only tests/mocks/configs |
Validation
Skipped per CI adaptation — Build / Unit Tests / SonarCloud run as separate required checks on this PR.
What's good
Zero production-code churn (safest possible shape for a coverage PR); consistently strong test quality across every sample (real edge cases, no padding, correct mock seams, clean comment policy); sonar.coverage.exclusions verified drift-free against vitest.config.ts; new RN mock primitives are minimal and precisely scoped to what's exercised; the PR's own "Honest ceiling" disclosure is accurate, which is what made the one undisclosed gap stand out by contrast.
Recommendation
Safe to merge — no Critical/High findings. Before closing out #243's coverage burn-down, add the five missing web tests identified above (small follow-up PR recommended given this PR's already-substantial size) so web's SonarCloud number reflects the same completeness the PR claims elsewhere.
1 participant
What
Second SonarCloud coverage burn-down toward the #243 SonarCloud-to-zero launch gate. Adds ~500 intelligent Vitest tests across all three workspaces (behavior + edge + failure paths — no assertion-free padding), closes the same instrument-gap on web that #508 closed on mobile, and enumerates a small set of genuinely-presentational web exclusions.
Coverage moved (local Vitest, lines)
packages/sharedapps/webapps/mobileEstimated combined SonarCloud line coverage ~87% (up from ~79%), computed over
sonar.sourcesminussonar.coverage.exclusions. The PR's own SonarCloud run is authoritative.¹ Web's real baseline once un-imported files are counted — see below.
Key correctness fix: web coverage instrument gap
apps/web/vitest.config.tshad nocoverage.include, so files that no test imported were absent fromlcovand invisible locally — while SonarCloud counted them as 0%. #508 fixed exactly this for mobile; this PR mirrors it onto web (include=app/components/hooks/stores/lib,excludemirroring the websonar.coverage.exclusions). Local web coverage now matches what SonarCloud measures.What got tested vs excluded (honesty)
use-friends,use-calendar-events, history-query hooks, …), thekeyboard-aware-scroll-viewscroll/keyboard logic (unblocked by adding an emit-capableKeyboard+findNodeHandleto the RN test mock), and many partial components pushed to 90-100%.sonar-project.properties) — genuinely-presentational files with no meaningfully-testable unit: Next route/error/not-found shells, static legal pages (privacy/terms/delete-account), theauth-callbackOAuth-effect glue, motion/nav glue (route-transition-shell, the effect-onlynavigation-history-tracker,web-nav), the desktopastra-copilot-railchrome (re-uses the already-testeduseChatComposer/chat components),calendar-week-viewlayout, and style/binding primitives. Mirrors test(ui): close mobile coverage instrument gap + add logic tests, de-flake web suite #508's mobile enumeration.bottom-tab-bar,social-entry-card,invite-hero,pair-detail,wrapped,tour-*,milestone-share-card,habit-row-menu,public-profile-view, …) are tested, not excluded.Ratchet
Raised the Vitest coverage
thresholdsin all three configs to lock the gains (set just below the measured numbers).Honest ceiling (left uncovered on purpose)
A few very large stateful mega-components remain partial — the two
habit-list.tsx(1199/1100-line web + mobile),habit-form-fields.tsx, and the 474/578-line calendar pages — plus deep SSE/step-up/queued-mutation branches. Pushing these to 100% would mean brittle integration-style tests, not honest unit coverage.Verification
shared/web/mobilefull suites green (2536 web, 1579 mobile, +shared); web verified under--sequence.shuffle(no cross-file pollution).turbo run lint type-checkclean across all three workspaces (only 3 pre-existing web lint warnings remain, unrelated to this PR).🤖 Generated with Claude Code