Skip to content
This repository was archived by the owner on Jul 30, 2020. It is now read-only.

update README with jest.useFakeTimers() gotcha #83

Closed
wants to merge 2 commits into from
Closed

update README with jest.useFakeTimers() gotcha #83

wants to merge 2 commits into from

Conversation

causztic
Copy link

@causztic causztic commented Nov 19, 2019

What:

Update README with a gotcha for async methods, based on the my own issues faced on #82

Why:

I think it will help newer users of the library to avoid this pitfall when using async methods in their tests to improve running times.

How:

By updating the README with a new section

Checklist:

  • Documentation added to the
    docs site
  • Typescript definitions updated (N/A)
  • Tests (N/A)
  • Ready to be merged

@causztic
Copy link
Author

causztic commented Nov 19, 2019

@bcarroll22 let me know whether this update is necessary!

@codecov
Copy link

codecov bot commented Nov 19, 2019

Codecov Report

Merging #83 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #83   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          20     20           
  Lines         268    268           
  Branches       67     67           
=====================================
  Hits          268    268

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 251c07d...8a5071e. Read the comment docs.

@bcarroll22
Copy link
Collaborator

I think I'd rather this be something that we include in the async utilities section of the docs site. That repo is located here.

That said though, it should be something similar to the one at the top of this section because this is expected behavior. If you use fake timers but don't advance them, then naturally async stuff is going to fail because it won't have been able to execute correctly.

@causztic
Copy link
Author

Thanks, I will make the PR on the other repo instead

@causztic causztic closed this Nov 20, 2019
@LRNZ09
Copy link

LRNZ09 commented Mar 16, 2020

Hey @causztic, have you ever opened that PR in the other repo linked by @bcarroll22 ?
I'm getting your same error, and I also think this should be clearly explained to devs while reading the docs

@causztic
Copy link
Author

@LRNZ09 no I haven't, do feel free to create a PR for that instead

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

Successfully merging this pull request may close these issues.

3 participants