-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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-fake-timers): Add feature to enable automatically advancing… #15300
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for jestjs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
835635d
to
fb18fa9
Compare
Is the behaviour of this new method different from the Lines 883 to 889 in bd1c6db
|
Good question! Yes, it's quite different, both in correctness and in practicality. I'll update the documentation with some version of this explanation (and please correct me if any of this seems wrong):
There are two key differences between these two:
Given that |
fb18fa9
to
7630c43
Compare
… timers Testing with mock clocks can often turn into a real struggle when dealing with situations where some work in the test is truly async and other work is captured by the mock clock. In addition, when using mock clocks, testers are always forced to write tests with intimate knowledge of when the mock clock needs to be ticked. Oftentimes, the purpose of using a mock clock is to speed up the execution time of the test when there are timeouts involved. It is not often a goal to test the exact timeout values. This can cause tests to be riddled with manual advancements of fake time. It ideal for test code to be written in a way that is independent of whether a mock clock is installed or which mock clock library is used. For example: ``` document.getElementById('submit'); // https://testing-library.com/docs/dom-testing-library/api-async/#waitfor await waitFor(() => expect(mockAPI).toHaveBeenCalledTimes(1)) ``` When mock clocks are involved, the above may not be possible if there is some delay involved between the click and the request to the API. Instead, developers would need to manually tick the clock beyond the delay to trigger the API call. This commit attempts to resolve these issues by adding a feature which allows jest to advance timers automatically with the passage of time, just as clocks do without mocks installed.
7630c43
to
de306d5
Compare
Interesting. Do I get it right that this new API is somewhat a variation of
|
Yes, that's right!
I think that's a pretty good idea for an alternative place for this API. Would |
The idea I was talking about was to replace |
Ah, you mean that it the automatic/manual would entirely replace the existing |
@mrazauskas happy to continue iterating on the API naming. Would you also be able to help with the CI failures? I’m not sure what’s going on there. |
Hm.. I think those failures are not related with your code. GHA run all the tests and if they pass, I think all is fine. |
540e0b2
to
3213eef
Compare
@mrazauskas I've changed the implementation to overload |
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.
Perhaps adding an example to the docs would be nice? The test that sets auto
and switches back to manual
looks like a good candidate. What you think?
Great idea! Thank you @atscott for putting this together. In terms of API, I think it is better to rollback to
Finally, I feel that this feature can be confused with the {advanceTime: true} // deprecated
{advanceTime: false} // deprecated
{advanceTime: 'auto'} // current behavior of `advanceTime: true`
{advanceTime: 'manual'} // current behavior of `advanceTime: false`
{advanceTime: 'instant'} // the behavior introduced by this PR
// and a new function that allows switching during execution
jest.setAdvanceTime(...) |
@yjaaidi Thanks for chiming in. I like your proposed design. If you read the discussion, my first intuition was that this feature is somehow connected with the Few details:
|
I've read the discussion and I think that there was some kind of misunderstanding because indeed this feature can be seen as a variation of advanceTimers. That is why I commented to make sure that we are all aligned 😊 This is indeed a typo and I meant I still wonder if What do you think? |
I'm all for making this a new option on the |
16bc8d2
to
a238999
Compare
I've updated the implementation to extend the existing |
5d84dbf
to
20aa0e9
Compare
20aa0e9
to
d0fe547
Compare
I added some more tests to ensure the API works well with calls to the async tick methods. In doing this, I've started to feel that the auto tick machinery may be better implemented in sinon. This boils down to the need for a way to wait a macrotask before automatically advancing timers to prevent the act of updating the mode from also advancing to the next timer. |
Yep. This is what I was thinking earlier today. You did a lot of nice work here, so it didn’t felt right to tell that straight forward. But the feeling is that this machinery indeed belongs in Sinon. They would benefit from it as well. Also you know their codebase already (at least a bit), so perhaps that’s the way forward? |
Yea, I'll convert this PR to draft until the story with implementing it in sinon becomes more clear since it'll still need to be exposed in the jest APIs and I think the tests and discussions around API are still valuable. edit: sinonjs/fake-timers#509 |
… timers
Summary
Testing with mock clocks can often turn into a real struggle when dealing with situations where some work in the test is truly async and other work is captured by the mock clock.
In addition, when using mock clocks, testers are always forced to write tests with intimate knowledge of when the mock clock needs to be ticked. Oftentimes, the purpose of using a mock clock is to speed up the execution time of the test when there are timeouts involved. It is not often a goal to test the exact timeout values. This can cause tests to be riddled with manual advancements of fake time. It ideal for test code to be written in a way that is independent of whether a mock clock is installed or which mock clock library is used. For example:
When mock clocks are involved, the above may not be possible if there is some delay involved between the click and the request to the API. Instead, developers would need to manually tick the clock beyond the delay to trigger the API call.
This commit attempts to resolve these issues by adding a feature which allows jest to advance timers automatically with the passage of time, just as clocks do without mocks installed.
Test plan
I wrote some unit tests but let me know if you need more.