From 34d393adc89f19f072574f132d5830fe2ab104b4 Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sun, 27 Sep 2026 07:47:24 +0200 Subject: [PATCH] fix(coverage): report retried-then-passing e2e tests as flaky (#1706) * fix(coverage): report retried-then-passing e2e tests as flaky The semantic summary flattened every Playwright attempt of a spec and checked for `failed` first, so a test that failed and then passed on a retry was reported as `failed` and its critical journey as `failing`, although Playwright counts it as flaky with zero unexpected results. The `flaky` branch was unreachable. Derive the status from the final attempt: only a failed or timed-out final attempt is `failed`; a pass after earlier failures is `flaky`. Skipped handling is unchanged. A journey with flaky tests is therefore `covered`; the Statuses line already lists the flaky count. Co-Authored-By: Claude Fable 5.1 * 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 --------- Co-authored-by: 4gray Co-authored-by: Claude Fable 5.1 --- tools/coverage/e2e-semantic-summary.mjs | 41 +++++--- tools/coverage/e2e-shard-reports.test.mjs | 113 +++++++++++++++++++++- 2 files changed, 140 insertions(+), 14 deletions(-) diff --git a/tools/coverage/e2e-semantic-summary.mjs b/tools/coverage/e2e-semantic-summary.mjs index 3b7a112dc..0ac1ddf36 100644 --- a/tools/coverage/e2e-semantic-summary.mjs +++ b/tools/coverage/e2e-semantic-summary.mjs @@ -81,6 +81,34 @@ function fail(message) { process.exit(1); } +const FAILURE_STATUSES = new Set(['failed', 'timedOut']); +// 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 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'; + } + const anyFailure = statuses.some((status) => FAILURE_STATUSES.has(status)); + if (finalStatus === 'passed') { + return anyFailure ? 'flaky' : 'passed'; + } + return anyFailure ? 'failed' : finalStatus; +} + +function specStatus(spec) { + const statuses = (spec.tests ?? []).map((test) => attemptsStatus(test.results ?? [])); + return ( + STATUS_PRECEDENCE.find((status) => statuses.includes(status)) ?? statuses[0] ?? 'unknown' + ); +} + function collectFromPlaywrightJson(report, projectName) { const tests = []; @@ -92,18 +120,7 @@ function collectFromPlaywrightJson(report, projectName) { ...(spec.tags ?? []).map(normalizeTag), ...tagsFromTitle(title), ]); - const statuses = (spec.tests ?? []).flatMap((test) => - (test.results ?? []).map((result) => result.status) - ); - const status = statuses.includes('failed') - ? 'failed' - : statuses.includes('timedOut') - ? 'failed' - : statuses.includes('skipped') - ? 'skipped' - : statuses.length > 1 && statuses.includes('passed') - ? 'flaky' - : statuses[0] ?? 'unknown'; + const status = specStatus(spec); tests.push({ project: projectName, diff --git a/tools/coverage/e2e-shard-reports.test.mjs b/tools/coverage/e2e-shard-reports.test.mjs index b7a58a867..6f576ab84 100644 --- a/tools/coverage/e2e-shard-reports.test.mjs +++ b/tools/coverage/e2e-shard-reports.test.mjs @@ -38,7 +38,18 @@ function makeTemporaryDir(prefix) { return root; } -function playwrightReport({ shard, file, titles, status = 'passed' }) { +// `results` lists the status of every Playwright attempt (retries included) +// and applies to each title; `status` is the single-attempt shorthand. +// `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: [ @@ -47,7 +58,9 @@ function playwrightReport({ shard, file, titles, status = 'passed' }) { specs: titles.map((title) => ({ title, tags: [], - tests: [{ results: [{ status }] }], + tests: projectResults.map((attempts) => ({ + results: attempts.map((attempt) => ({ status: attempt })), + })), })), }, ], @@ -277,6 +290,102 @@ describe('e2e-semantic-summary CLI', () => { assert.match(result.stepSummary, /Reports: 3\/3 shards/); }); + it('reports a test that passes on retry as flaky and its journey as covered', () => { + const root = makeWorkspace(); + const reportPath = writeReport( + root, + 'dist/test-results/electron-backend-e2e', + playwrightReport({ + file: 'src/search.e2e.ts', + titles: ['finds @search'], + results: ['failed', '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 \|/); + const tests = JSON.parse(readFileSync(result.jsonPath, 'utf8')); + assert.deepEqual( + tests.map((test) => test.status), + ['flaky'] + ); + }); + + it('keeps a test whose final retry fails as failed and its journey as failing', () => { + const root = makeWorkspace(); + const reportPath = writeReport( + root, + 'dist/test-results/electron-backend-e2e', + playwrightReport({ + file: 'src/search.e2e.ts', + titles: ['finds @search'], + results: ['failed', 'failed', 'failed'], + }) + ); + + 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/); + assert.match(markdown, /\| Workspace search across providers \| @search \| 1 \| failing \|/); + const tests = JSON.parse(readFileSync(result.jsonPath, 'utf8')); + assert.deepEqual( + tests.map((test) => test.status), + ['failed'] + ); + }); + + 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');