Skip to content

Limit use of realTimers to Jest's fakeTimers #652

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

Merged
merged 1 commit into from
Jun 17, 2020

Conversation

kgregory
Copy link
Collaborator

What:

Modify runWithRealTimers so that real timers are only used if they were faked by Jest's legacy or modern implementation of fake timers.

Also, when restoring Jest's fakes, be explicit in specifying whether we're restoring the modern or legacy fakes.

Why:

A recent change to runWithRealTimers added a check for the clock attribute on setTimeout so that we can determine whether timers were faked with Jest's modern fake timers (which are based on @sinonjs/fake-timers). See issue thread.

Unfortunately, this attribute can be present when users are not using Jest's fake timers and are using sinonjs. Also, when useFakeTimers are invoked without parameters in Jest 26, the legacy timers are used by default.

How:

Determine if Jest legacy timers are used by checking globalObj.setTimeout._isMockFunction.

Determine if Jest modern timers are used by checking for the presence of globalObj.setTimeout.clock and also by verifying that use of jest.getRealSystemTime is supported. It was introduced in Jest 26 and is only available when modern fake timers are used.

Checklist:

  • Documentation added to the
    docs site
  • Tests
  • Typescript definitions updated
  • Ready to be merged

A recent change to `runWithRealTimers` added a check for the `clock` attribute on `setTimeout` so that we can determine whether timers were faked with Jest's modern fake timers (which are based on `@sinonjs/fake-timers`).

Unfortunately, this attribute can be present when users are not using Jest's fake timers and are using sinonjs.  Also, when `useFakeTimers` are invoked without parameters in Jest 26, the legacy timers are used by default.

This PR addresses two issues:

1. Don't use realTimers if the timers were not faked by Jest.
2. If we're using realTimers, make sure we're explicit when calling `useFakeTimers` so that the fakes are restored to their original state.
@codesandbox-ci
Copy link

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 f86b41e:

Sandbox Source
awesome-banach-cfg89 Configuration

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #652 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #652   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          575       582    +7     
  Branches       145       149    +4     
=========================================
+ Hits           575       582    +7     
Impacted Files Coverage Δ
src/helpers.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c19f4f...f86b41e. Read the comment docs.

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid. Thank you!

@kentcdodds kentcdodds merged commit ea22d73 into testing-library:master Jun 17, 2020
@kentcdodds
Copy link
Member

🎉 This PR is included in version 7.16.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@kgregory kgregory deleted the pr/run-with-real-timers branch June 17, 2020 20:24
@kgregory
Copy link
Collaborator Author

Thanks @kentcdodds, feels good to have contributed.

@kentcdodds
Copy link
Member

@all-contributors please add @kgregory for code and tests

@allcontributors
Copy link
Contributor

@kentcdodds

I've put up a pull request to add @kgregory! 🎉

@kentcdodds
Copy link
Member

Thanks so much for your help! I've added you as a collaborator on the project. Please make sure that you review the other/MAINTAINING.md and CONTRIBUTING.md files (specifically the bit about the commit messages and the git hooks) and familiarize yourself with the code of conduct (we're using the contributor covenant). You might also want to watch the repo to be notified when someone files an issue/PR. Please continue to make PRs as you feel the need (you can make your branches directly on the repo rather than your fork if you want). Thanks! And welcome to the team :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants