Skip to content
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

feat: add a waitBeforeRetry option to jest.retryTimes (#14737) #14738

Merged
merged 9 commits into from
Dec 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Features

- `[jest-circus, jest-cli, jest-config]` Add `waitNextEventLoopTurnForUnhandledRejectionEvents` flag to minimise performance impact of correct detection of unhandled promise rejections introduced in [#14315](https://github.com/jestjs/jest/pull/14315) ([#14681](https://github.com/jestjs/jest/pull/14681))
- `[jest-circus]` Add a `waitBeforeRetry` option to `jest.retryTimes` ([#14738](https://github.com/jestjs/jest/pull/14738))
- `[jest-config]` [**BREAKING**] Add `mts` and `cts` to default `moduleFileExtensions` config ([#14369](https://github.com/facebook/jest/pull/14369))
- `[jest-config]` [**BREAKING**] Update `testMatch` and `testRegex` default option for supporting `mjs`, `cjs`, `mts`, and `cts` ([#14584](https://github.com/jestjs/jest/pull/14584))
- `[jest-config]` Loads config file from provided path in `package.json` ([#14044](https://github.com/facebook/jest/pull/14044))
Expand Down
10 changes: 10 additions & 0 deletions docs/JestObjectAPI.md
Original file line number Diff line number Diff line change
Expand Up @@ -1071,6 +1071,16 @@ test('will fail', () => {
});
```

`waitBeforeRetry` is the number of milliseconds to wait before retrying.

```js
jest.retryTimes(3, {waitBeforeRetry: 1000});

test('will fail', () => {
expect(true).toBe(false);
});
```

Returns the `jest` object for chaining.

:::caution
Expand Down
76 changes: 76 additions & 0 deletions e2e/__tests__/__snapshots__/testRetries.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,79 @@ exports[`Test Retries logs error(s) before retry 1`] = `
PASS __tests__/logErrorsBeforeRetries.test.js
✓ retryTimes set"
`;

exports[`Test Retries wait before retry 1`] = `
"LOGGING RETRY ERRORS retryTimes set
RETRY 1

expect(received).toBeFalsy()

Received: true

15 | expect(new Date().getTime() - startTimeInSeconds).toBeGreaterThan(200);
16 | } else {
> 17 | expect(true).toBeFalsy();
| ^
18 | }
19 | });
20 |

at Object.toBeFalsy (__tests__/waitBeforeRetry.test.js:17:18)

RETRY 2

expect(received).toBeFalsy()

Received: true

15 | expect(new Date().getTime() - startTimeInSeconds).toBeGreaterThan(200);
16 | } else {
> 17 | expect(true).toBeFalsy();
| ^
18 | }
19 | });
20 |

at Object.toBeFalsy (__tests__/waitBeforeRetry.test.js:17:18)

PASS __tests__/waitBeforeRetry.test.js
✓ retryTimes set"
`;

exports[`Test Retries wait before retry with fake timers 1`] = `
"LOGGING RETRY ERRORS retryTimes set with fake timers
RETRY 1

expect(received).toBeFalsy()

Received: true

16 | expect(new Date().getTime() - startTimeInSeconds).toBeGreaterThan(200);
17 | } else {
> 18 | expect(true).toBeFalsy();
| ^
19 | jest.runAllTimers();
20 | }
21 | });

at Object.toBeFalsy (__tests__/waitBeforeRetryFakeTimers.test.js:18:18)

RETRY 2

expect(received).toBeFalsy()

Received: true

16 | expect(new Date().getTime() - startTimeInSeconds).toBeGreaterThan(200);
17 | } else {
> 18 | expect(true).toBeFalsy();
| ^
19 | jest.runAllTimers();
20 | }
21 | });

at Object.toBeFalsy (__tests__/waitBeforeRetryFakeTimers.test.js:18:18)

