-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
feat(jest-circus): add onTestCaseStart hook Reporter
#14174
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(jest-circus): add onTestCaseStart hook Reporter
#14174
Conversation
…nd transparent approach - clean previous approach - add `test_started` event to Circus runner - fix tests for `test_started` event
✅ Deploy Preview for jestjs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
onTestCaseStart hook Reporter
|
@SimenB Would you please take a look at this review? |
|
Is someone even assigned to this pull request? What's happening? It's Jest's problem, I guess, but at the moment it means jest-circus is unusable in my IDE, and so I'm not using it. |
Oh yeah I know. The two of you are really stepping up here. That was my way of putting pressure on the Jest people. You've been talking to them about the issue for a long time, and they're a pretty significant bottleneck. |
|
Hey folks! Sorry about the slow answer. I'm currently preparing for a new job, so open source doesn't get much attention atm. Thanks for pinging - anything that helps IDE integration (especially intellij which I use myself 😀) should be prioritized. |
SimenB
left a comment
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.
👍
|
(are you on open js slack? That forum would work better to ping me on than GitHub tags or emails. (If there's some other forum you like prefer, I'd be happy to join. The open ones doesn't really work)) |
|
Hey @SimenB, Thank you so much 🙏
Have awesome time with new job ✨
Thanks. Nice to hear.
I joined after you mentioned it and I'm interesting to try it.
I don't know of any suitable forum, however I'll think about it. |
|
I'll release this after landing #14062 (unless it takes a long time). That might be useful for your cases as well? |
|
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. |
Summary
testoritvia reporter API #13112.onTestCaseStarthook for Reporter API for giving ability to handle beginning of everytestoritvia reporter for jest with Jest-Circus Runner.Test plan
Unit tests added
Detailed design
At start, I tried using the current
jest-circusevents to detect when a test case starts by listening to thetest_startevent and calling theonTestCaseStarthook of theReporterafter it. However, when a test is marked astodoor skipped for any reason,jest-circusstill dispatches thetest_startevent, followed by thetest_skiportest_todoevents, which makes it incorrect to call theonTestCaseStarthook on thetest_startevent, because the test suite hasn't actually started yet. This behavior forces tools that will handle theonTestCaseStarthook to filter out the tests that have actually started. Unfortunately, also ontest_startevent we don't have full information about test skipping, because most reasons for skipping a test resolve after the event dispatching.Moving the
test_startevent after thetest_skipandtest_todoevents could be a solution, but it could also break things, because jest has an internal handler that listens to thetest_startevent and modifies global state. Therefore, I decided not to move thetest_startevent.I investigated other events that are dispatched after the
test_skipandtest_todoevents and found thehook_startevent dispatching and thetest_fn_startevent dispatching. Calling theonTestCaseStarthook on thetest_fn_startevent would skip thebefore*hooks calling phase, which is not suitable for the feature request this PR is solving, because catching output from thebefore*hooks calling phase is important.Calling the
onTestCaseStarthook on thehook_startevent also looks incorrect, because thehook_startevent will be avoided for most tests and can be called many times for one test. This requires listening to a combination ofhook_startandtest_fn_startevents, which makes the solution stateful and less stable.So after all these investigations and discussing with colleagues, I decided to add a new event called
test_startedto thejest-circustest's run flow, which is dispatched after thetest_skipandtest_todoevents and before thehook_startandtest_fn_startevents. This is the best place in the test's running flow to understand when a test case really starts, and connecting theonTestCaseStarthook of theReporterto this event is simple and transparent.I found this solution. If you have any other suggestions, you are welcome.