This site runs best with JavaScript enabled.

Avoid Nesting when you're Testing

Software Engineer, React Training, Testing JavaScript Training

Photo by Kate Remmer


Why using hooks like beforeEach as a mechanism for code reuse leads to unmaintainable tests and how to avoid it.

I want to show you something. What I'm going to show is a general testing principle, applied to a React component test. So even though the example is a React one, hopefully it helps communicate the concept properly.

Note: my point isn't that nesting is bad by itself, but rather that it naturally encourages using test hooks (such as beforeEach) as a mechanism for code reuse which does lead to unmaintainable tests. Please read on...

Here's a React component that I want to test:

1// login.js
2import React from 'react'
3
4function Login({onSubmit}) {
5 const [error, setError] = React.useState('')
6
7 function handleSubmit(event) {
8 event.preventDefault()
9 const {
10 usernameInput: {value: username},
11 passwordInput: {value: password},
12 } = event.target.elements
13
14 if (!username) {
15 setError('username is required')
16 } else if (!password) {
17 setError('password is required')
18 } else {
19 setError('')
20 onSubmit({username, password})
21 }
22 }
23
24 return (
25 <div>
26 <form onSubmit={handleSubmit}>
27 <div>
28 <label htmlFor="usernameInput">Username</label>
29 <input id="usernameInput" />
30 </div>
31 <div>
32 <label htmlFor="passwordInput">Password</label>
33 <input id="passwordInput" type="password" />
34 </div>
35 <button type="submit">Submit</button>
36 </form>
37 {error ? <div role="alert">{error}</div> : null}
38 </div>
39 )
40}
41
42export default Login

And here's what that renders (it actually works, try it):

Here's a test suite that resembles the kind of testing I've seen over the years.

1// __tests__/login.js
2import React from 'react'
3import {render, fireEvent} from '@testing-library/react'
4import Login from '../login'
5
6describe('Login', () => {
7 let utils,
8 handleSubmit,
9 user,
10 changeUsernameInput,
11 changePasswordInput,
12 clickSubmit
13
14 beforeEach(() => {
15 handleSubmit = jest.fn()
16 user = {username: 'michelle', password: 'smith'}
17 utils = render(<Login onSubmit={handleSubmit} />)
18 changeUsernameInput = value =>
19 fireEvent.change(utils.getByLabelText(/username/i), {target: {value}})
20 changePasswordInput = value =>
21 fireEvent.change(utils.getByLabelText(/password/i), {target: {value}})
22 clickSubmit = () => fireEvent.click(utils.getByText(/submit/i))
23 })
24
25 describe('when username and password is provided', () => {
26 beforeEach(() => {
27 changeUsernameInput(user.username)
28 changePasswordInput(user.password)
29 })
30
31 describe('when the submit button is clicked', () => {
32 beforeEach(() => {
33 clickSubmit()
34 })
35
36 it('should call onSubmit with the username and password', () => {
37 expect(handleSubmit).toHaveBeenCalledTimes(1)
38 expect(handleSubmit).toHaveBeenCalledWith(user)
39 })
40 })
41 })
42
43 describe('when the password is not provided', () => {
44 beforeEach(() => {
45 changeUsernameInput(user.username)
46 })
47
48 describe('when the submit button is clicked', () => {
49 let errorMessage
50 beforeEach(() => {
51 clickSubmit()
52 errorMessage = utils.getByRole('alert')
53 })
54
55 it('should show an error message', () => {
56 expect(errorMessage).toHaveTextContent(/password is required/i)
57 })
58 })
59 })
60
61 describe('when the username is not provided', () => {
62 beforeEach(() => {
63 changePasswordInput(user.password)
64 })
65
66 describe('when the submit button is clicked', () => {
67 let errorMessage
68 beforeEach(() => {
69 clickSubmit()
70 errorMessage = utils.getByRole('alert')
71 })
72
73 it('should show an error message', () => {
74 expect(errorMessage).toHaveTextContent(/username is required/i)
75 })
76 })
77 })
78})

That should give us 100% confidence that this component works and will continue to work as designed. And it does. But here are the things I don't like about that test:

Over-abstraction

I feel like the utilities like changeUsernameInput and clickSubmit can be nice, but the tests are simple enough that duplicating that code instead could simplify our test code a bit. It's just that the abstraction of the function doesn't really give us a whole lot of benefit for this small set of tests, and we incur the cost for maintainers to have to look around the file for where those functions are defined.

Nesting

The tests above are written with Jest APIs, but you'll find similar APIs in all major JavaScript frameworks. I'm talking specifically about describe which is used for grouping tests, beforeEach for common setup/actions, and it for the actual assertions.

