-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: add enhanced retry options with delay and condition support #8548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ff657ae
2bca65e
ce7f779
b2ff23d
4d3432b
1f113ff
889c5df
7dc0db2
e99f667
2b66c7c
d38b495
6c02f41
02b9403
877ee32
87edabf
42fcbca
d4cc802
8c98cdb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -442,18 +442,19 @@ type ChainableTestAPI<ExtraContext = object> = ChainableFunction< | |
|
||
type TestCollectorOptions = Omit<TestOptions, 'shuffle'> | ||
|
||
export interface RetryConfig { | ||
count?: number | ||
strategy?: 'immediate' | 'test-file' | 'deferred' | ||
delay?: number | ||
condition?: string | RegExp | ((error: Error) => boolean) | ||
} | ||
|
||
export interface TestOptions { | ||
/** | ||
* Test timeout. | ||
*/ | ||
timeout?: number | ||
/** | ||
* Times to retry the test if fails. Useful for making flaky tests more stable. | ||
* When retries is up, the last test error will be thrown. | ||
* | ||
* @default 0 | ||
*/ | ||
retry?: number | ||
Comment on lines
-450
to
-456
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the doc-comment be replaced/extended instead of removed completely? |
||
retry?: number | RetryConfig | ||
/** | ||
* How many times the test will run again. | ||
* Only inner tests will repeat if set on `describe()`, nested `describe()` will inherit parent's repeat by default. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
import type { RetryConfig } from '../types/tasks' | ||
|
||
export function normalizeRetryConfig( | ||
retry: number | RetryConfig | undefined, | ||
): { | ||
count: number | ||
strategy: 'immediate' | 'test-file' | 'deferred' | ||
delay: number | ||
condition?: string | RegExp | ((error: Error) => boolean) | ||
} { | ||
if (typeof retry === 'number') { | ||
return { | ||
count: retry, | ||
strategy: 'immediate', | ||
delay: 0, | ||
} | ||
} | ||
|
||
if (!retry) { | ||
return { | ||
count: 0, | ||
strategy: 'immediate', | ||
delay: 0, | ||
} | ||
} | ||
|
||
return { | ||
count: retry.count ?? 0, | ||
strategy: retry.strategy ?? 'immediate', | ||
delay: retry.delay ?? 0, | ||
condition: retry.condition, | ||
} | ||
} | ||
|
||
export function shouldRetryOnError( | ||
error: Error, | ||
condition?: string | RegExp | ((error: Error) => boolean), | ||
): boolean { | ||
if (!condition) { | ||
return true | ||
} | ||
|
||
if (typeof condition === 'string') { | ||
return error.message.includes(condition) | ||
} | ||
|
||
if (condition instanceof RegExp) { | ||
return condition.test(error.message) | ||
} | ||
|
||
if (typeof condition === 'function') { | ||
try { | ||
return condition(error) | ||
} | ||
catch { | ||
return false | ||
} | ||
} | ||
|
||
return true | ||
} | ||
|
||
export function sleep(ms: number): Promise<void> { | ||
return new Promise(resolve => setTimeout(resolve, ms)) | ||
} | ||
|
||
export function calculateBackoffDelay( | ||
baseDelay: number, | ||
retryCount: number, | ||
): number { | ||
return baseDelay * 2 ** retryCount | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -602,8 +602,26 @@ export const cliOptionsConfig: VitestCLIOptions = { | |
}, | ||
retry: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, add a test for CLI input to |
||
description: | ||
'Retry the test specific number of times if it fails (default: `0`)', | ||
argument: '<times>', | ||
'Retry configuration. Can be a number or advanced options (default: `0`)', | ||
argument: '<times|config>', | ||
subcommands: { | ||
count: { | ||
description: 'Number of times to retry the test if it fails (default: 0)', | ||
argument: '<times>', | ||
}, | ||
strategy: { | ||
description: 'Retry strategy: immediate, test-file, or deferred (default: immediate)', | ||
argument: '<strategy>', | ||
}, | ||
delay: { | ||
description: 'Delay in milliseconds between retries (default: 0)', | ||
argument: '<milliseconds>', | ||
}, | ||
condition: { | ||
description: 'Condition for retrying: string pattern to match error messages', | ||
argument: '<pattern>', | ||
} as any, | ||
}, | ||
}, | ||
diff: { | ||
description: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -742,11 +742,40 @@ export interface InlineConfig { | |
bail?: number | ||
|
||
/** | ||
* Retry the test specific number of times if it fails. | ||
* Retry configuration for tests. | ||
* Can be a number (for backward compatibility) or an object with advanced options. | ||
* | ||
* @default 0 | ||
*/ | ||
retry?: number | ||
retry?: number | { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's keep comments, please |
||
/** | ||
* Number of times to retry the test if it fails. | ||
* @default 0 | ||
*/ | ||
count?: number | ||
|
||
/** | ||
* Strategy for when retries are executed. | ||
* - "immediate": retry failed test immediately (default) | ||
* - "test-file": run retries until the end of the test file | ||
* - "deferred": defer retries after all other tests have run across all test files | ||
* @default "immediate" | ||
*/ | ||
strategy?: 'immediate' | 'test-file' | 'deferred' | ||
|
||
/** | ||
* Delay in milliseconds between retries. | ||
* @default 0 | ||
*/ | ||
delay?: number | ||
|
||
/** | ||
* Condition for when to retry. Can be a RegExp pattern to match error messages | ||
* or a function that receives the error and returns boolean. | ||
* @default undefined (retry on any error) | ||
*/ | ||
condition?: string | RegExp | ((error: Error) => boolean) | ||
} | ||
|
||
/** | ||
* Show full diff when snapshot fails instead of a patch. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
import type { SerializedConfig } from '../runtime/config' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole file should be in |
||
|
||
export function normalizeRetryConfig( | ||
retry: SerializedConfig['retry'] | ||
): { | ||
count: number | ||
strategy: 'immediate' | 'test-file' | 'deferred' | ||
delay: number | ||
condition?: string | RegExp | ((error: Error) => boolean) | ||
} { | ||
if (typeof retry === 'number') { | ||
return { | ||
count: retry, | ||
strategy: 'immediate', | ||
delay: 0, | ||
} | ||
} | ||
|
||
return { | ||
count: retry.count ?? 0, | ||
strategy: retry.strategy ?? 'immediate', | ||
delay: retry.delay ?? 0, | ||
condition: retry.condition, | ||
} | ||
} | ||
|
||
export function shouldRetryOnError( | ||
error: Error, | ||
condition?: string | RegExp | ((error: Error) => boolean) | ||
): boolean { | ||
if (!condition) { | ||
return true | ||
} | ||
|
||
if (typeof condition === 'string') { | ||
return error.message.includes(condition) | ||
} | ||
|
||
if (condition instanceof RegExp) { | ||
return condition.test(error.message) | ||
} | ||
|
||
if (typeof condition === 'function') { | ||
try { | ||
return condition(error) | ||
} catch { | ||
return false | ||
} | ||
} | ||
|
||
return true | ||
} | ||
|
||
export function sleep(ms: number): Promise<void> { | ||
return new Promise(resolve => setTimeout(resolve, ms)) | ||
} | ||
|
||
export function calculateBackoffDelay( | ||
baseDelay: number, | ||
retryCount: number, | ||
algorithm: 'linear' | 'exponential' = 'linear' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am a bit confused why this needs to be customizable when we always pass down |
||
): number { | ||
switch (algorithm) { | ||
case 'exponential': | ||
return baseDelay * Math.pow(2, retryCount) | ||
case 'linear': | ||
default: | ||
return baseDelay | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not remove comments not related to the changes made