From a22e436bf628fcbb49da55d952203d40ab2849e2 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sat, 26 Sep 2026 23:17:12 +0200 Subject: [PATCH] fix(coverage): keep non-passing retries failed and surface flaky over skipped Review feedback on the final-attempt status: a failure followed by a skipped or interrupted retry returned the final status and dropped the failure, so the journey read as covered. Only a final pass now turns earlier failures into flaky; any other ending after a failure stays failed. A spec runs once per Playwright project, and `skipped` outranked `flaky`, so a skip in one browser hid a retried-then-passing test in another. Flaky now outranks skipped. Co-Authored-By: Claude Opus 5.5 --- tools/coverage/e2e-semantic-summary.mjs | 20 ++++---- tools/coverage/e2e-shard-reports.test.mjs | 61 ++++++++++++++++++++++- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/tools/coverage/e2e-semantic-summary.mjs b/tools/coverage/e2e-semantic-summary.mjs index ba984f8e4..0ac1ddf36 100644 --- a/tools/coverage/e2e-semantic-summary.mjs +++ b/tools/coverage/e2e-semantic-summary.mjs @@ -82,24 +82,24 @@ function fail(message) { } const FAILURE_STATUSES = new Set(['failed', 'timedOut']); -const STATUS_PRECEDENCE = ['failed', 'skipped', 'flaky']; +// A spec runs once per Playwright project (browser). Flaky outranks skipped so +// a retried test in one browser is not hidden by a skip in another. +const STATUS_PRECEDENCE = ['failed', 'flaky', 'skipped']; -// Playwright retries a failing test and records every attempt; only the final -// attempt decides the outcome. A pass after earlier failures is flaky, which -// Playwright itself does not count as unexpected. +// Playwright retries a failing test and records every attempt. Only a final +// pass turns earlier failures into flaky, which Playwright itself does not +// count as unexpected; any other ending after a failure stays failed. function attemptsStatus(results) { const statuses = results.map((result) => result.status); const finalStatus = statuses.at(-1); if (finalStatus === undefined) { return 'unknown'; } - if (FAILURE_STATUSES.has(finalStatus)) { - return 'failed'; + const anyFailure = statuses.some((status) => FAILURE_STATUSES.has(status)); + if (finalStatus === 'passed') { + return anyFailure ? 'flaky' : 'passed'; } - if (finalStatus === 'passed' && statuses.some((status) => FAILURE_STATUSES.has(status))) { - return 'flaky'; - } - return finalStatus; + return anyFailure ? 'failed' : finalStatus; } function specStatus(spec) { diff --git a/tools/coverage/e2e-shard-reports.test.mjs b/tools/coverage/e2e-shard-reports.test.mjs index 9cd446da2..6f576ab84 100644 --- a/tools/coverage/e2e-shard-reports.test.mjs +++ b/tools/coverage/e2e-shard-reports.test.mjs @@ -40,7 +40,16 @@ function makeTemporaryDir(prefix) { // `results` lists the status of every Playwright attempt (retries included) // and applies to each title; `status` is the single-attempt shorthand. -function playwrightReport({ shard, file, titles, status = 'passed', results = [status] }) { +// `projectResults` holds one such attempt list per Playwright project, for +// specs that run in several browsers. +function playwrightReport({ + shard, + file, + titles, + status = 'passed', + results = [status], + projectResults = [results], +}) { return { config: { shard }, suites: [ @@ -49,7 +58,9 @@ function playwrightReport({ shard, file, titles, status = 'passed', results = [s specs: titles.map((title) => ({ title, tags: [], - tests: [{ results: results.map((attempt) => ({ status: attempt })) }], + tests: projectResults.map((attempts) => ({ + results: attempts.map((attempt) => ({ status: attempt })), + })), })), }, ], @@ -329,6 +340,52 @@ describe('e2e-semantic-summary CLI', () => { ); }); + it('keeps a failure followed by a non-passing retry as failed', () => { + for (const finalStatus of ['skipped', 'interrupted']) { + const root = makeWorkspace(); + const reportPath = writeReport( + root, + 'dist/test-results/electron-backend-e2e', + playwrightReport({ + file: 'src/search.e2e.ts', + titles: ['finds @search'], + results: ['failed', finalStatus], + }) + ); + + const result = runSummary(root, [`--input=${reportPath}`]); + + assert.equal(result.status, 0, result.stderr); + const markdown = readFileSync(result.summaryPath, 'utf8'); + assert.match(markdown, /Statuses: failed: 1/, finalStatus); + assert.match( + markdown, + /\| Workspace search across providers \| @search \| 1 \| failing \|/, + finalStatus + ); + } + }); + + it('reports a spec skipped in one browser and flaky in another as flaky', () => { + const root = makeWorkspace(); + const reportPath = writeReport( + root, + 'dist/test-results/electron-backend-e2e', + playwrightReport({ + file: 'src/search.e2e.ts', + titles: ['finds @search'], + projectResults: [['skipped'], ['failed', 'passed']], + }) + ); + + const result = runSummary(root, [`--input=${reportPath}`]); + + assert.equal(result.status, 0, result.stderr); + const markdown = readFileSync(result.summaryPath, 'utf8'); + assert.match(markdown, /Statuses: flaky: 1/); + assert.match(markdown, /\| Workspace search across providers \| @search \| 1 \| covered \|/); + }); + it('refuses to summarize an incomplete shard set', () => { const root = makeWorkspace(); const shards = path.join(root, 'shards');