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

Conversation

WillianAgostini
Copy link
Contributor

Summary

Fixes #14737 adding a new option waitBeforeRetry to jest.retryTimes

Test plan

The tests have been updated.

Copy link

linux-foundation-easycla bot commented Dec 4, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link

netlify bot commented Dec 4, 2023

Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 89ed2d1
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/65895ed3f9d16300085855fb
😎 Deploy Preview https://deploy-preview-14738--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

thanks! in addition to my inline question, could you update the changelog?

@@ -102,6 +106,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

@SimenB
Copy link
Member

SimenB commented Dec 24, 2023

Wait, CI seems unhappy

@WillianAgostini
Copy link
Contributor Author

hi @SimenB
This bug step:7:61 is related to what?

@SimenB
Copy link
Member

SimenB commented Dec 25, 2023

hi @SimenB This bug step:7:61 is related to what?

That's just some CI flake. Which is very annoying, but fine to ignore 🙂

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

thanks!

@SimenB SimenB merged commit e20a4be into jestjs:main Dec 25, 2023
71 of 73 checks passed
@WillianAgostini WillianAgostini deleted the feat/14737 branch December 25, 2023 11:48
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: add a waitBeforeRetry option to jest.retryTimes
2 participants