-
-
Notifications
You must be signed in to change notification settings - Fork 595
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
Stop running jest in parallel in integ tests #3517
Conversation
... it makes the output impossible to understand.
c4ebcac
to
fb11f12
Compare
|
||
// force a save to indexeddb to make sure that we don't have any saves in flight when the test completes. | ||
await idbClient.store.save(true); |
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.
once we limit jest to one worker, it realises that something is logging after shutdown, and complains about it. This makes sure that the test cleans up properly.
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.
This goes directly against one of the goals of the Web Weekly Reduce time required to run tests so needs discussion there
I don't think it makes any difference. Compare https://github.com/matrix-org/matrix-js-sdk/actions/runs/5389744563/jobs/9784333178?pr=3517 with this PR (1m53s) with https://github.com/matrix-org/matrix-js-sdk/actions/runs/5389733941/jobs/9784477473?pr=3521 without it (1m49s) |
That's a 3.7% increase in time, which whilst we have few tests is negligible, but if we grow our number of tests we'd end up wanting to revert this, I don't think its the right approach personally. Happy to be overriden. -0 for me |
closing this for now due to general lack of consensus. I may reopen in future if it continues to grind my gears |
... it makes the output impossible to understand.
This change is marked as an internal change (Task), so will not be included in the changelog.