The test summarisers fail the job on an unreadable results file, which is the one thing they exist not to do #178

New Issue

2026-08-25T09:18:54-05:00

bermudalamb commented

2026-08-25 09:18:54 -05:00

Run 525 — the first run with #174's graceful failure — did what it was meant to: the integration suite failed, the end-to-end suite ran, and SonarQube Scan succeeded for the first time since #154 started. But Summarize integration tests failed, and it should not be able to.

Both summarisers already carry the principle in a comment. From summarize-jest.js:

This runs with if: always(), so it also fires when an earlier step failed and no results file was ever written. Report that plainly and exit 0 — the job still fails on the real step, and a stack trace here would only bury it.

That is exactly right, and it only covers the file being absent. A file that exists and cannot be read crashes them.

Reproduced

(a) truncated JSON
    SyntaxError: Unterminated string in JSON at position 103
    exit=1

(b) a suite entry with no testResults array
    for (const t of suite.testResults) {
                          ^
    exit=1

Both are reachable in this workflow. test:integration:cov passes --forceExit, which exists to paper over a post-run hang and can end the process around the write, leaving a partial file. And a suite that fails to run — as opposed to failing its assertions — is exactly the shape this suite has been producing under #154.

summarize-playwright.js guards its traversal thoroughly with || [] at every level but leaves JSON.parse unguarded, so it has the same hole by a narrower path.

Why it matters beyond tidiness

The failure lands on a step named "Summarize integration tests". That reads as a broken summary script rather than as failing tests, which is precisely the misdirection #142 fixed once already for the end-to-end summariser — and the reason that fix exists is that the job then reports its failure under a name describing summarising rather than testing.

It is also self-defeating. The summariser's whole job is to make a failing run readable. Crashing on the output of the worst failures is failing at the one moment it is most needed.

Fix

Guard reading and rendering, not just existence. Report what went wrong with the file, in the summary and on stdout, and exit 0 — leaving the gate to fail the job, which it does.

The reason has to reach the log. "Could not read the results file" with the parse error is diagnostic; a silent empty summary is not.

Run 525 — the first run with #174's graceful failure — did what it was meant to: the integration suite failed, the end-to-end suite ran, and **`SonarQube Scan` succeeded** for the first time since #154 started. But `Summarize integration tests` failed, and it should not be able to. Both summarisers already carry the principle in a comment. From `summarize-jest.js`: > This runs with `if: always()`, so it also fires when an earlier step failed and no results file was ever written. Report that plainly and exit 0 — the job still fails on the real step, and a stack trace here would only bury it. That is exactly right, and it only covers the file being *absent*. A file that exists and cannot be read crashes them. ## Reproduced ``` (a) truncated JSON SyntaxError: Unterminated string in JSON at position 103 exit=1 (b) a suite entry with no testResults array for (const t of suite.testResults) { ^ exit=1 ``` Both are reachable in this workflow. `test:integration:cov` passes `--forceExit`, which exists to paper over a post-run hang and can end the process around the write, leaving a partial file. And a suite that fails to *run* — as opposed to failing its assertions — is exactly the shape this suite has been producing under #154. `summarize-playwright.js` guards its traversal thoroughly with `|| []` at every level but leaves `JSON.parse` unguarded, so it has the same hole by a narrower path. ## Why it matters beyond tidiness The failure lands on a step named "Summarize integration tests". That reads as a broken summary script rather than as failing tests, which is precisely the misdirection #142 fixed once already for the end-to-end summariser — and the reason that fix exists is that the job then reports its failure under a name describing summarising rather than testing. It is also self-defeating. The summariser's whole job is to make a failing run readable. Crashing on the output of the worst failures is failing at the one moment it is most needed. ## Fix Guard reading and rendering, not just existence. Report what went wrong with the file, in the summary and on stdout, and exit 0 — leaving the gate to fail the job, which it does. The reason has to reach the log. "Could not read the results file" with the parse error is diagnostic; a silent empty summary is not.

bermudalamb referenced this issue from a commit

2026-08-25 09:20:38 -05:00

fix(ci): stop the test summarisers failing the job on an unreadable results file (#178)

bermudalamb referenced this issue

2026-08-25 09:21:02 -05:00

The integration suite's Postgres service loses its schema mid-run in CI, failing 12 of 17 suites #154

bermudalamb referenced a pull request that will close this issue

2026-08-25 09:29:05 -05:00

fix(ci): stop the test summarisers failing the job on an unreadable results file (#178) #179

bermudalamb referenced this issue

2026-08-25 11:34:23 -05:00

Clear the five code smells, which are the 21 minutes of new technical debt #181