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

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

bermudalamb commented

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

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

Both scripts already carried the principle in a comment — report plainly and exit 0, because the job fails on the real step and a stack trace here would only bury it — and both only implemented it for the file being absent. A file that exists and cannot be read crashed them.

Two ways to reach that, both reproduced. --forceExit, which the integration script passes to paper over a post-run hang, can end the process around the write and leave partial JSON. And a suite that fails to run rather than to assert arrives without the array the failure renderer walks, which is exactly the shape this suite has been producing under #154.

Reading is now guarded as thoroughly as summarize-playwright.js already guarded its traversal, and that traversal's || [] discipline is extended to the jest renderer. summarize-playwright.js had the same hole by the narrower path of an unguarded JSON.parse.

The reason reaches the log rather than being swallowed. "Could not read the results file" with the parse error is diagnostic; a silent empty summary is not.

This matters beyond tidiness because of where the failure lands. A crash here reports the job as failing at a step named for summarising rather than for testing, which is the misdirection #142 fixed once already — and a summariser whose job is to make a failing run readable should not crash on the output of the worst failures, which is the moment it is most needed.

Verified against a truncated file, a suite entry with no testResults, an absent file, and a real 278-test run: the first three now exit 0 naming the reason, the absent case is unchanged, and the happy path still reports its counts.

Closes #178

Run 525 was the first with #174's graceful failure, and it worked: the integration suite failed, the end-to-end suite ran again, and `SonarQube Scan` succeeded for the first time since #154 started. But `Summarize integration tests` failed, and that step should not be able to. Both scripts already carried the principle in a comment — report plainly and exit 0, because the job fails on the real step and a stack trace here would only bury it — and both only implemented it for the file being absent. A file that exists and cannot be read crashed them. Two ways to reach that, both reproduced. `--forceExit`, which the integration script passes to paper over a post-run hang, can end the process around the write and leave partial JSON. And a suite that fails to *run* rather than to assert arrives without the array the failure renderer walks, which is exactly the shape this suite has been producing under #154. Reading is now guarded as thoroughly as `summarize-playwright.js` already guarded its traversal, and that traversal's `|| []` discipline is extended to the jest renderer. `summarize-playwright.js` had the same hole by the narrower path of an unguarded `JSON.parse`. The reason reaches the log rather than being swallowed. "Could not read the results file" with the parse error is diagnostic; a silent empty summary is not. This matters beyond tidiness because of where the failure lands. A crash here reports the job as failing at a step named for summarising rather than for testing, which is the misdirection #142 fixed once already — and a summariser whose job is to make a failing run readable should not crash on the output of the worst failures, which is the moment it is most needed. Verified against a truncated file, a suite entry with no `testResults`, an absent file, and a real 278-test run: the first three now exit 0 naming the reason, the absent case is unchanged, and the happy path still reports its counts. Closes #178

bermudalamb self-assigned this 2026-08-25 09:29:05 -05:00

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

bermudalamb added 1 commit 2026-08-25 09:29:06 -05:00

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

SonarQube Analysis / sonarqube (pull_request) Failing after 21m21s

Details

Linting / lint (pull_request) Successful in 2m0s

Details

5d427bc1a1

Run 525 was the first with #174's graceful failure, and it worked: the integration suite failed, the end-to-end suite ran again, and `SonarQube Scan` succeeded for the first time since #154 started. But `Summarize integration tests` failed, and that step should not be able to.

Both scripts already carried the principle in a comment — report plainly and exit 0, because the job fails on the real step and a stack trace here would only bury it — and both only implemented it for the file being absent. A file that exists and cannot be read crashed them.

Two ways to reach that, both reproduced. `--forceExit`, which the integration script passes to paper over a post-run hang, can end the process around the write and leave partial JSON. And a suite that fails to *run* rather than to assert arrives without the array the failure renderer walks, which is exactly the shape this suite has been producing under #154.

Reading is now guarded as thoroughly as `summarize-playwright.js` already guarded its traversal, and that traversal's `|| []` discipline is extended to the jest renderer. `summarize-playwright.js` had the same hole by the narrower path of an unguarded `JSON.parse`.

The reason reaches the log rather than being swallowed. "Could not read the results file" with the parse error is diagnostic; a silent empty summary is not.

This matters beyond tidiness because of where the failure lands. A crash here reports the job as failing at a step named for summarising rather than for testing, which is the misdirection #142 fixed once already — and a summariser whose job is to make a failing run readable should not crash on the output of the worst failures, which is the moment it is most needed.

Verified against a truncated file, a suite entry with no `testResults`, an absent file, and a real 278-test run: the first three now exit 0 naming the reason, the absent case is unchanged, and the happy path still reports its counts.

Closes #178

bermudalamb moved this to Review in Code Quality and Hardening 2 on 2026-08-25 09:29:10 -05:00

bermudalamb merged commit e3514e8ef4 into main

2026-08-25 11:49:12 -05:00

bermudalamb deleted branch feature/178-resilient-summarisers

2026-08-25 11:49:12 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#179