Feature/61 coverage import #73
@@ -0,0 +1,52 @@
|
||||
# CI Contract — Test Coverage
|
||||
|
||||
**Status:** Live as of #61
|
||||
**Applies to:** `.gitea/workflows/sonarqube.yml`, `sonar-project.properties`, both workspaces' test tooling
|
||||
|
||||
What CI has to keep doing for SonarQube's coverage number to stay real. This exists because coverage does not fail loudly when it breaks — it reports a smaller number, which is indistinguishable from tests genuinely covering less.
|
||||
|
||||
## The contract
|
||||
|
||||
Every one of these is load-bearing. Breaking any of them produces a plausible-looking number rather than an error.
|
||||
|
||||
| # | Requirement | What breaks if it lapses |
|
||||
| --- | --- | --- |
|
||||
| 1 | Both backend suites run with coverage before the scan | The unit suite alone reports ~11%, because everything in `src/routes` is exercised only by the integration suite |
|
||||
| 2 | The two backend reports go to separate directories | Jest writes `coverage/lcov.info` by default; the second run overwrites the first and half the coverage vanishes |
|
||||
| 3 | The end-to-end run uses `test:e2e:cov`, not `test:e2e` | An uninstrumented dev server collects nothing while every test still passes |
|
||||
| 4 | `coverage:report` runs and is allowed to fail the job | It is the only thing that notices an empty collection |
|
||||
| 5 | `sonar.javascript.lcov.reportPaths` lists all three reports | A dropped path silently removes that suite's contribution |
|
||||
| 6 | The integration suite keeps `--forceExit` | It hangs after completing; on 2026-08-18 that cost 3h12m of runner time |
|
||||
| 7 | Nothing in the production path sets `COVERAGE` | An instrumented bundle ships to customers: larger, slower, and publishing the source structure through `window.__coverage__` |
|
||||
|
||||
## The failure mode this is written against
|
||||
|
||||
This project has now been bitten three times by a tool succeeding while measuring nothing:
|
||||
|
||||
- **#67** — SonarQube skipped all 34 frontend files because their tsconfig used `moduleResolution: "bundler"`, and still exited `EXECUTION SUCCESS`. The quality gate reported on a third of the codebase for months while looking complete.
|
||||
- **#60** — an ESLint matcher during development matched no files at all. The run was green because there was nothing to complain about.
|
||||
- **Coverage** has the same shape by construction. If Playwright reuses an already-running, uninstrumented dev server — which `reuseExistingServer` makes likely on a developer machine — every test passes, `window.__coverage__` is undefined, and the report is empty but valid.
|
||||
|
||||
The lesson each time was the same: a green tool is weak evidence. The specific defence here is `frontend/scripts/coverage-report.js`, which refuses to write a report when `.nyc_output` holds no samples and explains the two likely causes. It exists instead of calling `nyc report` directly, and that is the whole reason it exists.
|
||||
|
||||
## Checking it still holds
|
||||
|
||||
After any change to the workflow, the Vite config, or the test tooling:
|
||||
|
||||
1. `npm run build` in `frontend`, then grep the bundle for `__coverage__`. Zero occurrences is required. This is the one that ships to customers if it regresses.
|
||||
2. Run the coverage suites and confirm all three `lcov.info` files exist and are non-empty.
|
||||
3. Delete `.nyc_output` and run `npm run coverage:report`. It must exit non-zero. A guard nobody has seen fire is a guard nobody knows works.
|
||||
4. After a scan, check the coverage percentage moved in a direction the change explains. A sharp drop is far more likely to be broken collection than lost tests.
|
||||
|
||||
## Reading the number
|
||||
|
||||
Backend and frontend coverage do not mean the same thing, and averaging them hides that.
|
||||
|
||||
Backend coverage comes from tests that assert on responses — a covered line is usually a checked line. Frontend coverage comes from Playwright driving an instrumented browser, and istanbul marks a line covered when it executes. A component rendered during an end-to-end test reports as covered with nothing asserting anything about it, so the frontend number reads considerably better than the testing behind it.
|
||||
|
||||
The practical consequence: the 80% gate on new code is easier to clear on frontend changes than backend ones. Treat a high frontend number as evidence the code ran, not that it works. The real fix is a frontend unit suite, which does not exist yet.
|
||||
|
||||
## Related
|
||||
|
||||
- `docs/superpowers/specs/2026-08-20-coverage-import-design.md` — the design and why each choice was made
|
||||
- `docs/ci/sonarqube-ci-identity.md` — the separate question of which account CI authenticates as
|
||||
@@ -0,0 +1,49 @@
|
||||
# CI Identity — SonarQube
|
||||
|
||||
**Status:** Not done. Tracked as its own issue.
|
||||
**Applies to:** the `SONAR_TOKEN` Gitea Actions secret, and the SonarQube account behind it
|
||||
|
||||
## Current state
|
||||
|
||||
Gitea Actions authenticates to SonarQube using a token belonging to the **`admin`** account. This was flagged early in the project and never revisited.
|
||||
|
||||
The token reaches the scanner only through the `SONAR_TOKEN` environment variable — #67 removed the `-Dsonar.login=` command-line copy, so it is no longer passed on a command line where it could reach a log. That part is already fixed. What remains is *whose* token it is.
|
||||
|
||||
## Why it should change
|
||||
|
||||
An analysis job needs one permission: submit an analysis report for one project. The `admin` token carries every permission the server has — creating and deleting projects, changing quality gates and profiles, managing users, reading every project including `sql-utilities`.
|
||||
|
||||
That gap matters in three ordinary situations, none of which require anyone to be malicious:
|
||||
|
||||
- **A leaked token is a leaked server.** CI secrets end up in more places than intended: a debug run with `set -x`, a third-party action, a fork's workflow. The blast radius of an analysis token is one project's analysis history. The blast radius of this one is everything.
|
||||
- **A misconfigured scan can destroy history.** `sonar.projectKey` is a string in a properties file. A wrong value plus admin rights silently creates projects; other admin endpoints can delete them. A restricted token simply fails.
|
||||
- **Rotation is currently painful.** Rotating `admin`'s token means finding every other place that account is used. A dedicated account can be rotated on its own.
|
||||
|
||||
There is also a plainer reason: when CI shows up as `admin` in the analysis history, the audit trail cannot distinguish an automated scan from a person making a change.
|
||||
|
||||
## The change
|
||||
|
||||
1. On SonarQube, create a user — `gitea-ci` — with **no** global permissions.
|
||||
2. Grant it **Execute Analysis** on the `redefined-designs` project only. That is the single permission a scan needs.
|
||||
3. Generate a **user token** for that account. Not a project or global analysis token: SonarSource's own MCP server, and other tooling, require the user type, and this server is old enough that the distinction matters.
|
||||
4. Update the `SONAR_TOKEN` secret in the repository's Actions settings.
|
||||
5. Run the SonarQube workflow and confirm it still passes.
|
||||
6. **Revoke the `admin` token** that CI was using. Skipping this leaves the old credential valid and the change cosmetic.
|
||||
|
||||
Step 6 is the one worth naming explicitly, because the workflow will already be green after step 5 and it is easy to stop there.
|
||||
|
||||
## Also worth deciding at the same time
|
||||
|
||||
The scratch project `redefined-designs-local`, used by `scripts/scan-local.sh` so local scans do not overwrite CI's analysis of `main`, is currently written by a personal token from the developer's environment. If local scanning becomes a habit rather than an occasional check, it deserves the same treatment: its own restricted account rather than whichever token is to hand.
|
||||
|
||||
## Constraints on this server
|
||||
|
||||
SonarQube here is **9.9.8 LTA, Community edition**. Two consequences:
|
||||
|
||||
- It has no branch analysis, so every scan overwrites the single `main` analysis for whichever project key it is given. This is why the scratch project exists.
|
||||
- It does not accept `Authorization: Bearer` — a token is supplied as the basic-auth username. Any script checking the new account's permissions must use `curl -u "$TOKEN:"`, not a bearer header, or it will look like the permissions are wrong when the auth scheme is.
|
||||
|
||||
## Related
|
||||
|
||||
- `docs/ci/coverage-pipeline-contract.md` — what the pipeline must do for coverage to stay honest
|
||||
- #61 raised this as a "Related" note; it was split out so a permissions change would not be buried inside a CI-config commit
|
||||
Reference in New Issue
Block a user