Feature/61 coverage import #73
@@ -251,6 +251,10 @@ Take a green SonarQube job as weak evidence. It exits success on a partially-fai
|
||||
- **Unit tests**: `cd backend && npm run test:unit` — no DB required
|
||||
- **Integration tests**: `npm run db:test:up` (disposable tmpfs Postgres via `docker-compose.test.yml`) → `npm run migrate:up` → `npm run test:integration` → `npm run db:test:down`
|
||||
- **E2e (Playwright)**: needs backend running against a migrated DB; `cd frontend && npm run test:e2e`
|
||||
- **Coverage**: `npm run test:unit:cov` and `npm run test:integration:cov` in `backend` write `coverage/unit/lcov.info` and `coverage/integration/lcov.info` — separate directories because jest writes `coverage/lcov.info` by default and the second run would otherwise overwrite the first. Frontend coverage is `npm run test:e2e:cov` then `npm run coverage:report`. Added in #61.
|
||||
- **Frontend coverage comes from Playwright through an istanbul-instrumented dev server**, so read it with suspicion: istanbul marks a line covered when the browser ran it, meaning a component rendered during an end-to-end test reports as covered with nothing asserting anything about it. Backend coverage, coming from tests that assert on responses, means considerably more per percentage point. The 80% gate on new code is correspondingly easier to clear on frontend changes.
|
||||
- **Instrumentation is gated behind `COVERAGE=true`** and must stay that way — an instrumented bundle is larger, slower, and publishes the source structure through `window.__coverage__`. The Dockerfile runs a plain `npm run build`, which never sets it. `vite-plugin-istanbul` is also loaded by dynamic import because it is ESM-only while `vite.config.ts` evaluates as CommonJS; a static import fails the build outright.
|
||||
- **`npm run coverage:report` fails when nothing was collected** rather than writing an empty report. If Playwright reuses an uninstrumented dev server every test still passes while gathering nothing, and the resulting 0% reads as "the tests stopped covering things" rather than "collection was never switched on".
|
||||
- CI (`tests.yml`) runs all three as separate jobs, each posting a pass/fail summary to Gitea's job Summary tab. The `frontend-e2e` job runs `node migrate.js up` against a Postgres service container — same migration mechanism as everywhere else, no schema duplication anywhere in the project anymore.
|
||||
|
||||
### The local Node version will not run the integration or e2e suites
|
||||
|
||||
@@ -10,6 +10,44 @@ on:
|
||||
jobs:
|
||||
sonarqube:
|
||||
runs-on: ubuntu-latest
|
||||
# The scanner needs every coverage report in one workspace, so the suites run
|
||||
# here rather than being passed between jobs as artifacts. That makes this the
|
||||
# long job. The timeout is a stop, not a budget: the integration suite has
|
||||
# hung after completing before (see backend-integration.yml) and cost three
|
||||
# hours of runner time.
|
||||
timeout-minutes: 30
|
||||
services:
|
||||
postgres:
|
||||
image: postgres:16
|
||||
env:
|
||||
POSTGRES_USER: redefined_test
|
||||
POSTGRES_PASSWORD: redefined_test
|
||||
POSTGRES_DB: redefined_test
|
||||
options: >-
|
||||
--health-cmd "pg_isready -U redefined_test"
|
||||
--health-interval 5s
|
||||
--health-timeout 5s
|
||||
--health-retries 10
|
||||
env:
|
||||
# Consumed by the integration suite's setup files.
|
||||
TEST_PGHOST: postgres
|
||||
TEST_PGPORT: 5432
|
||||
TEST_PGUSER: redefined_test
|
||||
TEST_PGPASSWORD: redefined_test
|
||||
TEST_PGDATABASE: redefined_test
|
||||
# Consumed by the backend process the end-to-end run drives.
|
||||
PGHOST: postgres
|
||||
PGPORT: 5432
|
||||
PGUSER: redefined_test
|
||||
PGPASSWORD: redefined_test
|
||||
PGDATABASE: redefined_test
|
||||
PORT: 3000
|
||||
DEMO_MODE: 'true'
|
||||
UPLOADS_DIR: /tmp/redefined-uploads
|
||||
# NODE_ENV is deliberately unset: `npm install` omits devDependencies when
|
||||
# NODE_ENV=production, which strips tsc/vite/@playwright/test and breaks the
|
||||
# build. It would also flip the session cookie to Secure, which the e2e run
|
||||
# serves over plain http.
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -37,17 +75,61 @@ jobs:
|
||||
run: npm run build
|
||||
working-directory: frontend
|
||||
|
||||
# frontend/tsconfig.sonar.json is a standalone copy that cannot use
|
||||
# `extends`, so it can drift. Drift does not fail the scan — it silently
|
||||
# returns to skipping the frontend while still reporting success, which is
|
||||
# the failure #67 was about. Checked before scanning, so the scan is never
|
||||
# the thing that discovers it.
|
||||
- name: Check the Sonar tsconfig has not drifted
|
||||
run: node scripts/check-sonar-tsconfig.js
|
||||
|
||||
# Scan settings live in sonar-project.properties at the repo root, so a
|
||||
# local scan and this one analyse the same thing. Only the host and token
|
||||
# come from secrets.
|
||||
- name: Run migrations
|
||||
run: node migrate.js up
|
||||
working-directory: backend
|
||||
|
||||
- name: Backend unit tests with coverage
|
||||
run: npm run test:unit:cov
|
||||
working-directory: backend
|
||||
|
||||
# Covers everything in src/routes, which the unit suite does not touch —
|
||||
# without this the backend reports around 11% rather than the ~73% it
|
||||
# actually has.
|
||||
- name: Backend integration tests with coverage
|
||||
run: npm run test:integration:cov
|
||||
working-directory: backend
|
||||
|
||||
- name: Start backend for the end-to-end run
|
||||
run: |
|
||||
mkdir -p /tmp/redefined-uploads
|
||||
node dist/server.js > /tmp/backend.log 2>&1 &
|
||||
for i in $(seq 1 30); do
|
||||
if node -e "require('http').get('http://localhost:3000/api/config', r => process.exit(r.statusCode === 200 ? 0 : 1)).on('error', () => process.exit(1))"; then
|
||||
echo "Backend ready after ${i}s"
|
||||
exit 0
|
||||
fi
|
||||
sleep 1
|
||||
done
|
||||
echo "Backend did not become ready within 30s:"
|
||||
cat /tmp/backend.log
|
||||
exit 1
|
||||
working-directory: backend
|
||||
|
||||
- name: Install Playwright browsers
|
||||
run: npx playwright install --with-deps chromium
|
||||
working-directory: frontend
|
||||
|
||||
# Runs against an istanbul-instrumented dev server, which is what produces
|
||||
# window.__coverage__ for the fixture to collect.
|
||||
- name: Frontend end-to-end tests with coverage
|
||||
run: npm run test:e2e:cov
|
||||
working-directory: frontend
|
||||
|
||||
- name: Backend log
|
||||
if: failure()
|
||||
run: cat /tmp/backend.log
|
||||
|
||||
# Fails when nothing was collected rather than writing an empty report. An
|
||||
# uninstrumented dev server lets every test pass while gathering nothing,
|
||||
# and the resulting 0% reads as "the tests stopped covering things".
|
||||
- name: Merge frontend coverage
|
||||
run: npm run coverage:report
|
||||
working-directory: frontend
|
||||
|
||||
- name: SonarQube Scan
|
||||
uses: sonarsource/sonarqube-scan-action@v4
|
||||
env:
|
||||
|
||||
@@ -11,3 +11,4 @@ test-results/
|
||||
.env
|
||||
.superpowers/
|
||||
.scannerwork/
|
||||
.nyc_output/
|
||||
|
||||
@@ -2,6 +2,15 @@ module.exports = {
|
||||
preset: 'ts-jest',
|
||||
testEnvironment: 'node',
|
||||
testMatch: ['<rootDir>/tests/integration/**/*.test.ts'],
|
||||
// Separate directory per suite: jest writes coverage/lcov.info by default, so
|
||||
// running unit and integration in the same job would have the second silently
|
||||
// overwrite the first. SonarQube is pointed at both and merges them.
|
||||
coverageDirectory: '<rootDir>/coverage/integration',
|
||||
coverageReporters: ['lcov', 'text-summary'],
|
||||
// Every source file, not just the ones a test happens to import — otherwise an
|
||||
// entirely untested file is absent from the report rather than reported as 0%,
|
||||
// which flatters the total.
|
||||
collectCoverageFrom: ['<rootDir>/src/**/*.ts'],
|
||||
setupFiles: ['<rootDir>/tests/integration/setup/env.setup.ts'],
|
||||
globalSetup: '<rootDir>/tests/integration/setup/globalSetup.ts',
|
||||
testTimeout: 20000
|
||||
|
||||
@@ -1,5 +1,14 @@
|
||||
module.exports = {
|
||||
preset: 'ts-jest',
|
||||
testEnvironment: 'node',
|
||||
testMatch: ['<rootDir>/tests/unit/**/*.test.ts']
|
||||
testMatch: ['<rootDir>/tests/unit/**/*.test.ts'],
|
||||
// Separate directory per suite: jest writes coverage/lcov.info by default, so
|
||||
// running unit and integration in the same job would have the second silently
|
||||
// overwrite the first. SonarQube is pointed at both and merges them.
|
||||
coverageDirectory: '<rootDir>/coverage/unit',
|
||||
coverageReporters: ['lcov', 'text-summary'],
|
||||
// Every source file, not just the ones a test happens to import — otherwise an
|
||||
// entirely untested file is absent from the report rather than reported as 0%,
|
||||
// which flatters the total.
|
||||
collectCoverageFrom: ['<rootDir>/src/**/*.ts']
|
||||
};
|
||||
|
||||
@@ -11,8 +11,10 @@
|
||||
"test": "npm run test:unit",
|
||||
"test:unit": "jest -c jest.unit.config.js",
|
||||
"test:unit:json": "jest -c jest.unit.config.js --json --outputFile=unit-results.json",
|
||||
"test:unit:cov": "jest -c jest.unit.config.js --coverage",
|
||||
"test:integration": "jest -c jest.integration.config.js --runInBand",
|
||||
"test:integration:json": "jest -c jest.integration.config.js --runInBand --json --outputFile=integration-results.json",
|
||||
"test:integration:cov": "jest -c jest.integration.config.js --runInBand --coverage --forceExit",
|
||||
"db:test:up": "docker compose -f docker-compose.test.yml up -d",
|
||||
"db:test:down": "docker compose -f docker-compose.test.yml down -v",
|
||||
"migrate:up": "node migrate.js up",
|
||||
|
||||
@@ -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
|
||||
@@ -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.
|
||||
Generated
+1464
-1
File diff suppressed because it is too large
Load Diff
@@ -6,7 +6,9 @@
|
||||
"dev": "vite",
|
||||
"build": "tsc && vite build",
|
||||
"lint": "eslint src",
|
||||
"test:e2e": "playwright test"
|
||||
"test:e2e": "playwright test",
|
||||
"test:e2e:cov": "cross-env COVERAGE=true playwright test",
|
||||
"coverage:report": "node scripts/coverage-report.js"
|
||||
},
|
||||
"dependencies": {
|
||||
"@ant-design/icons": "^5.4.0",
|
||||
@@ -25,14 +27,17 @@
|
||||
"@types/react": "^18.3.3",
|
||||
"@types/react-dom": "^18.3.0",
|
||||
"@vitejs/plugin-react": "^4.3.1",
|
||||
"cross-env": "^10.1.0",
|
||||
"eslint": "^9.39.5",
|
||||
"eslint-plugin-jsx-a11y": "^6.10.2",
|
||||
"eslint-plugin-react-hooks": "^7.1.1",
|
||||
"eslint-plugin-sonarjs": "^4.2.0",
|
||||
"globals": "^17.11.0",
|
||||
"nyc": "^18.0.0",
|
||||
"pg": "^8.23.0",
|
||||
"typescript": "^5.5.4",
|
||||
"typescript-eslint": "^8.67.0",
|
||||
"vite": "^5.4.0"
|
||||
"vite": "^5.4.0",
|
||||
"vite-plugin-istanbul": "^6.0.2"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -17,7 +17,14 @@ export default defineConfig({
|
||||
webServer: {
|
||||
command: 'npm run dev',
|
||||
url: 'http://localhost:5173',
|
||||
reuseExistingServer: !process.env.CI,
|
||||
// Forwarded so the dev server instruments its modules when the run is a
|
||||
// coverage run. Without this the tests still pass and every page reports no
|
||||
// coverage at all, which publishes a 0% that reads as "the tests stopped
|
||||
// covering things" rather than "collection was never switched on".
|
||||
env: { COVERAGE: process.env.COVERAGE ?? '' },
|
||||
// Reusing a server that was started without COVERAGE would silently collect
|
||||
// nothing, so a coverage run always starts its own.
|
||||
reuseExistingServer: !process.env.CI && process.env.COVERAGE !== 'true',
|
||||
timeout: 30000
|
||||
},
|
||||
projects: [{ name: 'chromium', use: { ...devices['Desktop Chrome'] } }]
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
#!/usr/bin/env node
|
||||
// Turns the per-test istanbul samples in .nyc_output into coverage/lcov.info for
|
||||
// SonarQube to import.
|
||||
//
|
||||
// The check below is the point of this script existing rather than the pipeline
|
||||
// just calling `nyc report`. The failure mode that matters here is not an error,
|
||||
// it is silence: if the dev server was started without COVERAGE, every test still
|
||||
// passes and every page reports no instrumentation, so .nyc_output stays empty
|
||||
// and nyc cheerfully writes a valid, empty report. SonarQube then shows frontend
|
||||
// coverage at 0%, which reads as "the tests stopped covering things" rather than
|
||||
// "collection was never switched on".
|
||||
//
|
||||
// This project has been bitten twice by tools succeeding while measuring nothing
|
||||
// — SonarQube skipping the entire frontend and still exiting EXECUTION SUCCESS
|
||||
// (#67), and an ESLint matcher silently matching no files (#60). Coverage has the
|
||||
// same shape, so it fails loudly instead.
|
||||
const { existsSync, readdirSync } = require('fs');
|
||||
const { execFileSync } = require('child_process');
|
||||
const path = require('path');
|
||||
|
||||
const root = path.resolve(__dirname, '..');
|
||||
const nycOutput = path.join(root, '.nyc_output');
|
||||
|
||||
const samples = existsSync(nycOutput)
|
||||
? readdirSync(nycOutput).filter((f) => f.endsWith('.json'))
|
||||
: [];
|
||||
|
||||
if (samples.length === 0) {
|
||||
console.error('No coverage was collected — .nyc_output holds no samples.');
|
||||
console.error('');
|
||||
console.error('The tests may well have passed; that is the problem. Coverage is only');
|
||||
console.error('gathered when the dev server is instrumented, which happens when COVERAGE=true');
|
||||
console.error('reaches it. Run the suite with `npm run test:e2e:cov` rather than `npm run test:e2e`.');
|
||||
console.error('');
|
||||
console.error('If it was run that way, check that Playwright started its own dev server rather');
|
||||
console.error('than reusing one you already had open — an uninstrumented server collects nothing.');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
console.log(`Merging ${samples.length} coverage sample(s) from ${samples.length} test(s).`);
|
||||
|
||||
// Invoked through nyc's own entry point rather than npx: on Windows, spawning
|
||||
// the npx.cmd shim fails with EINVAL under current Node unless a shell is used,
|
||||
// and running the JS directly avoids needing one at all.
|
||||
execFileSync(
|
||||
process.execPath,
|
||||
[
|
||||
path.join(root, 'node_modules', 'nyc', 'bin', 'nyc.js'),
|
||||
'report',
|
||||
'--reporter=lcov',
|
||||
'--reporter=text-summary',
|
||||
'--report-dir',
|
||||
'coverage'
|
||||
],
|
||||
{ cwd: root, stdio: 'inherit' }
|
||||
);
|
||||
|
||||
const lcov = path.join(root, 'coverage', 'lcov.info');
|
||||
if (!existsSync(lcov)) {
|
||||
console.error(`nyc reported success but ${lcov} was not written.`);
|
||||
process.exit(1);
|
||||
}
|
||||
console.log(`Wrote ${path.relative(root, lcov)}`);
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect, Page } from '@playwright/test';
|
||||
import { test, expect, Page } from './fixtures';
|
||||
|
||||
const PASSWORD = 'supersecret123';
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect, Page } from '@playwright/test';
|
||||
import { test, expect, Page } from './fixtures';
|
||||
|
||||
const PASSWORD = 'supersecret123';
|
||||
const uniqueEmail = () => `disable-${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}@example.com`;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { test, expect } from './fixtures';
|
||||
|
||||
const suffix = () => `i${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`;
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect, Page } from '@playwright/test';
|
||||
import { test, expect, Page } from './fixtures';
|
||||
|
||||
const RUN = `v${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`;
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { test, expect } from './fixtures';
|
||||
|
||||
const suffix = () => `r${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`;
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { test, expect } from './fixtures';
|
||||
|
||||
const suffix = () => `s${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`;
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { test, expect } from './fixtures';
|
||||
|
||||
// The e2e database is shared and never reset, so every fixture name carries a
|
||||
// unique suffix and assertions are scoped to the nodes this run created. The
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect, Page } from '@playwright/test';
|
||||
import { test, expect, Page } from './fixtures';
|
||||
|
||||
// Relative luminance per WCAG, used to tell "light" from "dark" without
|
||||
// asserting exact hex values, which would break on any palette tweak.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect, Page } from '@playwright/test';
|
||||
import { test, expect, Page } from './fixtures';
|
||||
|
||||
const PASSWORD = 'supersecret123';
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect, Page } from '@playwright/test';
|
||||
import { test, expect, Page } from './fixtures';
|
||||
|
||||
const PASSWORD = 'supersecret123';
|
||||
// The storefront runs against a shared database that is never reset, so every
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect, Page } from '@playwright/test';
|
||||
import { test, expect, Page } from './fixtures';
|
||||
|
||||
const PASSWORD = 'supersecret123';
|
||||
const RUN = `f${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect, APIRequestContext } from '@playwright/test';
|
||||
import { test, expect, APIRequestContext } from './fixtures';
|
||||
|
||||
// The storefront shows every item ever seeded, and the e2e database is not
|
||||
// reset between runs. Every fixture below is therefore suffixed with a unique
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
import { test as base } from '@playwright/test';
|
||||
import { mkdirSync, writeFileSync } from 'fs';
|
||||
import { randomUUID } from 'crypto';
|
||||
import path from 'path';
|
||||
|
||||
// Re-exported so specs can import everything from here — expect, Page,
|
||||
// APIRequestContext — and get the coverage-collecting `test` at the same time.
|
||||
// The explicit `test` below wins over the star export.
|
||||
export * from '@playwright/test';
|
||||
|
||||
const NYC_OUTPUT = path.resolve(__dirname, '..', '..', '.nyc_output');
|
||||
const collectingCoverage = process.env.COVERAGE === 'true';
|
||||
|
||||
// Set once any page reports instrumentation. Checked by the collection script so
|
||||
// an empty run fails loudly instead of publishing 0% — see the note below.
|
||||
const MARKER = path.join(NYC_OUTPUT, '.collected');
|
||||
|
||||
/**
|
||||
* Flushes istanbul's per-page counters after each test.
|
||||
*
|
||||
* Coverage lives in `window.__coverage__` on the page and dies with it, so it
|
||||
* has to be read before the page closes — one file per test, because the suite
|
||||
* runs fullyParallel and workers would otherwise overwrite each other.
|
||||
*/
|
||||
export const test = base.extend<{ collectCoverage: void }>({
|
||||
collectCoverage: [
|
||||
async ({ page }, use) => {
|
||||
await use();
|
||||
|
||||
if (!collectingCoverage) return;
|
||||
|
||||
// The page may already be closed by a test that navigated away or crashed;
|
||||
// a missing sample is not worth failing a passing test over. The run-level
|
||||
// check catches the case that actually matters — no samples at all.
|
||||
const coverage = await page
|
||||
.evaluate(() => (window as unknown as { __coverage__?: unknown }).__coverage__)
|
||||
.catch(() => undefined);
|
||||
|
||||
if (!coverage) return;
|
||||
|
||||
mkdirSync(NYC_OUTPUT, { recursive: true });
|
||||
writeFileSync(path.join(NYC_OUTPUT, `${randomUUID()}.json`), JSON.stringify(coverage));
|
||||
writeFileSync(MARKER, 'ok');
|
||||
},
|
||||
{ auto: true }
|
||||
]
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect, Page } from '@playwright/test';
|
||||
import { test, expect, Page } from './fixtures';
|
||||
import { Client } from 'pg';
|
||||
|
||||
const PASSWORD = 'supersecret123';
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { test, expect } from './fixtures';
|
||||
|
||||
test.describe('Storefront failure states', () => {
|
||||
test('reports a server failure instead of claiming the store is empty', async ({ page }) => {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { test, expect } from './fixtures';
|
||||
|
||||
test.describe('Storefront', () => {
|
||||
test('loads and shows the site title', async ({ page }) => {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { test, expect } from './fixtures';
|
||||
|
||||
test.describe('Theme switching', () => {
|
||||
test('toggling the switch changes the body theme attribute', async ({ page }) => {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { test, expect } from './fixtures';
|
||||
|
||||
// The /verify-email route was missing entirely, so the link in every verification
|
||||
// email rendered a blank page. These cover the route existing and reporting an
|
||||
|
||||
+25
-4
@@ -1,8 +1,29 @@
|
||||
import { defineConfig } from 'vite';
|
||||
import { defineConfig, type PluginOption } from 'vite';
|
||||
import react from '@vitejs/plugin-react';
|
||||
|
||||
export default defineConfig({
|
||||
plugins: [react()],
|
||||
// Coverage instrumentation is opt-in and must stay that way. The plugin rewrites
|
||||
// every source file to record execution, which is what makes end-to-end coverage
|
||||
// possible and what must never reach a customer: an instrumented bundle is
|
||||
// substantially larger and slower, and it publishes the source structure through
|
||||
// window.__coverage__.
|
||||
//
|
||||
// Nothing in the production path sets COVERAGE — the Dockerfile runs a plain
|
||||
// `npm run build` — so the shipped bundle is uninstrumented. Only the Playwright
|
||||
// coverage run turns this on, via playwright.config.ts passing it to the dev
|
||||
// server it starts.
|
||||
//
|
||||
// Loaded with a dynamic import rather than at the top of the file because
|
||||
// vite-plugin-istanbul is ESM-only and this config is evaluated as CommonJS, the
|
||||
// package having no "type": "module". A static import fails the build outright.
|
||||
// The upside is that a production build never even resolves the package.
|
||||
async function coveragePlugins(): Promise<PluginOption[]> {
|
||||
if (process.env.COVERAGE !== 'true') return [];
|
||||
const { default: istanbul } = await import('vite-plugin-istanbul');
|
||||
return [istanbul({ include: 'src/**/*', extension: ['.ts', '.tsx'], requireEnv: false })];
|
||||
}
|
||||
|
||||
export default defineConfig(async () => ({
|
||||
plugins: [react(), ...(await coveragePlugins())],
|
||||
build: { outDir: 'dist' },
|
||||
server: {
|
||||
proxy: {
|
||||
@@ -11,4 +32,4 @@ export default defineConfig({
|
||||
'/webhooks': 'http://localhost:3000'
|
||||
}
|
||||
}
|
||||
});
|
||||
}));
|
||||
|
||||
@@ -14,3 +14,18 @@ sonar.sourceEncoding=UTF-8
|
||||
# EXECUTION SUCCESS. See #67. tsconfig.sonar.json is an analysis-only mirror;
|
||||
# both it and this line go away once the server can parse "bundler".
|
||||
sonar.typescript.tsconfigPaths=backend/tsconfig.json,frontend/tsconfig.sonar.json
|
||||
|
||||
# Test sources, so they are analysed under the test rule set rather than as
|
||||
# production code — or, as before, not at all.
|
||||
sonar.tests=backend/tests,frontend/tests
|
||||
|
||||
# Coverage. Three reports because the suites cover genuinely different things and
|
||||
# jest would otherwise overwrite one with the other: the unit suite alone reports
|
||||
# ~11% because everything in src/routes is exercised by the integration suite,
|
||||
# not by it. SonarQube merges them, so a line covered by any suite counts.
|
||||
#
|
||||
# Read the frontend number with suspicion. It comes from Playwright through an
|
||||
# istanbul-instrumented dev server, and istanbul marks a line covered when the
|
||||
# browser ran it — a component renders during an end-to-end test and reports as
|
||||
# covered with nothing asserting anything about it. See #61's design doc.
|
||||
sonar.javascript.lcov.reportPaths=backend/coverage/unit/lcov.info,backend/coverage/integration/lcov.info,frontend/coverage/lcov.info
|
||||
|
||||
Reference in New Issue
Block a user