Testing-library: avoid these mistakes in async tests

Testing is a crucial part of any large application development. The more code you write, the more tests you want to add to make sure all the parts still work together as expected. Here in Revolut, a lot of things happen behind our mobile super-app. We have a lot of backoffice apps with complex logic, and need to be sure nothing is broken when new features are added.

Sometimes, tests start to unexpectedly fail even if no changes were made to the business logic. It may happen after e.g. you updated some underlying library, made changes to the network layer, etc. Good and stable tests should still reliably assert component output against the given input, no matter what happens at the lower levels. Another even worse case is when tests still pass even when the component logic got broken.

In this article, I would like to show a few common mistakes that could lead to such issues, how to fix these, and how to make your tests stable and predictable. Initially, I picked this topic for our internal Revolut knowledge share session, but I feel like it could be helpful for a broader audience.

These and a few more examples could be found in this repository.

Table of Contents

1. Await with sync methods
2. Async methods without await
3. Side-effects inside waitFor

Await with sync methods

Simple asynchronous request

This is the most common mistake I'm running into while refactoring code. Let's say, you have a simple component that fetches and shows user info. For the sake of simplicity, our API will only capitalize the given user id and return it as a user name. I'm also using react-query-alike hooks, but not the library itself, to make things more transparent:

const getUser = async (id: string): Promise<string> =>
  id[0].toUpperCase().concat(id.slice(1))

const useUserQuery = (id: string | null) => {
  const [data, setData] = useState<string | null>(null)
  useEffect(() => {
    if (!id) {
      setData(null)
      return
    }
    getUser(id).then(setData)
  }, [id])
  return data
}

const UserView = ({ id }: { id: string | null }) => {
  const data = useUserQuery(id)
  if (data === null) return <div>Loading...</div>
  return <>{data}</>
}

We want to write a test for it, so we are rendering our component with React Testing Library (RTL for short) and asserting that an expected string is visible to our user:

it('should render user info', async () => {
  await render(<UserView id="bob" />)
  expect(screen.getByText('Bob')).not.toBeNull()
})

So far, this test works perfectly well.

Complex asyncronous request

Later, a new requirement comes in to display not only a user but also their partner name. Easy-peasy! Let's just change our fetch function a little bit, and then update an assertion.
In getUser, we will now wait for two consecutive requests and only then return the aggregated data:

const getUser = async (id: string): Promise<string> => {
  const user = await getUser(id)
  const partner = await (user[0] === 'A'
    ? getUser('charlie')
    : getUser('daisy'))
  return `${user} and ${partner}`
}

And let's update our test as well:

it('should render user info', async () => {
  await render(<UserView id="bob" />)
  expect(screen.getByText('Alice and Charlie')).not.toBeNull()
})

Our changes made perfect sense, but suddenly our test will start to fail with "Unable to find an element with the text: Alice and Charlie". Oh-oh! But we didn't change any representation logic, and even the query hook is the same. Also, RTL output shows "Loading..." text in our DOM, though it looks like we are awaiting for render to complete in the very first line of our test.

Explanation

Alright, let's find out what's going on here. render is a synchronous function, but await is designed to work with asynchronous ones. What's going on when render is awaited? Well, MDN is very clear about it:

If the value of the expression following the await operator is not a Promise, it's converted to a resolved Promise.

In our test, when we are calling render with await, JavaScript implicitly wraps the result into a promise and waits for it to be settled. Meanwhile, we already have another pending promise scheduled in the fetch function. By the time implicit awaited promise is resolved, our fetch is resolved as well, as it was scheduled earlier. So we have the correct output on the screen.

But after the latest changes, our fetch function waits for the two consecutive promises, thus data is not fully ready after implicit render promise is resolved. In fact, even in the first green test, react warned us about something going wrong with an "act warning", because actual update after fetch promise was resolved happened outside of RTL's act wrappers:

Warning: An update to UserAndPartnerView 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://reactjs.org/link/wrap-tests-with-act
    at UserAndPartnerView (./common-testing-library-mistakes/src/a-await-sync-methods/UserAndPartnerView.tsx:3:38)

Now, that we know what exactly caused the error, let's update our test. RTL provides a set of methods that return promises and are resolved when an element is found. This is the perfect case to use one of these:

it('should render user info', async () => {
  render(<UserView id="bob" />)
  expect(await screen.findByText('Alice and Charlie')).not.toBeNull()
})

Now, we don't care how many requests happen while the component is being rendered. findByText will wait for the given text to appear in the DOM.

