2026-08-20 10:30:30 -05:00
1 changed files with 103 additions and 0 deletions
@@ -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.