-
Notifications
You must be signed in to change notification settings - Fork 46.7k
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
[Mock Scheduler] Mimic browser's advanceTime #17967
Conversation
The mock Scheduler that we use in our tests has its own fake timer implementation. The `unstable_advanceTime` method advances the timeline. Currently, a call to `unstable_advanceTime` will also flush any pending expired work. But that's not how it works in the browser: when a timer fires, the corresponding task is added to the Scheduler queue. However, we will still wait until the next message event before flushing it. This commit changes `unstable_advanceTime` to more closely resemble the browser behavior, by removing the automatic flushing of expired work. ```js // Before this commit Scheduler.unstable_advanceTime(ms); // Equivalent behavior after this commit Scheduler.unstable_advanceTime(ms); Scheduler.unstable_flushExpired(); ``` The general principle is to prefer separate APIs for scheduling tasks and flushing them. This change does not affect any public APIs. `unstable_advanceTime` is only used by our own test suite. It is not used by `act`. However, we may need to update tests in www, like Relay's.
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit fe17821:
|
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 is more in line with the naming of the API but I'm not sure it's actually better. Seems easy to forget to flush. It's not really how I think about testing. I think of it as E2E. Play time forward a bit and see what happens.
I'd prefer an API like expect(Scheduler).toAdvanceTimeByAndToFlushAndYield(1000, ...)
I think I'd prefer a helper that plays time forward for a certain time with a reasonable amount of flushes in between. It shouldn't just jump to the end as if the CPU was stalled for the whole time. Seems like we'd test very unrealistic scenarios that way. The order might matter. |
I dig @sebmarkbage's suggestion of a method like |
Yeah I think that API would make sense because it's a test matcher which discourages us from calling it inside another task. |
The mock Scheduler that we use in our tests has its own fake timer implementation. The
unstable_advanceTime
method advances the timeline.Currently, a call to
unstable_advanceTime
will also flush any pending expired work. But that's not how it works in the browser: when a timer fires, the corresponding task is added to the Scheduler queue. However, we will still wait until the next message event before flushing it.This commit changes
unstable_advanceTime
to more closely resemble the browser behavior, by removing the automatic flushing of expired work.The general principle is to prefer separate APIs for scheduling tasks and flushing them.
This change does not affect any public APIs.
unstable_advanceTime
is only used by our own test suite. It is not used byact
.However, we may need to update tests in www, like Relay's.