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

fix(jest-circus) correct concurrent event ordering #14643

Conversation

madcapnmckay
Copy link
Contributor

@madcapnmckay madcapnmckay commented Oct 25, 2023

Summary

I'm unsure if this is a major, minor, or patch change. It does not change the API, but it changes the order of events emitted from jest-circus

Fixes the issue documented here.

When using test.concurrent events such as test_start test_started and test_fn_start would execute after the body of the test. This was due to the _runTest not being executed as part of the parallel execution runner instead it was executed synchronously like non-concurrent tests.

The solution was to use _runTest within startTestsConcurrently() and then wait on that process to complete instead of replacing the test.fn as was being done before.

Test plan

  • Run yarn test packages/jest-circus/src/__tests__/baseTest.test.ts at commit 8175d9f` to observe the snapshot content for the concurrent test.
  • Checkout a2fd8c7, rerun yarn test packages/jest-circus/src/__tests__/baseTest.test.ts and note that the output has been reordered to match the expectation that the circus actions occur before and after the test execution.

@netlify
Copy link

netlify bot commented Oct 25, 2023

Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 146fa15
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/65398bee81488c0008075669
😎 Deploy Preview https://deploy-preview-14643--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.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 25, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@madcapnmckay
Copy link
Contributor Author

Question: should this be going into main? I originally branched off of v29.7.0

@SimenB
Copy link
Member

SimenB commented Oct 27, 2023

@madcapnmckay main is correct 🙂 I see there's a bunch of test failures here, tho

Copy link

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Jan 25, 2024
Copy link

This PR was closed because it has been stalled for 30 days with no activity. Please open a new PR if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this Feb 24, 2024
Copy link

This PR was closed because it has been stalled for 30 days with no activity. Please open a new PR if the issue is still relevant, linking to this one.

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 Mar 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants