Improve docs to avoid 'act' warning when using react-query in tests

[dustinsoftware]

This is sort of related to #270. Filing in case we can improve the docs to help others avoid this issue.

When running tests, I ran into this error:

console.error node_modules/react-dom/cjs/react-dom.development.js:88
    Warning: An update to _default inside a test was not wrapped in act(...).

Here's the test:

test('renders the component', async () => {
  await act(async () => {
    const { getByText } = render(<Component />);
    const linkElement = getByText(/Hello/i);
    expect(linkElement).toBeInTheDocument();
  });
});

And here's the component (simplified):

const getPing = async () => {
  const { data } = await axios.get<{ content: string }>(
    getRoutePrefix() + 'ping'
  );
  return data;
};

export default function () {
  const { status, data, error, isFetching } = useQuery('ping', getPing);

  return (
    <>
      <div>Hello!</div>

      <div>{JSON.stringify(status)}</div>
      <div data-testid="testdata">{JSON.stringify(data)}</div>
      <div>Err: {JSON.stringify(error)}</div>
      <div>{JSON.stringify(isFetching)}</div>
    </>
  );
}

In #271 there was some cleanup logic added, however the error in my case appears to have been thrown before the cleanup call was fired.

I can fix the issue by adding to my tests at the end of each act call:

- import { render, wait } from '@testing-library/react';
+ import { render, wait, cleanup } from '@testing-library/react';

test('renders the component', async () => {
  await act(async () => {
    const { getByText } = render(<Component />);
    const linkElement = getByText(/Hello/i);
    expect(linkElement).toBeInTheDocument();
+   cleanup();
  });
});

Doing the cleanup outside the act does not help; that appears to be too late.

Is it necessary to add the cleanup call inside every act call? If so, can we update the react-query docs on how to test with jest + react-testing-library? I bet others will run into this issue as well and hoping to save some time as more users adopt the library.

Here's some screenshots with a debugger attached indicating the warning is originally caused by a call to refetch:

Versions:

@testing-library/react@9.5.0
jest@24.9.0
miragejs@0.1.37
react-dom@16.13.1
react-query@1.2.9
react-scripts@3.4.1
react@16.13.1

[dustinsoftware]

Here's a test that waits on react-query to load the data, forgot to include that when I filed the issue.

test('fetches data on load', async () => {
  await act(async () => {
    const { getByTestId } = render(<Component />);
    const loadingStatus = getByTestId('testdata');

    await wait(() =>
      expect(loadingStatus).toHaveTextContent('{"content":"pong"}')
    );
    cleanup();
  });
});

[tannerlinsley]

We don't see this issue in our own tests, so I'm not really sure what's going on with all of this. Are you using react-testing-library?

[dustinsoftware]

Yeah. Let me see if I can put up a codesandbox for this...

…

On Fri, May 1, 2020 at 10:52, Tanner Linsley ***@***.***> wrote: We don't see this issue in our own tests, so I'm not really sure what's going on with all of this. Are you using react-testing-library? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#432 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHGCFXZPMGMMQ4LFXB53DTRPMD4TANCNFSM4MXGNR7Q> .

[dustinsoftware]

Alright here's the codesandbox: https://codesandbox.io/s/jovial-sanne-mhgb8?file=/src/app.test.js

You may have to run the tests a few times to get the warning to show up (screenshot)

[tannerlinsley]

Interesting. I have no idea why this should be required.

[ilovett]

The state is trying to update due to the hook, but the component / test harness has already torn down which throws the warning.

You could also await waitFor -- to wait for query to be done -- IE wait for spinner to disappear

[dustinsoftware]

The query has already been completed; the codesandbox demonstrates that case. The problem seems to be isolated to the refetch timer not getting cleaned up before the test exits the ‘act’ scope. The cleanup call is still being made by react testing library, just outside of the act.

Order of events:

• test enters act scope
• query is made, schedules timer
• query completes, state updates
• assertion passes
• test exits act scope
• query refetch timer fires, react prints warning
• react testing library calls cleanup, which is too late to cancel the pending refetch timer

Without manually calling cleanup I don’t see a way to prevent the state update from getting scheduled.

[ilovett]

I'm seeing this and it looks like isStale changing from false to true sometimes after the test is completed: https://codesandbox.io/s/sad-gauss-uwptc (removed use of act but added logging to show renders after end of test)

I then reloaded the same test until I got the act warning because isStale changes after end of test:

I'm not sure what the right thing to do is, perhaps the test harness ReactQueryConfigProvider should set staleTime to Infinity ? Perhaps @kentcdodds can chime in .

I'm also not sure if there should be two different configs for ReactQueryConfigProvider in the app context vs. the test context -- I would think it's better to use the same config for both.

[kentcdodds]

I've definitely observed this same behavior in https://github.com/kentcdodds/bookshelf. I haven't had time to debug, but it definitely is annoying.

@dustinsoftware's analysis is consistent with what I assumed is happening. And I agree with @ilovett that we should use the same config for tests and production.

With that in mind, I'm not 100% certain of the best way to approach this. It's really hard to reproduce in a debugging environment.

Right now, the manual cleanup at the end of the test is the best workaround I can think of. Really wish that the afterEach hook was called synchronously after each test 😐

[andrico1234]

@kentcdodds I've also come across this issue too. It gets tricky to debug exactly which test is causing the the act error to display in the console, especially when a test file has multiple.

I'm going to make a ticket for this in the RTL repo, but I think this highlights a potential need to improve the error message to display exactly which test is causing the issue.

I'm curious whether it's possible to log out the first argument of the it function

[kentcdodds]

Something interesting that seems to have worked for me is doing this:

render(<ComponentThatUsesReactQuery />)
await waitFor(() => {
  if (queryCache.isFetching) {
    throw new Error('The react-query queryCache is still fetching')
  }
})

If I stick that in my app's custom render function as default behavior then I don't have to worry about it anywhere (though it does force me to await all of my render calls, which is fine but a little annoying).

[kentcdodds]

The trick here is that it doesn't allow you to test the loading state, so for those you'll probably want to make your assertions between the regular render call and the waitFor call.

Still trying to figure out the best way to do this, but that's one idea.

[tannerlinsley]

Okay, with the latest release, can we confirm that as long as queryCache.clear is called after the test that all is well?

[gsong]

@tannerlinsley With the latest release, I can't reproduce the issue even without calling queryCache.clear.

So for me, the problem appears to be resolved.

[kentcdodds]

I'm definitely still experiencing this without calling queryCache.clear() (with the latest version). Still need to dig in a bit. Planning on doing that soon.

[kentcdodds]

Unfortunately it's intermittent. Sounds like a race condition. If I'm able to figure out exactly what the problem is then I'll raise this issue again.

[gsong]

@kentcdodds You're right, it is intermittent now that I've been using the new version more. It's definitely better than the previous version.

@tannerlinsley @kentcdodds If there's something you want me to try with either library, let me know and I'd be more than happy to help.

[kentcdodds]

I thought that this may be an issue with the afterEach not getting called immediately after the test (hence the race condition). However, that doesn't seem to be the case: https://repl.it/repls/YawningActiveCharacterset

let started = []

let testEnded = false
function start() {
  const interval = setInterval(() => {
    if (testEnded) {
      console.log('interval ran after test ended')
    }
  }, 0)
  started.push(interval)
}

afterEach(async () => {
  for (const interval of started) {
    clearInterval(interval)
  }
  testEnded = false
})

test('start 1', async () => {
  start()
  await new Promise(resolve => setTimeout(resolve, 10))
  testEnded = true
})

test('start 2', async () => {
  start()
  await new Promise(resolve => setTimeout(resolve, 10))
  testEnded = true
})

I assumed that I would see some logs, but in my testing, I do not.

This means something else fishy is going on. Still digging.

[kentcdodds]

Ah! I figured it out. The reason this is happening is because the cleanup process actually includes flushing existing effects/etc. which involves waiting for the next tick of the event loop (via setImmediate). So it's more like this:

let started = []

let testEnded = false
function start() {
  const interval = setInterval(() => {
    if (testEnded) {
      console.log('interval ran after test ended')
    }
  }, 0)
  started.push(interval)
}

afterEach(async () => {
  await new Promise(resolve => setImmediate(resolve))
  for (const interval of started) {
    clearInterval(interval)
  }
  testEnded = false
})

test('start 1', async () => {
  start()
  await new Promise(resolve => setTimeout(resolve, 10))
  testEnded = true
})

test('start 2', async () => {
  start()
  await new Promise(resolve => setTimeout(resolve, 10))
  testEnded = true
})

And that does result in getting the logs.

Looks like there was already a PR to React Testing Library to address this issue: testing-library/react-testing-library#632

I didn't really understand why it was necessary and ultimately @kamranayub created #332 to fix the underlying issue.

Come to think of it... It's been a while since I've seen act warnings so this might actually be fixed and I just confused myself (and used up a lot of time on this one 😅).

But, if anyone experiences act warnings from react-query, then the fix may be to merge testing-library/react-testing-library#632 afterall...

Leaving this as-is right now though.

[iamtekeste]

I am still facing this issue, can someone help and tell me what the fix is?

[tannerlinsley]

I'm working through this right now. The best solution to this I've found (but is not perfect) is to make sure you do queryCache.clear() after each test. Part of the problem is that when queries unmount, they dispatch a garbage collection action that attempts to update the query state and rerender the component (which is now unmounted), but that's what triggers the act warning I believe. I'm also working on shipping some updates in v2 (next branch) that will make this easier to manage.

[iamtekeste]

Thank you for the quick reply @tannerlinsley, just so I am clear does it mean I have to use a custom renderer to have access to queryCache.clear inside React Testing Library?

[iamtekeste]

For anyone else looking for how to actually do this, all you have to do is import queryCache and call the clear method on it. No need for custom renderer.

import { queryCache } from "react-query";`

afterEach(() => {
  queryCache.clear();
});

[kachkaev]

@iamtekeste your trick worked for us with react-query v1, but after upgrading to v2, we had to change it to:

import { queryCache } from "react-query";`

afterEach(() => {
  queryCache.clear({ notify: false });
});

Otherwise, each test was failing with a timeout:

Error: Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.

[iamtekeste]

@kachkaev thank you for sharing your solution, it will come in handy when I upgrade to v2.

[lgenzelis]

I'm facing an issue which I suppose is related to this. I wanted to leave a comment here to aid those googling for it (I've stumbled upon this thread by sheer luck).

My problem:

After my tests finish running, I get an annoying

A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --runInBand --detectOpenHandles to find leaks.

message from Jest. Moreover, in our CI/CD environment, the tests don't finish running until the timer for the cache garbage collection is executed (which delays our deploys by 5 min.. not terrible, but definitely annoying).

Solution

To avoid having to write queryCache.clear() for each of my test files, I added

afterAll(() => {
  require('react-query').queryCache.clear();
});

to my jest.setup.js files and voilà, problem gone.

Thanks a lot to all of you who suggested using queryCache.clear() after the tests. 😃 😄

[sbaechler]

I keep getting the warning unless I use both queryCache.clear and the waitFor hack from #432 (comment).

I wrote a small utility function that wraps a render function with both:

export function getRenderWithQueryCache<TRenderArgs, TResult = RenderResult>(renderFunction: (args?: TRenderArgs) => TResult): (args?: TRenderArgs) => Promise<TResult> {
    afterEach(() => {
        queryCache.clear({ notify: false });
    });

    return async function renderWithQueryCache(...args) {
        const renderResult: TResult = renderFunction(...args);
        await waitFor(() => !queryCache.isFetching);

        return renderResult;
    }
}

[kamranayub]

This is what we're doing currently. You may want to include queryCaches since there could be multiple caches available depending on how you're using react-query.

import { act, waitFor } from '@testing-library/react';
import { queryCaches } from 'react-query';

afterEach(async () => {

  act(() => {
    // Clear all query caches (including the default)
    queryCaches.forEach(cache => cache.clear({ notify: false }));
  });

  // Hack to wait for timers to avoid act warnings
  await waitFor(() => {});
});

We use the render function normally but the catch is you must use findBy* async methods to avoid the act warning immediately after rendering.

[liorfrenkel]

This did the trick for me when testing react hooks: (thanks @kentcdodds #432 (comment))

async function afterHack() {
  const { waitFor } = renderHook(() => null);
  await waitFor(() => {
    if (queryCache.isFetching) {
      throw new Error('The react-query queryCache is still fetching');
    }
  });
}

beforeEach(() => {
    queryCache.clear({ notify: false });
  });
  afterEach(() => {
    afterHack();
  });

[kopax]

Hi @liorfrenkel , we are experiencing tones of warning, I am looking at your trick and I can't tell what is renderHook import. Can you tell?

[liorfrenkel]

Hi @liorfrenkel , we are experiencing tones of warning, I am looking at your trick and I can't tell what is renderHook import. Can you tell?

@kopax

import { renderHook } from '@testing-library/react-hooks';

[kopax]

Thanks for sharing. I still experience a tons of act warning even with that fix, is this working for you in react-query@3.1.1? This is the test that break: https://gist.github.com/kopax/69ea35181d55593f8b3a51938e359ef4#file-forgottenpassword-test-tsx-L30

This is my logs:

  console.error
    Warning: An update to ForgottenPassword inside a test was not wrapped in act(...).
    
    When testing, code that causes React state updates should be wrapped into act(...):
    
    act(() => {
      /* fire events that update state */
    });
    /* assert on the output */
    
    This ensures that you're testing the behavior the user would see in the browser. Learn more at https://fb.me/react-wrap-tests-with-act
        in ForgottenPassword
        in QueryClientProvider

      at printWarning (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:120:30)
      at error (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:92:5)
      at warnIfNotCurrentlyActingUpdatesInDEV (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:13729:7)
      at dispatchAction (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6405:9)
      at node_modules/react-query/lib/react/useBaseQuery.js:48:9
      at node_modules/react-query/lib/core/queryObserver.js:436:13
      at notifyFn (node_modules/react-query/lib/core/notifyManager.js:13:3)

  console.error
    Warning: An update to ForgottenPassword inside a test was not wrapped in act(...).
    
    When testing, code that causes React state updates should be wrapped into act(...):
    
    act(() => {
      /* fire events that update state */
    });
    /* assert on the output */
    
    This ensures that you're testing the behavior the user would see in the browser. Learn more at https://fb.me/react-wrap-tests-with-act
        in ForgottenPassword
        in QueryClientProvider

      at printWarning (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:120:30)
      at error (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:92:5)
      at warnIfNotCurrentlyActingUpdatesInDEV (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:13729:7)
      at dispatchAction (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6405:9)
      at node_modules/react-query/lib/react/useBaseQuery.js:48:9
      at node_modules/react-query/lib/core/queryObserver.js:436:13
      at notifyFn (node_modules/react-query/lib/core/notifyManager.js:13:3)

We are experiencing tons of those error and we would gladly want to know how to remove or at least disable them, Can anyone tell a fix that work in the latest react-query version?

[liorfrenkel]

@kopax it seems that your test is finished with that line 30,
if this is the case, the test ends when you press on a button, and I assume the button click is hooked to some ajax request?

you will need to wait for the action that is hooked to the button click in your test and then expect something to happen

also, you do not test a hook but a component, the hack that you are using is for testing hooks, try this one for he normal render of testing library:

async function afterHack() {
  await waitFor(() => {
    if (queryCache.isFetching) {
      throw new Error('The react-query queryCache is still fetching');
    }
  });
}

[kopax]

Yes the action is react-query (all our act errors comes out from react-query usage), it exactly fails when the code execute refetch from useQuery.

We already added in setupTests.js this logic:https://github.com/pass-culture/pass-culture-app-native/blob/master/src/tests/setupTests.js#L18, this was added in last December. I believe we already clear our cache with this.

We use the isFetching props on the button that we press which perform the request:

<ButtonPrimary
  title={_(t`Valider`)}
  onPress={() => refetch()}
  disabled={shouldDisableValidateButton || isFetching}
/>

I thought I could wait until this switch to true or false but the props are dispatched differently in the final components and getByTestId did not help wait for that props change, do you see another way to wait for event considering the cache is already cleared?

@tannerlinsley as this is still a recurrent issue in projects that use react-query, I suggest this to be re-opened

[liorfrenkel]

@kopax I believe the problem is in the test and not in react-query, basically the act warning that you get is right, something is happening after the test is finished and there is no check for it in the test, check this out act warning

try doing something like this:

it.only('should redirect to ResetPasswordEmailSent when password reset request is successful', async () => {
    const { getByPlaceholderText, findByText } = renderPage()
    const emailInput = getByPlaceholderText('tonadresse@email.com')
    fireEvent.changeText(emailInput, 'john.doe@gmail.com')
    const validateEmailButton = await findByText('Valider')
    fireEvent.press(validateEmailButton) // <==== line that break

  // pseudo code from here, we need to test the component state which is changing after the async call
   expect(the button).toBeDisabled()

  // and then a check that the button is not disabled after the query had finished
  await waitFor(() => expect(the button).not.toBeDisabled());
  })

[kopax]

Thanks for trying to help, I already tried to get the button state change and couldn't find a method to test the passed props (such as disabled) on the button. I also need to state that our button component will finally use the props disabled to configure internally and will not render a disabled state in the dom which cause me issue to identify the state change with the testing tool I known. Do you have another clue how I can test this component and fix this test ?

If you clone the code base and run yarn test, you'll see that we have tons of error like this we can't solve at the moment and they were all included by the usage of react-query in our components.

https://github.com/pass-culture/pass-culture-app-native/blob/PC-6434-2/src/features/auth/forgottenPassword/ForgottenPassword.tsx

[gpbaculio]

@kopax i was able to do a useMutation hook test like this, @liorfrenkel suggestion worked for me

it('mutation', async done => {
    await waitFor(async () => {
      const nockScope = nock(api.defaults.baseURL as string)
        .post(
          `/api/order/${
            store.getState().dashboard.orderModal?.orderId
          }/complete`,
        )
        .reply(200, {
          ...store.getState().dashboard.orderModal,
          orderStatus: 1,
        } as OrderType);

      const completeOrderBtn = screen.getByTestId(
        `completeOrderBtn/${store.getState().dashboard.orderModal?.orderId}`,
      );
      fireEvent.press(completeOrderBtn);
      const completeLoader = screen.getByTestId('complete-loader');
      expect(completeLoader).toBeTruthy();
      await waitForElementToBeRemoved(() =>
        screen.getByTestId('complete-loader'),
      );
      nockScope.done();
      done();
    });
  });