Conclusion

You should never await for syncronous functions, and render in particular. Use the proper asyncronous utils instead:

expect(await screen.findByText('some text')).not.toBe(null)
// or
await waitFor(() => {
  expect(screen.getByText('some text')).not.toBe(null)
})

Async methods without await

Positive case

Let's face the truth: JavaScript gives us hundreds of ways to shoot in a leg. And while async/await syntax is very convenient, it is very easy to write a call that returns a promise without an await in front of it.

Let's see how this could cause issues in our tests. I will be writing a test for the same UserView component we created in a previous example:

it('should render user info', async () => {
  render(<UserView id="alice" />)
  waitFor(() => {
    expect(screen.getByText('Alice')).not.toBeNull()
  })
})

This test passes, and everything looks good. Indeed, for a user with an id "alice", our request should return the name "Alice".

False-positive case

Now, let's see if our test fails when we pass the incorrect id

it('should render user info', async () => {
  render(<UserView id="bob" />)
  waitFor(() => {
    expect(screen.getByText('Alice')).not.toBeNull()
  })
})

Oops, it's still passing. But "bob"'s name should be Bob, not Alice.

Explanation

The reason is the missing await before asyncronous waitFor call. Asyncronous method call will always return a promise, which will not be awaited on its own. Jest simply calls this line and finishes the test. No assertions fail, so the test is green. But if we add await in front of waitFor, the test will fail as expected:

it('should render user info', async () => {
  render(<UserView id="bob" />)
  await waitFor(() => {
    expect(screen.getByText('Alice')).not.toBeNull()
  })
})

Unable to find an element with the text: Alice.

Ignored nodes: comments, <script />, <style />
<body>
  <div>
    Bob
  </div>
</body>

Conclusion

Never forget to await for async functions or return promises from the test (jest will wait for this promise to be resolved in this case). Otherwise, you may end up running tests that always pass.

Side-effects inside waitFor

How waitFor works

First of all, let's recall what is waitFor. It's an async RTL utility that accepts a callback and returns a promise. This promise is resolved as soon as the callback doesn't throw, or is rejected in a given timeout (one second by default). waitFor will call the callback a few times, either on DOM changes or simply with an interval.

Now, keeping all that in mind, let's see how side-effects inside waitFor could lead to unexpected test behavior.

Green test

Here, we have a component that renders a list of user transactions. Each list entry could be clicked to reveal more details.

const TransactionDetails = ({
  description,
  merchant,
}: {
  description?: string | null
  merchant?: string | null
}) => (
  <ul>
    {description && <li>Description: {description}</li>}
    {merchant && <li>Merchant: {merchant}</li>}
  </ul>
)

const Transactions = () => {
  const [selectedTransactionId, setSelectedTransactionId] = useState<
    string | null
  >(null)

  const transactions = useTransactionsQuery()
  if (transactions === null) return <div>Loading...</div>

  return (
    <ul>
      {transactions.map(tx => (
        <li
          key={tx.id}
          onClick={() =>
            setSelectedTransactionId(
              selectedTransactionId === tx.id ? null : tx.id,
            )
          }
        >
          <div>Id: {tx.id}</div>
          {selectedTransactionId === tx.id && (
            <TransactionDetails description={tx.description} />
          )}
        </li>
      ))}
    </ul>
  )
}

And the test to cover this logic:

it('should render transaction details', async () => {
  render(<Transactions />)

  await waitFor(() => {
    fireEvent.click(screen.getByText('Id: one'))
    expect(screen.getByText('Description: Coffee')).not.toBeNull()
  })
})

As the transactions list appears only after the request is done, we can't simply call screen.getByText('Id: one') because it will throw due to missing "Id: one" text. To avoid it, we put all the code inside waitFor which will retry on error. So we are waiting for the list entry to appear, clicking on it and asserting that description appears.

Hanging test

Now, let's add a bit more logic and fetch the transaction details as soon as it is clicked. Again, as in the very first example, we should not significantly change the test as the component basically stays the same. So we only want to add another assertion to make sure that the details were indeed fetched.

We will slightly change the component to fetch more data when one of the transactions is selected, and to pass fetched merchant name inside TransactionDetails. When nothing is selected, useTransactionDetailsQuery returns null, and the request is only triggered when an id is passed.

const TransactionsWithDetails = () => {
  // ...

  const transactions = useTransactionsQuery()
  const details = useTransactionDetailsQuery(selectedTransactionId)

  // ...
          <div>Id: {tx.id}</div>
          {selectedTransactionId === tx.id && (
            <TransactionDetails
              description={tx.description}
              merchant={details?.merchant}
            />
          )}
  // ...
}

First, the user sees the list of transactions. Then, as soon as one is clicked, details are fetched and shown.

As was mentioned earlier, in our test we will only add another assertion to check that merchant name from the details is rendered:

it('should render transaction details', async () => {
  render(<TransactionsWithDetails />)

  await waitFor(() => {
    fireEvent.click(screen.getByText('Id: one'))
    expect(screen.getByText('Description: Coffee')).not.toBeNull()
    expect(screen.getByText('Merchant: Mega Mall one')).not.toBeNull()
  })
})

When we run our updated test, we could notice that the test runner hangs. And while it's relatively easy to find the problem when we deal with a single test, it's a pain to find such a broken one in another few hundred.

Explanation

Let's figure out what is happenning here. waitFor is triggered multiple times because at least one of the assertions fails. Let's go through the sequence of calls, where each list entry represents the next waitFor call:

1. fireEvent.click(screen.getByText('Id: one')) fails as transactions list is not yet fetched, and "Id: one" text is not at the screen.
2. "Id: one" is present and clicked, but now expect(screen.getByText('Merchant: Mega Mall one')).not.toBeNull() fails as details are not yet fetched.
3. The above successful fireEvent.click triggered a DOM mutation, so waitFor executes the callback once again. fireEvent.click is triggered once again, closing the transaction description, and expect(screen.getByText('Description: Coffee')).not.toBeNull() fails.

As at the third call fireEvent.click caused another DOM mutation, we stuck in 2-3 loop. Transaction details are being opened and closed over and over again with no chance for the details request to complete and to render all the needed info.

The fix for the issue is very straightforward: we simply need to move our side-effect (fireEvent.click) out of waitFor.

it('should render transaction details', async () => {
  render(<TransactionsWithDetails />)

  const transaction = await screen.findByText('Id: one'))
  fireEvent.click(transaction)

  await waitFor(() => {
    expect(screen.getByText('Description: Coffee')).not.toBeNull()
    expect(screen.getByText('Merchant: Mega Mall one')).not.toBeNull()
  })
})

Conclusion

As waitFor is non-deterministic and you cannot say for sure how many times it will be called, you should never run side-effects inside it. Instead, wait for certain elements to appear on the screen, and trigger side-effects synchronously.

How to avoid these issues

The simplest way to stop making these mistakes is to add eslint-plugin-testing-library to your eslint.

• testing-library/await-async-utils makes sure you are awaiting for async methods like waitFor and waitForElementToBeRemoved
• testing-library/await-async-query protects you against missing awaits with asyncronous findBy... and findAllBy...
• testing-library/no-wait-for-side-effects doesn't allow you to write side-effects inside waitFor

The only thing it doesn't catch is await render, but works perfectly well for everything else.

Summary

Debugging asynchronous tests could be pretty difficult, but you could simply make your tests more failure-proof avoiding the mistakes I described above.

Unfortunately, most of the "common mistakes" articles only highlight bad practices, without providing a detailed explanation. I hope I closed this gap, and my post gave you enough details on why the above mistakes should be avoided.

And make sure you didn't miss rather old but still relevant Kent C. Dodds' Common mistakes with React Testing Library where more issues are described.

Comments

[Mario Beltrán]

eslint-plugin-testing-library creator here, great post!

The rule to prevent awaited renders is really interesting, so I've created myself an issue in the plugin to implement a rule for it: github.com/testing-library/eslint-...

Thanks for sharing all these detailed explanations!

[Aleksei Tsikov]

Thank you for the awesome linter plugin 😍

[Mohammad Jawad (Kasir) Barati]

QUESTION, Do you think it would save me a few dollars if I solidify a test like this:

it("should save me money", () => {
  // in the service layer we have a method named "get". And inside that method I call findOne method. Here I just mocked the returned value of that method. It will return a promise which will resolve and return null
  mockedRepository.findOne.resolve(null);

  const result = service.get("id");

  expect(result).resolves.toBeNull()
})

As you can see I prevent using async await syntax and now I was wondering if this will help my test to be executed faster and as a result my CI/CD piplines. Based on my limited knowledge when we use async await it wraps our code with another Promise when it is compiled to es5 or IDK whatever.

Please kindly correct me if I am wrong ❤️