A failing integration suite takes the SonarQube scan down with it, and four other steps can do the same #174

New Issue

2026-08-25T08:40:18-05:00

bermudalamb commented

2026-08-25 08:40:18 -05:00

sonarqube.yml already has a documented design for failing gracefully: run every suite, produce coverage, scan, summarise, and then fail at the end from recorded step outcomes. The comments on Fail if either suite failed spell it out, and #142 fixed it once already.

The integration suite was never wired into it.

- name: Backend integration tests with coverage    # no id, no continue-on-error
  run: npm run test:integration:cov

Compare the unit step above it and the end-to-end step below it, both of which carry id and continue-on-error: true and both of which the final gate checks. Integration carries neither and is absent from the gate.

So a failing integration suite aborts the job where the other two do not. That is what has been happening on every push since #154 started: run 523 on current main shows step 9 failing and steps 10 through 15 skipped, including SonarQube Scan.

The cost

There has been no SonarQube analysis at all for the duration of #154. Not a degraded one — none. The scan is skipped, so code smells, bugs and duplication go unreported on every commit, and the dashboard reflects whenever this last worked.

The end-to-end suite has not run in CI either, which is also why #116 cannot currently be verified: its fix is a CI-only database change, and the step that would demonstrate it is skipped rather than failing.

This is worth fixing on its own terms and independently of #154's cause, which is still unknown and still needs runner-side access.

Not just the one step

Four more steps sit between the integration suite and the scan with no guard, so each can skip the scan the same way:

Start backend for the end-to-end run — exits 1 when the backend does not come up within 30s
Install Playwright browsers — a network fetch
Merge frontend coverage — deliberately fails when nothing was collected
SonarQube Scan itself, which currently takes the summaries down with it if the scanner errors

Fixing only the integration step would leave the same shape in four other places.

The shape of the fix

Extend the existing pattern rather than inventing a new one. Every step from the first suite to the scan gets an id and continue-on-error: true, and the final gate checks all of them. The preconditions before that — checkout, installs, build checks, migrations — keep failing hard, because when they fail there is genuinely nothing to analyse.

The job still fails. It fails at the end, having produced everything it could, which is what the existing comments already argue for.

Also worth doing here

Integration has no summariser. Unit and end-to-end both get readable pass/fail counts; integration produces raw jest output. summarize-jest.js already takes a label, so this is reuse rather than new code. #154 records that the failure presenting as 36 assertion errors about categories and price filters is what made the original diagnosis expensive.

Nothing guards the gate. The bug being fixed here is precisely "a step was added and the gate was not updated", which is the same failure composeEnvironment.test.ts exists to prevent for docker-compose. A unit test that reads the workflow and asserts every guarded step is represented in the gate would stop the next one.

Accepted trade-off

If a coverage report is genuinely missing — the end-to-end run collecting nothing, say — the scan now runs anyway and SonarQube reports those files as uncovered, which looks like a coverage regression rather than a missing input.

That is accepted, for three reasons: in the failure actually occurring, jest still writes coverage for a suite whose tests fail, so all three reports exist; the scan has no sonar.qualitygate.wait, so a degraded run marks the dashboard and is overwritten by the next good one rather than blocking anything; and the job fails regardless, so no run in this state is mistaken for a clean one.

Overriding sonar.javascript.lcov.reportPaths in CI to skip missing files was considered and rejected — sonar-project.properties deliberately keeps that configuration out of the workflow so a local scan and a CI scan analyse the same thing.

`sonarqube.yml` already has a documented design for failing gracefully: run every suite, produce coverage, scan, summarise, and then fail at the end from recorded step outcomes. The comments on `Fail if either suite failed` spell it out, and #142 fixed it once already. The integration suite was never wired into it. ```yaml - name: Backend integration tests with coverage # no id, no continue-on-error run: npm run test:integration:cov ``` Compare the unit step above it and the end-to-end step below it, both of which carry `id` and `continue-on-error: true` and both of which the final gate checks. Integration carries neither and is absent from the gate. So a failing integration suite aborts the job where the other two do not. That is what has been happening on **every push since #154 started**: run 523 on current `main` shows step 9 failing and steps 10 through 15 skipped, including `SonarQube Scan`. ## The cost **There has been no SonarQube analysis at all for the duration of #154.** Not a degraded one — none. The scan is skipped, so code smells, bugs and duplication go unreported on every commit, and the dashboard reflects whenever this last worked. The end-to-end suite has not run in CI either, which is also why #116 cannot currently be verified: its fix is a CI-only database change, and the step that would demonstrate it is skipped rather than failing. This is worth fixing on its own terms and independently of #154's cause, which is still unknown and still needs runner-side access. ## Not just the one step Four more steps sit between the integration suite and the scan with no guard, so each can skip the scan the same way: - `Start backend for the end-to-end run` — exits 1 when the backend does not come up within 30s - `Install Playwright browsers` — a network fetch - `Merge frontend coverage` — deliberately fails when nothing was collected - `SonarQube Scan` itself, which currently takes the summaries down with it if the scanner errors Fixing only the integration step would leave the same shape in four other places. ## The shape of the fix Extend the existing pattern rather than inventing a new one. Every step from the first suite to the scan gets an `id` and `continue-on-error: true`, and the final gate checks all of them. The preconditions before that — checkout, installs, build checks, migrations — keep failing hard, because when they fail there is genuinely nothing to analyse. The job still fails. It fails at the end, having produced everything it could, which is what the existing comments already argue for. ## Also worth doing here **Integration has no summariser.** Unit and end-to-end both get readable pass/fail counts; integration produces raw jest output. `summarize-jest.js` already takes a label, so this is reuse rather than new code. #154 records that the failure presenting as 36 assertion errors about categories and price filters is what made the original diagnosis expensive. **Nothing guards the gate.** The bug being fixed here is precisely "a step was added and the gate was not updated", which is the same failure `composeEnvironment.test.ts` exists to prevent for docker-compose. A unit test that reads the workflow and asserts every guarded step is represented in the gate would stop the next one. ## Accepted trade-off If a coverage report is genuinely missing — the end-to-end run collecting nothing, say — the scan now runs anyway and SonarQube reports those files as uncovered, which looks like a coverage regression rather than a missing input. That is accepted, for three reasons: in the failure actually occurring, jest still writes coverage for a suite whose tests fail, so all three reports exist; the scan has no `sonar.qualitygate.wait`, so a degraded run marks the dashboard and is overwritten by the next good one rather than blocking anything; and the job fails regardless, so no run in this state is mistaken for a clean one. Overriding `sonar.javascript.lcov.reportPaths` in CI to skip missing files was considered and rejected — `sonar-project.properties` deliberately keeps that configuration out of the workflow so a local scan and a CI scan analyse the same thing.

bermudalamb referenced this issue from a commit

2026-08-25 08:47:57 -05:00

ci: fail at the end rather than part way through, so the scan still runs (#174)

bermudalamb referenced a pull request that will close this issue

2026-08-25 08:49:27 -05:00

ci: fail at the end rather than part way through, so the scan still runs (#174) #176

bermudalamb added reference feature/174-graceful-sonarqube-failure

2026-08-25 08:49:35 -05:00

bermudalamb added this to the Code Quality and Hardening 2 project 2026-08-25 08:49:42 -05:00

bermudalamb moved this to In Progress in Code Quality and Hardening 2 on 2026-08-25 08:49:48 -05:00

bermudalamb self-assigned this 2026-08-25 08:49:56 -05:00

bermudalamb closed this issue