Don't DRY your tests
For me, tests provide several benefits:
- As the first consumers of your code (at least when following TDD), tests are a great way to inform API decisions for your code.
- Tests are living documentation for your code. Unlike written documentation (also important) or code comments, tests are guaranteed to be in sync with your code if they're passing.
- Tests are a great way to clarify requirements and identify gaps in those requirements.
- Testable code tends to be well structured code, so coding with testing in mind can lead to code that is easier to understand and maintain.
- A solid suite of tests is a great safety net for catching unintended breaking changes when refactoring your code and adding features.
The problem
A lot of developers hear, DRY (Don't Repeat Yourself) and apply the concept to code duplication rather than it's intended target, knowledge duplication. This often leads to overly complex code, aimed at removing a handful of characters that just happen to be the same shape if you squint hard enough.
Each of your tests should stand on its own. Your tests should be isolated and free of side-effects so they are not order-dependent and one test can't create side-effects that lead to false positives (or negatives) in other tests.
When people try to "DRY out" their tests, this leads to tests that rely on a lot of shared utility functions and values stored in a surrounding scope aimed at cutting down on duplication. This leads to a lot of indirection and opens up the door to unintended side-effects that are hard to see and can be frustrating to debug.
In addition to the potential issues with side-effects, all the indirection makes it harder to maintain your test code in the long run and hurts the overall readability of individual tests. If you can't look at a test and understand what is being tested and the intended use of the code under test, it's really not living up to it potential as live documentation. It's also hard to write tests in this way and have them inform API decisions, since you up being scattering the code implementation throughout your test test utilities and you don't directly use the code as a consumer would in your test.
The solution
Don't DRY your tests!
Instead, you might consider leaning towards Write Everything Twice (WET), or in the case of your tests... as many times as you need to.
Let's look at an example
First, to set a little context, this is a test for a React component and the project is configured to use Jest, Testing Library and jest-dom for extending Jest's matchers to include toBeInTheDocument
.
it('should correctly apply the testId', () => {
const testId = 'test-id'
const { getByTestId } = renderBasicButton({ testId })
expect(getByTestId(testId)).toBeInTheDocument()
})
So what's happening here? Let's start with what we can determine from these 3 lines of test code:
- This test is rendering a "Basic" button, using a function called
renderBasicButton
that must be defined somewhere in the test's surrounding scope - That
renderBasicButton
function accepts an object that can have atestId
property - It uses the
getByTestId
function provided as part of the object returned fromrenderBasicButton
to check that the element can be found in the document.
Based on the test description, some knowledge of the libraries used here, and the things we were able to surmise from the code itself, it's not a stretch to imagine what's happening here:
renderBasicButton
must usetesting-library
'srender
function under the hood, this is wheregetByTestId
is coming from. This isn't a given, but it seems logical if you're familiar withtesting-library
.- It must render a button and apply the
testId
value to thedata-testid
attribute on the resultingbutton
element. This also assumes knowledge oftesting-library
.
Now, before we go an look at the code that supports this test, let's list out the questions we still have:
- What else happens when the button is rendered?
- Does it get other props?
- Does it have any children?
- Do those things even matter for this test?
- If someone changes something inside that
renderBasicButton
function, will this test start failing?
- We're pretty sure it's returning an object that includes functions returned from
testing-library
'srender
function.- Is it directly returning that?
- Is it returning a subset of that?
- Is it augmenting that return value and returning a superset of that?
Let's take a look at the renderBasicButton
function and see if we can answer these questions
const renderBasicButton = (myProps = {}) => {
const props = {
...BASE_BUTTON_PROPS,
...myProps,
}
return render(<Button {...props}>{TEXT}</Button>)
}
Now we're getting somewhere. We can answer some of our questions now:
- This does in-fact use
render
under the hood and directly returns that result. No subset or superset. - We can also see that whatever I pass in that object argument is merged with
BASE_BUTTON_PROPS
to create the props that are spread into theButton
component. - The
Button
component is being rendered withTEXT
as children
So we're one step removed from the test code, and the picture is coming into focus, but it still isn't 100% clear. We have some values coming in from the surrounding scope, so let's look at TEXT
and BASE_BUTTON_PROPS
to see what's actually being rendered in the test.
const TEXT = 'Test Text'
const BASE_BUTTON_PROPS = {
id: 'testId',
onClick: jest.fn(),
}
Now we know that TEXT
is just a string, so nothing more than you'd expect in a "basic" button. And BASE_BUTTON_PROPS
provides an id
and an onClick
that uses a spy function from Jest.
Now that I have all the pieces, I can reconstruct the button that gets rendered in this test. If I were to write out the JSX for it, it would look like:
<Button onClick={jest.fn()} id="testId" testId="test-id">
Test Text
</Button>
Now that we know what that the resulting JSX would be, we can revise the test to just use the render
function from RTL directly:
it('should correctly apply the testId', () => {
const testId = 'test-id'
const { getByTestId } = render(
<Button onClick={jest.fn()} id="testId" testId={testId}>
Test Text
</Button>
)
expect(getByTestId(testId)).toBeInTheDocument()
})
I've kept the testId
variable to avoid potential errors from duplicating that string value, but it's right in the test along with everything else, so you don't have to go anywhere else in the code to see that value.
This is better, but we can still do a little bit better. Since we're ultimately just verifying that the testId
prop is correctly rendered to the DOM as data-testid
, we don't really need this button to have an id
value or a click handler. Let's remove the props that aren't required for this test:
it('should correctly apply the testId', () => {
const testId = 'test-id'
const { getByTestId } = render(<Button testId="test-id">Test Text</Button>)
expect(getByTestId(testId)).toBeInTheDocument()
})
We don't really need the text here either, but a button with no content isn't a great example of usage, but I don't want it to distract from the primary point of the test, so let's make it a little more subtle:
it('should correctly apply the testId', () => {
const testId = 'test-id'
const { getByTestId } = render(<Button testId="test-id">click</Button>)
expect(getByTestId(testId)).toBeInTheDocument()
})
I think by using the shorter, all-lowercase string here, we've minimized its visual appearance, making it easier to focus on the important bits of the test code.
You could easily argue that it isn't worth it, or that the text isn't what's being tested so it could be removed. I could go either way on this and don't have a strong opinion either way.
What have we gained?
Reduced cognitive load
By putting all the relevant bits of our test inside the test function, we've made it easier to see what is being tested, what props are important for this particular test, and what the consuming code for this component would look like, albeit, an incomplete code snippet. It was possible to piece this all together in our original test, but it required us to jump around the code to gather all the information we need.
Clear signals of intent
We've removed bits of irrelevant code and made it clear that the id
and onClick
props have nothing to do with the testId
. If all of our tests reveal only the important bits of code, a test that has multiple props on the button will signal that all the included bits are related to that test. In a more complex scenario, you very well may have mocks that are important for the functionality under test, when you define their behavior right in your test, it makes the impact of side-effects in your code clear without you having to track that mock logic down. If the behavior for those mocks were defined elsewhere, you could easily miss them. On the flip side, if a mock is required for your code to work, but isn't part of the intended test, you can signal if a mock is unimportant by giving it a generic value, like resolving a mock service with an empty array, object for a string default
, depending on what that service should return
Simplified test code maintenance
If the feature under test here changes in the future, we have a single place to go and make changes. If we no longer need the feature in the future, we can delete this test and be confident that we aren't leaving behind orphaned utility functions. Sure, a linter could catch this for us, but I thought it was worth mentioning as a nice bonus feature of this approach.
Of course, a lot of this can be very subjective, but I've found in my experience that when I'm coming into a new code base with tests, the more "DRY" a test suite is, the harder it is for me to parse. I spend more time piecing things together than I do in cases where tests are self-contained.
If faced with the choice between duplicating code across tests and creating abstractions for that test code, consider the implications that will have for new team members, or even the current team members when you've forgotten about some of the nuance in a section of your code. If you have clear, easily parsable tests, you'll be grateful for the reduced cognitive load required to understand the code and tests.