I have a strong dislike for nesting like this. I've written and maintained thousands of tests that were written like this and I can tell you that as painful as it is for these three simple tests, it's way worse when you have thousands of lines of tests and wind up nesting even further.

What makes it so complex? Take this bit for example:

1it('should call onSubmit with the username and password', () => {
2 expect(handleSubmit).toHaveBeenCalledTimes(1)
3 expect(handleSubmit).toHaveBeenCalledWith(user)
4})

Where is handleSubmit coming from and what's its value? Where is user coming from? What's its value? Oh sure, you can go find where it's defined:

1describe('Login', () => {
2 let utils,
3 handleSubmit,
4 user,
5 changeUsernameInput,
6 changePasswordInput,
7 clickSubmit
8 // ...
9})

But then you also have to figure out where it's assigned:

1beforeEach(() => {
2 handleSubmit = jest.fn()
3 user = {username: 'michelle', password: 'smith'}
4 // ...
5})

And then, you have to make sure that it's not actually being assigned to something else in a further nested beforeEach. Tracing through the code to keep track of the variables and their values over time is the number one reason I strongly recommend against nested tests. The more you have to hold in your head for menial things like that, the less room there is for accomplishing the important task at hand.

You can argue that variable reassignment is an "anti-pattern" and should be avoided, and I would agree with you, but adding more linting rules to your suite of possibly already overbearing linting rules is not an awesome solution. What if there were a way to share this common setup without having to worry about variable reassignment at all?

Inline it!

For this simple component, I think the best solution is to just remove as much abstraction as possible. Check this out:

1// __tests__/login.js
2import React from 'react'
3import {render, fireEvent} from '@testing-library/react'
4import Login from '../login'
5
6test('calls onSubmit with the username and password when submit is clicked', () => {
7 const handleSubmit = jest.fn()
8 const {getByLabelText, getByText} = render(<Login onSubmit={handleSubmit} />)
9 const user = {username: 'michelle', password: 'smith'}
10
11 fireEvent.change(getByLabelText(/username/i), {
12 target: {value: user.username},
13 })
14 fireEvent.change(getByLabelText(/password/i), {
15 target: {value: user.password},
16 })
17 fireEvent.click(getByText(/submit/i))
18
19 expect(handleSubmit).toHaveBeenCalledTimes(1)
20 expect(handleSubmit).toHaveBeenCalledWith(user)
21})
22
23test('shows an error message when submit is clicked and no username is provided', () => {
24 const handleSubmit = jest.fn()
25 const {getByLabelText, getByText, getByRole} = render(
26 <Login onSubmit={handleSubmit} />,
27 )
28
29 fireEvent.change(getByLabelText(/password/i), {target: {value: 'anything'}})
30 fireEvent.click(getByText(/submit/i))
31
32 const errorMessage = getByRole('alert')
33 expect(errorMessage).toHaveTextContent(/username is required/i)
34 expect(handleSubmit).not.toHaveBeenCalled()
35})
36
37test('shows an error message when submit is clicked and no password is provided', () => {
38 const handleSubmit = jest.fn()
39 const {getByLabelText, getByText, getByRole} = render(
40 <Login onSubmit={handleSubmit} />,
41 )
42
43 fireEvent.change(getByLabelText(/username/i), {target: {value: 'anything'}})
44 fireEvent.click(getByText(/submit/i))
45
46 const errorMessage = getByRole('alert')
47 expect(errorMessage).toHaveTextContent(/password is required/i)
48 expect(handleSubmit).not.toHaveBeenCalled()
49})

Note: test is an alias for it and I just prefer using test when I'm not nested in a describe.

You'll notice that there is a bit of duplication there (we'll get to that), but look at how clear these tests are. With the exception of some test utilities and the Login component itself, the entire test is self-contained. This significantly improves the ability for us to understand what's going on in each test without having to do any scrolling around. If this component had a few dozen more tests, the benefits would be even more potent.

Notice also that we aren't nesting everything in a describe block, because it's really not necessary. Everything in the file is clearly testing the login component, and including even a single level of nesting is pointless.

Apply AHA (Avoid Hasty Abstractions)

The AHA principle states that you should:

prefer duplication over the wrong abstraction and optimize for change first.

For our simple Login component here, I'd probably leave the test as-is, but let's imagine that it's a bit more complicated and we're starting to see some problems with code duplication and we'd like to reduce it. Should we reach for beforeEach for that? I mean, that's what it's there for right?

Well, we could, but then we have to start worrying about mutable variable assignments again and we'd like to avoid that. How else could we share code between our tests? AHA! We could use functions!

1import React from 'react'
2import {render, fireEvent} from '@testing-library/react'
3import Login from '../login'
4
5// here we have a bunch of setup functions that compose together for our test cases
6// I only recommend doing this when you have a lot of tests that do the same thing.
7// I'm including it here only as an example. These tests don't necessitate this
8// much abstraction. Read more: https://kcd.im/aha-testing
9function setup() {
10 const handleSubmit = jest.fn()
11 const utils = render(<Login onSubmit={handleSubmit} />)
12 const user = {username: 'michelle', password: 'smith'}
13 const changeUsernameInput = value =>
14 fireEvent.change(utils.getByLabelText(/username/i), {target: {value}})
15 const changePasswordInput = value =>
16 fireEvent.change(utils.getByLabelText(/password/i), {target: {value}})
17 const clickSubmit = () => fireEvent.click(utils.getByText(/submit/i))
18 return {
19 ...utils,
20 handleSubmit,
21 user,
22 changeUsernameInput,
23 changePasswordInput,
24 clickSubmit,
25 }
26}
27
28function setupSuccessCase() {
29 const utils = setup()
30 utils.changeUsernameInput(utils.user.username)
31 utils.changePasswordInput(utils.user.password)
32 utils.clickSubmit()
33 return utils
34}
35
36function setupWithNoPassword() {
37 const utils = setup()
38 utils.changeUsernameInput(utils.user.username)
39 utils.clickSubmit()
40 const errorMessage = utils.getByRole('alert')
41 return {...utils, errorMessage}
42}
43
44function setupWithNoUsername() {
45 const utils = setup()
46 utils.changePasswordInput(utils.user.password)
47 utils.clickSubmit()
48 const errorMessage = utils.getByRole('alert')
49 return {...utils, errorMessage}
50}
51
52test('calls onSubmit with the username and password', () => {
53 const {handleSubmit, user} = setupSuccessCase()
54 expect(handleSubmit).toHaveBeenCalledTimes(1)
55 expect(handleSubmit).toHaveBeenCalledWith(user)
56})
57
58test('shows an error message when submit is clicked and no username is provided', () => {
59 const {handleSubmit, errorMessage} = setupWithNoUsername()
60 expect(errorMessage).toHaveTextContent(/username is required/i)
61 expect(handleSubmit).not.toHaveBeenCalled()
62})
63
64test('shows an error message when password is not provided', () => {
65 const {handleSubmit, errorMessage} = setupWithNoPassword()
66 expect(errorMessage).toHaveTextContent(/password is required/i)
67 expect(handleSubmit).not.toHaveBeenCalled()
68})

Now we could have dozens of tests that use these simple setup functions, and notice also that they can be composed together to give us a similar behavior as the nested beforeEach that we had before if that makes sense. But we avoid having mutable variables that we have to worry about keeping track of in our mind.

You can learn more about the benefits of AHA with testing from AHA Testing.

What about grouping tests?

The describe function is intended to group related tests together and can provide for a nice way to visually separate different tests, especially when the test file gets big. But I don't like it when the test file gets big. So instead of grouping tests by describe blocks, I group them by file. So if there's a logical grouping of different tests for the same "unit" of code, I'll separate them by putting them in completely different files. And if there's some code that really needs to be shared between them, then I'll create a __tests__/helpers/login.js file which has the shared code.

This comes with the benefit of logically grouping tests, completely separating any setup that's unique for them, reducing the cognitive load of working on a particular part of the unit of code I'm working on, and if your testing framework can run tests in parallel, then my tests will probably run faster as well.

What about cleanup?

This blog post isn't an attack on utilities like beforeEach/afterEach/etc. It's more of a caution against mutable variables in tests, and being mindful of your abstractions.

For cleanup, sometimes you're stuck with a situation where the thing you're testing makes some changes to the global environment and you need to cleanup after it. If you try to put that code inline within your test, then a test failure would result in your cleanup not running which could then lead to other tests failing, ultimately resulting in a lot of error output that is harder to debug.

For example, React Testing Library will insert your component into the document, and if you don't cleanup after each test, then your tests can run over themselves:

1import React from 'react'
2import {render, fireEvent} from '@testing-library/react'
3import Login from '../login'
4
5test('example 1', () => {
6 const handleSubmit = jest.fn()
7 const {getByLabelText} = render(<Login onSubmit={handleSubmit} />)
8 fireEvent.change(getByLabelText(/username/i), {target: {value: 'mary'}})
9 fireEvent.change(getByLabelText(/password/i), {
10 target: {value: 'little lamb'},
11 })
12 // more test here
13})
14
15test('example 2', () => {
16 const handleSubmit = jest.fn()
17 const {getByLabelText} = render(<Login onSubmit={handleSubmit} />)
18 // 💣 this will blow up because the `getByLabelText` is actually querying the
19 // entire document, and because we didn't cleanup after the previous test
20 // we'll get an error indicating that RTL found more than one field with the
21 // label "username"
22 fireEvent.change(getByLabelText(/username/i), {target: {value: 'mary'}})
23 // more test here
24})

