Common Mistakes With RTL

Not using Testing Library ESLint plugins

Importance: medium

If you'd like to avoid several of these common mistakes, then the official ESLint plugins could help out a lot:

• eslint-plugin-testing-library

• eslint-plugin-jest-dom

Note: If you are using create-react-app, eslint-plugin-testing-library is already included as a dependency.

Advice: Install and use the ESLint plugin for Testing Library.

Using wrapper as the variable name for the return value from render

Importance: low

// ❌
const wrapper = render(<Example prop="1" />)
wrapper.rerender(<Example prop="2" />)

// ✅
const {rerender} = render(<Example prop="1" />)
rerender(<Example prop="2" />)

The name wrapper is old cruft from enzyme and we don't need that here. The return value from render is not "wrapping" anything. It's simply a collection of utilities that (thanks to the next thing) you should actually not often need anyway.

Advice: destructure what you need from render or call it view.

Using cleanup

Importance: medium

// ❌
import {render, screen, cleanup} from '@testing-library/react'

afterEach(cleanup)

// ✅
import {render, screen} from '@testing-library/react'

For a long time now cleanup happens automatically (supported for most major testing frameworks) and you no longer need to worry about it. Learn more.

Advice: don't use cleanup

Not using screen

Importance: medium

// ❌
const {getByRole} = render(<Example />)
const errorMessageNode = getByRole('alert')

// ✅
render(<Example />)
const errorMessageNode = screen.getByRole('alert')

screen was added in DOM Testing Library v6.11.0 (which means you should have access to it in @testing-library/react@>=9). It comes from the same import statement you get render from:

import {render, screen} from '@testing-library/react'

The benefit of using screen is you no longer need to keep the render call destructure up-to-date as you add/remove the queries you need. You only need to type screen. and let your editor's magic autocomplete take care of the rest.

The only exception to this is if you're setting the container or baseElement which you probably should avoid doing (I honestly can't think of a legitimate use case for those options anymore and they only exist for historical reasons at this point).

You can also call screen.debug instead of debug

Advice: use screen for querying and debugging.

Using the wrong assertion

Importance: high

const button = screen.getByRole('button', {name: /disabled button/i})

// ❌
expect(button.disabled).toBe(true)
// error message:
//  expect(received).toBe(expected) // Object.is equality
//
//  Expected: true
//  Received: false

// ✅
expect(button).toBeDisabled()
// error message:
//   Received element is not disabled:
//     <button />

That toBeDisabled assertion comes from jest-dom. It's strongly recommended to use jest-dom because the error messages you get with it are much better.

Advice: install and use @testing-library/jest-dom**

Wrapping things in act unnecessarily

Importance: medium

// ❌
act(() => {
  render(<Example />)
})

const input = screen.getByRole('textbox', {name: /choose a fruit/i})
act(() => {
  fireEvent.keyDown(input, {key: 'ArrowDown'})
})

// ✅
render(<Example />)
const input = screen.getByRole('textbox', {name: /choose a fruit/i})
fireEvent.keyDown(input, {key: 'ArrowDown'})

I see people wrapping things in act like this because they see these "act" warnings all the time and are just desperately trying anything they can to get them to go away, but what they don't know is that render and fireEvent are already wrapped in act! So those are doing nothing useful.

Most of the time, if you're seeing an act warning, it's not just something to be silenced, but it's actually telling you that something unexpected is happening in your test. You can learn more about this from my blog post (and videos): Fix the "not wrapped in act(...)" warning.

Advice: Learn when act is necessary and don't wrap things in act unnecessarily.

Using the wrong query

Importance: high

// ❌
// assuming you've got this DOM to work with:
// <label>Username</label><input data-testid="username" />
screen.getByTestId('username')

// ✅
// change the DOM to be accessible by associating the label and setting the type
// <label for="username">Username</label><input id="username" type="text" />
screen.getByRole('textbox', {name: /username/i})

We maintain a page called "Which query should I use?" of the queries you should attempt to use in the order you should attempt to use them. If your goal is aligned with ours of having tests that give you confidence that your app will work when your users use them, then you'll want to query the DOM as closely to the way your end-users do so as possible. The queries we provide will help you to do this, but not all queries are created equally.

Using container to query for elements

As a sub-section of "Using the wrong query" I want to talk about querying on the container directly.

// ❌
const {container} = render(<Example />)
const button = container.querySelector('.btn-primary')
expect(button).toHaveTextContent(/click me/i)

