From bb795ac00b8bb93580a9ce2069b18e23b6b145fc Mon Sep 17 00:00:00 2001 From: 4gray Date: Sat, 26 Sep 2026 21:52:16 +0200 Subject: [PATCH] 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 --- tools/coverage/e2e-semantic-summary.mjs | 41 ++++++++++++----- tools/coverage/e2e-shard-reports.test.mjs | 56 ++++++++++++++++++++++- 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/tools/coverage/e2e-semantic-summary.mjs b/tools/coverage/e2e-semantic-summary.mjs index 3b7a112dc..ba984f8e4 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']); +const STATUS_PRECEDENCE = ['failed', 'skipped', 'flaky']; + +// 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. +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'; + } + if (finalStatus === 'passed' && statuses.some((status) => FAILURE_STATUSES.has(status))) { + return 'flaky'; + } + return 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..9cd446da2 100644 --- a/tools/coverage/e2e-shard-reports.test.mjs +++ b/tools/coverage/e2e-shard-reports.test.mjs @@ -38,7 +38,9 @@ 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. +function playwrightReport({ shard, file, titles, status = 'passed', results = [status] }) { return { config: { shard }, suites: [ @@ -47,7 +49,7 @@ function playwrightReport({ shard, file, titles, status = 'passed' }) { specs: titles.map((title) => ({ title, tags: [], - tests: [{ results: [{ status }] }], + tests: [{ results: results.map((attempt) => ({ status: attempt })) }], })), }, ], @@ -277,6 +279,56 @@ 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('refuses to summarize an incomplete shard set', () => { const root = makeWorkspace(); const shards = path.join(root, 'shards');