fix(test-runner): run project teardown after max-failures stop

packages/playwright/src/runner/dispatcher.ts

@@ -38,6 +38,7 @@ export class Dispatcher {
   private _queue: TestGroup[] = [];
   private _workerLimitPerProjectId = new Map<string, number>();
   private _queuedOrRunningHashCount = new Map<string, number>();
+  private _ignoreMaxFailures = false;
   private _finished = new ManualPromise<void>();
   private _isStopped = true;
 
@@ -92,7 +93,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.stop().catch(() => {}), this._ignoreMaxFailures);
     this._workerSlots[workerIndex].jobDispatcher = jobDispatcher;
 
     // 4. Run the job. This is the only async operation.
@@ -188,15 +189,17 @@ export class Dispatcher {
     this._queuedOrRunningHashCount.set(hash, delta + (this._queuedOrRunningHashCount.get(hash) || 0));
   }
 
-  async run(testGroups: TestGroup[], extraEnvByProjectId: EnvByProjectId) {
+  async run(testGroups: TestGroup[], extraEnvByProjectId: EnvByProjectId, options?: { ignoreMaxFailures?: boolean }) {
     this._extraEnvByProjectId = extraEnvByProjectId;
+    this._ignoreMaxFailures = !!options?.ignoreMaxFailures;
     this._queue = testGroups;
     for (const group of testGroups)
       this._updateCounterForWorkerHash(group.workerHash, +1);
     this._isStopped = false;
     this._workerSlots = [];
     // 0. Stop right away if we have reached max failures.
-    if (this._testRun.hasReachedMaxFailures())
+    // Teardown projects still run so cleanup is not skipped when --max-failures stops the suite.
+    if (!this._ignoreMaxFailures && this._testRun.hasReachedMaxFailures())
       void this.stop();
     // 1. Allocate workers.
     for (let i = 0; i < this._testRun.config.config.workers; i++)
@@ -285,6 +288,7 @@ class JobDispatcher {
   readonly job: TestGroup;
   private _testRun: TestRun;
   private _stopCallback: () => void;
+  private _ignoreMaxFailures: boolean;
   private _listeners: RegisteredListener[] = [];
   private _failedTests = new Set<testNs.TestCase>();
   private _failedWithNonRetriableError = new Set<testNs.TestCase|testNs.Suite>();
@@ -294,10 +298,11 @@ 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, stopCallback: () => void, ignoreMaxFailures = false) {
     this.job = job;
     this._testRun = testRun;
     this._stopCallback = stopCallback;
+    this._ignoreMaxFailures = ignoreMaxFailures;
     this._remainingByTestId = new Map(this.job.tests.map(e => [e.id, e]));
   }
 
@@ -317,7 +322,7 @@ class JobDispatcher {
   }
 
   private _onTestEnd(params: ipc.TestEndPayload) {
-    if (this._testRun.hasReachedMaxFailures()) {
+    if (!this._ignoreMaxFailures && this._testRun.hasReachedMaxFailures()) {
       // 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';
@@ -648,7 +653,7 @@ class JobDispatcher {
     // 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()) {
+    if (!this._ignoreMaxFailures && !hadMaxFailures && this._testRun.hasReachedMaxFailures()) {
       this._stopCallback();
       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

@@ -443,13 +443,17 @@ function createRunTestsTask(): Task<TestRun> {
     setup: async testRun => {
       const successfulProjects = new Set<commonConfig.FullProjectInternal>();
       const extraEnvByProjectId: EnvByProjectId = new Map();
-      const teardownToSetups = buildTeardownToSetupsMap(testRun.phases.map(phase => phase.projects.map(p => p.project)).flat());
+      const allProjects = testRun.phases.map(phase => phase.projects.map(p => p.project)).flat();
+      const teardownToSetups = buildTeardownToSetupsMap(allProjects);
+      const teardownProjects = new Set(teardownToSetups.keys());
 
       for (const { dispatcher, projects } of testRun.phases) {
         // Each phase contains dispatcher and a set of test groups.
         // We don't want to run the test groups belonging to the projects
         // that depend on the projects that failed previously.
+        // When --max-failures stops the suite, still run teardown projects for cleanup.
         const phaseTestGroups: TestGroup[] = [];
+        const maxFailuresReached = testRun.hasReachedMaxFailures();
         for (const { project, testGroups } of projects) {
           // Inherit extra environment variables from dependencies.
           let extraEnv: Record<string, string | undefined> = {};
@@ -460,12 +464,19 @@ function createRunTestsTask(): Task<TestRun> {
           extraEnvByProjectId.set(project.id, extraEnv);
 
           const hasFailedDeps = project.deps.some(p => !successfulProjects.has(p));
-          if (!hasFailedDeps)
-            phaseTestGroups.push(...testGroups);
+          if (hasFailedDeps)
+            continue;
+          if (maxFailuresReached && !teardownProjects.has(project))
+            continue;
+          phaseTestGroups.push(...testGroups);
         }
 
         if (phaseTestGroups.length) {
-          await dispatcher!.run(phaseTestGroups, extraEnvByProjectId);
+          const onlyTeardowns = phaseTestGroups.every(group => {
+            const project = projects.find(p => p.project.id === group.projectId)?.project;
+            return !!project && teardownProjects.has(project);
+          });
+          await dispatcher!.run(phaseTestGroups, extraEnvByProjectId, { ignoreMaxFailures: onlyTeardowns });
           await dispatcher.stop();
           for (const [projectId, envProduced] of dispatcher.producedEnvByProjectId()) {
             const extraEnv = extraEnvByProjectId.get(projectId) || {};

tests/playwright-test/max-failures.spec.ts

@@ -209,3 +209,48 @@ test('max-failures should not consider retries as failures', async ({ runInlineT
   expect(result.flaky).toBe(2);
   expect(result.passed).toBe(0);
 });
+
+test('max-failures should still run project teardown', async ({ runInlineTest }) => {
+  test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41713' });
+
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      import { defineConfig } from '@playwright/test';
+      export default defineConfig({
+        maxFailures: 1,
+        projects: [
+          { name: 'setup', testMatch: /global.setup.ts/, teardown: 'teardown' },
+          { name: 'teardown', testMatch: /global.teardown.ts/ },
+          { name: 'main', testMatch: /example.spec.ts/, dependencies: ['setup'] },
+        ],
+      });
+    `,
+    'global.setup.ts': `
+      import { test as setup } from '@playwright/test';
+      setup('setup', async () => {
+        console.log('\\n%%SETUP-RAN');
+      });
+    `,
+    'global.teardown.ts': `
+      import { test as teardown } from '@playwright/test';
+      teardown('teardown', async () => {
+        console.log('\\n%%TEARDOWN-RAN');
+      });
+    `,
+    'example.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('failing', async () => {
+        expect(1).toBe(2);
+      });
+      test('passing', async () => {
+        expect(1).toBe(1);
+      });
+    `,
+  }, { workers: 1 });
+
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+  expect(result.interrupted).toBe(0);
+  expect(result.outputLines).toContain('SETUP-RAN');
+  expect(result.outputLines).toContain('TEARDOWN-RAN');
+});