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.

Sign up for GitHub

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

Jump to bottom

Tests timeout with jest fakeTimers and waitFor for on Promise.resolve calls #873

Open
5c077yP opened this issue Jul 15, 2022 · 5 comments · May be fixed by #878
Open

Tests timeout with jest fakeTimers and waitFor for on Promise.resolve calls #873

5c077yP opened this issue Jul 15, 2022 · 5 comments · May be fixed by #878
Assignees
@eps1lon
Labels
bug Something isn't working

Comments

@5c077yP
Copy link

5c077yP commented Jul 15, 2022

Hey, I get some of my tests timing out when using waitFor and jest.useFakeTimers, but not using a timer internally, but only Promise.resolve. As a context I'm trying to migrate a bigger code base from v4 to the latest version from v5 on some tests are failing. I'm also using jests faketimers by default for the tests.

This might be a bit related to #631

  • react-hooks-testing-library version: 8.0.1
  • react version: 17.02
  • react-dom version (if applicable): 17.02
  • react-test-renderer version (if applicable): 17.02
  • node version: 16.14.2
  • yarn version: 1.22.10

Relevant code or config:

const React = require('react');
const { renderHook } = require('@testing-library/react-hooks');

jest.useFakeTimers(); // modern or fake doesn't matter here

const useStateAndAsyncEffect = (fn) => {
  const [s, set] = React.useState(false);

  React.useEffect(() => {
    Promise.resolve().then(() => set(true));
  }, []);

  React.useEffect(() => {
    if (s) fn();
  }, [s]);

  return;
};

test('test hook', async () => {
  const fn = jest.fn();
  const { waitFor } = renderHook(() => useStateAndAsyncEffect(fn));
  await waitFor(() => expect(fn).toHaveBeenCalled());
});

What happened:

The test fails from v5 and onwards, but worked in v4.

Suggested solution:

I was digging a bit into the code and saw v4 is calling act inside async-utils inside the while(true) loop, while from v5 upwards act is only called once. I've tried to figure out the details, but not really sure why calling act more than once is making this work. I'm thinking about react flushing micro tasks more often, but also not very familiar with react internals/fibers.

I've played with patch-package on got this diff working for me. Please let me know what you think about it 🙏

diff --git a/node_modules/@testing-library/react-hooks/lib/core/asyncUtils.js b/node_modules/@testing-library/react-hooks/lib/core/asyncUtils.js
index dbf1a49..77b9785 100644
--- a/node_modules/@testing-library/react-hooks/lib/core/asyncUtils.js
+++ b/node_modules/@testing-library/react-hooks/lib/core/asyncUtils.js
@@ -26,9 +26,11 @@ function asyncUtils(act, addResolver) {
 
     const waitForResult = async () => {
       while (true) {
-        const intervalSignal = (0, _createTimeoutController.createTimeoutController)(interval);
-        timeoutSignal.onTimeout(() => intervalSignal.cancel());
-        await intervalSignal.wrap(new Promise(addResolver));
+        await act(async () => {
+          const intervalSignal = (0, _createTimeoutController.createTimeoutController)(interval);
+          timeoutSignal.onTimeout(() => intervalSignal.cancel());
+          await intervalSignal.wrap(new Promise(addResolver));
+        })
 
         if (checkResult() || timeoutSignal.timedOut) {
           return;
@@ -37,7 +39,7 @@ function asyncUtils(act, addResolver) {
     };
 
     if (!checkResult()) {
-      await act(() => timeoutSignal.wrap(waitForResult()));
+      await timeoutSignal.wrap(waitForResult());
     }
 
     return !timeoutSignal.timedOut;
@5c077yP 5c077yP added the bug Something isn't working label Jul 15, 2022
@eps1lon
Copy link
Member

eps1lon commented Jul 18, 2022 •
edited

Probably another instance of #589. @mpeyper does /react-hooks manually flush the microtask queue when you're detecting fake timers? It looks like /react-hooks doesn't.

For comparison, /react manually flushes the microtask queue (although hacky) if we detect fake timers.

@5c077yP Could you check if the test still times out when you use

import {render, waitForm} from '@testing-library/react'; // IMPORTANT: Use 12.x if using React 17
const fn = jest.fn();
function Component() {
  useStateAndAsyncEffect(fn);
  return null;  
}
render(<Component />)
await waitFor(() => expect(fn).toHaveBeenCalled());

?

@5c077yP
Copy link
Author

5c077yP commented Jul 18, 2022

Hey @eps1lon , yes the test does work with /react out of the box

@mpeyper
Copy link
Member

mpeyper commented Jul 18, 2022

No, we have never supported fake times. An attempt was made in a alpha build some time ago, but was shelved after the decision was made to move renderHook into /react for react 18.

@eps1lon
Copy link
Member

eps1lon commented Jul 18, 2022

Yeah makes sense. I had some ideas for a simpler waitFor implementation in /dom (which /react) is using. I'll try to revisit them since that might enable us to use waitFor from /react when using /react-hooks i.e. make waitForm from /react-hooks obsolete.

@eps1lon eps1lon self-assigned this Jul 18, 2022
@eps1lon eps1lon linked a pull request Jul 19, 2022 that will close this issue
Draft
8 tasks
@lgibso34
Copy link

Came here to report the same thing.

I fixed my issue by using the waitFor from @testing-library/react. It checks for fake timers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@eps1lon eps1lon

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(waitFor): Automatically advance Jest fake timers eps1lon/react-hooks-testing-library
4 participants
@5c077yP @eps1lon @mpeyper @lgibso34