From 5d427bc1a1207bba62f906a69a3a7aa595a2d817 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Tue, 25 Aug 2026 09:20:34 -0500
Subject: [PATCH] fix(ci): stop the test summarisers failing the job on an
 unreadable results file (#178)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 scripts/summarize-jest.js       | 26 ++++++++++++++++++++++----
 scripts/summarize-playwright.js | 16 ++++++++++++++--
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/scripts/summarize-jest.js b/scripts/summarize-jest.js
index d705ebd..7cbb283 100644
--- a/scripts/summarize-jest.js
+++ b/scripts/summarize-jest.js
@@ -12,8 +12,24 @@ if (!fs.existsSync(resultsPath)) {
   process.exit(0);
 }
 
-const raw = fs.readFileSync(resultsPath, 'utf-8');
-const data = JSON.parse(raw);
+// Existing but unreadable is a separate case from absent, and it was the one
+// that could still fail the job. `--forceExit` can end the process around the
+// write and leave a partial file, and a suite that fails to *run* rather than
+// to assert produces a shape the renderer below did not expect. Either crashed
+// the script, which failed a step named for summarising and buried the real
+// failure — the exact misdirection #142 fixed once already. See #178.
+//
+// The reason is reported rather than swallowed: "could not read the results
+// file" plus the parse error is diagnostic, where a silent empty summary is not.
+let data;
+try {
+  data = JSON.parse(fs.readFileSync(resultsPath, 'utf-8'));
+} catch (err) {
+  const why = `Could not read \`${resultsPath}\` — ${err.message}`;
+  report([`## ${label || 'Test'} Results`, '', why, '']);
+  console.log(`${label || 'Test'}: ${why}`);
+  process.exit(0);
+}
 
 const lines = [];
 lines.push(`## ${label || 'Test'} Results`);
@@ -29,8 +45,10 @@ lines.push('');
 if (data.numFailedTests > 0) {
   lines.push('### Failures');
   lines.push('');
-  for (const suite of data.testResults) {
-    for (const t of suite.testResults) {
+  // Defensive at every level, like summarize-playwright.js's traversal: a suite
+  // that failed to run can arrive without the array this walks.
+  for (const suite of data.testResults || []) {
+    for (const t of suite.testResults || []) {
       if (t.status === 'failed') {
         lines.push(`- **${t.fullName}**`);
         const msg = t.failureMessages && t.failureMessages[0]
diff --git a/scripts/summarize-playwright.js b/scripts/summarize-playwright.js
index 82fe5bd..62d30cd 100644
--- a/scripts/summarize-playwright.js
+++ b/scripts/summarize-playwright.js
@@ -12,8 +12,20 @@ if (!fs.existsSync(resultsPath)) {
   process.exit(0);
 }
 
-const raw = fs.readFileSync(resultsPath, 'utf-8');
-const data = JSON.parse(raw);
+// The traversal below is defensive at every level; reading was not. A run
+// killed part way through leaves a file that exists and does not parse, and
+// crashing on it fails a step named for summarising rather than for testing.
+// See #178.
+let data;
+try {
+  data = JSON.parse(fs.readFileSync(resultsPath, 'utf-8'));
+} catch (err) {
+  const why = `Could not read \`${resultsPath}\` — ${err.message}`;
+  report(['## Playwright E2E Results', '', why, '']);
+  console.log(`Playwright: ${why}`);
+  process.exit(0);
+}
+
 const stats = data.stats || {};
 
 const lines = [];