-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix Jest runner timeout accounting races #11148
Conversation
Fixes oven-sh#11147 Jest runner would report incorrect timeouts, due to races in the timer accounting code. This commit arms and disarms timers before and after every test to ensure correct timeouts without false-positives.
Thank you for the PR, these changes look like they'll reduce some of the flakiness around timeouts |
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.
If CI passes I think we should merge this
❌ @stefreak, your commit has failing tests :( 💪 1 failing tests Darwin AARCH64
🐧🖥 1 failing tests Linux x64
🪟💻 6 failing tests Windows x64 baseline
🪟💻 7 failing tests Windows x64
|
@stefreak Some of the new timeout tests seem to fail on windows. This is most likely because timers on windows are less accurate. I suggest increasing the tolerances in the tests and seeing if that fixes it. |
@gvilums thank you for the feedback! I'll have a look at that |
I'm trying to figure out which of these test failures are related to my change, and which are flakes. Could you approve the workflows again @gvilums? Thank you ❤️ |
I finally managed to get a working windows dev environment, but actually this has been fixed in the mean time by #11419 |
What does this PR do?
Fixes #11147
Jest runner would report incorrect timeouts, due to races in the timer
accounting code.
This commit arms and disarms timers before and after every test to
ensure correct timeouts without false-positives.
How did you verify your code works?
I ran the test suite in #11147 manually and added to the automated test suite.
bun-debug test test-file-name.test
)