From c4862af2ef3474696dffd941ffdeb81bf876e0a8 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 15 Jul 2026 13:17:54 +0100 Subject: [PATCH] feat(test): replace 'deferred' retry strategy with 'isolated' With the 'isolated' retry strategy, retries run at the end of the run, one by one in a single worker. Tests that did not have a chance to run because of a failure keep their priority in the queue. --- docs/src/test-api/class-testconfig.md | 6 +- packages/playwright/src/common/config.ts | 2 +- .../playwright/src/common/configLoader.ts | 4 +- packages/playwright/src/runner/dispatcher.ts | 43 +++++++---- packages/playwright/types/test.d.ts | 9 ++- tests/playwright-test/retry.spec.ts | 73 ++++++++++++++++++- 6 files changed, 111 insertions(+), 26 deletions(-) diff --git a/docs/src/test-api/class-testconfig.md b/docs/src/test-api/class-testconfig.md index 169b8c943f11e..848882fe2ec20 100644 --- a/docs/src/test-api/class-testconfig.md +++ b/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', }); ``` diff --git a/packages/playwright/src/common/config.ts b/packages/playwright/src/common/config.ts index fc4e378625ea5..0287e87d3ee4f 100644 --- a/packages/playwright/src/common/config.ts +++ b/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[] = []; diff --git a/packages/playwright/src/common/configLoader.ts b/packages/playwright/src/common/configLoader.ts index 9beee268caa5c..f9ec6545177c5 100644 --- a/packages/playwright/src/common/configLoader.ts +++ b/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) { diff --git a/packages/playwright/src/runner/dispatcher.ts b/packages/playwright/src/runner/dispatcher.ts index aa30f5fb85579..87261df5dfd5c 100644 --- a/packages/playwright/src/runner/dispatcher.ts +++ b/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(); private _workerLimitPerProjectId = new Map(); private _queuedOrRunningHashCount = new Map(); private _finished = new ManualPromise(); @@ -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); } diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts index d30c7f240de59..d949bfaca4ee1 100644 --- a/packages/playwright/types/test.d.ts +++ b/packages/playwright/types/test.d.ts @@ -1644,8 +1644,9 @@ interface TestConfig { * 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 { * * 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 }`. diff --git a/tests/playwright-test/retry.spec.ts b/tests/playwright-test/retry.spec.ts index 1800eaded07d8..a251d228d3e4f 100644 --- a/tests/playwright-test/retry.spec.ts +++ b/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', + ]); +});