// ✅
render(<Example />)
screen.getByRole('button', {name: /click me/i})

We want to ensure that your users can interact with your UI and if you query around using querySelector we lose a lot of that confidence, the test is harder to read, and it will break more frequently. This goes hand-in-hand with the next sub-section:

Not querying by text

As a sub-section of "Using the wrong query", I want to talk about why I recommend you query by the actual text (in the case of localization, I recommend the default locale), rather than using test IDs or other mechanisms everywhere.

// ❌
screen.getByTestId('submit-button')

// ✅
screen.getByRole('button', {name: /submit/i})

If you don't query by the actual text, then you have to do extra work to make sure that your translations are getting applied correctly. The biggest complaint I hear about this is that it leads to content writers breaking your tests. My rebuttal to that is that first, if a content writer changes "Username" to "Email" that's a change I definitely want to know about (because I'll need to change my implementation). Also, if there is a situation where they break something, fixing that issue takes no time at all. It's easy to triage and easy to fix.

So the cost is pretty low, and the benefit is you get increased confidence that your translations are applied correctly and your tests are easier to write and read.

I should mention that not everyone agrees with me on this, feel free to read more about it in this tweet thread.

Not using *ByRole most of the time

As a sub-section of "Using the wrong query" I want to talk about *ByRole. In recent versions, the *ByRole queries have been seriously improved (primarily thanks to great work by Sebastian Silbermann) and are now the number one recommended approach to query your component's output. Here are some of my favorite features.

The name option allows you to query elements by their "Accessible Name" which is what screen readers will read for the element and it works even if your element has its text content split up by different elements. For example:

// assuming we've got this DOM structure to work with
// <button><span>Hello</span> <span>World</span></button>

screen.getByText(/hello world/i)
// ❌ fails with the following error:
// Unable to find an element with the text: /hello world/i. 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.

screen.getByRole('button', {name: /hello world/i})
// ✅ works!

One reason people don't use *ByRole queries is because they're not familiar with the implicit roles placed on elements. Here's a list of Roles on MDN. So another one of my favorite features of the *ByRole queries is that if we're unable to find an element with the role you've specified, not only will we log the entire DOM to you like we do with normal get* or find* variants, but we also log all the available roles you can query by!

// assuming we've got this DOM structure to work with
// <button><span>Hello</span> <span>World</span></button>
screen.getByRole('blah')

This will fail with the following error message:

TestingLibraryElementError: Unable to find an accessible element with the role "blah"

Here are the accessible roles:

  button:

  Name "Hello World":
  <button />

  --------------------------------------------------

<body>
  <div>
    <button>
      <span>
        Hello
      </span>

      <span>
        World
      </span>
    </button>
  </div>
</body>

Notice that we didn't have to add the role=button to our button for it to have the role of button. That's an implicit role, which leads us perfectly into our next one...

Advice: Read and follow the recommendations The "Which Query Should I Use" Guide.**

Adding aria-, role, and other accessibility attributes incorrectly

Importance: high

// ❌
render(<button role="button">Click me</button>)

// ✅
render(<button>Click me</button>)

Slapping accessibility attributes willy nilly is not only unnecessary (as in the case above), but it can also confuse screen readers and their users. The accessibility attributes should really only be used when semantic HTML doesn't satisfy your use case (like if you're building a non-native UI that you want to make accessible like an autocomplete). If that's what you're building, be sure to use an existing library that does this accessibly or follow the WAI-ARIA practices. They often have great examples.

Note: to make inputs accessible via a "role" you'll want to specify the type attribute!

Advice: Avoid adding unnecessary or incorrect accessibility attributes.

Not using @testing-library/user-event

Importance: medium

// ❌
fireEvent.change(input, {target: {value: 'hello world'}})

// ✅
userEvent.type(input, 'hello world')

@testing-library/user-event is a package that's built on top of fireEvent, but it provides several methods that resemble the user interactions more closely. In the example above, fireEvent.change will simply trigger a single change event on the input. However the type call, will trigger keyDown, keyPress, and keyUp events for each character as well. It's much closer to the user's actual interactions. This has the benefit of working well with libraries that you may use which don't actually listen for the change event.

We're still working on @testing-library/user-event to ensure that it delivers what it promises: firing all the same events the user would fire when performing a specific action. I don't think we're quite there yet and this is why it's not baked-into @testing-library/dom (though it may be at some point in the future). However, I'm confident enough in it to recommend you give it a look and use it's utilities over fireEvent.

