feat(test): replace 'deferred' retry strategy with 'isolated'

docs/src/test-api/class-testconfig.md

@@ -517,11 +517,11 @@ export default defineConfig({
 
 ## property: TestConfig.retryStrategy
 * since: v1.62
-- type: ?<[RetryStrategy]<"immediate"|"deferred">>
+- type: ?<[RetryStrategy]<"immediate"|"isolated">>
 
 Controls when failed tests are retried. Defaults to `'immediate'`.
 * `'immediate'` - A failed test is retried as soon as a worker is available, interleaved with the rest of the run. This is the default.
-* `'deferred'` - Retries are run only after all tests have had their first attempt, in parallel up to the configured number of [workers](#test-config-workers).
+* `'isolated'` - Retries are run at the end, after all other tests have finished, one by one in a single worker. This minimizes the interference between retried tests and the rest of the suite, at the expense of the total run time.
 
 Learn more about [test retries](../test-retries.md#retries).
 
@@ -532,7 +532,7 @@ import { defineConfig } from '@playwright/test';
 
 export default defineConfig({
   retries: 2,
-  retryStrategy: 'deferred',
+  retryStrategy: 'isolated',
 });
 ```
 

packages/playwright/src/common/config.ts

@@ -48,7 +48,7 @@ export class FullConfigInternal {
   readonly projects: FullProjectInternal[] = [];
   readonly singleTSConfigPath?: string;
   readonly captureGitInfo: Config['captureGitInfo'];
-  readonly retryStrategy: 'immediate' | 'deferred';
+  readonly retryStrategy: 'immediate' | 'isolated';
   defineConfigWasUsed = false;
 
   globalSetups: string[] = [];

packages/playwright/src/common/configLoader.ts

@@ -244,8 +244,8 @@ function validateConfig(file: string, config: Config) {
   }
 
   if ('retryStrategy' in config && config.retryStrategy !== undefined) {
-    if (typeof config.retryStrategy !== 'string' || !['immediate', 'deferred'].includes(config.retryStrategy))
-      throw errorWithFile(file, `config.retryStrategy must be one of "immediate" or "deferred"`);
+    if (typeof config.retryStrategy !== 'string' || !['immediate', 'isolated'].includes(config.retryStrategy))
+      throw errorWithFile(file, `config.retryStrategy must be one of "immediate" or "isolated"`);
   }
 
   if ('tsconfig' in config && config.tsconfig !== undefined) {

packages/playwright/src/runner/dispatcher.ts

@@ -36,6 +36,7 @@ export class Dispatcher {
   // Worker slot is claimed when it has jobDispatcher assigned.
   private _workerSlots: { worker?: WorkerHost, jobDispatcher?: JobDispatcher }[] = [];
   private _queue: TestGroup[] = [];
+  private _isolatedJobs = new Set<TestGroup>();
   private _workerLimitPerProjectId = new Map<string, number>();
   private _queuedOrRunningHashCount = new Map<string, number>();
   private _finished = new ManualPromise<void>();
@@ -58,6 +59,9 @@ 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];
+      // Isolated retries only run one at a time, after all other jobs have finished.
+      if (this._isolatedJobs.has(job) && this._workerSlots.some(w => !!w.jobDispatcher))
+        continue;
       const projectIdWorkerLimit = this._workerLimitPerProjectId.get(job.projectId);
       if (!projectIdWorkerLimit)
         return index;
@@ -150,13 +154,17 @@ export class Dispatcher {
     else if (this._isWorkerRedundant(worker))
       void worker.stop();
 
-    // 5. Possibly queue a new job with leftover tests and/or retries.
-    if (!this._isStopped && result.newJob) {
-      if (this._testRun.config.retryStrategy === 'deferred')
-        this._queue.push(result.newJob);
-      else
-        this._queue.unshift(result.newJob);
-      this._updateCounterForWorkerHash(result.newJob.workerHash, +1);
+    // 5. Possibly queue new jobs with leftover tests and/or retries.
+    if (!this._isStopped) {
+      if (result.remainingJob) {
+        this._queue.unshift(result.remainingJob);
+        this._updateCounterForWorkerHash(result.remainingJob.workerHash, +1);
+      }
+      if (result.isolatedRetriesJob) {
+        this._isolatedJobs.add(result.isolatedRetriesJob);
+        this._queue.push(result.isolatedRetriesJob);
+        this._updateCounterForWorkerHash(result.isolatedRetriesJob.workerHash, +1);
+      }
     }
   }
 
@@ -280,7 +288,7 @@ export class Dispatcher {
 }
 
 class JobDispatcher {
-  jobResult = new ManualPromise<{ newJob?: TestGroup, didFail: boolean }>();
+  jobResult = new ManualPromise<{ remainingJob?: TestGroup, isolatedRetriesJob?: TestGroup, didFail: boolean }>();
 
   readonly job: TestGroup;
   private _testRun: TestRun;
@@ -537,14 +545,21 @@ class JobDispatcher {
     }
 
     const remaining = [...this._remainingByTestId.values()];
+    const isolatedRetries: testNs.TestCase[] = [];
     for (const test of retryCandidates) {
-      if (test.results.length < test.retries + 1)
-        remaining.push(test);
+      if (test.results.length < test.retries + 1) {
+        // Immediate retries run together with the remaining tests, in a single job.
+        if (this._testRun.config.retryStrategy === 'immediate')
+          remaining.push(test);
+        else
+          isolatedRetries.push(test);
+      }
     }
 
-    // This job is over, we will schedule another one.
-    const newJob = remaining.length ? { ...this.job, tests: remaining } : undefined;
-    this._finished({ didFail: true, newJob });
+    // This job is over, we will schedule new jobs for the remaining tests and isolated retries.
+    const remainingJob = remaining.length ? { ...this.job, tests: remaining } : undefined;
+    const isolatedRetriesJob = isolatedRetries.length ? { ...this.job, tests: isolatedRetries } : undefined;
+    this._finished({ didFail: true, remainingJob, isolatedRetriesJob });
   }
 
   onExit(data: ProcessExitData) {
@@ -554,7 +569,7 @@ class JobDispatcher {
     this._onDone({ skipTestsDueToSetupFailure: [], fatalErrors: [], unexpectedExitError });
   }
 
-  private _finished(result: { newJob?: TestGroup, didFail: boolean }) {
+  private _finished(result: { remainingJob?: TestGroup, isolatedRetriesJob?: TestGroup, didFail: boolean }) {
     eventsHelper.removeEventListeners(this._listeners);
     this.jobResult.resolve(result);
   }

packages/playwright/types/test.d.ts

@@ -1644,8 +1644,9 @@ interface TestConfig<TestArgs = {}, WorkerArgs = {}> {
    * Controls when failed tests are retried. Defaults to `'immediate'`.
    * - `'immediate'` - A failed test is retried as soon as a worker is available, interleaved with the rest of the
    *   run. This is the default.
-   * - `'deferred'` - Retries are run only after all tests have had their first attempt, in parallel up to the
-   *   configured number of [workers](#test-config-workers).
+   * - `'isolated'` - Retries are run at the end, after all other tests have finished, one by one in a single worker.
+   *   This minimizes the interference between retried tests and the rest of the suite, at the expense of the total
+   *   run time.
    *
    * Learn more about [test retries](https://playwright.dev/docs/test-retries#retries).
    *
@@ -1657,12 +1658,12 @@ interface TestConfig<TestArgs = {}, WorkerArgs = {}> {
    *
    * export default defineConfig({
    *   retries: 2,
-   *   retryStrategy: 'deferred',
+   *   retryStrategy: 'isolated',
    * });
    * ```
    *
    */
-  retryStrategy?: "immediate"|"deferred";
+  retryStrategy?: "immediate"|"isolated";
 
   /**
    * Shard tests and execute only the selected shard. Specify in the one-based form like `{ total: 5, current: 2 }`.

tests/playwright-test/retry.spec.ts

@@ -263,10 +263,10 @@ test('failed and skipped on retry should be marked as flaky', async ({ runInline
   expect(result.report.suites[0].specs[0].tests[0].annotations).toEqual([{ type: 'skip', description: 'Skipped on first retry', location: expect.anything() }]);
 });
 
-test('should defer retries to the end of the run', async ({ runInlineTest }) => {
+test('should run isolated retries at the end of the run', async ({ runInlineTest }) => {
   const result = await runInlineTest({
     'playwright.config.js': `
-      module.exports = { retries: 3, retryStrategy: 'deferred' };
+      module.exports = { retries: 3, retryStrategy: 'isolated' };
     `,
     'a.test.js': `
       import { test, expect } from '@playwright/test';
@@ -298,3 +298,72 @@ test('should defer retries to the end of the run', async ({ runInlineTest }) =>
     'a-3',
   ]);
 });
+
+test('should run isolated retries one by one in a single worker', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'playwright.config.js': `
+      module.exports = { retries: 1, retryStrategy: 'isolated', workers: 2 };
+    `,
+    'a.test.js': `
+      import { test, expect } from '@playwright/test';
+      test('a', ({}, testInfo) => {
+        console.log('\\n%%a-' + testInfo.retry + '/' + testInfo.workerIndex);
+        expect(testInfo.retry).toBe(1);
+      });
+    `,
+    'b.test.js': `
+      import { test, expect } from '@playwright/test';
+      test('b', ({}, testInfo) => {
+        console.log('\\n%%b-' + testInfo.retry + '/' + testInfo.workerIndex);
+        expect(testInfo.retry).toBe(1);
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.flaky).toBe(2);
+  expect(result.results.length).toBe(4);
+  const lines = result.outputLines;
+  // First attempts run in parallel workers, before any retry.
+  expect(lines.slice(0, 2).map(line => line.split('/')[0]).sort()).toEqual(['a-0', 'b-0']);
+  expect(lines.slice(2).map(line => line.split('/')[0]).sort()).toEqual(['a-1', 'b-1']);
+  // Both retries run in the same worker, one by one.
+  const retryWorkers = lines.slice(2).map(line => line.split('/')[1]);
+  expect(retryWorkers[0]).toBe(retryWorkers[1]);
+});
+
+test('should not defer remaining tests with isolated retries', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'playwright.config.js': `
+      module.exports = { retries: 1, retryStrategy: 'isolated' };
+    `,
+    'a.test.js': `
+      import { test, expect } from '@playwright/test';
+      test('a1', ({}, testInfo) => {
+        console.log('\\n%%a1-' + testInfo.retry);
+        expect(testInfo.retry).toBe(1);
+      });
+      test('a2', ({}, testInfo) => {
+        console.log('\\n%%a2-' + testInfo.retry);
+      });
+    `,
+    'b.test.js': `
+      import { test, expect } from '@playwright/test';
+      test('b1', ({}, testInfo) => {
+        console.log('\\n%%b1-' + testInfo.retry);
+        expect(testInfo.retry).toBe(1);
+      });
+    `,
+  }, { workers: 1 });
+  expect(result.exitCode).toBe(0);
+  expect(result.flaky).toBe(2);
+  expect(result.passed).toBe(1);
+  // After "a1" fails, "a2" keeps its priority in the queue and runs
+  // before "b1", while the retry of "a1" is deferred to the end.
+  expect(result.outputLines).toEqual([
+    'a1-0',
+    'a2-0',
+    'b1-0',
+    'a1-1',
+    'b1-1',
+  ]);
+});