PASS __tests__/waitBeforeRetryFakeTimers.test.js
✓ retryTimes set with fake timers"
`;
20 changes: 19 additions & 1 deletion e2e/__tests__/testRetries.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,24 @@ describe('Test Retries', () => {
expect(extractSummary(result.stderr).rest).toMatchSnapshot();
});

it('wait before retry', () => {
const result = runJest('test-retries', ['waitBeforeRetry.test.js']);
expect(result.exitCode).toBe(0);
expect(result.failed).toBe(false);
expect(result.stderr).toContain(logErrorsBeforeRetryErrorMessage);
expect(extractSummary(result.stderr).rest).toMatchSnapshot();
});

it('wait before retry with fake timers', () => {
const result = runJest('test-retries', [
'waitBeforeRetryFakeTimers.test.js',
]);
expect(result.exitCode).toBe(0);
expect(result.failed).toBe(false);
expect(result.stderr).toContain(logErrorsBeforeRetryErrorMessage);
expect(extractSummary(result.stderr).rest).toMatchSnapshot();
});

it('reporter shows more than 1 invocation if test is retried', () => {
let jsonResult;

Expand All @@ -54,7 +72,7 @@ describe('Test Retries', () => {
runJest('test-retries', [
'--config',
JSON.stringify(reporterConfig),
'retry.test.js',
'__tests__/retry.test.js',
]);

const testOutput = fs.readFileSync(outputFilePath, 'utf8');
Expand Down
19 changes: 19 additions & 0 deletions e2e/test-retries/__tests__/waitBeforeRetry.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

let i = 0;
const startTimeInSeconds = new Date().getTime();
jest.retryTimes(3, {logErrorsBeforeRetry: true, waitBeforeRetry: 100});
it('retryTimes set', () => {
i++;
if (i === 3) {
expect(new Date().getTime() - startTimeInSeconds).toBeGreaterThan(200);
} else {
expect(true).toBeFalsy();
}
});
21 changes: 21 additions & 0 deletions e2e/test-retries/__tests__/waitBeforeRetryFakeTimers.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

let i = 0;
const startTimeInSeconds = new Date().getTime();
jest.retryTimes(3, {logErrorsBeforeRetry: true, waitBeforeRetry: 100});
it('retryTimes set with fake timers', () => {
jest.useFakeTimers();
i++;
if (i === 3) {
expect(new Date().getTime() - startTimeInSeconds).toBeGreaterThan(200);
} else {
expect(true).toBeFalsy();
jest.runAllTimers();
}
});
14 changes: 13 additions & 1 deletion packages/jest-circus/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import shuffleArray, {
rngBuilder,
} from './shuffleArray';
import {dispatch, getState} from './state';
import {RETRY_TIMES} from './types';
import {RETRY_TIMES, WAIT_BEFORE_RETRY} from './types';
import {
callAsyncCircusFn,
getAllHooksForDescribe,
Expand All @@ -24,6 +24,10 @@ import {
makeRunResult,
} from './utils';

// Global values can be overwritten by mocks or tests. We'll capture
// the original values in the variables before we require any files.
const {setTimeout} = globalThis;

type ConcurrentTestEntry = Omit<Circus.TestEntry, 'fn'> & {
fn: Circus.ConcurrentTestFn;
};
Expand Down Expand Up @@ -67,6 +71,10 @@ const _runTestsForDescribeBlock = async (
const retryTimes =
// eslint-disable-next-line no-restricted-globals
parseInt((global as Global.Global)[RETRY_TIMES] as string, 10) || 0;

const waitBeforeRetry =
// eslint-disable-next-line no-restricted-globals
parseInt((global as Global.Global)[WAIT_BEFORE_RETRY] as string, 10) || 0;
const deferredRetryTests = [];

if (rng) {
Expand Down Expand Up @@ -102,6 +110,10 @@ const _runTestsForDescribeBlock = async (
// Clear errors so retries occur
await dispatch({name: 'test_retry', test});

if (waitBeforeRetry > 0) {
await new Promise(resolve => setTimeout(resolve, waitBeforeRetry));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this setTimeout work if fake timers are active? This code runs within the sandbox of user code, so we have to for instance make sure Date.now is the real one:

const nowDeclaration = template(`
var jestNow = globalThis[Symbol.for('jest-native-now')] || globalThis.Date.now;
`);

Copy link
Contributor Author

@WillianAgostini WillianAgostini Dec 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does using const {setTimeout} = globalThis; solve this?

3707c1a

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that should work

}

await _runTest(test, isSkipped);
numRetriesAvailable--;
}
Expand Down
1 change: 1 addition & 0 deletions packages/jest-circus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

export const STATE_SYM = Symbol('JEST_STATE_SYMBOL');
export const RETRY_TIMES = Symbol.for('RETRY_TIMES');
export const WAIT_BEFORE_RETRY = Symbol.for('WAIT_BEFORE_RETRY');
// To pass this value from Runtime object to state we need to use global[sym]
export const TEST_TIMEOUT_SYMBOL = Symbol.for('TEST_TIMEOUT_SYMBOL');
export const LOG_ERRORS_BEFORE_RETRY = Symbol.for('LOG_ERRORS_BEFORE_RETRY');
4 changes: 3 additions & 1 deletion packages/jest-environment/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,14 @@ export interface Jest {
* the test to fail to the console, providing visibility on why a retry occurred.
* retries is exhausted.
*
* `waitBeforeRetry` is the number of milliseconds to wait before retrying
*
* @remarks
* Only available with `jest-circus` runner.
*/
retryTimes(
numRetries: number,
options?: {logErrorsBeforeRetry?: boolean},
options?: {logErrorsBeforeRetry?: boolean; waitBeforeRetry?: number},
): Jest;
/**
* Exhausts tasks queued by `setImmediate()`.
Expand Down
3 changes: 3 additions & 0 deletions packages/jest-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ type ResolveOptions = Parameters<typeof require.resolve>[1] & {

const testTimeoutSymbol = Symbol.for('TEST_TIMEOUT_SYMBOL');
const retryTimesSymbol = Symbol.for('RETRY_TIMES');
const waitBeforeRetrySymbol = Symbol.for('WAIT_BEFORE_RETRY');
const logErrorsBeforeRetrySymbol = Symbol.for('LOG_ERRORS_BEFORE_RETRY');

const NODE_MODULES = `${path.sep}node_modules${path.sep}`;
Expand Down Expand Up @@ -2265,6 +2266,8 @@ export default class Runtime {
this._environment.global[retryTimesSymbol] = numTestRetries;
this._environment.global[logErrorsBeforeRetrySymbol] =
options?.logErrorsBeforeRetry;
this._environment.global[waitBeforeRetrySymbol] =
options?.waitBeforeRetry;

return jestObject;
};
Expand Down
Loading