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

Always recreate mocks in FakeTimers.useFakeTimers() #2109

Merged

Conversation

itszero
Copy link
Contributor

@itszero itszero commented Nov 16, 2016

Summary

If the flag resetMocks or jest.resetAllMocks is used in the test, all the timer-related tests following that will fail with no way of recovering.

Cause

jest.resetAllMocks resets everything, including mocks created by fakeTimers. There are also no public methods that we can use to re-instantiate a new FakeTimer object to mitigate the problem.

Solution

Given that both mock and fake timers are part of the jest, they should be working together out-of-the box without extra code gluing them together. We change the useFakeTimers method to always create new mocks so the mocks will not be reverted by resetAllMocks. If there is a better way to manage the lifecycle of those mocks, please let me know. I will be happy to make changes.

Test plan

I have provided a new integration test suite named timer_after_resetAllMocks. It fails on jest 17.0.2 and is passing after this patch.

Note

This patch makes flow unhappy and I'm confused as to why it fails. I would love to hear some feedbacks on how to fix this.

> @ typecheck /Users/zcho/src/jest-work/jest
> flow check

packages/jest-util/src/FakeTimers.js:90
 90:       this._timerAPIs.nextTick = global.process.nextTick;
                           ^^^^^^^^ property `nextTick`. Property not found in
 90:       this._timerAPIs.nextTick = global.process.nextTick;
           ^^^^^^^^^^^^^^^ object literal

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact [email protected] if you have any questions.

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Zero Cho and others added 2 commits November 29, 2016 18:27
If the flag resetMocks or jest.resetAllMocks is used in the test, all the timer
tests following that will fail with no way of recovering.

jest.resetAllMocks resets everything, including the one created by fakeTimers.
There is also no public methods that we can use to reinstantiate a new
FakeTimer object to mitigate that.

Given both mock and fake timers are part of the jest, they should be working
together out-of-the box without extra code gluing them. We change the
useFakeTimers method to always create new mocks so it would not be reverted.
@cpojer
Copy link
Member

cpojer commented Nov 29, 2016

Thanks for the PR! I just pushed an update that should fix your Flow issue :)

@cpojer cpojer force-pushed the zcho/always_recreate_mocks_in_useFakeTimers branch from 886718c to faeb3e5 Compare November 29, 2016 18:28
@cpojer cpojer merged commit aadcda4 into jestjs:master Nov 29, 2016
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
* Always recreate mocks in FakeTimers.useFakeTimers()

If the flag resetMocks or jest.resetAllMocks is used in the test, all the timer
tests following that will fail with no way of recovering.

jest.resetAllMocks resets everything, including the one created by fakeTimers.
There is also no public methods that we can use to reinstantiate a new
FakeTimer object to mitigate that.

Given both mock and fake timers are part of the jest, they should be working
together out-of-the box without extra code gluing them. We change the
useFakeTimers method to always create new mocks so it would not be reverted.

* Cleanup FakeTimers handling of `process.nextTick`.
@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants