From 5cf46895982ad1f422c13d3c4799f8e4ebd307eb Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 2 Jun 2026 15:58:49 +0200 Subject: [PATCH 1/8] feat(test-runner): add Reporter.plan() hook for test filtering Fixes https://github.com/microsoft/playwright/issues/40934 --- docs/src/test-reporter-api/class-reporter.md | 33 ++ docs/src/test-reporter-api/class-suite.md | 38 ++ docs/src/test-reporter-api/class-testcase.md | 38 ++ packages/playwright/src/common/ipc.ts | 1 + packages/playwright/src/common/suiteUtils.ts | 42 +- packages/playwright/src/common/test.ts | 73 ++++ .../playwright/src/isomorphic/teleReceiver.ts | 26 ++ .../src/reporters/internalReporter.ts | 10 +- .../playwright/src/reporters/multiplexer.ts | 17 + .../playwright/src/reporters/reporterV2.ts | 10 + packages/playwright/src/runner/dispatcher.ts | 2 +- packages/playwright/src/runner/loadUtils.ts | 51 +-- packages/playwright/src/runner/reporters.ts | 7 +- packages/playwright/src/worker/workerMain.ts | 8 +- packages/playwright/types/testReporter.d.ts | 114 +++++ tests/playwright-test/reporter-plan.spec.ts | 389 ++++++++++++++++++ 16 files changed, 808 insertions(+), 51 deletions(-) create mode 100644 tests/playwright-test/reporter-plan.spec.ts diff --git a/docs/src/test-reporter-api/class-reporter.md b/docs/src/test-reporter-api/class-reporter.md index 8cc8e48676d85..ac959f252d438 100644 --- a/docs/src/test-reporter-api/class-reporter.md +++ b/docs/src/test-reporter-api/class-reporter.md @@ -297,3 +297,36 @@ Result of the test run. - returns: <[boolean]> Whether this reporter uses stdio for reporting. When it does not, Playwright Test could add some output to enhance user experience. If your reporter does not print to the terminal, it is strongly recommended to return `false`. + +## optional async method: Reporter.plan +* since: v1.61 + +Called after the configuration has been resolved and before [`method: Reporter.onBegin`]. Allows a reporter to inspect the discovered test corpus and call disposition methods on individual [TestCase]s or whole [Suite]s before the run starts. Available disposition methods: + +* [`method: TestCase.skip`] / [`method: Suite.skip`] — mark expected status as `'skipped'` and append a `skip` annotation. The test body is not executed. +* [`method: TestCase.fixme`] / [`method: Suite.fixme`] — same as `skip`, but appends a `fixme` annotation. +* [`method: TestCase.fail`] / [`method: Suite.fail`] — mark expected status as `'failed'` and append a `fail` annotation. +* [`method: TestCase.exclude`] / [`method: Suite.exclude`] — remove the test from the run entirely. Excluded tests do not appear in the report and their body is not executed. + +When multiple reporters implement `plan`, they are called in registration order and disposition methods accumulate; the last write wins on conflicting `expectedStatus` changes. Built-in sharding (see [`property: TestConfig.shard`]) is applied **after** `plan()` returns, so dispositions are not silently dropped by sharding. If your reporter handles sharding itself, also implement [`method: Reporter.implementsSharding`] to disable the built-in shard filter. + +Throwing from `plan` aborts the run before [`method: Reporter.onBegin`] is called. + +### param: Reporter.plan.config +* since: v1.61 +- `config` <[FullConfig]> + +Resolved configuration. + +### param: Reporter.plan.suite +* since: v1.61 +- `suite` <[Suite]> + +The root suite that contains all projects, files and test cases. + +## optional method: Reporter.implementsSharding +* since: v1.61 +- returns: <[boolean]> + +When `true`, Playwright skips its built-in shard filter for this run, leaving sharding to the reporter (typically implemented inside [`method: Reporter.plan`] by calling [`method: TestCase.exclude`] on out-of-shard tests). If [`property: TestConfig.shard`] is also explicitly set, a warning is printed indicating the request is being ignored. + diff --git a/docs/src/test-reporter-api/class-suite.md b/docs/src/test-reporter-api/class-suite.md index 1d458c842b3f0..8f0b1923e95f4 100644 --- a/docs/src/test-reporter-api/class-suite.md +++ b/docs/src/test-reporter-api/class-suite.md @@ -85,3 +85,41 @@ Returns a list of titles from the root down to this suite. Returns the type of the suite. The Suites form the following hierarchy: `root` -> `project` -> `file` -> `describe` -> ...`describe` -> `test`. + +## method: Suite.skip +* since: v1.61 + +Mark every descendant [TestCase] of this suite as skipped. Intended for use from [`method: Reporter.plan`]. See [`method: TestCase.skip`] for per-test semantics. + +### param: Suite.skip.reason +* since: v1.61 +- `reason` ?<[string]> + +Optional explanation surfaced as the annotation description. + +## method: Suite.fixme +* since: v1.61 + +Mark every descendant [TestCase] of this suite as fixme. Intended for use from [`method: Reporter.plan`]. See [`method: TestCase.fixme`] for per-test semantics. + +### param: Suite.fixme.reason +* since: v1.61 +- `reason` ?<[string]> + +Optional explanation surfaced as the annotation description. + +## method: Suite.fail +* since: v1.61 + +Mark every descendant [TestCase] of this suite as expected-to-fail. Intended for use from [`method: Reporter.plan`]. See [`method: TestCase.fail`] for per-test semantics. + +### param: Suite.fail.reason +* since: v1.61 +- `reason` ?<[string]> + +Optional explanation surfaced as the annotation description. + +## method: Suite.exclude +* since: v1.61 + +Remove every descendant [TestCase] of this suite from the run. Excluded tests do not appear in the report and their body is not executed. Must be called from inside [`method: Reporter.plan`] — calling later has no effect. diff --git a/docs/src/test-reporter-api/class-testcase.md b/docs/src/test-reporter-api/class-testcase.md index 22a8588fb0934..96bcf83a36047 100644 --- a/docs/src/test-reporter-api/class-testcase.md +++ b/docs/src/test-reporter-api/class-testcase.md @@ -107,3 +107,41 @@ Returns a list of titles from the root down to this test. - returns: <[TestCaseType]<"test">> Returns "test". Useful for detecting test cases in [`method: Suite.entries`]. + +## method: TestCase.skip +* since: v1.61 + +Mark this test as skipped. Intended for use from inside [`method: Reporter.plan`] — that is the only call-site at which a mutation propagates to workers. Sets [`property: TestCase.expectedStatus`] to `'skipped'` and appends a `skip` annotation. + +### param: TestCase.skip.reason +* since: v1.61 +- `reason` ?<[string]> + +Optional explanation surfaced as the annotation description. + +## method: TestCase.fixme +* since: v1.61 + +Mark this test as fixme. Intended for use from inside [`method: Reporter.plan`] — that is the only call-site at which a mutation propagates to workers. Sets [`property: TestCase.expectedStatus`] to `'skipped'` and appends a `fixme` annotation. + +### param: TestCase.fixme.reason +* since: v1.61 +- `reason` ?<[string]> + +Optional explanation surfaced as the annotation description. + +## method: TestCase.fail +* since: v1.61 + +Mark this test as expected-to-fail. Intended for use from inside [`method: Reporter.plan`] — that is the only call-site at which a mutation propagates to workers. Sets [`property: TestCase.expectedStatus`] to `'failed'` (unless the test was already marked as skipped) and appends a `fail` annotation. + +### param: TestCase.fail.reason +* since: v1.61 +- `reason` ?<[string]> + +Optional explanation surfaced as the annotation description. + +## method: TestCase.exclude +* since: v1.61 + +Remove this test from the run entirely. Excluded tests do not appear in the report, and their body is not executed. Must be called from inside [`method: Reporter.plan`] — calling later has no effect. diff --git a/packages/playwright/src/common/ipc.ts b/packages/playwright/src/common/ipc.ts index 3fdaeccfcb6d5..27fa4287fe552 100644 --- a/packages/playwright/src/common/ipc.ts +++ b/packages/playwright/src/common/ipc.ts @@ -142,6 +142,7 @@ export type StepEndPayload = { export type TestEntry = { testId: string; retry: number; + planAnnotations: { type: string, description?: string, location?: { file: string, line: number, column: number } }[]; }; export type RunPayload = { diff --git a/packages/playwright/src/common/suiteUtils.ts b/packages/playwright/src/common/suiteUtils.ts index 6c7128cca6cf9..f40be3f98fbf9 100644 --- a/packages/playwright/src/common/suiteUtils.ts +++ b/packages/playwright/src/common/suiteUtils.ts @@ -25,14 +25,6 @@ import type { FullProjectInternal } from './config'; import type { Suite, TestCase } from './test'; import type { Matcher, TestCaseFilter } from '../util'; -export function filterTestsRemoveEmptySuites(suite: Suite, filter: TestCaseFilter): boolean { - const filteredSuites = suite.suites.filter(child => filterTestsRemoveEmptySuites(child, filter)); - const filteredTests = suite.tests.filter(filter); - const entries = new Set([...filteredSuites, ...filteredTests]); - suite._entries = suite._entries.filter(e => entries.has(e)); // Preserve the order. - return !!suite._entries.length; -} - export function bindFileSuiteToProject(project: FullProjectInternal, suite: Suite): Suite { const relativeFile = path.relative(project.project.testDir, suite.location!.file); const fileId = calculateSha1(toPosixPath(relativeFile)).slice(0, 20); @@ -93,23 +85,27 @@ export function applyRepeatEachIndex(project: FullProjectInternal, fileSuite: Su }); } -export function filterOnly(suite: Suite) { - if (!suite._getOnlyItems().length) - return; - const suiteFilter = (suite: Suite) => suite._only; - const testFilter = (test: TestCase) => test._only; - return filterSuiteWithOnlySemantics(suite, suiteFilter, testFilter); -} - -function filterSuiteWithOnlySemantics(suite: Suite, suiteFilter: (suite: Suite) => boolean, testFilter: TestCaseFilter) { - const onlySuites = suite.suites.filter(child => filterSuiteWithOnlySemantics(child, suiteFilter, testFilter) || suiteFilter(child)); - const onlyTests = suite.tests.filter(testFilter); - const onlyEntries = new Set([...onlySuites, ...onlyTests]); - if (onlyEntries.size) { - suite._entries = suite._entries.filter(e => onlyEntries.has(e)); // Preserve the order. +export function filterOnly(suite: Suite): boolean { + const toExclude: (Suite | TestCase)[] = []; + let hasOnlyInside = false; + for (const child of suite.suites) { + if (filterOnly(child)) + hasOnlyInside = true; + else + toExclude.push(child); + } + for (const test of suite.tests) { + if (test._only) + hasOnlyInside = true; + else + toExclude.push(test); + } + if (hasOnlyInside) { + for (const e of toExclude) + e.exclude(); return true; } - return false; + return !!suite._only; } export function createFiltersFromArguments(args: string[]): { fileFilter: Matcher, testFilter: TestCaseFilter } { diff --git a/packages/playwright/src/common/test.ts b/packages/playwright/src/common/test.ts index fa8d8c20032d0..6054c805aad41 100644 --- a/packages/playwright/src/common/test.ts +++ b/packages/playwright/src/common/test.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import path from 'path'; +import { parseStackFrame } from '@isomorphic/stackTrace'; import { rootTestType } from './testType'; import { computeTestCaseOutcome } from '../isomorphic/teleReceiver'; @@ -96,6 +98,14 @@ export class Suite extends Base { this._entries.unshift(suite); } + _detach(child: Suite | TestCase) { + const idx = this._entries.indexOf(child); + if (idx !== -1) + this._entries.splice(idx, 1); + if (this._entries.length === 0) + this.parent?._detach(this); + } + allTests(): TestCase[] { const result: TestCase[] = []; const visit = (suite: Suite) => { @@ -252,6 +262,28 @@ export class Suite extends Base { project(): FullProject | undefined { return this._fullProject?.project || this.parent?.project(); } + + skip(reason?: string): void { + for (const entry of this.entries()) + entry.skip(reason); + } + + fixme(reason?: string): void { + for (const entry of this.entries()) + entry.fixme(reason); + } + + fail(reason?: string): void { + for (const entry of this.entries()) + entry.fail(reason); + } + + exclude(): void { + if (this.parent) + this.parent._detach(this); + else + this._entries = []; + } } export class TestCase extends Base implements reporterTypes.TestCase { @@ -275,6 +307,7 @@ export class TestCase extends Base implements reporterTypes.TestCase { _projectId = ''; // Explicitly declared tags that are not a part of the title. _tags: string[] = []; + _planAnnotations: TestAnnotation[] = []; constructor(title: string, fn: Function, testType: TestTypeImpl, location: Location) { super(title); @@ -309,6 +342,32 @@ export class TestCase extends Base implements reporterTypes.TestCase { ]; } + skip(reason?: string): void { + const annotation: TestAnnotation = { type: 'skip', description: reason, location: captureCallerLocation(this.skip) }; + this.annotations.push(annotation); + this._planAnnotations.push(annotation); + this.expectedStatus = 'skipped'; + } + + fixme(reason?: string): void { + const annotation: TestAnnotation = { type: 'fixme', description: reason, location: captureCallerLocation(this.fixme) }; + this.annotations.push(annotation); + this._planAnnotations.push(annotation); + this.expectedStatus = 'skipped'; + } + + fail(reason?: string): void { + const annotation: TestAnnotation = { type: 'fail', description: reason, location: captureCallerLocation(this.fail) }; + this.annotations.push(annotation); + this._planAnnotations.push(annotation); + if (this.expectedStatus !== 'skipped') + this.expectedStatus = 'failed'; + } + + exclude(): void { + this.parent._detach(this); + } + _serialize(): any { return { kind: 'test', @@ -384,3 +443,17 @@ export class TestCase extends Base implements reporterTypes.TestCase { return path.join(' '); } } + +function captureCallerLocation(belowFn: Function): Location | undefined { + const err: { stack?: string } = {}; + Error.captureStackTrace(err, belowFn); + const lines = (err.stack ?? '').split('\n'); + for (const line of lines) { + const frame = parseStackFrame(line, path.sep, false); + if (frame?.file && !frame.file.includes(`${path.sep}packages${path.sep}isomorphic${path.sep}`) + && !frame.file.includes(`${path.sep}packages${path.sep}playwright${path.sep}`) + && !frame.file.includes(`${path.sep}packages${path.sep}playwright-core${path.sep}`)) + return { file: frame.file, line: frame.line ?? 0, column: frame.column ?? 0 }; + } + return undefined; +} diff --git a/packages/playwright/src/isomorphic/teleReceiver.ts b/packages/playwright/src/isomorphic/teleReceiver.ts index bfde074d8d1c1..b8a15723b8408 100644 --- a/packages/playwright/src/isomorphic/teleReceiver.ts +++ b/packages/playwright/src/isomorphic/teleReceiver.ts @@ -648,6 +648,19 @@ export class TeleSuite implements reporterTypes.Suite { suite.parent = this; this._entries.push(suite); } + + skip(_reason?: string): void { + throw new Error('Disposition methods are not supported on a TeleSuite (read-only).'); + } + fixme(_reason?: string): void { + throw new Error('Disposition methods are not supported on a TeleSuite (read-only).'); + } + fail(_reason?: string): void { + throw new Error('Disposition methods are not supported on a TeleSuite (read-only).'); + } + exclude(): void { + throw new Error('Disposition methods are not supported on a TeleSuite (read-only).'); + } } export class TeleTestCase implements reporterTypes.TestCase { @@ -693,6 +706,19 @@ export class TeleTestCase implements reporterTypes.TestCase { this.results.push(result); return result; } + + skip(_reason?: string): void { + throw new Error('Disposition methods are not supported on a TeleTestCase (read-only).'); + } + fixme(_reason?: string): void { + throw new Error('Disposition methods are not supported on a TeleTestCase (read-only).'); + } + fail(_reason?: string): void { + throw new Error('Disposition methods are not supported on a TeleTestCase (read-only).'); + } + exclude(): void { + throw new Error('Disposition methods are not supported on a TeleTestCase (read-only).'); + } } class TeleTestStep implements reporterTypes.TestStep { diff --git a/packages/playwright/src/reporters/internalReporter.ts b/packages/playwright/src/reporters/internalReporter.ts index 23429ffbee1a4..deb40c71315dc 100644 --- a/packages/playwright/src/reporters/internalReporter.ts +++ b/packages/playwright/src/reporters/internalReporter.ts @@ -50,6 +50,10 @@ export class InternalReporter implements ReporterV2 { this._reporter.onConfigure?.(config); } + async plan(config: FullConfig, suite: testNs.Suite) { + await this._reporter.plan?.(config, suite); + } + onBegin(suite: testNs.Suite) { this._didBegin = true; this._reporter.onBegin?.(suite); @@ -108,7 +112,11 @@ export class InternalReporter implements ReporterV2 { } printsToStdio() { - return this._reporter.printsToStdio ? this._reporter.printsToStdio() : true; + return this._reporter.printsToStdio?.() ?? true; + } + + implementsSharding() { + return this._reporter.implementsSharding?.() ?? false; } private _addSnippetToTestErrors(test: TestCase, result: TestResult) { diff --git a/packages/playwright/src/reporters/multiplexer.ts b/packages/playwright/src/reporters/multiplexer.ts index 6596a2bd66f72..efb7a802d10d5 100644 --- a/packages/playwright/src/reporters/multiplexer.ts +++ b/packages/playwright/src/reporters/multiplexer.ts @@ -34,6 +34,15 @@ export class Multiplexer implements ReporterV2 { wrap(() => reporter.onConfigure?.(config)); } + async plan(config: FullConfig, suite: test.Suite) { + // Unlike other reporter callbacks, `plan` errors are NOT swallowed — + // they propagate so the run aborts before onBegin. Reporters use plan + // to mutate the corpus; silently dropping a planning error would let + // an inconsistent (partial-mutation) state reach the workers. + for (const reporter of this._reporters) + await reporter.plan?.(config, suite); + } + onBegin(suite: test.Suite) { for (const reporter of this._reporters) wrap(() => reporter.onBegin?.(suite)); @@ -110,6 +119,14 @@ export class Multiplexer implements ReporterV2 { return prints; }); } + + implementsSharding(): boolean { + return this._reporters.some(r => { + let shards = false; + wrap(() => shards = r.implementsSharding ? r.implementsSharding() : false); + return shards; + }); + } } async function wrapAsync(callback: () => T | Promise) { diff --git a/packages/playwright/src/reporters/reporterV2.ts b/packages/playwright/src/reporters/reporterV2.ts index 23cffcb4bb916..77e96d867165a 100644 --- a/packages/playwright/src/reporters/reporterV2.ts +++ b/packages/playwright/src/reporters/reporterV2.ts @@ -28,6 +28,7 @@ export interface ReportEndParams { export interface ReporterV2 { onConfigure?(config: FullConfig): void; + plan?(config: FullConfig, suite: Suite): void | Promise; onBegin?(suite: Suite): void; onTestBegin?(test: TestCase, result: TestResult): void; onStdOut?(chunk: string | Buffer, test?: TestCase, result?: TestResult): void; @@ -42,6 +43,7 @@ export interface ReporterV2 { onStepBegin?(test: TestCase, result: TestResult, step: TestStep): void; onStepEnd?(test: TestCase, result: TestResult, step: TestStep): void; printsToStdio?(): boolean; + implementsSharding?(): boolean; version(): 'v2'; } @@ -79,6 +81,10 @@ class ReporterV2Wrapper implements ReporterV2 { this._config = config; } + async plan(config: FullConfig, suite: Suite) { + await this._reporter.plan?.(config, suite); + } + onBegin(suite: Suite) { this._reporter.onBegin?.(this._config, suite); @@ -145,4 +151,8 @@ class ReporterV2Wrapper implements ReporterV2 { printsToStdio() { return this._reporter.printsToStdio ? this._reporter.printsToStdio() : true; } + + implementsSharding() { + return this._reporter.implementsSharding ? this._reporter.implementsSharding() : false; + } } diff --git a/packages/playwright/src/runner/dispatcher.ts b/packages/playwright/src/runner/dispatcher.ts index 29f4f4e84eb1a..a0fbcccf04083 100644 --- a/packages/playwright/src/runner/dispatcher.ts +++ b/packages/playwright/src/runner/dispatcher.ts @@ -563,7 +563,7 @@ class JobDispatcher { const runPayload: ipc.RunPayload = { file: this.job.requireFile, entries: this.job.tests.map(test => { - return { testId: test.id, retry: test.results.length }; + return { testId: test.id, retry: test.results.length, planAnnotations: test._planAnnotations }; }), }; worker.runTestGroup(runPayload); diff --git a/packages/playwright/src/runner/loadUtils.ts b/packages/playwright/src/runner/loadUtils.ts index a2cce3b386459..3ae087f5dc751 100644 --- a/packages/playwright/src/runner/loadUtils.ts +++ b/packages/playwright/src/runner/loadUtils.ts @@ -23,7 +23,7 @@ import { toPosixPath } from '@utils/fileUtils'; import { InProcessLoaderHost, OutOfProcessLoaderHost } from './loaderHost'; import { createTitleMatcher, errorWithFile, parseLocationArg } from '../util'; import { buildProjectsClosure, collectFilesForProject } from './projectUtils'; -import { createTestGroups, filterForShard } from './testGroups'; +import { createTestGroups, filterForShard } from './testGroups'; import { cc, config as commonConfig, FullConfigInternal, suiteUtils, test as testNs, transform } from '../common'; import type { RawSourceMap } from 'source-map'; @@ -129,9 +129,15 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho for (const [project, fileSuites] of testRun.projectSuites) { const projectSuite = createProjectSuite(project, fileSuites); projectSuites.set(project, projectSuite); - - const filteredProjectSuite = filterProjectSuite(projectSuite, testRun.preOnlyTestFilters); - filteredProjectSuites.set(project, filteredProjectSuite); + filteredProjectSuites.set(project, projectSuite); + if (testRun.preOnlyTestFilters.length) { + const filteredProjectSuite = projectSuite._deepClone(); + for (const test of filteredProjectSuite.allTests()) { + if (!testRun.preOnlyTestFilters.every(f => f(test))) + test.exclude(); + } + filteredProjectSuites.set(project, filteredProjectSuite); + } } } @@ -165,8 +171,9 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho } } + await testRun.reporter.plan(config.config, rootSuite); // Shard only the top-level projects. - if (config.config.shard) { + if (config.config.shard && !testRun.reporter.implementsSharding()) { // Create test groups for top-level projects. const testGroups: TestGroup[] = []; for (const projectSuite of rootSuite.suites) { @@ -184,12 +191,19 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho testsInThisShard.add(test); } - // Update project suites, removing empty ones. - suiteUtils.filterTestsRemoveEmptySuites(rootSuite, test => testsInThisShard.has(test)); + // Drop all tests that are not in this shard. + for (const t of rootSuite.allTests()) { + if (!testsInThisShard.has(t)) + t.exclude(); + } } - if (testRun.postShardTestFilters.length) - suiteUtils.filterTestsRemoveEmptySuites(rootSuite, test => testRun.postShardTestFilters.every(filter => filter(test))); + if (testRun.postShardTestFilters.length){ + for (const test of rootSuite.allTests()) { + if (!testRun.postShardTestFilters.every(f => f(test))) + test.exclude(); + } + } const topLevelProjects = []; // Now prepend dependency projects without filtration. @@ -218,25 +232,14 @@ function createProjectSuite(project: commonConfig.FullProjectInternal, fileSuite const grepMatcher = createTitleMatcher(project.project.grep); const grepInvertMatcher = project.project.grepInvert ? createTitleMatcher(project.project.grepInvert) : null; - suiteUtils.filterTestsRemoveEmptySuites(projectSuite, (test: testNs.TestCase) => { + for (const test of projectSuite.allTests()) { const grepTitle = test._grepTitleWithTags(); - if (grepInvertMatcher?.(grepTitle)) - return false; - return grepMatcher(grepTitle); - }); + if (grepInvertMatcher?.(grepTitle) || !grepMatcher(grepTitle)) + test.exclude(); + } return projectSuite; } -function filterProjectSuite(projectSuite: testNs.Suite, testFilters: TestCaseFilter[]): testNs.Suite { - // Fast path. - if (!testFilters.length) - return projectSuite; - - const result = projectSuite._deepClone(); - suiteUtils.filterTestsRemoveEmptySuites(result, test => testFilters.every(filter => filter(test))); - return result; -} - function buildProjectSuite(project: commonConfig.FullProjectInternal, projectSuite: testNs.Suite): testNs.Suite { const result = new testNs.Suite(project.project.name, 'project'); result._fullProject = project; diff --git a/packages/playwright/src/runner/reporters.ts b/packages/playwright/src/runner/reporters.ts index 00dd732b63991..7bb85f87547b5 100644 --- a/packages/playwright/src/runner/reporters.ts +++ b/packages/playwright/src/runner/reporters.ts @@ -74,7 +74,7 @@ export async function createReporters(config: FullConfigInternal, mode: 'list' | } } - const someReporterPrintsToStdio = reporters.some(r => r.printsToStdio ? r.printsToStdio() : true); + const someReporterPrintsToStdio = reporters.some(r => r.printsToStdio?.() ?? true); if (reporters.length && !someReporterPrintsToStdio) { // Add a line/dot/list-mode reporter for convenience. // Important to put it first, just in case some other reporter stalls onEnd. @@ -83,6 +83,11 @@ export async function createReporters(config: FullConfigInternal, mode: 'list' | else if (mode !== 'merge') reporters.unshift(!process.env.CI ? new LineReporter() : new DotReporter()); } + + const shardingReporters = reporters.filter(r => r.implementsSharding?.() ?? false); + if (shardingReporters.length > 1) + throw new Error(`Multiple reporters declare 'implementsSharding': ${shardingReporters.map(r => r.constructor?.name ?? 'reporter').join(', ')}. Only one reporter may handle sharding.`); + return reporters; } diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index 22e8f46451639..c1c9f69f9b6cf 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -220,7 +220,13 @@ export class WorkerMain extends ProcessRunner { const suite = suiteUtils.bindFileSuiteToProject(this._project, fileSuite); if (this._params.repeatEachIndex) suiteUtils.applyRepeatEachIndex(this._project, suite, this._params.repeatEachIndex); - suiteUtils.filterTestsRemoveEmptySuites(suite, test => entries.has(test.id)); + for (const test of suite.allTests()) { + const entry = entries.get(test.id); + if (!entry) + test.exclude(); + else + test.annotations.push(...entry.planAnnotations); + } const tests = suite.allTests(); // Collect test IDs that were not found in the worker diff --git a/packages/playwright/types/testReporter.d.ts b/packages/playwright/types/testReporter.d.ts index e5bd52c42995e..b40871b689956 100644 --- a/packages/playwright/types/testReporter.d.ts +++ b/packages/playwright/types/testReporter.d.ts @@ -158,6 +158,15 @@ export interface Reporter { * - `'interrupted'` - Interrupted by the user. */ onEnd?(result: FullResult): Promise<{ status?: FullResult['status'] } | undefined | void> | void; + /** + * When `true`, Playwright skips its built-in shard filter for this run, leaving sharding to the reporter (typically + * implemented inside [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) by + * calling [testCase.exclude()](https://playwright.dev/docs/api/class-testcase#test-case-exclude) on out-of-shard + * tests). If [testConfig.shard](https://playwright.dev/docs/api/class-testconfig#test-config-shard) is also + * explicitly set, a warning is printed indicating the request is being ignored. + */ + implementsSharding?(): boolean; + /** * Called once before running tests. All tests have been already discovered and put into a hierarchy of * [Suite](https://playwright.dev/docs/api/class-suite)s. @@ -227,6 +236,40 @@ export interface Reporter { */ onTestEnd?(test: TestCase, result: TestResult): void; + /** + * Called after the configuration has been resolved and before + * [reporter.onBegin(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-on-begin). Allows a + * reporter to inspect the discovered test corpus and call disposition methods on individual + * [TestCase](https://playwright.dev/docs/api/class-testcase)s or whole + * [Suite](https://playwright.dev/docs/api/class-suite)s before the run starts. Available disposition methods: + * - [testCase.skip([reason])](https://playwright.dev/docs/api/class-testcase#test-case-skip) / + * [suite.skip([reason])](https://playwright.dev/docs/api/class-suite#suite-skip) — mark expected status as + * `'skipped'` and append a `skip` annotation. The test body is not executed. + * - [testCase.fixme([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fixme) / + * [suite.fixme([reason])](https://playwright.dev/docs/api/class-suite#suite-fixme) — same as `skip`, but appends + * a `fixme` annotation. + * - [testCase.fail([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fail) / + * [suite.fail([reason])](https://playwright.dev/docs/api/class-suite#suite-fail) — mark expected status as + * `'failed'` and append a `fail` annotation. + * - [testCase.exclude()](https://playwright.dev/docs/api/class-testcase#test-case-exclude) / + * [suite.exclude()](https://playwright.dev/docs/api/class-suite#suite-exclude) — remove the test from the run + * entirely. Excluded tests do not appear in the report and their body is not executed. + * + * When multiple reporters implement `plan`, they are called in registration order and disposition methods accumulate; + * the last write wins on conflicting `expectedStatus` changes. Built-in sharding (see + * [testConfig.shard](https://playwright.dev/docs/api/class-testconfig#test-config-shard)) is applied **after** + * `plan()` returns, so dispositions are not silently dropped by sharding. If your reporter handles sharding itself, + * also implement + * [reporter.implementsSharding()](https://playwright.dev/docs/api/class-reporter#reporter-implements-sharding) to + * disable the built-in shard filter. + * + * Throwing from `plan` aborts the run before + * [reporter.onBegin(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-on-begin) is called. + * @param config Resolved configuration. + * @param suite The root suite that contains all projects, files and test cases. + */ + plan?(config: FullConfig, suite: Suite): Promise; + /** * Whether this reporter uses stdio for reporting. When it does not, Playwright Test could add some output to enhance * user experience. If your reporter does not print to the terminal, it is strongly recommended to return `false`. @@ -368,11 +411,44 @@ export interface Suite { */ entries(): Array; + /** + * Remove every descendant [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite from the run. + * Excluded tests do not appear in the report and their body is not executed. Must be called from inside + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — calling later has no + * effect. + */ + exclude(): void; + + /** + * Mark every descendant [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite as expected-to-fail. + * Intended for use from [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan). + * See [testCase.fail([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fail) for per-test + * semantics. + * @param reason Optional explanation surfaced as the annotation description. + */ + fail(reason?: string): void; + + /** + * Mark every descendant [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite as fixme. Intended + * for use from [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan). See + * [testCase.fixme([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fixme) for per-test semantics. + * @param reason Optional explanation surfaced as the annotation description. + */ + fixme(reason?: string): void; + /** * Configuration of the project this suite belongs to, or [void] for the root suite. */ project(): FullProject|undefined; + /** + * Mark every descendant [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite as skipped. Intended + * for use from [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan). See + * [testCase.skip([reason])](https://playwright.dev/docs/api/class-testcase#test-case-skip) for per-test semantics. + * @param reason Optional explanation surfaced as the annotation description. + */ + skip(reason?: string): void; + /** * Returns a list of titles from the root down to this suite. */ @@ -427,6 +503,34 @@ export interface Suite { * projects' suites. */ export interface TestCase { + /** + * Remove this test from the run entirely. Excluded tests do not appear in the report, and their body is not executed. + * Must be called from inside + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — calling later has no + * effect. + */ + exclude(): void; + + /** + * Mark this test as expected-to-fail. Intended for use from inside + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — that is the only + * call-site at which a mutation propagates to workers. Sets + * [testCase.expectedStatus](https://playwright.dev/docs/api/class-testcase#test-case-expected-status) to `'failed'` + * (unless the test was already marked as skipped) and appends a `fail` annotation. + * @param reason Optional explanation surfaced as the annotation description. + */ + fail(reason?: string): void; + + /** + * Mark this test as fixme. Intended for use from inside + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — that is the only + * call-site at which a mutation propagates to workers. Sets + * [testCase.expectedStatus](https://playwright.dev/docs/api/class-testcase#test-case-expected-status) to `'skipped'` + * and appends a `fixme` annotation. + * @param reason Optional explanation surfaced as the annotation description. + */ + fixme(reason?: string): void; + /** * Whether the test is considered running fine. Non-ok tests fail the test run with non-zero exit code. */ @@ -440,6 +544,16 @@ export interface TestCase { */ outcome(): "skipped"|"expected"|"unexpected"|"flaky"; + /** + * Mark this test as skipped. Intended for use from inside + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — that is the only + * call-site at which a mutation propagates to workers. Sets + * [testCase.expectedStatus](https://playwright.dev/docs/api/class-testcase#test-case-expected-status) to `'skipped'` + * and appends a `skip` annotation. + * @param reason Optional explanation surfaced as the annotation description. + */ + skip(reason?: string): void; + /** * Returns a list of titles from the root down to this test. */ diff --git a/tests/playwright-test/reporter-plan.spec.ts b/tests/playwright-test/reporter-plan.spec.ts new file mode 100644 index 0000000000000..149bfbbccf458 --- /dev/null +++ b/tests/playwright-test/reporter-plan.spec.ts @@ -0,0 +1,389 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test, expect } from './playwright-test-fixtures'; + +test('plan is called between project setup and onBegin and can skip tests', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + async plan(config, suite) { + console.log('plan: ' + suite.allTests().length + ' tests'); + for (const t of suite.allTests()) { + if (t.title.includes('skip-me')) + t.skip('planned skip'); + } + } + onBegin(config, suite) { + console.log('onBegin: ' + suite.allTests().length + ' tests'); + } + onTestEnd(test, result) { + console.log('end ' + test.title + ' status=' + result.status + ' expected=' + test.expectedStatus + ' ann=' + test.annotations.map(a => a.type + ':' + (a.description || '')).join(',')); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('run-me', async () => {}); + test('skip-me', async () => { throw new Error('should not run'); }); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.output).toContain('plan: 2 tests'); + expect(result.output).toContain('onBegin: 2 tests'); + expect(result.output).toMatch(/end skip-me status=skipped expected=skipped ann=.*skip:planned skip/); + expect(result.output).toMatch(/end run-me status=passed expected=passed/); + // Ordering: plan < onBegin + const idxPlan = result.output.indexOf('plan:'); + const idxBegin = result.output.indexOf('onBegin:'); + expect(idxPlan).toBeLessThan(idxBegin); +}); + +test('disposition methods are intended for plan() but not enforced at runtime', async ({ runInlineTest }) => { + // We document plan() as the intended call-site for skip/fixme/fail/exclude + // but do not enforce it at runtime. Calling them from e.g. onBegin will + // mutate the in-process suite (visible to reporters) but won't affect the + // workers, since the run payload is built earlier. + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + onBegin(config, suite) { + // Should not throw. + suite.allTests()[0].skip('late'); + console.log('late skip ok: ' + suite.allTests()[0].expectedStatus); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('one', async () => {}); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.output).toContain('late skip ok: skipped'); +}); + +test('TestCase.exclude removes test from run and report', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + async plan(config, suite) { + for (const t of suite.allTests()) + if (t.title === 'excluded') t.exclude(); + } + onBegin(config, suite) { + console.log('begin tests: ' + suite.allTests().map(t => t.title).join(',')); + } + onTestEnd(test, result) { + console.log('ran ' + test.title); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('kept', async () => {}); + test('excluded', async () => { throw new Error('should not run'); }); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.output).toContain('begin tests: kept'); + expect(result.output).toContain('ran kept'); + expect(result.output).not.toContain('ran excluded'); +}); + +test('Suite.skip cascades to all descendants', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + async plan(config, suite) { + // Skip every test under the 'doomed' describe. + const visit = (s) => { + if (s.title === 'doomed') s.skip('whole group'); + for (const child of s.suites || []) visit(child); + }; + visit(suite); + } + onTestEnd(test, result) { + console.log(test.title + ':' + result.status + ':' + test.expectedStatus); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test.describe('doomed', () => { + test('one', async () => { throw new Error('nope'); }); + test('two', async () => { throw new Error('nope'); }); + }); + test('keep', async () => {}); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.output).toContain('one:skipped:skipped'); + expect(result.output).toContain('two:skipped:skipped'); + expect(result.output).toContain('keep:passed:passed'); +}); + +test('plan throwing aborts the run before onBegin', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + async plan(config, suite) { + throw new Error('plan-aborted'); + } + onBegin(config, suite) { + // InternalReporter synthesizes an empty-suite onBegin when the run + // aborted before normal onBegin — that's expected. The point is that + // we never see the real corpus. + console.log('onBegin suite size: ' + suite.allTests().length); + } + onError(err) { + console.log('error: ' + err.message); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('one', async () => {}); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).not.toBe(0); + expect(result.output).toContain('plan-aborted'); + // Synthetic empty-suite onBegin OK, real onBegin (size 1) must NOT happen. + expect(result.output).not.toContain('onBegin suite size: 1'); +}); + +test('multiple reporters: plan called in order, annotations accumulate', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'first.ts': ` + class R { + async plan(config, suite) { + console.log('first plan'); + suite.allTests()[0].fail('first reason'); + } + onTestEnd(test, result) { + console.log('first onTestEnd: ' + test.expectedStatus + ' ann=' + test.annotations.map(a => a.type).join(',')); + } + } + module.exports = R; + `, + 'second.ts': ` + class R { + async plan(config, suite) { + console.log('second plan'); + suite.allTests()[0].skip('second reason'); + } + } + module.exports = R; + `, + 'playwright.config.ts': `module.exports = { reporter: [['./first.ts'], ['./second.ts']] };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('one', async () => {}); + `, + }, { reporter: '', workers: 1 }); + + const idxFirst = result.output.indexOf('first plan'); + const idxSecond = result.output.indexOf('second plan'); + expect(idxFirst).toBeGreaterThan(-1); + expect(idxSecond).toBeGreaterThan(idxFirst); + // Annotations accumulate from both reporters. skip beats fail in + // worker-side expectedStatus derivation (mirroring testInfo semantics). + expect(result.output).toContain('first onTestEnd: skipped'); + expect(result.output).toContain('fail,skip'); +}); + +test('exclude prunes the tree eagerly: later reporter does not see excluded test', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'first.ts': ` + class R { + async plan(config, suite) { + for (const t of suite.allTests()) + if (t.title === 'gone') t.exclude(); + console.log('first sees: ' + suite.allTests().map(t => t.title).join(',')); + } + } + module.exports = R; + `, + 'second.ts': ` + class R { + async plan(config, suite) { + console.log('second sees: ' + suite.allTests().map(t => t.title).join(',')); + } + } + module.exports = R; + `, + 'playwright.config.ts': `module.exports = { reporter: [['./first.ts'], ['./second.ts']] };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('kept', async () => {}); + test('gone', async () => { throw new Error('should not run'); }); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.output).toContain('first sees: kept'); + expect(result.output).toContain('second sees: kept'); + expect(result.output).not.toMatch(/second sees:[^\n]*gone/); +}); + +test('implementsSharding disables built-in shard filter', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class R { + implementsSharding() { return true; } + async plan(config, suite) { + // Custom "shard": keep only every other test. + let i = 0; + for (const t of suite.allTests()) { + if (i++ % 2 === 1) t.exclude(); + } + } + onBegin(config, suite) { + console.log('begin count: ' + suite.allTests().length); + } + } + module.exports = R; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts', shard: { current: 1, total: 2 } };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + for (let i = 0; i < 4; i++) + test('t' + i, async () => {}); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + // Reporter sees all 4 tests, excludes every other → 2 kept (would have been ~2 from built-in shard). + // The point: built-in shard didn't run on top of reporter's exclusions. + expect(result.output).toContain('begin count: 2'); +}); + +test('multiple reporters declaring implementsSharding throws', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter-a.ts': ` + class A { + implementsSharding() { return true; } + } + module.exports = A; + `, + 'reporter-b.ts': ` + class B { + implementsSharding() { return true; } + } + module.exports = B; + `, + 'playwright.config.ts': `module.exports = { reporter: [['./reporter-a.ts'], ['./reporter-b.ts']] };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('t', async () => {}); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).not.toBe(0); + expect(result.rawOutput).toMatch(/Multiple reporters declare 'implementsSharding'/); +}); + +test('built-in shard runs when no reporter implements sharding', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class R { + async plan(config, suite) { + console.log('plan: ' + suite.allTests().length); + } + onBegin(config, suite) { + console.log('begin: ' + suite.allTests().length); + } + } + module.exports = R; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts', shard: { current: 1, total: 2 } };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test.describe.configure({ mode: 'parallel' }); + for (let i = 0; i < 4; i++) + test('t' + i, async () => {}); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + // plan sees full corpus (pre-shard), onBegin sees sharded subset. + expect(result.output).toContain('plan: 4'); + expect(result.output).toContain('begin: 2'); +}); + +test('plan sees the .only-narrowed corpus', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class R { + async plan(config, suite) { + console.log('plan tests: ' + suite.allTests().map(t => t.title).join(',')); + } + } + module.exports = R; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('one', async () => {}); + test.only('two', async () => {}); + test('three', async () => {}); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.output).toContain('plan tests: two'); +}); + +test('plan annotations capture caller location pointing at reporter', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + async plan(config, suite) { + for (const t of suite.allTests()) + t.skip('planned'); + } + onTestEnd(test, result) { + const a = test.annotations.find(a => a.type === 'skip'); + console.log('loc=' + (a?.location ? require('path').basename(a.location.file) + ':' + a.location.line : 'NONE')); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('t', async () => {}); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.output).toMatch(/loc=reporter\.ts:\d+/); +}); From cf670d7ef8dca94a0ecef8a1b78874037bc0058c Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 2 Jun 2026 16:08:52 +0200 Subject: [PATCH 2/8] chore(test-runner): simplify captureCallerLocation --- packages/playwright/src/common/test.ts | 27 ++++++++++---------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/packages/playwright/src/common/test.ts b/packages/playwright/src/common/test.ts index 6054c805aad41..a5fc5e401d827 100644 --- a/packages/playwright/src/common/test.ts +++ b/packages/playwright/src/common/test.ts @@ -14,10 +14,10 @@ * limitations under the License. */ -import path from 'path'; -import { parseStackFrame } from '@isomorphic/stackTrace'; +import { captureRawStack } from '@isomorphic/stackTrace'; import { rootTestType } from './testType'; import { computeTestCaseOutcome } from '../isomorphic/teleReceiver'; +import { filteredStackTrace } from '../util'; import type { FixturesWithLocation, FullProjectInternal } from './config'; import type { FixturePool } from './fixtures'; @@ -343,21 +343,21 @@ export class TestCase extends Base implements reporterTypes.TestCase { } skip(reason?: string): void { - const annotation: TestAnnotation = { type: 'skip', description: reason, location: captureCallerLocation(this.skip) }; + const annotation: TestAnnotation = { type: 'skip', description: reason, location: captureCallerLocation() }; this.annotations.push(annotation); this._planAnnotations.push(annotation); this.expectedStatus = 'skipped'; } fixme(reason?: string): void { - const annotation: TestAnnotation = { type: 'fixme', description: reason, location: captureCallerLocation(this.fixme) }; + const annotation: TestAnnotation = { type: 'fixme', description: reason, location: captureCallerLocation() }; this.annotations.push(annotation); this._planAnnotations.push(annotation); this.expectedStatus = 'skipped'; } fail(reason?: string): void { - const annotation: TestAnnotation = { type: 'fail', description: reason, location: captureCallerLocation(this.fail) }; + const annotation: TestAnnotation = { type: 'fail', description: reason, location: captureCallerLocation() }; this.annotations.push(annotation); this._planAnnotations.push(annotation); if (this.expectedStatus !== 'skipped') @@ -444,16 +444,9 @@ export class TestCase extends Base implements reporterTypes.TestCase { } } -function captureCallerLocation(belowFn: Function): Location | undefined { - const err: { stack?: string } = {}; - Error.captureStackTrace(err, belowFn); - const lines = (err.stack ?? '').split('\n'); - for (const line of lines) { - const frame = parseStackFrame(line, path.sep, false); - if (frame?.file && !frame.file.includes(`${path.sep}packages${path.sep}isomorphic${path.sep}`) - && !frame.file.includes(`${path.sep}packages${path.sep}playwright${path.sep}`) - && !frame.file.includes(`${path.sep}packages${path.sep}playwright-core${path.sep}`)) - return { file: frame.file, line: frame.line ?? 0, column: frame.column ?? 0 }; - } - return undefined; +function captureCallerLocation(): Location | undefined { + const frame = filteredStackTrace(captureRawStack())[0]; + if (!frame?.file) + return undefined; + return { file: frame.file, line: frame.line ?? 0, column: frame.column ?? 0 }; } From 903c22b0ae6b3af8099f6856be8ded3e023a380d Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 2 Jun 2026 16:19:26 +0200 Subject: [PATCH 3/8] docs(test-runner): trim Reporter.plan / disposition docs --- docs/src/test-reporter-api/class-reporter.md | 14 +--- docs/src/test-reporter-api/class-suite.md | 8 +- docs/src/test-reporter-api/class-testcase.md | 8 +- packages/playwright/types/testReporter.d.ts | 83 ++++++-------------- 4 files changed, 33 insertions(+), 80 deletions(-) diff --git a/docs/src/test-reporter-api/class-reporter.md b/docs/src/test-reporter-api/class-reporter.md index ac959f252d438..a39f5afe2a8c0 100644 --- a/docs/src/test-reporter-api/class-reporter.md +++ b/docs/src/test-reporter-api/class-reporter.md @@ -301,16 +301,7 @@ Whether this reporter uses stdio for reporting. When it does not, Playwright Tes ## optional async method: Reporter.plan * since: v1.61 -Called after the configuration has been resolved and before [`method: Reporter.onBegin`]. Allows a reporter to inspect the discovered test corpus and call disposition methods on individual [TestCase]s or whole [Suite]s before the run starts. Available disposition methods: - -* [`method: TestCase.skip`] / [`method: Suite.skip`] — mark expected status as `'skipped'` and append a `skip` annotation. The test body is not executed. -* [`method: TestCase.fixme`] / [`method: Suite.fixme`] — same as `skip`, but appends a `fixme` annotation. -* [`method: TestCase.fail`] / [`method: Suite.fail`] — mark expected status as `'failed'` and append a `fail` annotation. -* [`method: TestCase.exclude`] / [`method: Suite.exclude`] — remove the test from the run entirely. Excluded tests do not appear in the report and their body is not executed. - -When multiple reporters implement `plan`, they are called in registration order and disposition methods accumulate; the last write wins on conflicting `expectedStatus` changes. Built-in sharding (see [`property: TestConfig.shard`]) is applied **after** `plan()` returns, so dispositions are not silently dropped by sharding. If your reporter handles sharding itself, also implement [`method: Reporter.implementsSharding`] to disable the built-in shard filter. - -Throwing from `plan` aborts the run before [`method: Reporter.onBegin`] is called. +Called after the configuration has been resolved and before [`method: Reporter.onBegin`]. Allows a reporter to mark individual tests as skipped, excluded, fixed or failing. ### param: Reporter.plan.config * since: v1.61 @@ -328,5 +319,4 @@ The root suite that contains all projects, files and test cases. * since: v1.61 - returns: <[boolean]> -When `true`, Playwright skips its built-in shard filter for this run, leaving sharding to the reporter (typically implemented inside [`method: Reporter.plan`] by calling [`method: TestCase.exclude`] on out-of-shard tests). If [`property: TestConfig.shard`] is also explicitly set, a warning is printed indicating the request is being ignored. - +When `true`, Playwright skips its built-in shard filter for this run, leaving sharding to the reporter (typically implemented inside [`method: Reporter.plan`] by calling [`method: TestCase.exclude`] on out-of-shard tests). diff --git a/docs/src/test-reporter-api/class-suite.md b/docs/src/test-reporter-api/class-suite.md index 8f0b1923e95f4..0bec189b9f1ca 100644 --- a/docs/src/test-reporter-api/class-suite.md +++ b/docs/src/test-reporter-api/class-suite.md @@ -89,7 +89,7 @@ Returns the type of the suite. The Suites form the following hierarchy: ## method: Suite.skip * since: v1.61 -Mark every descendant [TestCase] of this suite as skipped. Intended for use from [`method: Reporter.plan`]. See [`method: TestCase.skip`] for per-test semantics. +Mark every [TestCase] of this suite as skipped, see [`method: TestCase.skip`]. ### param: Suite.skip.reason * since: v1.61 @@ -100,7 +100,7 @@ Optional explanation surfaced as the annotation description. ## method: Suite.fixme * since: v1.61 -Mark every descendant [TestCase] of this suite as fixme. Intended for use from [`method: Reporter.plan`]. See [`method: TestCase.fixme`] for per-test semantics. +Mark every [TestCase] of this suite as fixme, see [`method: TestCase.fixme`]. ### param: Suite.fixme.reason * since: v1.61 @@ -111,7 +111,7 @@ Optional explanation surfaced as the annotation description. ## method: Suite.fail * since: v1.61 -Mark every descendant [TestCase] of this suite as expected-to-fail. Intended for use from [`method: Reporter.plan`]. See [`method: TestCase.fail`] for per-test semantics. +Mark every [TestCase] of this suite as expected-to-fail, see [`method: TestCase.fail`]. ### param: Suite.fail.reason * since: v1.61 @@ -122,4 +122,4 @@ Optional explanation surfaced as the annotation description. ## method: Suite.exclude * since: v1.61 -Remove every descendant [TestCase] of this suite from the run. Excluded tests do not appear in the report and their body is not executed. Must be called from inside [`method: Reporter.plan`] — calling later has no effect. +Must be called from inside [`method: Reporter.plan`], exclude this suite from the run. Excluded tests do not appear in the report and their body is not executed. diff --git a/docs/src/test-reporter-api/class-testcase.md b/docs/src/test-reporter-api/class-testcase.md index 96bcf83a36047..006d41f0bbb86 100644 --- a/docs/src/test-reporter-api/class-testcase.md +++ b/docs/src/test-reporter-api/class-testcase.md @@ -111,7 +111,7 @@ Returns "test". Useful for detecting test cases in [`method: Suite.entries`]. ## method: TestCase.skip * since: v1.61 -Mark this test as skipped. Intended for use from inside [`method: Reporter.plan`] — that is the only call-site at which a mutation propagates to workers. Sets [`property: TestCase.expectedStatus`] to `'skipped'` and appends a `skip` annotation. +Must be called from inside [`method: Reporter.plan`], skip this test. The test body is not executed and the test is reported as skipped. ### param: TestCase.skip.reason * since: v1.61 @@ -122,7 +122,7 @@ Optional explanation surfaced as the annotation description. ## method: TestCase.fixme * since: v1.61 -Mark this test as fixme. Intended for use from inside [`method: Reporter.plan`] — that is the only call-site at which a mutation propagates to workers. Sets [`property: TestCase.expectedStatus`] to `'skipped'` and appends a `fixme` annotation. +Must be called from inside [`method: Reporter.plan`], mark this test as fixme. The test body is not executed and the test is reported as skipped, with the intention to fix it. ### param: TestCase.fixme.reason * since: v1.61 @@ -133,7 +133,7 @@ Optional explanation surfaced as the annotation description. ## method: TestCase.fail * since: v1.61 -Mark this test as expected-to-fail. Intended for use from inside [`method: Reporter.plan`] — that is the only call-site at which a mutation propagates to workers. Sets [`property: TestCase.expectedStatus`] to `'failed'` (unless the test was already marked as skipped) and appends a `fail` annotation. +Must be called from inside [`method: Reporter.plan`], mark this test as "should fail". Playwright runs the test and ensures it is actually failing, useful for documenting broken functionality until it is fixed. ### param: TestCase.fail.reason * since: v1.61 @@ -144,4 +144,4 @@ Optional explanation surfaced as the annotation description. ## method: TestCase.exclude * since: v1.61 -Remove this test from the run entirely. Excluded tests do not appear in the report, and their body is not executed. Must be called from inside [`method: Reporter.plan`] — calling later has no effect. +Must be called from inside [`method: Reporter.plan`], exclude this test from the run. Excluded tests do not appear in the report and their body is not executed. diff --git a/packages/playwright/types/testReporter.d.ts b/packages/playwright/types/testReporter.d.ts index b40871b689956..c9030c8e4a69d 100644 --- a/packages/playwright/types/testReporter.d.ts +++ b/packages/playwright/types/testReporter.d.ts @@ -162,8 +162,7 @@ export interface Reporter { * When `true`, Playwright skips its built-in shard filter for this run, leaving sharding to the reporter (typically * implemented inside [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) by * calling [testCase.exclude()](https://playwright.dev/docs/api/class-testcase#test-case-exclude) on out-of-shard - * tests). If [testConfig.shard](https://playwright.dev/docs/api/class-testconfig#test-config-shard) is also - * explicitly set, a warning is printed indicating the request is being ignored. + * tests). */ implementsSharding?(): boolean; @@ -239,32 +238,7 @@ export interface Reporter { /** * Called after the configuration has been resolved and before * [reporter.onBegin(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-on-begin). Allows a - * reporter to inspect the discovered test corpus and call disposition methods on individual - * [TestCase](https://playwright.dev/docs/api/class-testcase)s or whole - * [Suite](https://playwright.dev/docs/api/class-suite)s before the run starts. Available disposition methods: - * - [testCase.skip([reason])](https://playwright.dev/docs/api/class-testcase#test-case-skip) / - * [suite.skip([reason])](https://playwright.dev/docs/api/class-suite#suite-skip) — mark expected status as - * `'skipped'` and append a `skip` annotation. The test body is not executed. - * - [testCase.fixme([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fixme) / - * [suite.fixme([reason])](https://playwright.dev/docs/api/class-suite#suite-fixme) — same as `skip`, but appends - * a `fixme` annotation. - * - [testCase.fail([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fail) / - * [suite.fail([reason])](https://playwright.dev/docs/api/class-suite#suite-fail) — mark expected status as - * `'failed'` and append a `fail` annotation. - * - [testCase.exclude()](https://playwright.dev/docs/api/class-testcase#test-case-exclude) / - * [suite.exclude()](https://playwright.dev/docs/api/class-suite#suite-exclude) — remove the test from the run - * entirely. Excluded tests do not appear in the report and their body is not executed. - * - * When multiple reporters implement `plan`, they are called in registration order and disposition methods accumulate; - * the last write wins on conflicting `expectedStatus` changes. Built-in sharding (see - * [testConfig.shard](https://playwright.dev/docs/api/class-testconfig#test-config-shard)) is applied **after** - * `plan()` returns, so dispositions are not silently dropped by sharding. If your reporter handles sharding itself, - * also implement - * [reporter.implementsSharding()](https://playwright.dev/docs/api/class-reporter#reporter-implements-sharding) to - * disable the built-in shard filter. - * - * Throwing from `plan` aborts the run before - * [reporter.onBegin(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-on-begin) is called. + * reporter to mark individual tests as skipped, excluded, fixed or failing. * @param config Resolved configuration. * @param suite The root suite that contains all projects, files and test cases. */ @@ -412,26 +386,22 @@ export interface Suite { entries(): Array; /** - * Remove every descendant [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite from the run. - * Excluded tests do not appear in the report and their body is not executed. Must be called from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — calling later has no - * effect. + * Must be called from inside + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), exclude this suite + * from the run. Excluded tests do not appear in the report and their body is not executed. */ exclude(): void; /** - * Mark every descendant [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite as expected-to-fail. - * Intended for use from [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan). - * See [testCase.fail([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fail) for per-test - * semantics. + * Mark every [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite as expected-to-fail, see + * [testCase.fail([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fail). * @param reason Optional explanation surfaced as the annotation description. */ fail(reason?: string): void; /** - * Mark every descendant [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite as fixme. Intended - * for use from [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan). See - * [testCase.fixme([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fixme) for per-test semantics. + * Mark every [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite as fixme, see + * [testCase.fixme([reason])](https://playwright.dev/docs/api/class-testcase#test-case-fixme). * @param reason Optional explanation surfaced as the annotation description. */ fixme(reason?: string): void; @@ -442,9 +412,8 @@ export interface Suite { project(): FullProject|undefined; /** - * Mark every descendant [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite as skipped. Intended - * for use from [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan). See - * [testCase.skip([reason])](https://playwright.dev/docs/api/class-testcase#test-case-skip) for per-test semantics. + * Mark every [TestCase](https://playwright.dev/docs/api/class-testcase) of this suite as skipped, see + * [testCase.skip([reason])](https://playwright.dev/docs/api/class-testcase#test-case-skip). * @param reason Optional explanation surfaced as the annotation description. */ skip(reason?: string): void; @@ -504,29 +473,25 @@ export interface Suite { */ export interface TestCase { /** - * Remove this test from the run entirely. Excluded tests do not appear in the report, and their body is not executed. * Must be called from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — calling later has no - * effect. + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), exclude this test + * from the run. Excluded tests do not appear in the report and their body is not executed. */ exclude(): void; /** - * Mark this test as expected-to-fail. Intended for use from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — that is the only - * call-site at which a mutation propagates to workers. Sets - * [testCase.expectedStatus](https://playwright.dev/docs/api/class-testcase#test-case-expected-status) to `'failed'` - * (unless the test was already marked as skipped) and appends a `fail` annotation. + * Must be called from inside + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), mark this test as + * "should fail". Playwright runs the test and ensures it is actually failing, useful for documenting broken + * functionality until it is fixed. * @param reason Optional explanation surfaced as the annotation description. */ fail(reason?: string): void; /** - * Mark this test as fixme. Intended for use from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — that is the only - * call-site at which a mutation propagates to workers. Sets - * [testCase.expectedStatus](https://playwright.dev/docs/api/class-testcase#test-case-expected-status) to `'skipped'` - * and appends a `fixme` annotation. + * Must be called from inside + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), mark this test as + * fixme. The test body is not executed and the test is reported as skipped, with the intention to fix it. * @param reason Optional explanation surfaced as the annotation description. */ fixme(reason?: string): void; @@ -545,11 +510,9 @@ export interface TestCase { outcome(): "skipped"|"expected"|"unexpected"|"flaky"; /** - * Mark this test as skipped. Intended for use from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) — that is the only - * call-site at which a mutation propagates to workers. Sets - * [testCase.expectedStatus](https://playwright.dev/docs/api/class-testcase#test-case-expected-status) to `'skipped'` - * and appends a `skip` annotation. + * Must be called from inside + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), skip this test. The + * test body is not executed and the test is reported as skipped. * @param reason Optional explanation surfaced as the annotation description. */ skip(reason?: string): void; From 4111355d3c2f7dad69a52a3fbba387e0f9f1304c Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 2 Jun 2026 16:25:11 +0200 Subject: [PATCH 4/8] test(test-runner): condense Reporter.plan tests --- tests/playwright-test/reporter-plan.spec.ts | 229 +++++--------------- 1 file changed, 56 insertions(+), 173 deletions(-) diff --git a/tests/playwright-test/reporter-plan.spec.ts b/tests/playwright-test/reporter-plan.spec.ts index 149bfbbccf458..ef618e85f0797 100644 --- a/tests/playwright-test/reporter-plan.spec.ts +++ b/tests/playwright-test/reporter-plan.spec.ts @@ -16,57 +16,20 @@ import { test, expect } from './playwright-test-fixtures'; -test('plan is called between project setup and onBegin and can skip tests', async ({ runInlineTest }) => { +test('plan runs between project setup and onBegin, sees the .only-narrowed corpus, and can skip tests', async ({ runInlineTest }) => { const result = await runInlineTest({ 'reporter.ts': ` class Reporter { async plan(config, suite) { - console.log('plan: ' + suite.allTests().length + ' tests'); - for (const t of suite.allTests()) { - if (t.title.includes('skip-me')) - t.skip('planned skip'); - } + console.log('%% plan: ' + suite.allTests().map(t => t.title).join(',')); + for (const t of suite.allTests()) + if (t.title.includes('skip-me')) t.skip('planned skip'); } onBegin(config, suite) { - console.log('onBegin: ' + suite.allTests().length + ' tests'); + console.log('%% onBegin: ' + suite.allTests().map(t => t.title).join(',')); } onTestEnd(test, result) { - console.log('end ' + test.title + ' status=' + result.status + ' expected=' + test.expectedStatus + ' ann=' + test.annotations.map(a => a.type + ':' + (a.description || '')).join(',')); - } - } - module.exports = Reporter; - `, - 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, - 'a.test.ts': ` - import { test } from '@playwright/test'; - test('run-me', async () => {}); - test('skip-me', async () => { throw new Error('should not run'); }); - `, - }, { reporter: '', workers: 1 }); - - expect(result.exitCode).toBe(0); - expect(result.output).toContain('plan: 2 tests'); - expect(result.output).toContain('onBegin: 2 tests'); - expect(result.output).toMatch(/end skip-me status=skipped expected=skipped ann=.*skip:planned skip/); - expect(result.output).toMatch(/end run-me status=passed expected=passed/); - // Ordering: plan < onBegin - const idxPlan = result.output.indexOf('plan:'); - const idxBegin = result.output.indexOf('onBegin:'); - expect(idxPlan).toBeLessThan(idxBegin); -}); - -test('disposition methods are intended for plan() but not enforced at runtime', async ({ runInlineTest }) => { - // We document plan() as the intended call-site for skip/fixme/fail/exclude - // but do not enforce it at runtime. Calling them from e.g. onBegin will - // mutate the in-process suite (visible to reporters) but won't affect the - // workers, since the run payload is built earlier. - const result = await runInlineTest({ - 'reporter.ts': ` - class Reporter { - onBegin(config, suite) { - // Should not throw. - suite.allTests()[0].skip('late'); - console.log('late skip ok: ' + suite.allTests()[0].expectedStatus); + console.log('%% end ' + test.title + ' status=' + result.status + ' expected=' + test.expectedStatus + ' ann=' + test.annotations.map(a => a.type + ':' + (a.description || '')).join(',')); } } module.exports = Reporter; @@ -74,12 +37,19 @@ test('disposition methods are intended for plan() but not enforced at runtime', 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, 'a.test.ts': ` import { test } from '@playwright/test'; - test('one', async () => {}); + test('ignored-by-only', async () => {}); + test.only('run-me', async () => {}); + test.only('skip-me', async () => { throw new Error('should not run'); }); `, }, { reporter: '', workers: 1 }); expect(result.exitCode).toBe(0); - expect(result.output).toContain('late skip ok: skipped'); + expect(result.outputLines).toEqual([ + 'plan: run-me,skip-me', + 'onBegin: run-me,skip-me', + 'end run-me status=passed expected=passed ann=', + 'end skip-me status=skipped expected=skipped ann=skip:planned skip', + ]); }); test('TestCase.exclude removes test from run and report', async ({ runInlineTest }) => { @@ -91,10 +61,10 @@ test('TestCase.exclude removes test from run and report', async ({ runInlineTest if (t.title === 'excluded') t.exclude(); } onBegin(config, suite) { - console.log('begin tests: ' + suite.allTests().map(t => t.title).join(',')); + console.log('%% begin: ' + suite.allTests().map(t => t.title).join(',')); } onTestEnd(test, result) { - console.log('ran ' + test.title); + console.log('%% ran ' + test.title); } } module.exports = Reporter; @@ -108,9 +78,10 @@ test('TestCase.exclude removes test from run and report', async ({ runInlineTest }, { reporter: '', workers: 1 }); expect(result.exitCode).toBe(0); - expect(result.output).toContain('begin tests: kept'); - expect(result.output).toContain('ran kept'); - expect(result.output).not.toContain('ran excluded'); + expect(result.outputLines).toEqual([ + 'begin: kept', + 'ran kept', + ]); }); test('Suite.skip cascades to all descendants', async ({ runInlineTest }) => { @@ -118,7 +89,6 @@ test('Suite.skip cascades to all descendants', async ({ runInlineTest }) => { 'reporter.ts': ` class Reporter { async plan(config, suite) { - // Skip every test under the 'doomed' describe. const visit = (s) => { if (s.title === 'doomed') s.skip('whole group'); for (const child of s.suites || []) visit(child); @@ -126,7 +96,7 @@ test('Suite.skip cascades to all descendants', async ({ runInlineTest }) => { visit(suite); } onTestEnd(test, result) { - console.log(test.title + ':' + result.status + ':' + test.expectedStatus); + console.log('%% ' + test.title + ':' + result.status + ':' + test.expectedStatus); } } module.exports = Reporter; @@ -143,9 +113,11 @@ test('Suite.skip cascades to all descendants', async ({ runInlineTest }) => { }, { reporter: '', workers: 1 }); expect(result.exitCode).toBe(0); - expect(result.output).toContain('one:skipped:skipped'); - expect(result.output).toContain('two:skipped:skipped'); - expect(result.output).toContain('keep:passed:passed'); + expect(result.outputLines.sort()).toEqual([ + 'keep:passed:passed', + 'one:skipped:skipped', + 'two:skipped:skipped', + ]); }); test('plan throwing aborts the run before onBegin', async ({ runInlineTest }) => { @@ -156,13 +128,10 @@ test('plan throwing aborts the run before onBegin', async ({ runInlineTest }) => throw new Error('plan-aborted'); } onBegin(config, suite) { - // InternalReporter synthesizes an empty-suite onBegin when the run - // aborted before normal onBegin — that's expected. The point is that - // we never see the real corpus. - console.log('onBegin suite size: ' + suite.allTests().length); + console.log('%% onBegin: ' + suite.allTests().length); } onError(err) { - console.log('error: ' + err.message); + console.log('%% error: ' + err.message); } } module.exports = Reporter; @@ -175,21 +144,24 @@ test('plan throwing aborts the run before onBegin', async ({ runInlineTest }) => }, { reporter: '', workers: 1 }); expect(result.exitCode).not.toBe(0); - expect(result.output).toContain('plan-aborted'); - // Synthetic empty-suite onBegin OK, real onBegin (size 1) must NOT happen. - expect(result.output).not.toContain('onBegin suite size: 1'); + expect(result.outputLines).toContain('error: Error: plan-aborted'); + // Synthetic empty-suite onBegin is OK; the real onBegin (size 1) must NOT happen. + expect(result.outputLines).not.toContain('onBegin: 1'); }); -test('multiple reporters: plan called in order, annotations accumulate', async ({ runInlineTest }) => { +test('multiple reporters: plan called in order, annotations accumulate, exclude prunes for next reporter', async ({ runInlineTest }) => { const result = await runInlineTest({ 'first.ts': ` class R { async plan(config, suite) { - console.log('first plan'); - suite.allTests()[0].fail('first reason'); + console.log('%% first plan sees: ' + suite.allTests().map(t => t.title).join(',')); + for (const t of suite.allTests()) { + if (t.title === 'gone') t.exclude(); + else t.fail('first reason'); + } } onTestEnd(test, result) { - console.log('first onTestEnd: ' + test.expectedStatus + ' ann=' + test.annotations.map(a => a.type).join(',')); + console.log('%% first onTestEnd: ' + test.expectedStatus + ' ann=' + test.annotations.map(a => a.type).join(',')); } } module.exports = R; @@ -197,50 +169,13 @@ test('multiple reporters: plan called in order, annotations accumulate', async ( 'second.ts': ` class R { async plan(config, suite) { - console.log('second plan'); + console.log('%% second plan sees: ' + suite.allTests().map(t => t.title).join(',')); suite.allTests()[0].skip('second reason'); } } module.exports = R; `, 'playwright.config.ts': `module.exports = { reporter: [['./first.ts'], ['./second.ts']] };`, - 'a.test.ts': ` - import { test } from '@playwright/test'; - test('one', async () => {}); - `, - }, { reporter: '', workers: 1 }); - - const idxFirst = result.output.indexOf('first plan'); - const idxSecond = result.output.indexOf('second plan'); - expect(idxFirst).toBeGreaterThan(-1); - expect(idxSecond).toBeGreaterThan(idxFirst); - // Annotations accumulate from both reporters. skip beats fail in - // worker-side expectedStatus derivation (mirroring testInfo semantics). - expect(result.output).toContain('first onTestEnd: skipped'); - expect(result.output).toContain('fail,skip'); -}); - -test('exclude prunes the tree eagerly: later reporter does not see excluded test', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'first.ts': ` - class R { - async plan(config, suite) { - for (const t of suite.allTests()) - if (t.title === 'gone') t.exclude(); - console.log('first sees: ' + suite.allTests().map(t => t.title).join(',')); - } - } - module.exports = R; - `, - 'second.ts': ` - class R { - async plan(config, suite) { - console.log('second sees: ' + suite.allTests().map(t => t.title).join(',')); - } - } - module.exports = R; - `, - 'playwright.config.ts': `module.exports = { reporter: [['./first.ts'], ['./second.ts']] };`, 'a.test.ts': ` import { test } from '@playwright/test'; test('kept', async () => {}); @@ -249,9 +184,12 @@ test('exclude prunes the tree eagerly: later reporter does not see excluded test }, { reporter: '', workers: 1 }); expect(result.exitCode).toBe(0); - expect(result.output).toContain('first sees: kept'); - expect(result.output).toContain('second sees: kept'); - expect(result.output).not.toMatch(/second sees:[^\n]*gone/); + // skip beats fail in expectedStatus, both annotations accumulate. + expect(result.outputLines).toEqual([ + 'first plan sees: kept,gone', + 'second plan sees: kept', + 'first onTestEnd: skipped ann=fail,skip', + ]); }); test('implementsSharding disables built-in shard filter', async ({ runInlineTest }) => { @@ -260,14 +198,13 @@ test('implementsSharding disables built-in shard filter', async ({ runInlineTest class R { implementsSharding() { return true; } async plan(config, suite) { - // Custom "shard": keep only every other test. let i = 0; for (const t of suite.allTests()) { if (i++ % 2 === 1) t.exclude(); } } onBegin(config, suite) { - console.log('begin count: ' + suite.allTests().length); + console.log('%% begin: ' + suite.allTests().map(t => t.title).join(',')); } } module.exports = R; @@ -281,23 +218,20 @@ test('implementsSharding disables built-in shard filter', async ({ runInlineTest }, { reporter: '', workers: 1 }); expect(result.exitCode).toBe(0); - // Reporter sees all 4 tests, excludes every other → 2 kept (would have been ~2 from built-in shard). - // The point: built-in shard didn't run on top of reporter's exclusions. - expect(result.output).toContain('begin count: 2'); + // Reporter sees all 4 tests and excludes every other → t0, t2 kept. + // Built-in shard would have produced a different split (e.g. t0, t1) and + // would further reduce the corpus; the assertion proves it did not run. + expect(result.outputLines).toEqual(['begin: t0,t2']); }); test('multiple reporters declaring implementsSharding throws', async ({ runInlineTest }) => { const result = await runInlineTest({ 'reporter-a.ts': ` - class A { - implementsSharding() { return true; } - } + class A { implementsSharding() { return true; } } module.exports = A; `, 'reporter-b.ts': ` - class B { - implementsSharding() { return true; } - } + class B { implementsSharding() { return true; } } module.exports = B; `, 'playwright.config.ts': `module.exports = { reporter: [['./reporter-a.ts'], ['./reporter-b.ts']] };`, @@ -308,58 +242,7 @@ test('multiple reporters declaring implementsSharding throws', async ({ runInlin }, { reporter: '', workers: 1 }); expect(result.exitCode).not.toBe(0); - expect(result.rawOutput).toMatch(/Multiple reporters declare 'implementsSharding'/); -}); - -test('built-in shard runs when no reporter implements sharding', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'reporter.ts': ` - class R { - async plan(config, suite) { - console.log('plan: ' + suite.allTests().length); - } - onBegin(config, suite) { - console.log('begin: ' + suite.allTests().length); - } - } - module.exports = R; - `, - 'playwright.config.ts': `module.exports = { reporter: './reporter.ts', shard: { current: 1, total: 2 } };`, - 'a.test.ts': ` - import { test } from '@playwright/test'; - test.describe.configure({ mode: 'parallel' }); - for (let i = 0; i < 4; i++) - test('t' + i, async () => {}); - `, - }, { reporter: '', workers: 1 }); - - expect(result.exitCode).toBe(0); - // plan sees full corpus (pre-shard), onBegin sees sharded subset. - expect(result.output).toContain('plan: 4'); - expect(result.output).toContain('begin: 2'); -}); - -test('plan sees the .only-narrowed corpus', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'reporter.ts': ` - class R { - async plan(config, suite) { - console.log('plan tests: ' + suite.allTests().map(t => t.title).join(',')); - } - } - module.exports = R; - `, - 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, - 'a.test.ts': ` - import { test } from '@playwright/test'; - test('one', async () => {}); - test.only('two', async () => {}); - test('three', async () => {}); - `, - }, { reporter: '', workers: 1 }); - - expect(result.exitCode).toBe(0); - expect(result.output).toContain('plan tests: two'); + expect(result.rawOutput).toContain(`Multiple reporters declare 'implementsSharding'`); }); test('plan annotations capture caller location pointing at reporter', async ({ runInlineTest }) => { @@ -372,7 +255,7 @@ test('plan annotations capture caller location pointing at reporter', async ({ r } onTestEnd(test, result) { const a = test.annotations.find(a => a.type === 'skip'); - console.log('loc=' + (a?.location ? require('path').basename(a.location.file) + ':' + a.location.line : 'NONE')); + console.log('%% loc=' + (a?.location ? require('path').basename(a.location.file) + ':' + a.location.line : 'NONE')); } } module.exports = Reporter; @@ -385,5 +268,5 @@ test('plan annotations capture caller location pointing at reporter', async ({ r }, { reporter: '', workers: 1 }); expect(result.exitCode).toBe(0); - expect(result.output).toMatch(/loc=reporter\.ts:\d+/); + expect(result.outputLines).toEqual(['loc=reporter.ts:5']); }); From 051d9649687e934df8196545f3e28b92a3ac9ecc Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 3 Jun 2026 12:34:28 +0200 Subject: [PATCH 5/8] test(test-runner): cover Reporter.plan suite filtering contract Add tests locking in that Reporter.plan's suite contains only top-level projects (setup/dependency projects are not exposed), respects --project and --grep, and ignores --shard (built-in sharding runs after plan). Clarify the Reporter.plan.suite docs accordingly. Fixes: https://github.com/microsoft/playwright/issues/40934 --- docs/src/test-reporter-api/class-reporter.md | 6 +- packages/playwright/types/testReporter.d.ts | 12 +- tests/playwright-test/reporter-plan.spec.ts | 125 +++++++++++++++++++ 3 files changed, 141 insertions(+), 2 deletions(-) diff --git a/docs/src/test-reporter-api/class-reporter.md b/docs/src/test-reporter-api/class-reporter.md index a39f5afe2a8c0..b1796b719a958 100644 --- a/docs/src/test-reporter-api/class-reporter.md +++ b/docs/src/test-reporter-api/class-reporter.md @@ -313,7 +313,11 @@ Resolved configuration. * since: v1.61 - `suite` <[Suite]> -The root suite that contains all projects, files and test cases. +The root suite that contains the projects, files and test cases that will run. + +The suite reflects `--project`, `--grep`/`--grep-invert` and `.only` filtering, so it only contains tests that match the current invocation. It contains only the top-level projects being run — setup and dependency projects are not included and cannot be excluded from here. + +The suite ignores the `--shard` argument: it always contains the full, un-sharded corpus. Playwright applies its built-in sharding after [`method: Reporter.plan`] returns, unless [`method: Reporter.implementsSharding`] returns `true`. ## optional method: Reporter.implementsSharding * since: v1.61 diff --git a/packages/playwright/types/testReporter.d.ts b/packages/playwright/types/testReporter.d.ts index c9030c8e4a69d..2c4a30d7c1573 100644 --- a/packages/playwright/types/testReporter.d.ts +++ b/packages/playwright/types/testReporter.d.ts @@ -240,7 +240,17 @@ export interface Reporter { * [reporter.onBegin(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-on-begin). Allows a * reporter to mark individual tests as skipped, excluded, fixed or failing. * @param config Resolved configuration. - * @param suite The root suite that contains all projects, files and test cases. + * @param suite The root suite that contains the projects, files and test cases that will run. + * + * The suite reflects `--project`, `--grep`/`--grep-invert` and `.only` filtering, so it only contains tests that + * match the current invocation. It contains only the top-level projects being run — setup and dependency projects are + * not included and cannot be excluded from here. + * + * The suite ignores the `--shard` argument: it always contains the full, un-sharded corpus. Playwright applies its + * built-in sharding after + * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) returns, unless + * [reporter.implementsSharding()](https://playwright.dev/docs/api/class-reporter#reporter-implements-sharding) + * returns `true`. */ plan?(config: FullConfig, suite: Suite): Promise; diff --git a/tests/playwright-test/reporter-plan.spec.ts b/tests/playwright-test/reporter-plan.spec.ts index ef618e85f0797..4dc5a8ef6b16d 100644 --- a/tests/playwright-test/reporter-plan.spec.ts +++ b/tests/playwright-test/reporter-plan.spec.ts @@ -245,6 +245,131 @@ test('multiple reporters declaring implementsSharding throws', async ({ runInlin expect(result.rawOutput).toContain(`Multiple reporters declare 'implementsSharding'`); }); +test('plan.suite contains only top-level projects, not dependency/setup projects', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + async plan(config, suite) { + // The suite only exposes top-level projects, so a reporter has no handle on + // setup/dependency project tests and therefore cannot exclude them. + console.log('%% plan projects: ' + suite.suites.map(s => s.title).join(',')); + console.log('%% plan tests: ' + suite.allTests().map(t => t.title).join(',')); + } + onTestEnd(test, result) { + console.log('%% ran ' + test.parent.project().name + '/' + test.title); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': ` + module.exports = { + reporter: './reporter.ts', + projects: [ + { name: 'setup', testMatch: /a\\.setup\\.ts/ }, + { name: 'main', testMatch: /a\\.test\\.ts/, dependencies: ['setup'] }, + ], + }; + `, + 'a.setup.ts': ` + import { test } from '@playwright/test'; + test('setup-test', async () => {}); + `, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('main-test', async () => {}); + `, + }, { reporter: '', workers: 1 }, undefined, { additionalArgs: ['--project=main'] }); + + expect(result.exitCode).toBe(0); + // plan only sees the top-level 'main' project; the 'setup' dependency is prepended afterwards. + expect(result.outputLines).toContain('plan projects: main'); + // 'setup-test' is absent from the plan suite, proving setup/dependency tests are not exposed. + expect(result.outputLines).toContain('plan tests: main-test'); + // Both the dependency and the main project still run. + expect(result.outputLines).toContain('ran setup/setup-test'); + expect(result.outputLines).toContain('ran main/main-test'); +}); + +test('plan.suite respects --grep filtering', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + async plan(config, suite) { + console.log('%% plan: ' + suite.allTests().map(t => t.title).join(',')); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('foo-one', async () => {}); + test('bar-two', async () => {}); + `, + }, { reporter: '', workers: 1, grep: 'foo' }); + + expect(result.exitCode).toBe(0); + expect(result.outputLines).toEqual(['plan: foo-one']); +}); + +test('plan.suite respects --project filtering', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + async plan(config, suite) { + console.log('%% plan projects: ' + suite.suites.map(s => s.title).join(',')); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': ` + module.exports = { + reporter: './reporter.ts', + projects: [{ name: 'one' }, { name: 'two' }], + }; + `, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('t', async () => {}); + `, + }, { reporter: '', workers: 1, project: 'one' }); + + expect(result.exitCode).toBe(0); + expect(result.outputLines).toEqual(['plan projects: one']); +}); + +test('plan.suite ignores --shard; built-in sharding applies after plan', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + async plan(config, suite) { + // plan sees the full, un-sharded corpus. + console.log('%% plan: ' + suite.allTests().map(t => t.title).join(',')); + } + onBegin(config, suite) { + // built-in sharding has narrowed the run after plan. + console.log('%% begin: ' + suite.allTests().map(t => t.title).join(',')); + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts', fullyParallel: true };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + for (let i = 0; i < 4; i++) + test('t' + i, async () => {}); + `, + }, { reporter: '', workers: 1, shard: '1/2' }); + + expect(result.exitCode).toBe(0); + // plan observes all four tests regardless of --shard. + expect(result.outputLines).toContain('plan: t0,t1,t2,t3'); + // The built-in shard filter runs after plan and reduces the corpus. + const beginLine = result.outputLines.find(l => l.startsWith('begin: ')); + expect(beginLine).toBeTruthy(); + expect(beginLine!.slice('begin: '.length).split(',').length).toBe(2); +}); + test('plan annotations capture caller location pointing at reporter', async ({ runInlineTest }) => { const result = await runInlineTest({ 'reporter.ts': ` From 17873513b8f87cda9ab2a872491bcd15d2259b20 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Thu, 4 Jun 2026 10:02:06 +0200 Subject: [PATCH 6/8] chore(test-runner): address Reporter.preprocessSuite review feedback --- docs/src/test-reporter-api/class-reporter.md | 16 ++--- docs/src/test-reporter-api/class-suite.md | 2 +- docs/src/test-reporter-api/class-testcase.md | 8 +-- packages/playwright/src/common/suiteUtils.ts | 42 ++++++------ packages/playwright/src/common/test.ts | 21 ++++++ .../src/reporters/internalReporter.ts | 13 ++-- .../playwright/src/reporters/multiplexer.ts | 25 ++++--- .../playwright/src/reporters/reporterV2.ts | 11 +-- packages/playwright/src/runner/loadUtils.ts | 52 +++++++------- packages/playwright/src/runner/reporters.ts | 7 +- packages/playwright/src/worker/workerMain.ts | 10 +-- packages/playwright/types/testReporter.d.ts | 67 ++++++++----------- tests/playwright-test/reporter-plan.spec.ts | 61 ++++++++++++----- .../overrides-testReporter.d.ts | 1 + 14 files changed, 181 insertions(+), 155 deletions(-) diff --git a/docs/src/test-reporter-api/class-reporter.md b/docs/src/test-reporter-api/class-reporter.md index b1796b719a958..26f8d619e22ca 100644 --- a/docs/src/test-reporter-api/class-reporter.md +++ b/docs/src/test-reporter-api/class-reporter.md @@ -298,18 +298,20 @@ Result of the test run. Whether this reporter uses stdio for reporting. When it does not, Playwright Test could add some output to enhance user experience. If your reporter does not print to the terminal, it is strongly recommended to return `false`. -## optional async method: Reporter.plan +## optional async method: Reporter.preprocessSuite * since: v1.61 +- `result` ?<[Object]> + - `implementsSharding` ?<[boolean]> When `true`, Playwright skips its built-in shard filter for this run, leaving sharding to the reporter (typically implemented by calling [`method: TestCase.exclude`] on out-of-shard tests). Called after the configuration has been resolved and before [`method: Reporter.onBegin`]. Allows a reporter to mark individual tests as skipped, excluded, fixed or failing. -### param: Reporter.plan.config +### param: Reporter.preprocessSuite.config * since: v1.61 - `config` <[FullConfig]> Resolved configuration. -### param: Reporter.plan.suite +### param: Reporter.preprocessSuite.suite * since: v1.61 - `suite` <[Suite]> @@ -317,10 +319,4 @@ The root suite that contains the projects, files and test cases that will run. The suite reflects `--project`, `--grep`/`--grep-invert` and `.only` filtering, so it only contains tests that match the current invocation. It contains only the top-level projects being run — setup and dependency projects are not included and cannot be excluded from here. -The suite ignores the `--shard` argument: it always contains the full, un-sharded corpus. Playwright applies its built-in sharding after [`method: Reporter.plan`] returns, unless [`method: Reporter.implementsSharding`] returns `true`. - -## optional method: Reporter.implementsSharding -* since: v1.61 -- returns: <[boolean]> - -When `true`, Playwright skips its built-in shard filter for this run, leaving sharding to the reporter (typically implemented inside [`method: Reporter.plan`] by calling [`method: TestCase.exclude`] on out-of-shard tests). +The suite ignores the `--shard` argument: it always contains the full, un-sharded corpus. Playwright applies its built-in sharding after [`method: Reporter.preprocessSuite`] returns, unless the returned `implementsSharding` is `true`. diff --git a/docs/src/test-reporter-api/class-suite.md b/docs/src/test-reporter-api/class-suite.md index 0bec189b9f1ca..f9b06e36a5025 100644 --- a/docs/src/test-reporter-api/class-suite.md +++ b/docs/src/test-reporter-api/class-suite.md @@ -122,4 +122,4 @@ Optional explanation surfaced as the annotation description. ## method: Suite.exclude * since: v1.61 -Must be called from inside [`method: Reporter.plan`], exclude this suite from the run. Excluded tests do not appear in the report and their body is not executed. +Must be called from inside [`method: Reporter.preprocessSuite`], exclude this suite from the run. Excluded tests do not appear in the report and their body is not executed. diff --git a/docs/src/test-reporter-api/class-testcase.md b/docs/src/test-reporter-api/class-testcase.md index 006d41f0bbb86..95e6052e32ae7 100644 --- a/docs/src/test-reporter-api/class-testcase.md +++ b/docs/src/test-reporter-api/class-testcase.md @@ -111,7 +111,7 @@ Returns "test". Useful for detecting test cases in [`method: Suite.entries`]. ## method: TestCase.skip * since: v1.61 -Must be called from inside [`method: Reporter.plan`], skip this test. The test body is not executed and the test is reported as skipped. +Must be called from inside [`method: Reporter.preprocessSuite`], skip this test. The test body is not executed and the test is reported as skipped. ### param: TestCase.skip.reason * since: v1.61 @@ -122,7 +122,7 @@ Optional explanation surfaced as the annotation description. ## method: TestCase.fixme * since: v1.61 -Must be called from inside [`method: Reporter.plan`], mark this test as fixme. The test body is not executed and the test is reported as skipped, with the intention to fix it. +Must be called from inside [`method: Reporter.preprocessSuite`], mark this test as fixme. The test body is not executed and the test is reported as skipped, with the intention to fix it. ### param: TestCase.fixme.reason * since: v1.61 @@ -133,7 +133,7 @@ Optional explanation surfaced as the annotation description. ## method: TestCase.fail * since: v1.61 -Must be called from inside [`method: Reporter.plan`], mark this test as "should fail". Playwright runs the test and ensures it is actually failing, useful for documenting broken functionality until it is fixed. +Must be called from inside [`method: Reporter.preprocessSuite`], mark this test as "should fail". Playwright runs the test and ensures it is actually failing, useful for documenting broken functionality until it is fixed. ### param: TestCase.fail.reason * since: v1.61 @@ -144,4 +144,4 @@ Optional explanation surfaced as the annotation description. ## method: TestCase.exclude * since: v1.61 -Must be called from inside [`method: Reporter.plan`], exclude this test from the run. Excluded tests do not appear in the report and their body is not executed. +Must be called from inside [`method: Reporter.preprocessSuite`], exclude this test from the run. Excluded tests do not appear in the report and their body is not executed. diff --git a/packages/playwright/src/common/suiteUtils.ts b/packages/playwright/src/common/suiteUtils.ts index f40be3f98fbf9..6c7128cca6cf9 100644 --- a/packages/playwright/src/common/suiteUtils.ts +++ b/packages/playwright/src/common/suiteUtils.ts @@ -25,6 +25,14 @@ import type { FullProjectInternal } from './config'; import type { Suite, TestCase } from './test'; import type { Matcher, TestCaseFilter } from '../util'; +export function filterTestsRemoveEmptySuites(suite: Suite, filter: TestCaseFilter): boolean { + const filteredSuites = suite.suites.filter(child => filterTestsRemoveEmptySuites(child, filter)); + const filteredTests = suite.tests.filter(filter); + const entries = new Set([...filteredSuites, ...filteredTests]); + suite._entries = suite._entries.filter(e => entries.has(e)); // Preserve the order. + return !!suite._entries.length; +} + export function bindFileSuiteToProject(project: FullProjectInternal, suite: Suite): Suite { const relativeFile = path.relative(project.project.testDir, suite.location!.file); const fileId = calculateSha1(toPosixPath(relativeFile)).slice(0, 20); @@ -85,27 +93,23 @@ export function applyRepeatEachIndex(project: FullProjectInternal, fileSuite: Su }); } -export function filterOnly(suite: Suite): boolean { - const toExclude: (Suite | TestCase)[] = []; - let hasOnlyInside = false; - for (const child of suite.suites) { - if (filterOnly(child)) - hasOnlyInside = true; - else - toExclude.push(child); - } - for (const test of suite.tests) { - if (test._only) - hasOnlyInside = true; - else - toExclude.push(test); - } - if (hasOnlyInside) { - for (const e of toExclude) - e.exclude(); +export function filterOnly(suite: Suite) { + if (!suite._getOnlyItems().length) + return; + const suiteFilter = (suite: Suite) => suite._only; + const testFilter = (test: TestCase) => test._only; + return filterSuiteWithOnlySemantics(suite, suiteFilter, testFilter); +} + +function filterSuiteWithOnlySemantics(suite: Suite, suiteFilter: (suite: Suite) => boolean, testFilter: TestCaseFilter) { + const onlySuites = suite.suites.filter(child => filterSuiteWithOnlySemantics(child, suiteFilter, testFilter) || suiteFilter(child)); + const onlyTests = suite.tests.filter(testFilter); + const onlyEntries = new Set([...onlySuites, ...onlyTests]); + if (onlyEntries.size) { + suite._entries = suite._entries.filter(e => onlyEntries.has(e)); // Preserve the order. return true; } - return !!suite._only; + return false; } export function createFiltersFromArguments(args: string[]): { fileFilter: Matcher, testFilter: TestCaseFilter } { diff --git a/packages/playwright/src/common/test.ts b/packages/playwright/src/common/test.ts index a5fc5e401d827..a3be0940cff9b 100644 --- a/packages/playwright/src/common/test.ts +++ b/packages/playwright/src/common/test.ts @@ -60,6 +60,9 @@ export class Suite extends Base { _parallelMode: 'none' | 'default' | 'serial' | 'parallel' = 'none'; _fullProject: FullProjectInternal | undefined; _fileId: string | undefined; + // Set on the root suite for the duration of Reporter.preprocessSuite(), gating + // the disposition methods (skip/fixme/fail/exclude) to that phase only. + _preprocessing = false; readonly _type: 'root' | 'project' | 'file' | 'describe'; constructor(title: string, type: 'root' | 'project' | 'file' | 'describe') { @@ -279,11 +282,17 @@ export class Suite extends Base { } exclude(): void { + if (!this._rootSuite()._preprocessing) + throw new Error(`Suite.exclude() can only be called from Reporter.preprocessSuite().`); if (this.parent) this.parent._detach(this); else this._entries = []; } + + _rootSuite(): Suite { + return this.parent?._rootSuite() ?? this; + } } export class TestCase extends Base implements reporterTypes.TestCase { @@ -343,6 +352,8 @@ export class TestCase extends Base implements reporterTypes.TestCase { } skip(reason?: string): void { + if (!this._rootSuite()._preprocessing) + throw new Error(`TestCase.skip() can only be called from Reporter.preprocessSuite().`); const annotation: TestAnnotation = { type: 'skip', description: reason, location: captureCallerLocation() }; this.annotations.push(annotation); this._planAnnotations.push(annotation); @@ -350,6 +361,8 @@ export class TestCase extends Base implements reporterTypes.TestCase { } fixme(reason?: string): void { + if (!this._rootSuite()._preprocessing) + throw new Error(`TestCase.fixme() can only be called from Reporter.preprocessSuite().`); const annotation: TestAnnotation = { type: 'fixme', description: reason, location: captureCallerLocation() }; this.annotations.push(annotation); this._planAnnotations.push(annotation); @@ -357,6 +370,8 @@ export class TestCase extends Base implements reporterTypes.TestCase { } fail(reason?: string): void { + if (!this._rootSuite()._preprocessing) + throw new Error(`TestCase.fail() can only be called from Reporter.preprocessSuite().`); const annotation: TestAnnotation = { type: 'fail', description: reason, location: captureCallerLocation() }; this.annotations.push(annotation); this._planAnnotations.push(annotation); @@ -365,9 +380,15 @@ export class TestCase extends Base implements reporterTypes.TestCase { } exclude(): void { + if (!this._rootSuite()._preprocessing) + throw new Error(`TestCase.exclude() can only be called from Reporter.preprocessSuite().`); this.parent._detach(this); } + _rootSuite(): Suite { + return this.parent._rootSuite(); + } + _serialize(): any { return { kind: 'test', diff --git a/packages/playwright/src/reporters/internalReporter.ts b/packages/playwright/src/reporters/internalReporter.ts index deb40c71315dc..3afea92b23ec0 100644 --- a/packages/playwright/src/reporters/internalReporter.ts +++ b/packages/playwright/src/reporters/internalReporter.ts @@ -50,8 +50,13 @@ export class InternalReporter implements ReporterV2 { this._reporter.onConfigure?.(config); } - async plan(config: FullConfig, suite: testNs.Suite) { - await this._reporter.plan?.(config, suite); + async preprocessSuite(config: FullConfig, suite: testNs.Suite) { + suite._preprocessing = true; + try { + return await this._reporter.preprocessSuite?.(config, suite); + } finally { + suite._preprocessing = false; + } } onBegin(suite: testNs.Suite) { @@ -115,10 +120,6 @@ export class InternalReporter implements ReporterV2 { return this._reporter.printsToStdio?.() ?? true; } - implementsSharding() { - return this._reporter.implementsSharding?.() ?? false; - } - private _addSnippetToTestErrors(test: TestCase, result: TestResult) { for (const error of result.errors) addLocationAndSnippetToError(this._config, error, test.location.file); diff --git a/packages/playwright/src/reporters/multiplexer.ts b/packages/playwright/src/reporters/multiplexer.ts index efb7a802d10d5..bf271824a034a 100644 --- a/packages/playwright/src/reporters/multiplexer.ts +++ b/packages/playwright/src/reporters/multiplexer.ts @@ -34,13 +34,20 @@ export class Multiplexer implements ReporterV2 { wrap(() => reporter.onConfigure?.(config)); } - async plan(config: FullConfig, suite: test.Suite) { - // Unlike other reporter callbacks, `plan` errors are NOT swallowed — - // they propagate so the run aborts before onBegin. Reporters use plan + async preprocessSuite(config: FullConfig, suite: test.Suite) { + // Unlike other reporter callbacks, `preprocessSuite` errors are NOT swallowed — + // they propagate so the run aborts before onBegin. Reporters use preprocessSuite // to mutate the corpus; silently dropping a planning error would let // an inconsistent (partial-mutation) state reach the workers. - for (const reporter of this._reporters) - await reporter.plan?.(config, suite); + const shardingReporters: ReporterV2[] = []; + for (const reporter of this._reporters) { + const result = await reporter.preprocessSuite?.(config, suite); + if (result?.implementsSharding) + shardingReporters.push(reporter); + } + if (shardingReporters.length > 1) + throw new Error(`Multiple reporters declare 'implementsSharding': ${shardingReporters.map(r => r.constructor?.name ?? 'reporter').join(', ')}. Only one reporter may handle sharding.`); + return { implementsSharding: shardingReporters.length > 0 }; } onBegin(suite: test.Suite) { @@ -119,14 +126,6 @@ export class Multiplexer implements ReporterV2 { return prints; }); } - - implementsSharding(): boolean { - return this._reporters.some(r => { - let shards = false; - wrap(() => shards = r.implementsSharding ? r.implementsSharding() : false); - return shards; - }); - } } async function wrapAsync(callback: () => T | Promise) { diff --git a/packages/playwright/src/reporters/reporterV2.ts b/packages/playwright/src/reporters/reporterV2.ts index 77e96d867165a..3d5a65a53d46a 100644 --- a/packages/playwright/src/reporters/reporterV2.ts +++ b/packages/playwright/src/reporters/reporterV2.ts @@ -28,7 +28,7 @@ export interface ReportEndParams { export interface ReporterV2 { onConfigure?(config: FullConfig): void; - plan?(config: FullConfig, suite: Suite): void | Promise; + preprocessSuite?(config: FullConfig, suite: Suite): { implementsSharding?: boolean } | Promise<{ implementsSharding?: boolean } | undefined | void> | void; onBegin?(suite: Suite): void; onTestBegin?(test: TestCase, result: TestResult): void; onStdOut?(chunk: string | Buffer, test?: TestCase, result?: TestResult): void; @@ -43,7 +43,6 @@ export interface ReporterV2 { onStepBegin?(test: TestCase, result: TestResult, step: TestStep): void; onStepEnd?(test: TestCase, result: TestResult, step: TestStep): void; printsToStdio?(): boolean; - implementsSharding?(): boolean; version(): 'v2'; } @@ -81,8 +80,8 @@ class ReporterV2Wrapper implements ReporterV2 { this._config = config; } - async plan(config: FullConfig, suite: Suite) { - await this._reporter.plan?.(config, suite); + async preprocessSuite(config: FullConfig, suite: Suite) { + return await this._reporter.preprocessSuite?.(config, suite); } onBegin(suite: Suite) { @@ -151,8 +150,4 @@ class ReporterV2Wrapper implements ReporterV2 { printsToStdio() { return this._reporter.printsToStdio ? this._reporter.printsToStdio() : true; } - - implementsSharding() { - return this._reporter.implementsSharding ? this._reporter.implementsSharding() : false; - } } diff --git a/packages/playwright/src/runner/loadUtils.ts b/packages/playwright/src/runner/loadUtils.ts index 3ae087f5dc751..9485a923ba5a1 100644 --- a/packages/playwright/src/runner/loadUtils.ts +++ b/packages/playwright/src/runner/loadUtils.ts @@ -23,7 +23,7 @@ import { toPosixPath } from '@utils/fileUtils'; import { InProcessLoaderHost, OutOfProcessLoaderHost } from './loaderHost'; import { createTitleMatcher, errorWithFile, parseLocationArg } from '../util'; import { buildProjectsClosure, collectFilesForProject } from './projectUtils'; -import { createTestGroups, filterForShard } from './testGroups'; +import { createTestGroups, filterForShard } from './testGroups'; import { cc, config as commonConfig, FullConfigInternal, suiteUtils, test as testNs, transform } from '../common'; import type { RawSourceMap } from 'source-map'; @@ -129,15 +129,9 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho for (const [project, fileSuites] of testRun.projectSuites) { const projectSuite = createProjectSuite(project, fileSuites); projectSuites.set(project, projectSuite); - filteredProjectSuites.set(project, projectSuite); - if (testRun.preOnlyTestFilters.length) { - const filteredProjectSuite = projectSuite._deepClone(); - for (const test of filteredProjectSuite.allTests()) { - if (!testRun.preOnlyTestFilters.every(f => f(test))) - test.exclude(); - } - filteredProjectSuites.set(project, filteredProjectSuite); - } + + const filteredProjectSuite = filterProjectSuite(projectSuite, testRun.preOnlyTestFilters); + filteredProjectSuites.set(project, filteredProjectSuite); } } @@ -171,9 +165,9 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho } } - await testRun.reporter.plan(config.config, rootSuite); + const preprocessResult = await testRun.reporter.preprocessSuite(config.config, rootSuite); // Shard only the top-level projects. - if (config.config.shard && !testRun.reporter.implementsSharding()) { + if (config.config.shard && !preprocessResult?.implementsSharding) { // Create test groups for top-level projects. const testGroups: TestGroup[] = []; for (const projectSuite of rootSuite.suites) { @@ -191,19 +185,12 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho testsInThisShard.add(test); } - // Drop all tests that are not in this shard. - for (const t of rootSuite.allTests()) { - if (!testsInThisShard.has(t)) - t.exclude(); - } + // Update project suites, removing empty ones. + suiteUtils.filterTestsRemoveEmptySuites(rootSuite, test => testsInThisShard.has(test)); } - if (testRun.postShardTestFilters.length){ - for (const test of rootSuite.allTests()) { - if (!testRun.postShardTestFilters.every(f => f(test))) - test.exclude(); - } - } + if (testRun.postShardTestFilters.length) + suiteUtils.filterTestsRemoveEmptySuites(rootSuite, test => testRun.postShardTestFilters.every(filter => filter(test))); const topLevelProjects = []; // Now prepend dependency projects without filtration. @@ -232,14 +219,25 @@ function createProjectSuite(project: commonConfig.FullProjectInternal, fileSuite const grepMatcher = createTitleMatcher(project.project.grep); const grepInvertMatcher = project.project.grepInvert ? createTitleMatcher(project.project.grepInvert) : null; - for (const test of projectSuite.allTests()) { + suiteUtils.filterTestsRemoveEmptySuites(projectSuite, (test: testNs.TestCase) => { const grepTitle = test._grepTitleWithTags(); - if (grepInvertMatcher?.(grepTitle) || !grepMatcher(grepTitle)) - test.exclude(); - } + if (grepInvertMatcher?.(grepTitle)) + return false; + return grepMatcher(grepTitle); + }); return projectSuite; } +function filterProjectSuite(projectSuite: testNs.Suite, testFilters: TestCaseFilter[]): testNs.Suite { + // Fast path. + if (!testFilters.length) + return projectSuite; + + const result = projectSuite._deepClone(); + suiteUtils.filterTestsRemoveEmptySuites(result, test => testFilters.every(filter => filter(test))); + return result; +} + function buildProjectSuite(project: commonConfig.FullProjectInternal, projectSuite: testNs.Suite): testNs.Suite { const result = new testNs.Suite(project.project.name, 'project'); result._fullProject = project; diff --git a/packages/playwright/src/runner/reporters.ts b/packages/playwright/src/runner/reporters.ts index 7bb85f87547b5..00dd732b63991 100644 --- a/packages/playwright/src/runner/reporters.ts +++ b/packages/playwright/src/runner/reporters.ts @@ -74,7 +74,7 @@ export async function createReporters(config: FullConfigInternal, mode: 'list' | } } - const someReporterPrintsToStdio = reporters.some(r => r.printsToStdio?.() ?? true); + const someReporterPrintsToStdio = reporters.some(r => r.printsToStdio ? r.printsToStdio() : true); if (reporters.length && !someReporterPrintsToStdio) { // Add a line/dot/list-mode reporter for convenience. // Important to put it first, just in case some other reporter stalls onEnd. @@ -83,11 +83,6 @@ export async function createReporters(config: FullConfigInternal, mode: 'list' | else if (mode !== 'merge') reporters.unshift(!process.env.CI ? new LineReporter() : new DotReporter()); } - - const shardingReporters = reporters.filter(r => r.implementsSharding?.() ?? false); - if (shardingReporters.length > 1) - throw new Error(`Multiple reporters declare 'implementsSharding': ${shardingReporters.map(r => r.constructor?.name ?? 'reporter').join(', ')}. Only one reporter may handle sharding.`); - return reporters; } diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index c1c9f69f9b6cf..163b2e455e4e7 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -220,14 +220,10 @@ export class WorkerMain extends ProcessRunner { const suite = suiteUtils.bindFileSuiteToProject(this._project, fileSuite); if (this._params.repeatEachIndex) suiteUtils.applyRepeatEachIndex(this._project, suite, this._params.repeatEachIndex); - for (const test of suite.allTests()) { - const entry = entries.get(test.id); - if (!entry) - test.exclude(); - else - test.annotations.push(...entry.planAnnotations); - } + suiteUtils.filterTestsRemoveEmptySuites(suite, test => entries.has(test.id)); const tests = suite.allTests(); + for (const test of tests) + test.annotations.push(...entries.get(test.id)!.planAnnotations); // Collect test IDs that were not found in the worker // (e.g. test titles changed between runner and worker). diff --git a/packages/playwright/types/testReporter.d.ts b/packages/playwright/types/testReporter.d.ts index 2c4a30d7c1573..3ae8fedda5f5c 100644 --- a/packages/playwright/types/testReporter.d.ts +++ b/packages/playwright/types/testReporter.d.ts @@ -145,6 +145,23 @@ export interface FullResult { * [reporter.onBegin(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-on-begin). */ export interface Reporter { + /** + * Called after the configuration has been resolved and before + * [reporter.onBegin(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-on-begin). Allows a + * reporter to mark individual tests as skipped, excluded, fixed or failing. + * @param config Resolved configuration. + * @param suite The root suite that contains the projects, files and test cases that will run. + * + * The suite reflects `--project`, `--grep`/`--grep-invert` and `.only` filtering, so it only contains tests that + * match the current invocation. It contains only the top-level projects being run — setup and dependency projects are + * not included and cannot be excluded from here. + * + * The suite ignores the `--shard` argument: it always contains the full, un-sharded corpus. Playwright applies its + * built-in sharding after + * [reporter.preprocessSuite(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-preprocess-suite) + * returns, unless the returned `implementsSharding` is `true`. + */ + preprocessSuite?(config: FullConfig, suite: Suite): Promise<{ implementsSharding?: boolean } | undefined | void> | { implementsSharding?: boolean } | void; /** * Called after all tests have been run, or testing has been interrupted. Note that this method may return a [Promise] * and Playwright Test will await it. Reporter is allowed to override the status and hence affect the exit code of the @@ -158,14 +175,6 @@ export interface Reporter { * - `'interrupted'` - Interrupted by the user. */ onEnd?(result: FullResult): Promise<{ status?: FullResult['status'] } | undefined | void> | void; - /** - * When `true`, Playwright skips its built-in shard filter for this run, leaving sharding to the reporter (typically - * implemented inside [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) by - * calling [testCase.exclude()](https://playwright.dev/docs/api/class-testcase#test-case-exclude) on out-of-shard - * tests). - */ - implementsSharding?(): boolean; - /** * Called once before running tests. All tests have been already discovered and put into a hierarchy of * [Suite](https://playwright.dev/docs/api/class-suite)s. @@ -235,25 +244,6 @@ export interface Reporter { */ onTestEnd?(test: TestCase, result: TestResult): void; - /** - * Called after the configuration has been resolved and before - * [reporter.onBegin(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-on-begin). Allows a - * reporter to mark individual tests as skipped, excluded, fixed or failing. - * @param config Resolved configuration. - * @param suite The root suite that contains the projects, files and test cases that will run. - * - * The suite reflects `--project`, `--grep`/`--grep-invert` and `.only` filtering, so it only contains tests that - * match the current invocation. It contains only the top-level projects being run — setup and dependency projects are - * not included and cannot be excluded from here. - * - * The suite ignores the `--shard` argument: it always contains the full, un-sharded corpus. Playwright applies its - * built-in sharding after - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan) returns, unless - * [reporter.implementsSharding()](https://playwright.dev/docs/api/class-reporter#reporter-implements-sharding) - * returns `true`. - */ - plan?(config: FullConfig, suite: Suite): Promise; - /** * Whether this reporter uses stdio for reporting. When it does not, Playwright Test could add some output to enhance * user experience. If your reporter does not print to the terminal, it is strongly recommended to return `false`. @@ -397,8 +387,8 @@ export interface Suite { /** * Must be called from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), exclude this suite - * from the run. Excluded tests do not appear in the report and their body is not executed. + * [reporter.preprocessSuite(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-preprocess-suite), + * exclude this suite from the run. Excluded tests do not appear in the report and their body is not executed. */ exclude(): void; @@ -484,24 +474,25 @@ export interface Suite { export interface TestCase { /** * Must be called from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), exclude this test - * from the run. Excluded tests do not appear in the report and their body is not executed. + * [reporter.preprocessSuite(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-preprocess-suite), + * exclude this test from the run. Excluded tests do not appear in the report and their body is not executed. */ exclude(): void; /** * Must be called from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), mark this test as - * "should fail". Playwright runs the test and ensures it is actually failing, useful for documenting broken - * functionality until it is fixed. + * [reporter.preprocessSuite(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-preprocess-suite), + * mark this test as "should fail". Playwright runs the test and ensures it is actually failing, useful for + * documenting broken functionality until it is fixed. * @param reason Optional explanation surfaced as the annotation description. */ fail(reason?: string): void; /** * Must be called from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), mark this test as - * fixme. The test body is not executed and the test is reported as skipped, with the intention to fix it. + * [reporter.preprocessSuite(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-preprocess-suite), + * mark this test as fixme. The test body is not executed and the test is reported as skipped, with the intention to + * fix it. * @param reason Optional explanation surfaced as the annotation description. */ fixme(reason?: string): void; @@ -521,8 +512,8 @@ export interface TestCase { /** * Must be called from inside - * [reporter.plan(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-plan), skip this test. The - * test body is not executed and the test is reported as skipped. + * [reporter.preprocessSuite(config, suite)](https://playwright.dev/docs/api/class-reporter#reporter-preprocess-suite), + * skip this test. The test body is not executed and the test is reported as skipped. * @param reason Optional explanation surfaced as the annotation description. */ skip(reason?: string): void; diff --git a/tests/playwright-test/reporter-plan.spec.ts b/tests/playwright-test/reporter-plan.spec.ts index 4dc5a8ef6b16d..f038e4a8bb3b5 100644 --- a/tests/playwright-test/reporter-plan.spec.ts +++ b/tests/playwright-test/reporter-plan.spec.ts @@ -20,7 +20,7 @@ test('plan runs between project setup and onBegin, sees the .only-narrowed corpu const result = await runInlineTest({ 'reporter.ts': ` class Reporter { - async plan(config, suite) { + async preprocessSuite(config, suite) { console.log('%% plan: ' + suite.allTests().map(t => t.title).join(',')); for (const t of suite.allTests()) if (t.title.includes('skip-me')) t.skip('planned skip'); @@ -56,7 +56,7 @@ test('TestCase.exclude removes test from run and report', async ({ runInlineTest const result = await runInlineTest({ 'reporter.ts': ` class Reporter { - async plan(config, suite) { + async preprocessSuite(config, suite) { for (const t of suite.allTests()) if (t.title === 'excluded') t.exclude(); } @@ -88,7 +88,7 @@ test('Suite.skip cascades to all descendants', async ({ runInlineTest }) => { const result = await runInlineTest({ 'reporter.ts': ` class Reporter { - async plan(config, suite) { + async preprocessSuite(config, suite) { const visit = (s) => { if (s.title === 'doomed') s.skip('whole group'); for (const child of s.suites || []) visit(child); @@ -120,11 +120,37 @@ test('Suite.skip cascades to all descendants', async ({ runInlineTest }) => { ]); }); +test('disposition methods throw when called outside preprocessSuite', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + onBegin(config, suite) { + try { + suite.allTests()[0].exclude(); + console.log('%% no-throw'); + } catch (e) { + console.log('%% threw: ' + e.message); + } + } + } + module.exports = Reporter; + `, + 'playwright.config.ts': `module.exports = { reporter: './reporter.ts' };`, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('t', async () => {}); + `, + }, { reporter: '', workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.outputLines).toContain('threw: TestCase.exclude() can only be called from Reporter.preprocessSuite().'); +}); + test('plan throwing aborts the run before onBegin', async ({ runInlineTest }) => { const result = await runInlineTest({ 'reporter.ts': ` class Reporter { - async plan(config, suite) { + async preprocessSuite(config, suite) { throw new Error('plan-aborted'); } onBegin(config, suite) { @@ -153,7 +179,7 @@ test('multiple reporters: plan called in order, annotations accumulate, exclude const result = await runInlineTest({ 'first.ts': ` class R { - async plan(config, suite) { + async preprocessSuite(config, suite) { console.log('%% first plan sees: ' + suite.allTests().map(t => t.title).join(',')); for (const t of suite.allTests()) { if (t.title === 'gone') t.exclude(); @@ -168,7 +194,7 @@ test('multiple reporters: plan called in order, annotations accumulate, exclude `, 'second.ts': ` class R { - async plan(config, suite) { + async preprocessSuite(config, suite) { console.log('%% second plan sees: ' + suite.allTests().map(t => t.title).join(',')); suite.allTests()[0].skip('second reason'); } @@ -196,12 +222,12 @@ test('implementsSharding disables built-in shard filter', async ({ runInlineTest const result = await runInlineTest({ 'reporter.ts': ` class R { - implementsSharding() { return true; } - async plan(config, suite) { + async preprocessSuite(config, suite) { let i = 0; for (const t of suite.allTests()) { if (i++ % 2 === 1) t.exclude(); } + return { implementsSharding: true }; } onBegin(config, suite) { console.log('%% begin: ' + suite.allTests().map(t => t.title).join(',')); @@ -227,11 +253,14 @@ test('implementsSharding disables built-in shard filter', async ({ runInlineTest test('multiple reporters declaring implementsSharding throws', async ({ runInlineTest }) => { const result = await runInlineTest({ 'reporter-a.ts': ` - class A { implementsSharding() { return true; } } + class A { + preprocessSuite() { return { implementsSharding: true }; } + onError(err) { console.log('%% error: ' + err.message); } + } module.exports = A; `, 'reporter-b.ts': ` - class B { implementsSharding() { return true; } } + class B { preprocessSuite() { return { implementsSharding: true }; } } module.exports = B; `, 'playwright.config.ts': `module.exports = { reporter: [['./reporter-a.ts'], ['./reporter-b.ts']] };`, @@ -242,14 +271,14 @@ test('multiple reporters declaring implementsSharding throws', async ({ runInlin }, { reporter: '', workers: 1 }); expect(result.exitCode).not.toBe(0); - expect(result.rawOutput).toContain(`Multiple reporters declare 'implementsSharding'`); + expect(result.outputLines.join('\n')).toContain(`Multiple reporters declare 'implementsSharding'`); }); test('plan.suite contains only top-level projects, not dependency/setup projects', async ({ runInlineTest }) => { const result = await runInlineTest({ 'reporter.ts': ` class Reporter { - async plan(config, suite) { + async preprocessSuite(config, suite) { // The suite only exposes top-level projects, so a reporter has no handle on // setup/dependency project tests and therefore cannot exclude them. console.log('%% plan projects: ' + suite.suites.map(s => s.title).join(',')); @@ -294,7 +323,7 @@ test('plan.suite respects --grep filtering', async ({ runInlineTest }) => { const result = await runInlineTest({ 'reporter.ts': ` class Reporter { - async plan(config, suite) { + async preprocessSuite(config, suite) { console.log('%% plan: ' + suite.allTests().map(t => t.title).join(',')); } } @@ -316,7 +345,7 @@ test('plan.suite respects --project filtering', async ({ runInlineTest }) => { const result = await runInlineTest({ 'reporter.ts': ` class Reporter { - async plan(config, suite) { + async preprocessSuite(config, suite) { console.log('%% plan projects: ' + suite.suites.map(s => s.title).join(',')); } } @@ -342,7 +371,7 @@ test('plan.suite ignores --shard; built-in sharding applies after plan', async ( const result = await runInlineTest({ 'reporter.ts': ` class Reporter { - async plan(config, suite) { + async preprocessSuite(config, suite) { // plan sees the full, un-sharded corpus. console.log('%% plan: ' + suite.allTests().map(t => t.title).join(',')); } @@ -374,7 +403,7 @@ test('plan annotations capture caller location pointing at reporter', async ({ r const result = await runInlineTest({ 'reporter.ts': ` class Reporter { - async plan(config, suite) { + async preprocessSuite(config, suite) { for (const t of suite.allTests()) t.skip('planned'); } diff --git a/utils/generate_types/overrides-testReporter.d.ts b/utils/generate_types/overrides-testReporter.d.ts index b67b850603af1..a4376aabca5b0 100644 --- a/utils/generate_types/overrides-testReporter.d.ts +++ b/utils/generate_types/overrides-testReporter.d.ts @@ -42,6 +42,7 @@ export interface FullResult { } export interface Reporter { + preprocessSuite?(config: FullConfig, suite: Suite): Promise<{ implementsSharding?: boolean } | undefined | void> | { implementsSharding?: boolean } | void; onEnd?(result: FullResult): Promise<{ status?: FullResult['status'] } | undefined | void> | void; } From a6a00f75f35fdfca1beba1534caf6eca4ecd3d4a Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Thu, 18 Jun 2026 12:42:58 +0200 Subject: [PATCH 7/8] test(test-runner): cover greedy time-based scheduling on preprocessSuite --- tests/playwright-test/reporter-plan.spec.ts | 92 +++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/playwright-test/reporter-plan.spec.ts b/tests/playwright-test/reporter-plan.spec.ts index f038e4a8bb3b5..a6b4c5e5df9e1 100644 --- a/tests/playwright-test/reporter-plan.spec.ts +++ b/tests/playwright-test/reporter-plan.spec.ts @@ -14,7 +14,9 @@ * limitations under the License. */ +import * as fs from 'fs'; import { test, expect } from './playwright-test-fixtures'; +import { Reporter, FullConfig, Suite, TestCase, TestResult } from 'packages/playwright-test/reporter'; test('plan runs between project setup and onBegin, sees the .only-narrowed corpus, and can skip tests', async ({ runInlineTest }) => { const result = await runInlineTest({ @@ -424,3 +426,93 @@ test('plan annotations capture caller location pointing at reporter', async ({ r expect(result.exitCode).toBe(0); expect(result.outputLines).toEqual(['loc=reporter.ts:5']); }); + +test('greedy time-based scheduling can be built on preprocessSuite', async ({ runInlineTest, mergeReports }) => { + test.slow(); + + const timingsFile = test.info().outputPath('timings.json'); + + const files = { + 'scheduler.ts': ` + const fs = require('fs'); + ${class Scheduler implements Reporter { + _durations = new Map(); + _config!: FullConfig; + preprocessSuite(config: FullConfig, suite: Suite) { + if (!config.shard) + throw new Error('Should not be called during merge step.'); + const file = process.env.TIMINGS_FILE; + const timings = file && fs.existsSync(file) ? JSON.parse(fs.readFileSync(file, 'utf8')) : null; + if (!timings) + return { implementsSharding: false }; + const total = config.shard.total; + const tests = suite.allTests(); + const sorted = [...tests].sort((a, b) => (timings[b.id] || 0) - (timings[a.id] || 0)); + const loads = new Array(total).fill(0); + const assignment = new Map(); + for (const t of sorted) { + let min = 0; + for (let i = 1; i < total; i++) { + if (loads[i] < loads[min]) + min = i; + } + loads[min] += (timings[t.id] || 0); + assignment.set(t, min); + } + for (const t of tests) { + if (assignment.get(t) !== config.shard.current - 1) + t.exclude(); + } + return { implementsSharding: true }; + } + onBegin(config: FullConfig, suite: Suite) { + this._config = config; + const shard = config.shard ? config.shard.current + '/' + config.shard.total : 'none'; + console.log('%% shard ' + shard + ': ' + suite.allTests().map(t => t.title).join(',')); + } + onTestEnd(test: TestCase, result: TestResult) { + this._durations.set(test.id, result.duration); + } + onEnd() { + if (this._config.shard) + return; + fs.writeFileSync(process.env.TIMINGS_FILE, JSON.stringify(Object.fromEntries(this._durations))); + } + }.toString()} + module.exports = Scheduler; + `, + 'playwright.config.ts': ` + module.exports = { + fullyParallel: true, + reporter: [['blob'], ['./scheduler.ts']], + }; + `, + 'a.test.ts': ` + import { test } from '@playwright/test'; + test('a', async () => { await new Promise(r => setTimeout(r, 3000)); }); + test('b', async () => { await new Promise(r => setTimeout(r, 1500)); }); + test('c', async () => { await new Promise(r => setTimeout(r, 100)); }); + `, + }; + + // Round 1: no timing file, falls back to built-in contiguous sharding. + const r1s1 = await runInlineTest(files, { shard: '1/2' }, { TIMINGS_FILE: timingsFile }); + const r1s2 = await runInlineTest(files, { shard: '2/2' }, { TIMINGS_FILE: timingsFile, PWTEST_BLOB_DO_NOT_REMOVE: '1' }); + expect(r1s1.exitCode).toBe(0); + expect(r1s2.exitCode).toBe(0); + expect(r1s1.outputLines).toEqual(['shard 1/2: a,b']); + expect(r1s2.outputLines).toEqual(['shard 2/2: c']); + + // Merge: the reporter sees every test's duration and writes the timing file. + const merge = await mergeReports('blob-report', { TIMINGS_FILE: timingsFile }, { additionalArgs: ['--reporter', 'scheduler.ts'] }); + expect(merge.exitCode).toBe(0); + expect(Object.keys(JSON.parse(fs.readFileSync(timingsFile, 'utf8')))).toHaveLength(3); + + // Round 2: with the timing file, LPT scheduling balances the shards. + const r2s1 = await runInlineTest(files, { shard: '1/2' }, { TIMINGS_FILE: timingsFile }); + const r2s2 = await runInlineTest(files, { shard: '2/2' }, { TIMINGS_FILE: timingsFile }); + expect(r2s1.exitCode).toBe(0); + expect(r2s2.exitCode).toBe(0); + expect(r2s1.outputLines).toEqual(['shard 1/2: a']); + expect(r2s2.outputLines).toEqual(['shard 2/2: b,c']); +}); From 0aa9806e641938bbc0874c6900f3ee35a0cbf62f Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Thu, 18 Jun 2026 12:45:51 +0200 Subject: [PATCH 8/8] test(test-runner): default unknown test timings to the average --- tests/playwright-test/reporter-plan.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/playwright-test/reporter-plan.spec.ts b/tests/playwright-test/reporter-plan.spec.ts index a6b4c5e5df9e1..2a9e0d4bedcb7 100644 --- a/tests/playwright-test/reporter-plan.spec.ts +++ b/tests/playwright-test/reporter-plan.spec.ts @@ -447,7 +447,9 @@ test('greedy time-based scheduling can be built on preprocessSuite', async ({ ru return { implementsSharding: false }; const total = config.shard.total; const tests = suite.allTests(); - const sorted = [...tests].sort((a, b) => (timings[b.id] || 0) - (timings[a.id] || 0)); + const known = Object.values(timings) as number[]; + const avg = known.length ? known.reduce((a, b) => a + b, 0) / known.length : 0; + const sorted = [...tests].sort((a, b) => (timings[b.id] || avg) - (timings[a.id] || avg)); const loads = new Array(total).fill(0); const assignment = new Map(); for (const t of sorted) { @@ -456,7 +458,7 @@ test('greedy time-based scheduling can be built on preprocessSuite', async ({ ru if (loads[i] < loads[min]) min = i; } - loads[min] += (timings[t.id] || 0); + loads[min] += (timings[t.id] || avg); assignment.set(t, min); } for (const t of tests) {