Fix #631, add jestFakeTimersAreEnabled and use it to detect faketimer in createTimeoutController

[chris110408]

What:

Fixes #631, add jestFakeTimersAreEnabled and use it detect the useFakeTimer.
when useFakeTimer has been detected and jest.advanceTimersByTime will advance time to the timeout-1
test:'should waitFor arbitrary expectation to pass when fake timers are not advanced explicitly' has been added

Why:
we need to add jestFakeTimersAreEnabled function the same function used on @testing-library/dom
and use it to detect the fake timer and use advanced time to make sure time is not stuck

How:
The problem was when the fake timer is used. the time will not move forward unless using the advanced time.
#682 createTimeoutController has been used to replace Promise.race.
the problem is to detect the fake timber and use advance time to move time forward to the last ms until the timeout.

Checklist:

• Documentation updated
• Tests
• Ready to be merged
  
• Added me to contributors table. @mpeyper my status is test contributor only could you please update my status to the code and test

[netlify]

✔️ Deploy Preview for react-hooks-testing-library ready!

🔨 Explore the source changes: d623572

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-hooks-testing-library/deploys/614738594b33ad0008148c1f

😎 Browse the preview: https://deploy-preview-688--react-hooks-testing-library.netlify.app

[codecov]

Codecov Report

Merging #688 (d623572) into alpha (8b72a87) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             alpha      #688   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        16    +1     
  Lines          244       264   +20     
  Branches        32        38    +6     
=========================================
+ Hits           244       264   +20     

Impacted Files	Coverage Δ
src/core/asyncUtils.ts	100.00% <100.00%> (ø)
src/helpers/createTimeoutController.ts	100.00% <100.00%> (ø)
src/helpers/fakeTimers.ts	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 8b72a87...d623572. Read the comment docs.

[mpeyper]

@chris110408 I've just pushed some more tests for the fake timers test suite (they are the same as the regular async utils, except one).

Theoretically all these tests should pass, when fake timers are enabled. With your current changes, most are timing out (the CI agents are not handling this very well as it's taking a long time for all of the tests to time out).

[mpeyper]

@chris110408, you probably got a notification about it (but in case anyone is reading along) but I've pushed up a branch with most of the suggested changes here and a modified advance timers implementation that passes all the tests (at least locally for me).

[chris110408]

@chris110408, you probably got a notification about it (but in case anyone is reading along) but I've pushed up a branch with most of the suggested changes here and a modified advance timers implementation that passes all the tests (at least locally for me).

Thank you. I will look at those change today

[chris110408]

@mpeyper The skip has been removed and I made some other revise to make this test pass
https://github.com/testing-library/react-hooks-testing-library/pull/688/files#r705710781
https://github.com/testing-library/react-hooks-testing-library/pull/688/files#r705708190

[chris110408]

I'm just reading a few things (here, here and here) that suggest te build error we were getting was actually a node error (from v8) and just very unlucky timing on our part.

I'm going to try reenabling that test a see if it passes now, but will revert if it's still an issue.

Update: it worked!

Nice. Is there any thing I need to double check before we merge this PR?

[mpeyper]

I'm just reading a few things (here, here and here) that suggest te build error we were getting was actually a node error (from v8) and just very unlucky timing on our part.
I'm going to try reenabling that test a see if it passes now, but will revert if it's still an issue.
Update: it worked!

Nice. Is there any thing I need to double check before we merge this PR?

I need to properly review it (spent most of my time looking at the build issue the other night), but I'm thinking of putting this out in a beta release first.

[mpeyper]

Thanks a lot @chris110408. I've made a few, most cosmetic changes.

The biggest change was no using DEFAULT_TIMEOUT for advancing timers when timeout = false to rather keep advancing while the promise is outstanding.

The only other thing I noticed that might have been an issue (no tests were failing) was that timers were being advance in both the timeout and interval controllers since they were moved out of the the if (timeout) check for the timeout = false, interval = false test case, but that just meant we needed to pick one to run the timers.

I'm going to push out an alpha build for this now.

[github-actions]

🎉 This PR is included in version 8.0.0-alpha.1 🎉

The release is available on:

• npm package (@alpha dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[mpeyper]

Just wondering if we need some a note in the async utils docs about fake timers?

[mpeyper]

I've got a fairly complicated real world example that I thought would be fixed by this change, but after testing out the alpha it's still not working. I'll try to get a boiled down version onto a test to help work through it, but so far it appears as though the microtask queue is not flushing as we advance timers. I'm not sure why this is occurring for the setup yet, but wanted to drop a comment here in case anyone else tries it with a more real world example and is also running into issues.

[chris110408]

Thanks a lot @chris110408. I've made a few, most cosmetic changes.

The biggest change was no using DEFAULT_TIMEOUT for advancing timers when timeout = false to rather keep advancing while the promise is outstanding.

The only other thing I noticed that might have been an issue (no tests were failing) was that timers were being advance in both the timeout and interval controllers since they were moved out of the the if (timeout) check for the timeout = false, interval = false test case, but that just meant we needed to pick one to run the timers.

I'm going to push out an alpha build for this now.

@mpeyper Could you please add me to the code contributor list?

[chris110408]

Just wondering if we need some a note in the async utils docs about fake timers?

I could work on that If we need to

[chris110408]

I've got a fairly complicated real world example that I thought would be fixed by this change, but after testing out the alpha it's still not working. I'll try to get a boiled down version onto a test to help work through it, but so far it appears as though the microtask queue is not flushing as we advance timers. I'm not sure why this is occurring for the setup yet, but wanted to drop a comment here in case anyone else tries it with a more real world example and is also running into issues.

will look at it this weekend

[mpeyper]

@mpeyper Could you please add me to the code contributor list?

I did, it's just in the alpha branch for now.

I could work on that If we need to

Thanks. Just target the alpha branch if it's not in main when you make the PR.

will look at it this weekend

Hopefully I will have boiled down the example by then. I'll post it here if I do. I'm actively working on this too as the hook/test in question is part of my day job.

[chris110408]

@mpeyper Could you please add me to the code contributor list?

I did, it's just in the alpha branch for now.

I could work on that If we need to

Thanks. Just target the alpha branch if it's not in main when you make the PR.

will look at it this weekend

Hopefully I will have boiled down the example by then. I'll post it here if I do. I'm actively working on this too as the hook/test in question is part of my day job.

Thank you

[mpeyper]

I've got a fairly complicated real world example that I thought would be fixed by this change, but after testing out the alpha it's still not working. I'll try to get a boiled down version onto a test to help work through it, but so far it appears as though the microtask queue is not flushing as we advance timers. I'm not sure why this is occurring for the setup yet, but wanted to drop a comment here in case anyone else tries it with a more real world example and is also running into issues.

So after a few hours of replacing portions of this very complex hook setup, I've managed to narrow it down to an API call being made with node-fetch that is being mocked with msw. If I replace that API call with a promise that resolves after a short timeout, the test works as expected.

I'm out of steam today to look at this anymore, but I'll try to setup a small example just with node-fetch and msw and see I can replicate it without everything else going on in this hook (it's also got event emitters, intervals, memoized functions and redux dispatching all thrown in mix) sometime over the next few days.

[icatalina]

Hi! thanks for fixing this! We are facing this issue, when are we planning to release it? thanks 🙇

[mpeyper]

Hi @icatalina, I'm not sure this will ever see a full release at this point, given what is happening over in RTL. I believe renderHook is available now in a stable release, so you should probably check that out. As a bonus, their version of waitFor already supports fake timers from jest.

[icatalina]

Ohhh!! I had no idea! thanks heaps for pointing that out! :)