Fixing this is pretty simple, you need to execute the cleanup method from @testing-library/react after each test.

1import React from 'react'
2import {render, fireEvent, cleanup} from '@testing-library/react'
3import Login from '../login'
4
5test('example 1', () => {
6 const handleSubmit = jest.fn()
7 const {getByLabelText} = render(<Login onSubmit={handleSubmit} />)
8 fireEvent.change(getByLabelText(/username/i), {target: {value: 'mary'}})
9 fireEvent.change(getByLabelText(/password/i), {
10 target: {value: 'little lamb'},
11 })
12 // more test here
13 cleanup()
14})
15
16test('example 2', () => {
17 const handleSubmit = jest.fn()
18 const {getByLabelText} = render(<Login onSubmit={handleSubmit} />)
19 fireEvent.change(getByLabelText(/username/i), {target: {value: 'mary'}})
20 // more test here
21 cleanup()
22})

However, if you don't use afterEach to do this then if a test fails your cleanup wont run, like this:

1test('example 1', () => {
2 const handleSubmit = jest.fn()
3 const {getByLabelText} = render(<Login onSubmit={handleSubmit} />)
4 fireEvent.change(getByLabelText(/username/i), {target: {value: 'mary'}})
5 // 💣 the following typo will result in a error thrown:
6 // "no field with the label matching passssword"
7 fireEvent.change(getByLabelText(/passssword/i), {
8 target: {value: 'little lamb'},
9 })
10 // more test here
11 cleanup()
12})

Because of this, the cleanup function in "example 1" will not run and then "example 2" wont run properly, so instead of only seeing 1 test failure, you'll see that all the tests failed and it'll make it much harder to debug.

So instead, you should use afterEach and that will ensure that even if your test fails, you can cleanup:

1import React from 'react'
2import {render, fireEvent, cleanup} from '@testing-library/react'
3import Login from '../login'
4
5afterEach(() => cleanup())
6
7test('example 1', () => {
8 const handleSubmit = jest.fn()
9 const {getByLabelText} = render(<Login onSubmit={handleSubmit} />)
10 fireEvent.change(getByLabelText(/username/i), {target: {value: 'mary'}})
11 fireEvent.change(getByLabelText(/password/i), {
12 target: {value: 'little lamb'},
13 })
14 // more test here
15})
16
17test('example 2', () => {
18 const handleSubmit = jest.fn()
19 const {getByLabelText} = render(<Login onSubmit={handleSubmit} />)
20 fireEvent.change(getByLabelText(/username/i), {target: {value: 'mary'}})
21 // more test here
22})

Even better, with React Testing Library, you can import @testing-library/react/cleanup-after-each and it'll do this for you automatically. Learn more in the docs

In addition, sometimes there are definitely good use cases for before*, but they're normally matched with a cleanup that's necessary in an after*. Like starting and stopping a server:

1let server
2beforeAll(async () => {
3 server = await startServer()
4})
5afterAll(() => server.close())

There's not really any other reliable way to do this. Another use case I can think of that I've used these hooks is for testing console.error calls:

1beforeAll(() => {
2 jest.spyOn(console, 'error').mockImplementation(() => {})
3})
4
5afterEach(() => {
6 console.error.mockClear()
7})
8
9afterAll(() => {
10 console.error.mockRestore()
11})

So there are definitely use cases for those kinds of hooks. I just don't recommend them as a mechanism for code reuse. We have functions for that.

Conclusion

I hope this helps clarify what I meant in this tweet:

I've written tens of thousands of tests with different frameworks and styles and in my experience, reducing the amount of variable mutation has resulted in vastly simpler test maintenance. Good luck!

P.S. If you'd like to play with the examples, here's a codesandbox

Discuss on TwitterEdit post on GitHub

Share article
loading relevant upcoming workshops...

TestingJavaScript.com

Your essential guide to flawless testing.

Jump on this self-paced workshop and learn the smart, efficient way to test any JavaScript application. 🏆

Start Now
Kent C. Dodds

Kent C. Dodds is a JavaScript software engineer and teacher. He's taught hundreds of thousands of people how to make the world a better place with quality software development tools and practices. He lives with his wife and four kids in Utah.