test(ui): close mobile coverage instrument gap + add logic tests, de-flake web suite - #508
Conversation
…flake web suite Root cause (#446): sonar.sources analyzed all of apps/mobile app/components/hooks/stores, but apps/mobile/vitest.config.ts only instrumented lib/**, stores/**, and one hook — so every tested mobile screen/component/hook was absent from lcov and scored 0% by SonarCloud, dragging combined coverage to ~57%. - Broaden mobile vitest coverage.include to the full app/components/hooks/stores/lib/widget surface (mirrors sonar.sources) so the existing suite's real coverage lands in lcov. - Add surgical sonar.coverage.exclusions + matching vitest exclude for genuinely-presentational RN code (JSX screens/sections, style tables, Expo Router shells, native bridges, bootstrap glue) — enumerated, never a blanket apps/mobile/** wildcard, so the ~178 render-tested components keep counting. Removed offline-queue.ts + auth-store.ts from the exclusions: now instrumented and well-covered. - Remove the invalid `all` coverage option (not a Vitest 4 type; no runtime effect). - 38 new/deepened mobile test files (behavior + edge + failure) across data hooks, 0%-logic hooks, app-level logic, request/response builders, error mapping, and pure helpers. Mobile kept-set coverage: ~53% raw -> 78.9% lines / 77.6% statements over 10,102 instrumented lines. - De-flake the web suite under parallel/coverage load: server-fetch.test.ts (per-test module reset + vi.stubEnv restore, fixing an APP_VERSION "undefined"-string env leak), push-prompt (reset leaked globalThis.PushManager), tour-store (reset persistent hiddenSections). Green in default and shuffle order. Refs #243 (coverage burn-down) 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.
Code Review: PR #508
Scope: PR #508 in orbit-ui-mobile (chore/ui-coverage-to-100 → main)
Recommendation: APPROVE
| Severity | Count |
|---|---|
| Critical (incl. |
0 |
| High | 0 |
| Medium | 0 |
| Low / Info | 0 |
Summary
This is a test/coverage-infrastructure PR only — no production source changed. It (1) fixes a real instrumentation gap in apps/mobile/vitest.config.ts (coverage.include was scoped to ~7 files while sonar.sources/eslint cover the whole app/components/hooks/stores/lib surface, silently scoring ~200 already-tested mobile files at 0%), (2) mirrors the fix in sonar-project.properties with a legitimate, enumerated (not wildcard) exclusion list for genuinely presentational/untestable files, (3) adds/deepens 35 mobile test files with real behavior+edge+failure assertions, and (4) fixes 3 pre-existing web test-pollution bugs (server-fetch.test.ts, push-prompt.test.tsx, tour-store.test.ts). Every changed test's assertions were checked against the real (unmodified) production source on the highest-risk surfaces (auth: google-auth/google-auth-callback; the 401-refresh-once logic in chat-stream; the deep-link open-redirect guard in use-push-notifications; Pro-gating/rollback logic in use-preference-controls and use-api-key-management). No console.log, no narration comments, no assertion-free padding, no hardcoded secrets anywhere in the diff.
Findings
Critical / High / Medium
None.
Low / Info
None rose to the actionable bar.
Parity verification (two independent passes)
A parity-checker subagent run initially did not return within the review session; the skill did independent manual verification and found no gap (web's vitest.config.ts never had the narrow-include bug mobile had, since it has no coverage.include restriction at all; the mobile test files use vi.hoisted() mocks reset in beforeEach, a pattern already immune to the pollution bugs the web fixes addressed).
A second, later-arriving parity-checker run surfaced three specific claims, each checked by hand against current file contents:
- "Web lacks the same coverage.include/exclude mobile got" — confirmed true but not a gap: web's config has no restrictive include at all (only thresholds), so it was never affected by the bug this PR fixes (mobile's narrow include silently zero-scored ~200 already-tested files). Nothing to mirror.
- "Mobile hook tests lack
vi.resetModules()/vi.unstubAllEnvs()like the web fixes" — checkeduse-friends.test.ts,use-goals.test.ts,use-habits.test.ts,api-client.test.tsforprocess.env/globalThismutation: zero matches. These tests don't use the polluting patterns the web fixes addressed, so the cleanup isn't needed. False positive. - "Mobile added
use-friends.test.tsbut web has no equivalent" — confirmed true: no direct unit test exists forapps/web/hooks/use-friends.ts(only indirect mocking in 8 consumer tests). This is a pre-existing gap, not introduced or touched by this diff, and outside this PR's stated scope (closing mobile's coverage-config gap + de-flaking specific web pollution bugs). Not a regression — noted for awareness, not blocking.
What's good
- The exclusion list in
sonar-project.propertiesis honestly mirrored againstvitest.config.ts'scoverage.exclude(~120 entries, no drift) and is enumerated per-file rather than a blanket wildcard. - The lowered branch/function coverage thresholds (62→58, 75→66) reflect the vastly larger now-instrumented surface, not metric-gaming — the PR's own reported numbers (77.6%/78.9%/69.4%/61.1%) sit safely above every new threshold.
- Test additions consistently include a real edge case and a real failure case, not just the happy path (offline guards, permission-denial, 401-retry-exactly-once, rollback-on-error, double-submit guards, open-redirect rejection on push-notification deep links).
- The 3 web de-flake fixes were root-caused correctly (missing
vi.resetModules()/vi.stubEnv, unresetglobalThis.PushManager, unreset ZustandhiddenSections) rather than papered over.
Recommendation
Approve and merge. No changes requested.
…ss web+mobile+shared (#243) (#509) 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>



Why
SonarCloud Coverage was stuck at ~57% (part of the #243 frozen end-state). Root cause is the #446 instrument gap:
sonar-project.propertiesanalyzes all ofapps/mobileapp/,components/,hooks/,stores/,lib/, butapps/mobile/vitest.config.tsonly instrumentedlib/**,stores/**, and one hook. So every one of the ~200 mobile test files that already exercises a screen/component/hook produced no lcov entry for it → SonarCloud scored the whole mobile surface 0% and dragged the number down.Two honest levers (both used)
1. Fix the instrument gap + surgical coverage exclusions
apps/mobile/vitest.config.tscoverage.includeto the fullapp/components/hooks/stores/lib/modules/orbit-widget/srcsurface (mirrorssonar.sources), so the existing suite's real coverage lands in lcov. Removed the invalidalloption (not a Vitest 4 type; it had no runtime effect and broketsc).sonar.coverage.exclusions(mirrored in the vitestexclude) now removes ONLY genuinely-presentational / untestable-glue code — the mobile harness runs in anodeenv and unit-tests extracted logic, not rendered RN JSX. Excluded, with rationale grouped in the properties file:**/*.styles.ts/*-styles.ts/styles.ts(style token tables),app/**/_layout.tsx(Expo Router shells),modules/**(native Android widget bridge).providers.tsx,theme-provider.tsx,use-app-theme.ts,supabase.ts,sentry-init.ts,plural.ts,orbit-widget.ts(requireNativeModule),version-gate-store.ts,preferences-labels.ts,celebration-motion.ts, and 5 thin store-selector/gesture hooks.apps/mobile/**wildcard, so the 178 component/screen files that ARE render-tested keep counting.offline-queue.ts(now 91%) andstores/auth-store.ts(now 77%) from the old exclusions — they are instrumented and well-covered now, so their reducer/JWT logic counts.2. Real behavior tests for uncovered logic (38 new/deepened files)
Every test asserts real behavior + an edge + a failure case (no assertion-free padding). Highlights:
use-habits41→86%,use-goals44→88%,use-notifications28→88%,use-push-notifications54→81%,use-friends34→98% (optimistic writes + rollback, cache invalidation,enabledgating).use-drill-navigation,use-habit-form,use-tag-selection,use-review-reminder,use-persistent-reminder,use-resolve-clarification,use-reschedule-suggestion,use-apply-onboarding,use-tour-mock-data.use-user-facts,advanced-api-keys(MAX cap + offline guard),use-preference-controls(rollback + Pro gating),chat-stream(401 refresh-and-retry exactly once),app-version,push-notification-permissions,idempotency-keyregistry,challenge-errorsmapping,drawer-content-inset,profile-subscription-display.use-chat-composer77→90%,use-login-flow60→83%,use-tags55→92%,use-wrapped21→100%,use-summary13→100%,google-auth37→97%,google-auth-callback68→98%,use-goal-progress-form-state56→94%,use-goal-status-actions36→92%,use-data-export25→100%,use-today-selection68→100%.Mobile kept-set coverage: ~53% raw → 78.9% lines / 77.6% statements / 69.4% functions / 61.1% branches over 10,102 instrumented lines. Regression-guard thresholds added just below (
76/75/66/58). No production source changed — behavior-preserving.Flaky web-suite fix (deterministic under parallel/coverage load)
The named
apps/web/__tests__/lib/server-fetch.test.tsflaked because it only calledvi.resetModules()in 2 of 9 tests and restoredAPP_VERSIONviaprocess.env.APP_VERSION = previous— which, whenpreviouswasundefined, wrote the literal string"undefined", leaking dirty env across tests/files. Fixed: per-testvi.resetModules()+vi.stubEnv/vi.unstubAllEnvs(). While verifying under--sequence.shuffleI found and fixed two more real cross/within-file pollution bugs (Orbit "fix what you see"):globalThis.PushManagerwas never reset, so "no SW support" fell through the guard tonavigator.serviceWorker.readyon an undefined SW when it didn't run first → reset it in before/afterEach.beforeEachonly calledendTour(), which by design keeps the persistenthiddenSections; "starts inactive" then failed after asetHiddenSectionstest → reset it too.Web suite verified green in default AND shuffle order (275 files / 2268 tests), multiple runs.
Verification (all foreground)
apps/mobile:vitest run --coveragegreen (226 files / 1355 tests, thresholds pass),tsc --noEmitclean,expo lintclean.apps/web:vitest rungreen ×2 (default + shuffle),tsc --noEmitclean, changed-file lint clean.packages/shared:vitest run --coveragegreen (93.8% lines, no instrument gap).--scope changed --base origin/main: 0 issues.The PR's own SonarCloud run is the authoritative validator; the mobile reindex + tests should move Coverage substantially above 57%.
Refs #243 (coverage burn-down)
🤖 Generated with Claude Code