Advice: Use @testing-library/user-event over fireEvent where possible.

Using query* variants for anything except checking for non-existence

Importance: high

// ❌
expect(screen.queryByRole('alert')).toBeInTheDocument()

// ✅
expect(screen.getByRole('alert')).toBeInTheDocument()
expect(screen.queryByRole('alert')).not.toBeInTheDocument()

The only reason the query* variant of the queries is exposed is for you to have a function you can call which does not throw an error if no element is found to match the query (it returns null if no element is found). The only reason this is useful is to verify that an element is not rendered to the page. The reason this is so important is because the get* and find* variants will throw an extremely helpful error if no element is found–it prints out the whole document so you can see what's rendered and maybe why your query failed to find what you were looking for. Whereas query* will only return null and the best toBeInTheDocument can do is say: "null isn't in the document" which is not very helpful.

Advice: Only use the query* variants for asserting that an element cannot be found.

Using waitFor to wait for elements that can be queried with find*

Importance: high

// ❌
const submitButton = await waitFor(() =>
  screen.getByRole('button', {name: /submit/i}),
)

// ✅
const submitButton = await screen.findByRole('button', {name: /submit/i})

Those two bits of code are basically equivalent (find* queries use waitFor under the hood), but the second is simpler and the error message you get will be better.

Advice: use find* any time you want to query for something that may not be available right away.

Passing an empty callback to waitFor

Importance: high

// ❌
await waitFor(() => {})
expect(window.fetch).toHaveBeenCalledWith('foo')
expect(window.fetch).toHaveBeenCalledTimes(1)

// ✅
await waitFor(() => expect(window.fetch).toHaveBeenCalledWith('foo'))
expect(window.fetch).toHaveBeenCalledTimes(1)

The purpose of waitFor is to allow you to wait for a specific thing to happen. If you pass an empty callback it might work today because all you need to wait for is "one tick of the event loop" thanks to the way your mocks work. But you'll be left with a fragile test which could easily fail if you refactor your async logic.

Advice: wait for a specific assertion inside waitFor.

Having multiple assertions in a single waitFor callback

Importance: low

// ❌
await waitFor(() => {
  expect(window.fetch).toHaveBeenCalledWith('foo')
  expect(window.fetch).toHaveBeenCalledTimes(1)
})

// ✅
await waitFor(() => expect(window.fetch).toHaveBeenCalledWith('foo'))
expect(window.fetch).toHaveBeenCalledTimes(1)

Let's say that for the example above, window.fetch was called twice. So the waitFor call will fail, however, we'll have to wait for the timeout before we see that test failure. By putting a single assertion in there, we can both wait for the UI to settle to the state we want to assert on, and also fail faster if one of the assertions do end up failing.

Advice: only put one assertion in a callback.

Performing side-effects in waitFor

Importance: high

// ❌
await waitFor(() => {
  fireEvent.keyDown(input, {key: 'ArrowDown'})
  expect(screen.getAllByRole('listitem')).toHaveLength(3)
})

// ✅
fireEvent.keyDown(input, {key: 'ArrowDown'})
await waitFor(() => {
  expect(screen.getAllByRole('listitem')).toHaveLength(3)
})

waitFor is intended for things that have a non-deterministic amount of time between the action you performed and the assertion passing. Because of this, the callback can be called (or checked for errors) a non-deterministic number of times and frequency (it's called both on an interval as well as when there are DOM mutations). So this means that your side-effect could run multiple times!

This also means that you can't use snapshot assertions within waitFor. If you do want to use a snapshot assertion, then first wait for a specific assertion, and then after that you can take your snapshot.

Advice: put side-effects outside waitFor callbacks and reserve the callback for assertions only.

Using get* variants as assertions

Importance: low

// ❌
screen.getByRole('alert', {name: /error/i})

// ✅
expect(screen.getByRole('alert', {name: /error/i})).toBeInTheDocument()

This one's not really a big deal actually, but I thought I'd mention it and give my opinion on it. If get* queries are unsuccessful in finding the element, they'll throw a really helpful error message that shows you the full DOM structure (with syntax highlighting) which will help you during debugging. Because of this, the assertion could never possibly fail (because the query will throw before the assertion has a chance to).

For this reason, many people skip the assertion. This really is fine honestly, but I personally normally keep the assertion in there just to communicate to readers of the code that it's not just an old query hanging around after a refactor but that I'm explicitly asserting that it exists.

Advice: If you want to assert that something exists, make that assertion explicit.