diff --git a/docs/superpowers/specs/2026-08-20-coverage-import-design.md b/docs/superpowers/specs/2026-08-20-coverage-import-design.md new file mode 100644 index 0000000..8f1e1d6 --- /dev/null +++ b/docs/superpowers/specs/2026-08-20-coverage-import-design.md @@ -0,0 +1,103 @@ +# Test Coverage Import — Design + +**Issue:** [#61 — SonarQube imports no test coverage](https://gitea.bermudalamb.synology.me/bermudalamb/redefined-designs/issues/61) +**Date:** 2026-08-20 +**Status:** Approved + +## Goal + +Make SonarQube's coverage number real, so the "coverage on new code" gate — the single most useful thing SonarQube offers a project this size — starts working instead of reading a permanent 0%. + +## What #61 still needs + +Two of the issue's four asks were already delivered by #67, which moved the scan configuration into `sonar-project.properties` and dropped the inline `-D` arguments: + +| #61 asks for | State | +| --- | --- | +| `sonar-project.properties` so the config is reviewable and runnable locally | Done in #67 | +| Replace deprecated `sonar.login`, stop passing the token on a command line | Done in #67 — the token now reaches the scanner only via `SONAR_TOKEN` | +| Import coverage | **This change** | +| Declare `sonar.tests` | **This change** | + +The issue's "Related" note — CI authenticating as `admin` rather than a dedicated `gitea-ci` user — is deliberately **not** here. It is its own issue, so a permissions change is not buried inside a CI-config commit. + +## The two constraints that shape this + +**Backend route logic is only covered by the integration suite.** The unit suite covers `asyncRoute`, `itemFilters`, `utils`, `tagColor` and the route-wrapping guard; everything in `src/routes` is exercised by the 134 integration tests. Importing unit coverage alone would report the route files near 0% when they are in fact well covered — a number worse than none, because it would look measured. + +That suite is `workflow_dispatch`-only, because on 2026-08-18 it held the runner for 3h12m: 89 tests in 87 seconds followed by a hang, with Jest reporting it "did not exit one second after the test run has completed". So the Sonar workflow runs it with `--forceExit` and a hard `timeout-minutes`. That treats the hang as the known post-run problem it is, rather than blocking coverage on root-causing it. + +**The frontend has no unit tests.** There is no Vitest, no jsdom, nothing — only 17 Playwright end-to-end specs. Frontend coverage therefore has to come from instrumenting the app and collecting from browser runs. + +## Decisions + +| Area | Decision | +| --- | --- | +| Backend coverage | Unit **and** integration, as two separate lcov files | +| Integration in CI | Runs in the Sonar workflow with `--forceExit` and `timeout-minutes` | +| Frontend coverage | `vite-plugin-istanbul` on the dev server, collected per test by Playwright | +| Production safety | Instrumentation is gated behind a `COVERAGE` environment variable | +| Empty coverage | Fails loudly rather than reporting a zero | +| CI identity | Out of scope — separate issue | + +### Why two lcov files rather than one + +Jest writes `coverage/lcov.info` by default, so running both suites would have the second overwrite the first. They go to `coverage/unit/` and `coverage/integration/`, and `sonar.javascript.lcov.reportPaths` lists both. SonarQube merges them, so a line covered by either suite counts as covered — which is the correct semantics: a route tested end to end through Express is genuinely exercised. + +### Why instrumentation must be gated + +`vite-plugin-istanbul` rewrites every source file to record execution. That is exactly what makes coverage possible and exactly what must never reach a customer: an instrumented bundle is substantially larger and slower, and it exposes the source structure in `window.__coverage__`. + +The plugin is therefore added only when `process.env.COVERAGE === 'true'`, which no production build sets. The Dockerfile runs a plain `npm run build`, so the shipped bundle is uninstrumented. This is verified by building normally and confirming the output contains no coverage instrumentation, rather than by assuming the gate works. + +### Why end-to-end coverage needs reading with suspicion + +This is recorded because the number will look better than the testing behind it. + +Istanbul marks a line executed when the browser runs it. An end-to-end test that renders a component marks its lines covered without asserting anything about them — so a component can report 90% while nothing checks its behaviour. Backend coverage, coming from tests that assert on responses, means considerably more per percentage point than frontend coverage does here. + +The consequence is that the 80% gate on new code will be easier to clear on frontend changes than on backend ones. That is a known weakness of the chosen approach, accepted deliberately: a flattering number that exists can be tightened later, whereas the alternative — instrumenting nothing on the frontend — leaves every frontend pull request failing a gate it can never satisfy. + +The honest long-term fix is a frontend unit suite. That is not this change. + +### Why empty coverage has to fail loudly + +The failure mode this project keeps hitting is not "the tool errors", it is "the tool succeeds while measuring nothing" — SonarQube skipping the whole frontend and still exiting `EXECUTION SUCCESS` (#67), and an ESLint matcher that silently matched no files during #60's development. + +Coverage has the same shape. If Playwright reuses an already-running, uninstrumented dev server — which `reuseExistingServer: !process.env.CI` makes likely on a developer machine — every test passes, `window.__coverage__` is undefined, and the report is empty. SonarQube would then show frontend coverage dropping to zero, which reads as "the tests stopped covering things" rather than "collection broke". + +So the collection step asserts it gathered something, and the CI job fails if it did not. + +## Files + +| File | Change | +| --- | --- | +| `backend/jest.unit.config.js` | Coverage settings, output to `coverage/unit` | +| `backend/jest.integration.config.js` | Coverage settings, output to `coverage/integration` | +| `backend/package.json` | `test:unit:cov`, `test:integration:cov` | +| `frontend/vite.config.ts` | Gated `vite-plugin-istanbul` | +| `frontend/playwright.config.ts` | Pass `COVERAGE` through to the dev server | +| `frontend/tests/e2e/fixtures.ts` | New — auto-fixture flushing `window.__coverage__` | +| `frontend/tests/e2e/*.spec.ts` | 17 import lines repointed at `./fixtures` | +| `frontend/package.json` | `test:e2e:cov`, `coverage:report`, new devDependencies | +| `sonar-project.properties` | `sonar.tests`, `sonar.javascript.lcov.reportPaths` | +| `.gitea/workflows/sonarqube.yml` | Postgres service, run all three suites with coverage | +| `.gitignore` | `.nyc_output/` | + +## Testing + +1. `npm run build` in `frontend` produces a bundle with **no** istanbul instrumentation. +2. `COVERAGE=true` produces a dev bundle that **does** carry it — both directions checked, so the gate is known to work rather than assumed. +3. Backend unit and integration runs each emit a non-empty `lcov.info` at the expected path. +4. The Playwright run emits `.nyc_output` files and `nyc report` turns them into a non-empty `coverage/lcov.info`. +5. A local scan reports a non-zero coverage percentage for both workspaces. +6. Deleting the coverage output and re-running the collection step fails the job rather than reporting 0%. + +Point 6 is the one worth actually performing rather than reasoning about, for the same reason the gate check in #60 was performed: a guard nobody has seen fire is a guard nobody knows works. + +## Out of scope + +- **A frontend unit suite.** The right answer to weak frontend coverage, and much larger than this. +- **CI identity hardening** — the `admin` to `gitea-ci` token swap. Its own issue. +- **Raising or lowering the gate thresholds.** Let the real numbers arrive first. +- **Root-causing the integration suite's post-run hang.** Worked around here with `--forceExit`; it deserves its own investigation.