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

Repeated status logging due to ordering inconsistency #76

Open
JoostK opened this issue Aug 7, 2023 · 0 comments · May be fixed by #81
Open

Repeated status logging due to ordering inconsistency #76

JoostK opened this issue Aug 7, 2023 · 0 comments · May be fixed by #81

Comments

@JoostK
Copy link

JoostK commented Aug 7, 2023

Thanks for this exciting project! I have been toying with the light runner a couple times now and it is hugely impressive, yielding up to two orders of magnitude performance improvements (going from ~400s to ~5s for ~2500 tests in ~400 suites, using --runInBand). Jest's isolation is hurting us a lot, even though we don't need it.

I am experiencing an issue where the start message that is sent over a test's MessageChannel may arrive after the tests' Promise has already resolved, i.e. the finished callback has already been executed. This results in excessive status message logging, as the late start event does activate the test as "running" even though it has already finished, rendering it in the "running" state forever (and consequently generating excessive status messages as more and more tests become considered "running").

I don't know exactly why/when this happens, nor do I know what the delivery guarantees are w.r.t. MessagePort message timing vs the micro task queue. My observations are with the InBandPiscina mode and this may be relevant for this to occur, considering that Piscina would otherwise communicate over message channels itself.

Potential solutions

I see two approaches that would fix my issue:

  1. Detect when a start message is received after the promise has resolved, then ignoring the start message entirely.
  2. Delaying the finish notification until after the start message has been received, then notifying the status runner of both events simultaneously (possibly with a microtick between the start and end notification, if needed)

I have tested locally with approach 1 and that seems to work just fine, but option 2 results in more consistently communicating test statuses without skipping status transitions (not sure if there's any invariants in this area).

I would be happy to contribute either fix, just opening this issue for discussion.

JoostK added a commit to JoostK/jest-light-runner that referenced this issue Nov 26, 2023
The start message that is sent over a test's MessageChannel may arrive after
the tests' Promise has already resolved, i.e. the finished callback has
already been executed. This results in excessive status message logging,
as the late start event does activate the test as "running" even though
it has already finished, rendering it in the "running" state forever (and
consequently generating excessive status messages as more and more tests become
considered "running").

Fixes nicolo-ribaudo#76
JoostK added a commit to JoostK/jest-light-runner that referenced this issue Nov 28, 2023
The start message that is sent over a test's MessageChannel may arrive after
the tests' Promise has already resolved, i.e. the finished callback has
already been executed. This results in excessive status message logging,
as the late start event does activate the test as "running" even though
it has already finished, rendering it in the "running" state forever (and
consequently generating excessive status messages as more and more tests become
considered "running").

Fixes nicolo-ribaudo#76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant