Flaky tests: Asynchronous queries can return unmounted elements

Edit: Skip comments about debugging and jump to the root cause.

If your issue is regarding one of the query APIs (getByText,
getByLabelText, etc), then please file it on the dom-testing-library
repository instead. If you file it here it will be closed. Thanks :)

This is related to asynchronous queries (findBy*) but the issue is not on dom-testing-library. See testing-library/dom-testing-library#876.

@testing-library/react version: "^11.1.0"
Testing Framework and version:
- fresh project generated with CRA 4.0.1
- jest@26.6.0
DOM Environment:
- jest-environment-jsdom@26.6.2
- jsdom@16.4.0

Relevant code or config:

Also available in codesandbox below.

This issue comes up randomly so below we are generating 200 tests:

import React, { useEffect, useState } from "react";
import { render, waitForElementToBeRemoved, screen } from "@testing-library/react";

function Component() {
    const [visible, setVisible] = useState(false);

    useEffect(() => {
        const timer1 = setTimeout(() => setVisible(true), 10);
        const timer2 = setTimeout(() => setVisible(false), 15);

        return () => {
            clearTimeout(timer1);
            clearTimeout(timer2);
        };
    }, []);

    return (
        <div>
            {visible && <span id="test-id">Content</span>}
        </div>
    );
}

for(const i of Array(200).fill(null).map((_, i) => i)) {
    test(`unstable test ${i}`, async () => {
        render(<Component />);

        const content = await screen.findByText("Content");
        await waitForElementToBeRemoved(content);
    });
}

Above is only a minimal repro. My real use case includes querying "Loading" text which is visible while MSW is handling the request. It does not use setTimeout at all. Something like:

// Click submit button
userEvent.click(screen.getByRole("button", { name: "Submit" }));

// Loader should appear
const loader = await screen.findByText("Loading");

// Loader should disappear once request resolves
await waitForElementToBeRemoved(loader);

What you did:

Queried element with asynchronous query findBy*. I would expect element returned to be present in the DOM. If it wasn't the query should throw error.
Passed the queried element to waitForElementToBeRemoved.

What happened:

Element returned from findByText was not in DOM when waitForElementToBeRemoved started handling it.

 FAIL  src/App.test.js
  × unstable test (66 ms)

  ● unstable test

    The element(s) given to waitForElementToBeRemoved are already removed. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal.

      33 |     render(<Component />);
      34 |     const content = await screen.findByText("Content");
    > 35 |     await waitForElementToBeRemoved(content);
         |           ^
      36 | });
      37 |
      38 |

      at initialCheck (node_modules/@testing-library/dom/dist/wait-for-element-to-be-removed.js:16:11)      
      at waitForElementToBeRemoved (node_modules/@testing-library/dom/dist/wait-for-element-to-be-removed.js:39:3)
      at Object.<anonymous> (src/App.test.js:35:11)

Most of the time these issues come up really randomly. We might have successful builds for months on CI and then this error occurs. Triggering new build usually fixes this. 😄

Reproduction:

The codesandbox is still showing incorrect error message Cannot read property 'parentElement' of null but this was fixed in testing-library/dom-testing-library#871.

Problem description:

Asynchronous queries can return elements which have unmounted the DOM.

I was hoping the initialCheck done by waitForElementToBeRemoved would have been the cause but it looks like the parentElement is not even present before element is given to it.

const content = await screen.findByText("Content");
console.log(content.outerHTML); // <span id="test-id">Content</span>
console.log(content.parentElement); // null
await waitForElementToBeRemoved(content); // The element(s) given to waitForElementToBeRemoved are already removed.

By setting the isAsyncActSupported to false from dist/act-compat.js this issue disappears. However errors fill stdout but tests are passing. I have no idea what I'm doing here but this works 😃 .

react-testing-library/src/act-compat.js

Line 20 in deafd51

let isAsyncActSupported = null

I think act-compact.js is causing asynchronous queries to run additional cycles/ticks/micro-queues instead of returning the element instantly. This causes setTimeouts and/or Promises to run before the element is returned. In practice the queried element can unmount before asynchronous query is resolved.

Suggested solution:

I'm not familiar with the act wrapper of RTL but I think the fix should be applied there.

There is a workaround for this: Don't use waitForElementToBeRemoved.

const content = await screen.findByText("Content");
await waitFor(() => expect(content).not.toBeInTheDocument());

I hope I'm getting this correctly but IMO the problem here is these two lines:

// Loader should appear
const loader = await screen.findByText("Loading");

// Loader should disappear once request resolves
await waitForElementToBeRemoved(loader);

waitForElementToBeRemoved uses waitFor which calls the callback function it accepts every tick in order to see if the return value of your callback is present.
IMO what happens here is that it only reads that element once (when you initiate your variable) and not at the next ticks.
Did you try something like this:

// Loader should appear
const loader = await screen.findByText("Loading");

// Loader should disappear once request resolves
await waitForElementToBeRemoved(() => screen.getByText("Loading")});

I do recall something about passing an element instead of a callback to waitForElementToBeRemoved though I can't find the related issue.

This is not really an issue with waitForElementToBeRemoved since element is already unmounted when returned from the query:

const content = await screen.findByText("Content");
console.log(content.outerHTML); // <span id="test-id">Content</span>
console.log(content.parentElement); // null <-- Unmounted from DOM

Test with the suggested approach:

  ● unstable test 87

    TestingLibraryElementError: Unable to find an element with the text: Content. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

    <body>
      <div>
        <div />
      </div>
    </body>

      31 | 
      32 |         await screen.findByText("Content");
    > 33 |         await waitForElementToBeRemoved(() => screen.getByText("Content"));
         |                                                      ^
      34 |     });
      35 | }
      36 | 

      at Object.getElementError (node_modules/@testing-library/dom/dist/config.js:37:19)
      at node_modules/@testing-library/dom/dist/query-helpers.js:90:38

This can also be reproduced without React. Here's example of DTL. Just change the import of screen. This really points out the problem is with react-testing-librarys way of handling the async queries.

https://codesandbox.io/s/dom-testing-libraryissues876-2-fe3fi?file=/src/Dom.test.js:100-308

import { screen } from "@testing-library/dom";
...
Tests:       200 passed, 200 total

import { screen } from "@testing-library/react";
... 
Tests:       59 failed, 141 passed, 200 total

If waitForElementToBeRemoved is confusing the test setup here too much it can be replaced with following:

const content = await screen.findByText("Content");
expect(content).toBeInTheDocument();

  ● unstable test 199

    expect(element).toBeInTheDocument()

    element could not be found in the document

      29 | 
      30 |         const content = await screen.findByText("Content");
    > 31 |         expect(content).toBeInTheDocument();
         |                         ^
      32 |     });
      33 | }
      34 | 

      at Object.<anonymous> (src/React.test.js:31:25)

Test Suites: 1 failed, 1 total
Tests:       148 failed, 52 passed, 200 total

Does this still happen if you make the timeout longer than the React batch time (16ms)?

With this setup:

const TIMEOUT = 5; // Adjusting this doesnt matter
const DIFF = 5;
...
useEffect(() => {
    const timer1 = setTimeout(() => setVisible(true), TIMEOUT);
    const timer2 = setTimeout(() => setVisible(false), TIMEOUT + DIFF);

const TIMEOUT = 5;
const DIFF = 1; // Tests: 316 failed, 684 passed, 1000 total
const DIFF = 2; // Tests: 9 failed, 991 passed, 1000 total
const DIFF = 3; // Tests: 1 failed, 999 passed, 1000 total
const DIFF = 4; // Tests: 1 failed, 999 passed, 1000 total
const DIFF = 5; // Tests: 1 failed, 999 passed, 1000 total
const DIFF = 6; // Tests: 1 failed, 999 passed, 1000 total
const DIFF = 7; // Tests: 1000 passed, 1000 total

So async queries are unstable if content updates are done fast enough. As said earlier my real use case utilizes MSW behind the queries. If I'm looking at this correctly it looks like MSW is using 5ms delay by default. https://github.com/mswjs/msw/blob/9b667c74f635735e1b941baf61db9f1b77c5a5e3/src/context/delay.ts#L7-L12

But this is just my use case. I think RTL should work with any delay values.

I had some time to debug this further. Looks like this is caused by nested awaits in asyncWrapper.

react-testing-library/src/pure.js

Lines 12 to 18 in a241cb8

    
           asyncWrapper: async cb => { 
        
             let result 
        
             await asyncAct(async () => { 
        
               result = await cb() 
        
             }) 
        
             return result 
        
           },

More debugging info, narrows the issue to asyncWrapper

Replace asyncAct with react-dom/testing-utils act just to narrow act-compact out.

diff --git a/src/pure.js b/src/pure.js
index 75098f7..c02003f 100644
--- a/src/pure.js
+++ b/src/pure.js
@@ -5,15 +5,19 @@ import {
   prettyDOM,
   configure as configureDTL,
 } from '@testing-library/dom'
-import act, {asyncAct} from './act-compat'
+import act from './act-compat'
 import {fireEvent} from './fire-event'
+import * as testUtils from 'react-dom/test-utils'
 
 configureDTL({
   asyncWrapper: async cb => {
     let result
-    await asyncAct(async () => {
+    await testUtils.act(async () => {
       result = await cb()
+      console.log('callback resolved', prettyDOM());
     })
+
+    console.log('asyncWrapper done', prettyDOM());
     return result
   },
   eventWrapper: cb => {

Element is present in DOM in the inner async method.
Element is already unmounted once act has resolved.

 FAIL  src/__tests__/unstable.test.js
  ✕ unstable test 1 (66 ms)

  ● unstable test 1

    expect(element).toBeInTheDocument()

    element could not be found in the document

      32 |     const content = await screen.findByText('Content')
      33 |     console.log('Finish', window.currentTest, prettyDOM())
    > 34 |     expect(content).toBeInTheDocument()
         |                     ^
      35 |   })
      36 | }
      37 |

      at Object.<anonymous> (src/__tests__/unstable.test.js:34:21)

  console.log
    callback resolved <body>
      <div>
        <div>
          <span>
            Content
          </span>
        </div>
      </div>
    </body>

      at src/pure.js:19:13

  console.log
    asyncWrapper done <body>
      <div>
        <div />
      </div>
    </body>

      at Object.toBeInTheDocument [as asyncWrapper] (src/pure.js:11:14)

  console.log
    Finish unstable test 1 <body>
      <div>
        <div />
      </div>
    </body>

      at Object.<anonymous> (src/__tests__/unstable.test.js:23:3)

A quick fix is to resolve the outer scope immediately when callback resolves:

diff --git a/src/pure.js b/src/pure.js
index 75098f7..61110d7 100644
--- a/src/pure.js
+++ b/src/pure.js
@@ -9,12 +9,13 @@ import act, {asyncAct} from './act-compat'
 import {fireEvent} from './fire-event'
 
 configureDTL({
-  asyncWrapper: async cb => {
-    let result
-    await asyncAct(async () => {
-      result = await cb()
+  asyncWrapper: cb => {
+    return new Promise(resolve => {
+      asyncAct(async () => {
+        const result = await cb()
+        resolve(result)
+      })
     })
-    return result
   },
   eventWrapper: cb => {
     let result

Now I can run 2000 tests with 1ms timeout difference and they all pass. There is however a new error thrown by act: Warning: You called act(async () => ...) without await. This could lead to unexpected testing behaviour, interleaving multiple act calls and mixing their scopes. You should - await act(async () => ...);.

Clearly this fix isn't perfect. I'll have to study the react-dom/test-utils next.

setup for testing-library/react-testing-library#865

React's act will flush micro tasks once given callback resolves. In practice this means that once callback created by findBy queries resolves the state of DOM may change before act resolves. There is no way to resolve act immeadiately when element queried by findBy appears. Asynchronous findBy queries don't provide stable state of DOM. It simply won't work as intended.

Repro setup in react repository with minimal waitFor:

git clone git@github.com:AriPerkkio/react.git
cd react
yarn # Install deps
yarn test packages/react-dom/src/__tests__/AsyncAct-test.js  --watchAll

I see two possible solutions for this issue:

1. Skip `flushWorkAndMicroTasks` of `act` calls

As done in this POC commit an option for bailing out of act is required. Resolve the act immediately as queried element is found. With this setup the tests pass even if element disappears 1ms after its mount.

2. Document flakiness of `findBy` queries in testing library docs

Documentation should clearly indicate that asynchronous queries may resolve in different state of DOM. Elements returned from these queries may already have unmounted. Even if your tests work today they may randomly fail in future.

Trying to recap:

The way await findBy... is supposed to work is 1) wait for effects to clear so that DOM is stable in terms of "known pending work" 2) query. In theory if the reason for a change is also wrapped in act() (e.g. a fireEvent call), 1) should generally be true. It sounds like a problem comes from when the effect is triggered without an initial act(), e.g. through a timer in the app code. In this case, the act() isn't wrapping an action, it is wrapping the query itself, which was triggered due to a test retry timer or MutationObserver event - and effects that happened in that sequence are flushed after the result.

And... I've seen this one in the wild now.

Basically this:

expect(await screen.findByText('Loading...')).toBeInTheDocument();

The expectation throws an error but not the findBy.
Since it's a transient state, removing the assertion can unblock the test, but still.

Faced same kind of issue:
happens mostly on CI during high CPU load ( in general suite takes 2sec to complete, with CPU load it might be 10x longer and even more ). Tried to reproduce it at laptop with CPU throttling ( let's zip smth ;) ) and randomly hit it.
Firstly I thought that it happened because of MSV used, but issues came in not chained with requests places.

So it is happening like this:

If you have a flag with value false (only when initiated with false) let us say it is loading flag, the async call will change the flag to true then false. I think waitForElementToBeRemoved api has no way to detect that there is async call is going on in the code so it just terminate the test by element not found already in dom or timeout.

In my case, async call is axios and i just mocked it using msw library. In the test, i added a simple line:

await new Promise((resolve) => setTimeout(resolve, 1000)); // 1000 is the time that i know my `msw` should be finished.

At that point of delay, I'm sure that async call is finished and component is stable with no state changes anymore.

That solved my issue so far, but I lost the check for the loading indicator because I don't know how to predect the loading element.

As a question, Is there aways to detect that react component has async call is going on and? Maybe a wrapper? Does <Suspense> works here?

@vadeneev

@vadeneev and @josh-biddick, it sounds like you have run exactly into this same issue. I'm not sure about @AhmedBHameed's case.

I've found one tricky way to identify such unstable tests in local environment. By running multiple npm run test scripts parallel, the CPU load is high enough to make these bugs appear. This way we've been able to fix these issues in local before pushing code to CI.

CI=true npm run test & \
CI=true npm run test & \
CI=true npm run test & \
CI=true npm run test & \
CI=true npm run test & \
wait

I wonder if this will be fixed by #937 in future since it's removing the asyncWrapper which was identified to be one of the root cause here.

@AriPerkkio

Hello @AriPerkkio , thanks for this post, I thought I was going nuts. I've followed your advice on the parallel local tests to see which ones were failing, but how did you actually solved them? I can't seem to find a way to make them run correctly every single time, some of them keep failing

I am using the same setup as you, with MSW for mocking the API. When I try to test a failing API call (e.g. 400), for some reason with the parallel tests the expectations keep on failing. As an example:

it("shows error message if API returns an error", async () => {
    server.use(
      rest.get(
        `http://localhost/balances/`,
        (req, res, ctx) => {
          return res(
            ctx.status(500),
          );
        }
      )
    );

    render(<HotWalletMutationsPanel  />);

    await waitFor(() => {
      expect(
        screen.getByText("Cannot retrieve hotwallet mutations")
      ).toBeInTheDocument();
    });
  });

@alessandrocapra

@alessandrocapra without seeing the whole test case + component, or a minimal repro of your specific problem it is difficult to give any accurate instructions. Based on your code snippet I'm not 100% sure this issue is its root cause.

But in general: If you query elements which can change their state/attributes after some time has passed, do not make any assumptions of their state. For example, if a "Loading" text is visible only when fetch request is active, do not assume the queried element is in the DOM after findBy* query returned it.

You can see this in practice at https://codesandbox.io/s/dom-testing-libraryissues876-2-fe3fi?file=/src/React.test.js.
With 1000 tests I'm seeing following results with average laptop: Test Summary | 483 failed | 517 passed | 1000 total. About 48% of the time findByText returned an element without .parentElement.

render(<Component />);

const content = await screen.findByText("Content");
await waitForElementToBeRemoved(content);
//    ^^ The element(s) given to waitForElementToBeRemoved are already removed. 
//       waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal.

By changing the waitForElementToBeRemoved I always get Test Summary | 1000 passed | 1000 total.

render(<Component />);

const content = await screen.findByText("Content");
- await waitForElementToBeRemoved(content);
+ await waitFor(() => {
+     expect(content).not.toBeInTheDocument(); // Does not care if findByText returns element without parentElement (= unmounted DOM node)
+ });

I've followed your advice on the parallel local tests to see which ones were failing, but how did you actually solved them?

With this parallel running I've found these flaky tests from many projects. We were simply making assertions on elements returned by asynchronous queries. By refactoring the test case a bit like shown above we were able to fix these issues. In all cases in the end we were able to run projects containing hundreds of tests parallel with 100% success.

Is this ever going to be fixed? We just ran into the same issue.

encounter such flaky issues with the following dependencies:
"@testing-library/jest-dom": "5.17.0",
"@testing-library/react": "12.1.5",
"@testing-library/react-hooks": "8.0.1",

issue is not easily reproducible on the local machine but prone to fail on CI/CD.

since most of us run jest on CI/CD with the --silent option.

I then reproduce it locally much more frequently with the --silent option (seems the output matters? 🤔️).

the error message is "element could not be found in the document" which means the element is indeed found/exist but not located in the current dom tree when expecting it to be in the document.

In my case I don't have to use findBy* actually, after replace findBy with getBy my issues seems fixed.

Still encountering this issue in CI/CD

Navigation Menu

Repository navigation

Flaky tests: Asynchronous queries can return unmounted elements #865

Relevant code or config:

What you did:

What happened:

Reproduction:

Problem description:

Suggested solution:

1. Skip `flushWorkAndMicroTasks` of `act` calls

2. Document flakiness of `findBy` queries in testing library docs

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	asyncWrapper: async cb => {
	let result
	await asyncAct(async () => {
	result = await cb()
	})
	return result
	},

Flaky tests: Asynchronous queries can return unmounted elements #865

Description

Relevant code or config:

What you did:

What happened:

Reproduction:

Problem description:

Suggested solution:

Activity

MatanBobi commented on Jan 18, 2021

AriPerkkio commented on Jan 18, 2021

AriPerkkio commented on Jan 22, 2021

alexkrolick commented on Jan 22, 2021

AriPerkkio commented on Jan 22, 2021

AriPerkkio commented on Feb 21, 2021

AriPerkkio commented on Mar 7, 2021

1. Skip flushWorkAndMicroTasks of act calls

2. Document flakiness of findBy queries in testing library docs

alexkrolick commented on Mar 9, 2021

alexkrolick commented on Mar 18, 2021

vadeneev commented on May 20, 2021

AhmedBHameed commented on Jun 11, 2021

AriPerkkio commented on Jul 1, 2021

alessandrocapra commented on Dec 21, 2021

AriPerkkio commented on Dec 21, 2021

denniseffing commented on May 26, 2023

Anthony-YY commented on Jan 23, 2024

sidrak19 commented on Jan 28, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

1. Skip `flushWorkAndMicroTasks` of `act` calls

2. Document flakiness of `findBy` queries in testing library docs