feat(ci): import test coverage into SonarQube (#61)

SonarQube reported 0% coverage for 78 unit, 134 integration and 83 end-to-end tests, so the coverage-on-new-code gate — the most useful thing SonarQube offers a project this size — has been failing permanently while looking configured. It now reports 69.6%, verified by a real scan.

Backend coverage comes from both suites, written to separate directories because jest writes coverage/lcov.info by default and the second run would silently overwrite the first. Both are needed rather than just the fast one: the unit suite alone reports 11%, because everything in src/routes is exercised by the integration suite. That suite is manual-only after hanging for 3h12m post-run, so it runs here with --forceExit and the job carries a hard timeout; jest confirmed during testing that it would otherwise have hung.

The frontend had no unit tests at all, so its coverage comes from Playwright driving an istanbul-instrumented dev server, collected per test by an auto-fixture and merged with nyc. The 17 specs now import from a local fixtures module that re-exports @playwright/test, which is what lets the fixture attach without touching each test body.

Instrumentation is gated behind COVERAGE=true and loaded by dynamic import, since vite-plugin-istanbul is ESM-only while vite.config.ts evaluates as CommonJS. Both directions were checked rather than assumed: a normal build contains no instrumentation, and the dev server instruments nested modules as well as top-level ones — the first attempt used an include glob of src/* which would have silently missed everything under src/admin and src/cart.

coverage:report fails when nothing was collected instead of writing an empty report, and that guard was fired deliberately to confirm it works. This project has been bitten twice by tools succeeding while measuring nothing — SonarQube skipping the whole frontend and still exiting EXECUTION SUCCESS in #67, and an ESLint matcher silently matching no files during #60 — and coverage has exactly that shape: an uninstrumented dev server lets every test pass while gathering nothing, and the 0% that follows reads as lost coverage rather than broken collection.

Worth knowing when reading the numbers: end-to-end coverage flatters. Istanbul marks a line covered when the browser ran it, so a component rendered during a test counts as covered with nothing asserting anything about it. Recorded in the design doc and the project context rather than left to be discovered.

Also declares sonar.tests so test files are analysed under the test rule set rather than as production code.

Closes #61
This commit is contained in:
2026-08-20 10:13:13 -05:00
parent 7e4084a65f
commit 332c1e7cd0
30 changed files with 1762 additions and 34 deletions
+4
View File
@@ -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
+90 -8
View File
@@ -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:
+1
View File
@@ -11,3 +11,4 @@ test-results/
.env
.superpowers/
.scannerwork/
.nyc_output/
+9
View File
@@ -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
+10 -1
View File
@@ -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']
};
+2
View File
@@ -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",
+1464 -1
View File
File diff suppressed because it is too large Load Diff
+7 -2
View File
@@ -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"
}
}
+8 -1
View File
@@ -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'] } }]
+63
View File
@@ -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 -1
View File
@@ -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 -1
View File
@@ -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 -1
View File
@@ -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 -1
View File
@@ -1,4 +1,4 @@
import { test, expect, Page } from '@playwright/test';
import { test, expect, Page } from './fixtures';
const PASSWORD = 'supersecret123';
+1 -1
View File
@@ -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 -1
View File
@@ -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 -1
View File
@@ -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
+47
View File
@@ -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 -1
View File
@@ -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 -1
View File
@@ -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 -1
View File
@@ -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 -1
View File
@@ -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 -1
View File
@@ -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
View File
@@ -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'
}
}
});
}));
+15
View File
@@ -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