From d37db16828672a2720d31c0394578ef0ef7cf9ef Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Sat, 17 Oct 2026 01:34:30 +0200 Subject: [PATCH 1/4] fix: distinguish between `skipped` and `not run` tests --- src/reportBuilder.ts | 13 +++++++- tests/retries.spec.ts | 72 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/reportBuilder.ts b/src/reportBuilder.ts index 89ea9b5..2c35505 100644 --- a/src/reportBuilder.ts +++ b/src/reportBuilder.ts @@ -156,14 +156,25 @@ async function toFKSuites(context: ProcessingContext, pwSuite: Suite, parentFKSu return [suite]; } +// A test skipped without calling test.skip() did not run: for example, a test in a +// serial suite after an earlier test failed. Playwright still reports a "skipped" result +// for it, but tells it apart from an explicit skip by `expectedStatus`. +function didNotRun(pwTest: TestCase, result: TestResult) { + return result.status === 'skipped' && pwTest.expectedStatus !== 'skipped'; +} + async function toFKTest(context: ProcessingContext, pwTest: TestCase, parentFKSuites: FK.Suite[]): Promise { + // Playwright ignores results of tests that did not run and interrupted results + // when it computes a test outcome, so we do not report them either. + const results = Array.from(context.results.get(pwTest) ?? new Set()) + .filter(result => !didNotRun(pwTest, result) && result.status !== 'interrupted'); const test: FK.Test = { title: pwTest.title, // Playwright Test tags must start with '@' so we cut it off. tags: pwTest.tags.map(tag => tag.startsWith('@') ? tag.substring(1) : tag), location: createLocation(context, pwTest.location), // de-duplication of tests will happen later, so here we will have all attempts. - attempts: await Promise.all(Array.from(context.results.get(pwTest) ?? new Set()).map(result => toFKRunAttempt(context, pwTest, result))), + attempts: await Promise.all(results.map(result => toFKRunAttempt(context, pwTest, result))), }; const fkTestId = computeFKTestId(test, parentFKSuites); diff --git a/tests/retries.spec.ts b/tests/retries.spec.ts index 2f9c7e6..32d44f0 100644 --- a/tests/retries.spec.ts +++ b/tests/retries.spec.ts @@ -28,3 +28,75 @@ test('should handle retries', async ({}, testInfo) => { assertCount(testNoRetry.attempts, 1); }); + +test('should drop attempts that did not run', async ({}, testInfo) => { + const { report } = await generateFlakinessReport(testInfo, { + 'smoke.spec.ts': ` + import { test, expect } from '@playwright/test'; + + test.describe.serial('serial', () => { + test('first', async ({}, testInfo) => { + expect(testInfo.retry).toBeGreaterThan(0); + }); + + test('second', async () => { + expect(1).toBe(2); + }); + + test('third', async () => {}); + }); + `, + }, {}, { + retries: 2, + }); + + const [fileSuite] = assertCount(report.suites, 1); + const [serialSuite] = assertCount(fileSuite.suites, 1); + const [first, second, third] = assertCount(serialSuite.tests, 3); + expect(first.attempts.map(a => a.status ?? 'passed')).toEqual(['failed', 'passed', 'passed']); + // Playwright: failed. The first attempt did not run, so it is not reported. + expect(second.attempts.map(a => a.status ?? 'passed')).toEqual(['failed', 'failed']); + // Playwright: did not run. Same as tests that never started because of --max-failures. + expect(third.attempts).toEqual([]); +}); + +test('should keep explicit skips on retry', async ({}, testInfo) => { + const { report } = await generateFlakinessReport(testInfo, { + 'smoke.spec.ts': ` + import { test, expect } from '@playwright/test'; + + test('skips on retry', async ({}, testInfo) => { + test.skip(testInfo.retry > 0, 'skip on retry'); + expect(1).toBe(2); + }); + `, + }, {}, { + retries: 1, + }); + + const [fileSuite] = assertCount(report.suites, 1); + const [skipsOnRetry] = assertCount(fileSuite.tests, 1); + expect(skipsOnRetry.attempts.map(a => a.status ?? 'passed')).toEqual(['failed', 'skipped']); + expect(skipsOnRetry.attempts.map(a => a.expectedStatus ?? 'passed')).toEqual(['skipped', 'skipped']); +}); + +test('should drop interrupted attempts', async ({}, testInfo) => { + const { report } = await generateFlakinessReport(testInfo, { + 'smoke.spec.ts': ` + import { test, expect } from '@playwright/test'; + + test('interrupted on retry', async ({}, testInfo) => { + expect(testInfo.retry).toBeGreaterThan(0); + // Interrupt the test runner, as if a user pressed Ctrl-C. + process.kill(process.ppid, 'SIGINT'); + await new Promise(() => {}); + }); + `, + }, {}, { + retries: 1, + }); + + const [fileSuite] = assertCount(report.suites, 1); + const [interruptedOnRetry] = assertCount(fileSuite.tests, 1); + expect(interruptedOnRetry.attempts.map(a => a.status ?? 'passed')).toEqual(['failed']); +}); From 79748b1f152d09a1ea5d81c5b3e4c4ef6db488f9 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Sat, 17 Oct 2026 14:34:01 -0700 Subject: [PATCH 2/4] fix --- tests/retries.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/retries.spec.ts b/tests/retries.spec.ts index 32d44f0..2aa7b67 100644 --- a/tests/retries.spec.ts +++ b/tests/retries.spec.ts @@ -81,6 +81,7 @@ test('should keep explicit skips on retry', async ({}, testInfo) => { }); test('should drop interrupted attempts', async ({}, testInfo) => { + test.skip(process.platform === 'win32', 'process.kill() cannot deliver SIGINT on Windows'); const { report } = await generateFlakinessReport(testInfo, { 'smoke.spec.ts': ` import { test, expect } from '@playwright/test'; From 3bbe8fefb48d2a4e44f00c093e47c2774ae3559d Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Sat, 17 Oct 2026 15:21:34 -0700 Subject: [PATCH 3/4] reword --- src/reportBuilder.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/reportBuilder.ts b/src/reportBuilder.ts index 2c35505..4f14d64 100644 --- a/src/reportBuilder.ts +++ b/src/reportBuilder.ts @@ -156,18 +156,24 @@ async function toFKSuites(context: ProcessingContext, pwSuite: Suite, parentFKSu return [suite]; } -// A test skipped without calling test.skip() did not run: for example, a test in a -// serial suite after an earlier test failed. Playwright still reports a "skipped" result -// for it, but tells it apart from an explicit skip by `expectedStatus`. -function didNotRun(pwTest: TestCase, result: TestResult) { - return result.status === 'skipped' && pwTest.expectedStatus !== 'skipped'; +// Tests might get results even if they were not executed. +// We want to filter out such results. +function isExecutionResult(pwTest: TestCase, result: TestResult) { + // A test that was interrupted in-flight gets the "interrupted" status. We don't want to report these. + if (result.status === 'interrupted') + return false; + // Filter out tests that were skipped because they were a part of a `.serial` suite + // that was aborted due to early failure, + if (result.status === 'skipped' && pwTest.expectedStatus !== 'skipped') + return false; + return true; } async function toFKTest(context: ProcessingContext, pwTest: TestCase, parentFKSuites: FK.Suite[]): Promise { // Playwright ignores results of tests that did not run and interrupted results // when it computes a test outcome, so we do not report them either. const results = Array.from(context.results.get(pwTest) ?? new Set()) - .filter(result => !didNotRun(pwTest, result) && result.status !== 'interrupted'); + .filter(result => isExecutionResult(pwTest, result)); const test: FK.Test = { title: pwTest.title, // Playwright Test tags must start with '@' so we cut it off. From ccccc0cdf82467da1c2d555d2ad4c3ead0fa77ab Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Sat, 17 Oct 2026 15:23:52 -0700 Subject: [PATCH 4/4] more rewording --- src/reportBuilder.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/reportBuilder.ts b/src/reportBuilder.ts index 4f14d64..be05095 100644 --- a/src/reportBuilder.ts +++ b/src/reportBuilder.ts @@ -156,22 +156,20 @@ async function toFKSuites(context: ProcessingContext, pwSuite: Suite, parentFKSu return [suite]; } -// Tests might get results even if they were not executed. -// We want to filter out such results. +// Tests might get results even if they did not run to completion. +// Playwright ignores such results when it computes a test outcome, so we do not report them either. function isExecutionResult(pwTest: TestCase, result: TestResult) { - // A test that was interrupted in-flight gets the "interrupted" status. We don't want to report these. + // A test that was interrupted in-flight gets the "interrupted" status. if (result.status === 'interrupted') return false; - // Filter out tests that were skipped because they were a part of a `.serial` suite - // that was aborted due to early failure, + // A test that was skipped without calling `test.skip()` did not run: e.g. it is a part + // of a `.serial` suite where an earlier test failed. if (result.status === 'skipped' && pwTest.expectedStatus !== 'skipped') return false; return true; } async function toFKTest(context: ProcessingContext, pwTest: TestCase, parentFKSuites: FK.Suite[]): Promise { - // Playwright ignores results of tests that did not run and interrupted results - // when it computes a test outcome, so we do not report them either. const results = Array.from(context.results.get(pwTest) ?? new Set()) .filter(result => isExecutionResult(pwTest, result)); const test: FK.Test = {