diff --git a/packages/playwright/src/common/DEPS.list b/packages/playwright/src/common/DEPS.list index ab654c7f90ba9..d134d5a4697d3 100644 --- a/packages/playwright/src/common/DEPS.list +++ b/packages/playwright/src/common/DEPS.list @@ -2,6 +2,7 @@ ../util.ts ../utilsBundle.ts ../transform +../isomorphic/teleReceiver.ts [testType.ts] ../matchers/expect.ts diff --git a/packages/playwright/src/common/test.ts b/packages/playwright/src/common/test.ts index 5f22f8fe3b05c..9cf88dc634f5d 100644 --- a/packages/playwright/src/common/test.ts +++ b/packages/playwright/src/common/test.ts @@ -20,6 +20,7 @@ import type { TestTypeImpl } from './testType'; import { rootTestType } from './testType'; import type { Annotation, FixturesWithLocation, FullProjectInternal } from './config'; import type { Location, FullProject } from '../../types/testReporter'; +import { computeTestCaseOutcome } from '../isomorphic/teleReceiver'; class Base { title: string; @@ -280,21 +281,7 @@ export class TestCase extends Base implements reporterTypes.TestCase { } outcome(): 'skipped' | 'expected' | 'unexpected' | 'flaky' { - // Ignore initial skips that may be a result of "skipped because previous test in serial mode failed". - const results = [...this.results]; - while (results[0]?.status === 'skipped' || results[0]?.status === 'interrupted') - results.shift(); - - // All runs were skipped. - if (!results.length) - return 'skipped'; - - const failures = results.filter(result => result.status !== 'skipped' && result.status !== 'interrupted' && result.status !== this.expectedStatus); - if (!failures.length) // all passed - return 'expected'; - if (failures.length === results.length) // all failed - return 'unexpected'; - return 'flaky'; // mixed bag + return computeTestCaseOutcome(this); } ok(): boolean { diff --git a/packages/playwright/src/isomorphic/teleReceiver.ts b/packages/playwright/src/isomorphic/teleReceiver.ts index 15feb60370bca..d2cc5e5cd8413 100644 --- a/packages/playwright/src/isomorphic/teleReceiver.ts +++ b/packages/playwright/src/isomorphic/teleReceiver.ts @@ -494,21 +494,7 @@ export class TeleTestCase implements reporterTypes.TestCase { } outcome(): 'skipped' | 'expected' | 'unexpected' | 'flaky' { - // Ignore initial skips that may be a result of "skipped because previous test in serial mode failed". - const results = [...this.results]; - while (results[0]?.status === 'skipped' || results[0]?.status === 'interrupted') - results.shift(); - - // All runs were skipped. - if (!results.length) - return 'skipped'; - - const failures = results.filter(result => result.status !== 'skipped' && result.status !== 'interrupted' && result.status !== this.expectedStatus); - if (!failures.length) // all passed - return 'expected'; - if (failures.length === results.length) // all failed - return 'unexpected'; - return 'flaky'; // mixed bag + return computeTestCaseOutcome(this); } ok(): boolean { @@ -643,3 +629,47 @@ export function parseRegexPatterns(patterns: JsonPattern[]): (string | RegExp)[] return new RegExp(p.r!.source, p.r!.flags); }); } + +export function computeTestCaseOutcome(test: reporterTypes.TestCase) { + let skipped = 0; + let didNotRun = 0; + let expected = 0; + let interrupted = 0; + let unexpected = 0; + for (const result of test.results) { + if (result.status === 'interrupted') { + ++interrupted; // eslint-disable-line @typescript-eslint/no-unused-vars + } else if (result.status === 'skipped' && test.expectedStatus === 'skipped') { + // Only tests "expected to be skipped" are skipped. These were specifically + // marked with test.skip or test.fixme. + ++skipped; + } else if (result.status === 'skipped') { + // Tests that were expected to run, but were skipped are "did not run". + // This happens when: + // - testing finished early; + // - test failure prevented other tests in the serial suite to run; + // - probably more cases! + ++didNotRun; // eslint-disable-line @typescript-eslint/no-unused-vars + } else if (result.status === test.expectedStatus) { + // Either passed and expected to pass, or failed and expected to fail. + ++expected; + } else { + ++unexpected; + } + } + + // Tests that were "skipped as expected" are considered equal to "expected" below, + // because that's the expected outcome. + // + // However, we specifically differentiate the case of "only skipped" + // and show it as "skipped" in all reporters. + // + // More exotic cases like "failed on first run and skipped on retry" are flaky. + if (expected === 0 && unexpected === 0) + return 'skipped'; // all results were skipped or interrupted + if (unexpected === 0) + return 'expected'; // no failures, just expected+skipped + if (expected === 0 && skipped === 0) + return 'unexpected'; // only failures + return 'flaky'; // expected+unexpected or skipped+unexpected +} diff --git a/packages/playwright/src/reporters/base.ts b/packages/playwright/src/reporters/base.ts index 0f1f7bb0ce059..f4da4a74a0f2b 100644 --- a/packages/playwright/src/reporters/base.ts +++ b/packages/playwright/src/reporters/base.ts @@ -216,7 +216,7 @@ export class BaseReporter implements ReporterV2 { if (test.results.some(result => !!result.error)) interruptedToPrint.push(test); interrupted.push(test); - } else if (!test.results.length) { + } else if (!test.results.length || test.expectedStatus !== 'skipped') { ++didNotRun; } else { ++skipped; diff --git a/tests/playwright-test/hooks.spec.ts b/tests/playwright-test/hooks.spec.ts index 4d8999ba3f149..d72cd7abb1431 100644 --- a/tests/playwright-test/hooks.spec.ts +++ b/tests/playwright-test/hooks.spec.ts @@ -392,14 +392,14 @@ test('beforeAll failure should prevent the test, but not afterAll', async ({ run test('failed', () => { console.log('\\n%%test1'); }); - test('skipped', () => { + test('does not run', () => { console.log('\\n%%test2'); }); `, }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); - expect(result.skipped).toBe(1); + expect(result.didNotRun).toBe(1); expect(result.outputLines).toEqual([ 'beforeAll', 'afterAll', @@ -492,14 +492,14 @@ test('beforeAll timeout should be reported and prevent more tests', async ({ run test('failed', () => { console.log('\\n%%test1'); }); - test('skipped', () => { + test('does not run', () => { console.log('\\n%%test2'); }); `, }, { timeout: 1000 }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); - expect(result.skipped).toBe(1); + expect(result.didNotRun).toBe(1); expect(result.outputLines).toEqual([ 'beforeAll', 'afterAll', @@ -681,14 +681,14 @@ test('unhandled rejection during beforeAll should be reported and prevent more t test('failed', () => { console.log('\\n%%test1'); }); - test('skipped', () => { + test('does not run', () => { console.log('\\n%%test2'); }); `, }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); - expect(result.skipped).toBe(1); + expect(result.didNotRun).toBe(1); expect(result.outputLines).toEqual([ 'beforeAll', 'afterAll', @@ -790,7 +790,7 @@ test('beforeAll failure should only prevent tests that are affected', async ({ r test('failed', () => { console.log('\\n%%test1'); }); - test('skipped', () => { + test('does not run', () => { console.log('\\n%%test2'); }); }); @@ -801,7 +801,7 @@ test('beforeAll failure should only prevent tests that are affected', async ({ r }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); - expect(result.skipped).toBe(1); + expect(result.didNotRun).toBe(1); expect(result.passed).toBe(1); expect(result.outputLines).toEqual([ 'beforeAll', diff --git a/tests/playwright-test/retry.spec.ts b/tests/playwright-test/retry.spec.ts index d6cad8a0801c8..b5dfc7a347f79 100644 --- a/tests/playwright-test/retry.spec.ts +++ b/tests/playwright-test/retry.spec.ts @@ -216,7 +216,7 @@ test('should retry beforeAll failure', async ({ runInlineTest }) => { expect(result.exitCode).toBe(1); expect(result.passed).toBe(0); expect(result.failed).toBe(1); - expect(result.skipped).toBe(1); + expect(result.didNotRun).toBe(1); expect(result.output.split('\n')[2]).toBe('×°×°F°'); expect(result.output).toContain('BeforeAll is bugged!'); }); diff --git a/tests/playwright-test/runner.spec.ts b/tests/playwright-test/runner.spec.ts index 955c8e1c0cafc..617dab904e8ce 100644 --- a/tests/playwright-test/runner.spec.ts +++ b/tests/playwright-test/runner.spec.ts @@ -103,7 +103,7 @@ test('should report subprocess creation error', async ({ runInlineTest }, testIn 'a.spec.js': ` import { test, expect } from '@playwright/test'; test('fails', () => {}); - test('skipped', () => {}); + test('does not run', () => {}); // Infect subprocesses to immediately exit when spawning a worker. process.env.NODE_OPTIONS = '--require ${JSON.stringify(testInfo.outputPath('preload.js').replace(/\\/g, '\\\\'))}'; ` @@ -111,7 +111,7 @@ test('should report subprocess creation error', async ({ runInlineTest }, testIn expect(result.exitCode).toBe(1); expect(result.passed).toBe(0); expect(result.failed).toBe(1); - expect(result.skipped).toBe(1); + expect(result.didNotRun).toBe(1); expect(result.output).toContain('Error: worker process exited unexpectedly (code=42, signal=null)'); }); diff --git a/tests/playwright-test/test-modifiers.spec.ts b/tests/playwright-test/test-modifiers.spec.ts index 0b947691078a1..2b206b448ac1f 100644 --- a/tests/playwright-test/test-modifiers.spec.ts +++ b/tests/playwright-test/test-modifiers.spec.ts @@ -677,13 +677,14 @@ test('static modifiers should be added in serial mode', async ({ runInlineTest } }); test.skip('skipped', async ({}) => { }); - test('ignored', async ({}) => { + test('does not run', async ({}) => { }); `, }); expect(result.exitCode).toBe(1); expect(result.passed).toBe(0); - expect(result.skipped).toBe(3); + expect(result.skipped).toBe(2); + expect(result.didNotRun).toBe(1); expect(result.report.suites[0].specs[0].tests[0].annotations).toEqual([{ type: 'slow' }]); expect(result.report.suites[0].specs[1].tests[0].annotations).toEqual([{ type: 'fixme' }]); expect(result.report.suites[0].specs[2].tests[0].annotations).toEqual([{ type: 'skip' }]); diff --git a/tests/playwright-test/test-serial.spec.ts b/tests/playwright-test/test-serial.spec.ts index 085d175097028..5059b8108b0c0 100644 --- a/tests/playwright-test/test-serial.spec.ts +++ b/tests/playwright-test/test-serial.spec.ts @@ -47,7 +47,7 @@ test('test.describe.serial should work', async ({ runInlineTest }) => { expect(result.exitCode).toBe(1); expect(result.passed).toBe(2); expect(result.failed).toBe(1); - expect(result.skipped).toBe(2); + expect(result.didNotRun).toBe(2); expect(result.outputLines).toEqual([ 'test1', 'test2', @@ -87,7 +87,7 @@ test('test.describe.serial should work in describe', async ({ runInlineTest }) = expect(result.exitCode).toBe(1); expect(result.passed).toBe(2); expect(result.failed).toBe(1); - expect(result.skipped).toBe(2); + expect(result.didNotRun).toBe(2); expect(result.outputLines).toEqual([ 'test1', 'test2', @@ -128,7 +128,7 @@ test('test.describe.serial should work with retry', async ({ runInlineTest }) => expect(result.passed).toBe(2); expect(result.flaky).toBe(1); expect(result.failed).toBe(1); - expect(result.skipped).toBe(1); + expect(result.didNotRun).toBe(1); expect(result.outputLines).toEqual([ 'test1', 'test2', @@ -272,7 +272,7 @@ test('test.describe.serial should work with test.fail', async ({ runInlineTest } expect(result.exitCode).toBe(1); expect(result.passed).toBe(2); expect(result.failed).toBe(1); - expect(result.skipped).toBe(1); + expect(result.didNotRun).toBe(1); expect(result.outputLines).toEqual([ 'zero', 'one', @@ -394,3 +394,55 @@ test('test.describe.serial should work with fullyParallel', async ({ runInlineTe 'two', ]); }); + +test('serial fail + skip is failed', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + test.describe.configure({ mode: 'serial', retries: 1 }); + test.describe.serial('serial suite', () => { + test('one', async () => { + expect(test.info().retry).toBe(0); + }); + test('two', async () => { + expect(1).toBe(2); + }); + test('three', async () => { + }); + }); + `, + }, { workers: 1 }); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(0); + expect(result.skipped).toBe(0); + expect(result.flaky).toBe(1); + expect(result.failed).toBe(1); + expect(result.interrupted).toBe(0); + expect(result.didNotRun).toBe(1); +}); + +test('serial skip + fail is failed', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + test.describe.configure({ mode: 'serial', retries: 1 }); + test.describe.serial('serial suite', () => { + test('one', async () => { + expect(test.info().retry).toBe(1); + }); + test('two', async () => { + expect(1).toBe(2); + }); + test('three', async () => { + }); + }); + `, + }, { workers: 1 }); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(0); + expect(result.skipped).toBe(0); + expect(result.flaky).toBe(1); + expect(result.failed).toBe(1); + expect(result.interrupted).toBe(0); + expect(result.didNotRun).toBe(1); +});