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

test: enable modern jest timers test #938

Closed
wants to merge 1 commit into from

Conversation

timdeschryver
Copy link
Member

What:

Explicit set jest's faker timers to modern.

Why:

Because in Jest 26.x the default is legacy.
https://jestjs.io/blog/2020/05/05/jest-26#new-fake-timers

How:

Changed the modern test by passing modern as the options.
I also had to advance the timers, and had to "clear" the timers after the test.

Checklist:

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 1, 2021

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 51fd8e7:

Sandbox Source
react-testing-library-examples Configuration

@codecov
Copy link

codecov bot commented May 1, 2021

Codecov Report

Merging #938 (51fd8e7) into master (ffc8f26) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #938   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        26           
  Lines          953       953           
  Branches       289       289           
=========================================
  Hits           953       953           
Flag Coverage Δ
node-10.14.2 100.00% <ø> (ø)
node-12 100.00% <ø> (ø)
node-14 100.00% <ø> (ø)
node-15 100.00% <ø> (ø)
node-16 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 ffc8f26...51fd8e7. Read the comment docs.

const response = 'data'
const doAsyncThing = () =>
new Promise(r => setTimeout(() => r(response), time))
let result
doAsyncThing().then(r => (result = r))

if (advanceTimers) {
jest.advanceTimersByTime(time)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't waitFor take care of this?

@timdeschryver I think this is fixed by #966 and #962 without needing to change these particular tests. Can we close this PR in favor of #962?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes of course.
Thanks for getting back to this PR, because I forgot about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't waitFor take care of this?

It could've been a bug in waitFor then, the reason why I added this was because the timers weren't advancing on their own. So, I thought that I had to do that manually.

@timdeschryver timdeschryver deleted the fix/modern-timers-test branch June 3, 2021 11:14
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 this pull request may close these issues.

None yet

2 participants