fix(test-runner): run teardown projects when maxFailures is reached

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,14 +279,27 @@ 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 {
   jobResult = new ManualPromise<{ newJob?: TestGroup, didFail: boolean }>();
 
   readonly job: TestGroup;
   private _testRun: TestRun;
-  private _stopCallback: () => void;
+  private _onMaxFailuresReached: () => void;
   private _listeners: RegisteredListener[] = [];
   private _failedTests = new Set<testNs.TestCase>();
   private _failedWithNonRetriableError = new Set<testNs.TestCase|testNs.Suite>();
@@ -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.`) });
     }
   }

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<TestRun> {
       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<commonConfig.FullProjectInternal, commonConfig.FullProjectInternal[]>();
       for (const [teardown, setups] of teardownToSetups) {
         const closure = buildDependentProjects(setups, allProjects);
@@ -424,6 +430,10 @@ function createPhasesTask(): Task<TestRun> {
           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);
           }

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: [],
     };
   };

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': `