From e14be11558d232227296ec9e9fd33c6013604330 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Fri, 10 Jul 2026 11:33:37 -0700 Subject: [PATCH] fix(test-runner): run teardown projects when maxFailures is reached When a run is interrupted by maxFailures, teardown projects were skipped, so their cleanup (e.g. clearing a database) never ran. Exempt teardown project test groups from the maxFailures interruption via a TestGroup.ignoreMaxFailures flag so they still run. Fixes: https://github.com/microsoft/playwright/issues/41713 --- packages/playwright/src/runner/dispatcher.ts | 47 ++++++++---- packages/playwright/src/runner/tasks.ts | 10 +++ packages/playwright/src/runner/testGroups.ts | 4 + tests/playwright-test/max-failures.spec.ts | 81 ++++++++++++++++++++ 4 files changed, 126 insertions(+), 16 deletions(-) diff --git a/packages/playwright/src/runner/dispatcher.ts b/packages/playwright/src/runner/dispatcher.ts index aa30f5fb85579..f1c779a4074d6 100644 --- a/packages/playwright/src/runner/dispatcher.ts +++ b/packages/playwright/src/runner/dispatcher.ts @@ -58,6 +58,8 @@ export class Dispatcher { // Always pick the first job that can be run while respecting the project worker limit. for (let index = 0; index < this._queue.length; index++) { const job = this._queue[index]; + if (this._testRun.isStoppedByMaxFailures(job)) + continue; const projectIdWorkerLimit = this._workerLimitPerProjectId.get(job.projectId); if (!projectIdWorkerLimit) return index; @@ -92,7 +94,7 @@ export class Dispatcher { // 3. Claim both the job and the worker slot. this._queue.splice(jobIndex, 1); - const jobDispatcher = new JobDispatcher(job, this._testRun, () => this.stop().catch(() => {})); + const jobDispatcher = new JobDispatcher(job, this._testRun, () => this._stopForMaxFailures()); this._workerSlots[workerIndex].jobDispatcher = jobDispatcher; // 4. Run the job. This is the only async operation. @@ -151,7 +153,7 @@ export class Dispatcher { void worker.stop(); // 5. Possibly queue a new job with leftover tests and/or retries. - if (!this._isStopped && result.newJob) { + if (!this._isStopped && result.newJob && !this._testRun.isStoppedByMaxFailures(result.newJob)) { if (this._testRun.config.retryStrategy === 'deferred') this._queue.push(result.newJob); else @@ -164,8 +166,9 @@ export class Dispatcher { if (this._finished.isDone()) return; - // Check that we have no more work to do. - if (this._queue.length && !this._isStopped) + // Check that we have no more work to do. Jobs stopped by maxFailures + // will never be scheduled, so they don't count. + if (!this._isStopped && this._queue.some(job => !this._testRun.isStoppedByMaxFailures(job))) return; // Make sure all workers have finished the current job. @@ -195,9 +198,8 @@ export class Dispatcher { this._updateCounterForWorkerHash(group.workerHash, +1); this._isStopped = false; this._workerSlots = []; - // 0. Stop right away if we have reached max failures. - if (this._testRun.hasReachedMaxFailures()) - void this.stop(); + // Note: when maxFailures is already reached, only jobs that ignore it (if + // any) will be scheduled; the rest are skipped by `isStoppedByMaxFailures`. // 1. Allocate workers. for (let i = 0; i < this._testRun.config.config.workers; i++) this._workerSlots.push({}); @@ -277,6 +279,19 @@ export class Dispatcher { await Promise.all(this._workerSlots.map(({ worker }) => worker?.stop())); this._checkFinished(); } + + private _stopForMaxFailures() { + // Max failures reached: stop workers whose jobs don't ignore maxFailures so + // their tests are reported as interrupted, but let the rest (e.g. teardown + // projects) keep running. + for (const slot of this._workerSlots) { + if (slot.jobDispatcher && this._testRun.isStoppedByMaxFailures(slot.jobDispatcher.job)) + void slot.worker?.stop(); + } + // Remaining stopped jobs won't be scheduled anymore. + this._checkFinished(); + this._scheduleJob(); + } } class JobDispatcher { @@ -284,7 +299,7 @@ class JobDispatcher { readonly job: TestGroup; private _testRun: TestRun; - private _stopCallback: () => void; + private _onMaxFailuresReached: () => void; private _listeners: RegisteredListener[] = []; private _failedTests = new Set(); private _failedWithNonRetriableError = new Set(); @@ -294,10 +309,10 @@ class JobDispatcher { private _workerIndex = 0; private _currentlyRunning: { test: testNs.TestCase, result: TestResult } | undefined; - constructor(job: TestGroup, testRun: TestRun, stopCallback: () => void) { + constructor(job: TestGroup, testRun: TestRun, onMaxFailuresReached: () => void) { this.job = job; this._testRun = testRun; - this._stopCallback = stopCallback; + this._onMaxFailuresReached = onMaxFailuresReached; this._remainingByTestId = new Map(this.job.tests.map(e => [e.id, e])); } @@ -317,7 +332,7 @@ class JobDispatcher { } private _onTestEnd(params: ipc.TestEndPayload) { - if (this._testRun.hasReachedMaxFailures()) { + if (this._testRun.isStoppedByMaxFailures(this.job)) { // Do not show more than one error to avoid confusion, but report // as interrupted to indicate that we did actually start the test. params.status = 'interrupted'; @@ -449,7 +464,7 @@ class JobDispatcher { for (const test of this._remainingByTestId.values()) { if (!testIds.has(test.id)) continue; - if (!this._testRun.hasReachedMaxFailures()) { + if (!this._testRun.isStoppedByMaxFailures(this.job)) { this._failTestWithErrors(test, errors); errors = []; // Only report errors for the first test. } @@ -624,7 +639,7 @@ class JobDispatcher { // with skipped tests mixed in-between non-skipped. This makes // for a better reporter experience. const allTestsSkipped = this.job.tests.every(test => test.expectedStatus === 'skipped'); - if (allTestsSkipped && !this._testRun.hasReachedMaxFailures()) { + if (allTestsSkipped && !this._testRun.isStoppedByMaxFailures(this.job)) { for (const test of this.job.tests) { const result = test._appendTestResult(); this._testRun.reporter.onTestBegin?.(test, result); @@ -644,12 +659,12 @@ class JobDispatcher { private _reportTestEnd(test: testNs.TestCase, result: TestResult) { this._testRun.reporter.onTestEnd?.(test, result); - const hadMaxFailures = this._testRun.hasReachedMaxFailures(); + const hadMaxFailures = this._testRun.isStoppedByMaxFailures(this.job); // Test is considered failing after the last retry. if (test.outcome() === 'unexpected' && test.results.length > test.retries) ++this._testRun.failedTestCount; - if (!hadMaxFailures && this._testRun.hasReachedMaxFailures()) { - this._stopCallback(); + if (!hadMaxFailures && this._testRun.isStoppedByMaxFailures(this.job)) { + this._onMaxFailuresReached(); this._testRun.reporter.onError?.({ message: colors.red(`Testing stopped early after ${this._testRun.config.config.maxFailures} maximum allowed failures.`) }); } } diff --git a/packages/playwright/src/runner/tasks.ts b/packages/playwright/src/runner/tasks.ts index a2b2d328d818e..7c24e99a15845 100644 --- a/packages/playwright/src/runner/tasks.ts +++ b/packages/playwright/src/runner/tasks.ts @@ -113,6 +113,11 @@ export class TestRun { return max > 0 && this.failedTestCount >= max; } + isStoppedByMaxFailures(testGroup: TestGroup) { + // Teardown project groups keep running so that cleanup is not skipped. + return this.hasReachedMaxFailures() && !testGroup.ignoreMaxFailures; + } + result(): 'failed' | 'passed' { const hasFailedTests = this.rootSuite?.allTests().some(test => !test.ok()); const hasFlakyTests = this.rootSuite?.allTests().some(test => test.outcome() === 'flaky'); @@ -395,6 +400,7 @@ function createPhasesTask(): Task { const projectToSuite = new Map(testRun.rootSuite!.suites.map(suite => [suite._fullProject!, suite])); const allProjects = [...projectToSuite.keys()]; const teardownToSetups = buildTeardownToSetupsMap(allProjects); + const teardownProjectIds = new Set([...teardownToSetups.keys()].map(project => project.id)); const teardownToSetupsDependents = new Map(); for (const [teardown, setups] of teardownToSetups) { const closure = buildDependentProjects(setups, allProjects); @@ -424,6 +430,10 @@ function createPhasesTask(): Task { for (const project of phaseProjects) { const projectSuite = projectToSuite.get(project)!; const testGroups = createTestGroups(projectSuite, testRun.config.config.workers); + if (teardownProjectIds.has(project.id)) { + for (const testGroup of testGroups) + testGroup.ignoreMaxFailures = true; + } phase.projects.push({ project, projectSuite, testGroups }); testGroupsInPhase += Math.min(project.workers ?? Number.MAX_SAFE_INTEGER, testGroups.length); } diff --git a/packages/playwright/src/runner/testGroups.ts b/packages/playwright/src/runner/testGroups.ts index f9d6dd6eb8109..57b99132d3af1 100644 --- a/packages/playwright/src/runner/testGroups.ts +++ b/packages/playwright/src/runner/testGroups.ts @@ -21,6 +21,9 @@ export type TestGroup = { requireFile: string; repeatEachIndex: number; projectId: string; + // Tests in this group keep running even after maxFailures is reached, so that + // teardown projects are not skipped. + ignoreMaxFailures: boolean; tests: test.TestCase[]; }; @@ -57,6 +60,7 @@ export function createTestGroups(projectSuite: test.Suite, expectedParallelism: requireFile: test._requireFile, repeatEachIndex: test.repeatEachIndex, projectId: test._projectId, + ignoreMaxFailures: false, tests: [], }; }; diff --git a/tests/playwright-test/max-failures.spec.ts b/tests/playwright-test/max-failures.spec.ts index 56ad9f466fe55..706443be390d8 100644 --- a/tests/playwright-test/max-failures.spec.ts +++ b/tests/playwright-test/max-failures.spec.ts @@ -182,6 +182,87 @@ test('max-failures should work across phases', async ({ runInlineTest }) => { expect(result.output).not.toContain('running d'); }); +test('max-failures should still run teardown project', async ({ runInlineTest }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41713' }); + + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { + projects: [ + { name: 'setup', testMatch: '**/setup.ts', teardown: 'teardown' }, + { name: 'teardown', testMatch: '**/teardown.ts' }, + { name: 'project', dependencies: ['setup'], testMatch: '**/a.spec.ts' }, + ], + };`, + 'setup.ts': ` + import { test } from '@playwright/test'; + test('setup', async ({}) => { console.log('\\n%%setup'); }); + `, + 'teardown.ts': ` + import { test } from '@playwright/test'; + test('teardown', async ({}) => { console.log('\\n%%teardown'); }); + `, + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('failing', async ({}) => { expect(1).toBe(2); }); + `, + }, { 'workers': 1, 'max-failures': 1 }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.output).toContain('%%setup'); + // Teardown should still run when the run is cut short by maxFailures. + expect(result.output).toContain('%%teardown'); +}); + +test('max-failures should skip non-teardown projects sharing a teardown phase', async ({ runInlineTest }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41713' }); + + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { + maxFailures: 1, + projects: [ + { name: 'setup', testMatch: '**/setup.ts', teardown: 'teardown' }, + { name: 'teardown', testMatch: '**/teardown.ts' }, + { name: 'a', dependencies: ['setup'], testMatch: '**/a.spec.ts' }, + // Independent chain that shares the last phase with 'teardown'. + { name: 'x', testMatch: '**/x.spec.ts' }, + { name: 'y', dependencies: ['x'], testMatch: '**/y.spec.ts' }, + { name: 'z', dependencies: ['y'], testMatch: '**/z.spec.ts' }, + ], + };`, + 'setup.ts': ` + import { test } from '@playwright/test'; + test('setup', async ({}) => {}); + `, + 'teardown.ts': ` + import { test } from '@playwright/test'; + test('teardown', async ({}) => { console.log('\\n%%teardown'); }); + `, + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('failing', async ({}) => { expect(1).toBe(2); }); + `, + 'x.spec.ts': ` + import { test } from '@playwright/test'; + test('x', async ({}) => { console.log('\\n%%x'); }); + `, + 'y.spec.ts': ` + import { test } from '@playwright/test'; + test('y', async ({}) => { console.log('\\n%%y'); }); + `, + 'z.spec.ts': ` + import { test } from '@playwright/test'; + test('z', async ({}) => { console.log('\\n%%z'); }); + `, + }, { workers: 1 }); + expect(result.exitCode).toBe(1); + // Teardown still runs despite maxFailures. + expect(result.output).toContain('%%teardown'); + // Regular project 'z' sharing the last phase with teardown must be skipped. + expect(result.output).not.toContain('%%z'); +}); + test('max-failures should not consider retries as failures', async ({ runInlineTest }) => { const result = await runInlineTest({ 'playwright.config.ts': `