DEV Community

Cover image for Why practicing DRY in tests is bad for you
Matti Bar-Zeev
Matti Bar-Zeev

Posted on

Why practicing DRY in tests is bad for you

This post is a bit different from the recent ones I’ve published. I’m going to share my point of view on practicing DRY in unit tests and why I think it is bad for you. Care to know why? Here we go -

What is DRY?

Assuming that not all of us know what DRY means here is a quick explanation:
“Don't Repeat Yourself (DRY) is a principle of software development aimed at reducing repetition of software patterns” (from here). We don’t like duplications since “Duplication can lead to maintenance nightmares, poor factoring, and logical contradictions.” (from here).
An example can be having a single service which is responsible for fetching data from the server instead of duplicating the code all over the code base.
The main benefit is clear - a single source of logic, where each modification for it applies to all which uses it.

Where does DRY apply in tests?

In tests we thrive to assert as much as needed in order to give us the future modification confidence we feel comfortable with. This means that there will be a lot of tests that differ in nuances in order to make sure we cover each of the edge cases well.
What the previous sentence means in code is that tests tend to have a lot of repetitive and duplicated code to them, this is where the DRY principle finds its way in.

Let me try and explain with examples from the React world -
We are testing a custom component and we’re using the React Testing Library (and jest-dom) in order to test the component’s rendering. It may look something like this:

describe('Confirmation component', () => {
   it('should render', () => {
       const {getByRole} = render(<Confirmation />);
       expect(getByRole('dialog')).toBeInTheDocument();
   });
});
Enter fullscreen mode Exit fullscreen mode

Here I’m testing that once the Confirmation component is being rendered, the element with the “dialog” role is present on the document.
This is great but it is just a single test among the many cases this component has, and that means that for each test you will have the same repetitive render code, which sometimes can be complex with props for the component, and perhaps wrapping it in a context provider.
So what many choose to do is to create a “helper” render function which encapsulates the rendering and then each test can call it, before starting its assertions:

function renderConfirmationComponent() {
   return render(<Confirmation />);
}

describe('Confirmation component', () => {
   it('should render', () => {
       const {getByRole} = renderConfirmationComponent();
       expect(getByRole('dialog')).toBeInTheDocument();
   });
});
Enter fullscreen mode Exit fullscreen mode

We gain the benefit of DRY, where if we want to change the rendering for all the tests, we do it in a single place.

Another example of DRY in tests is using loops in order to generate many different test cases. An example can be testing an “add” function which receives 2 arguments and returns the result for it.
Instead of duplicating the code many times for each case, you can loop over a “data-provider” (or "data-set") for the test and generate the test cases, something like this:

describe('Add function', () => {
   const dataProvider = [
       [1, 2, 3],
       [3, 21, 24],
       [1, 43, 44],
       [15, 542, 557],
       [5, 19, 24],
       [124, 22, 146],
   ];

   dataProvider.forEach((testCase) => {
       it(`should return a ${testCase[2]} result for adding ${testCase[0]} and ${testCase[1]}`, () => {
           const result = add(testCase[0], testCase[1]);
           expect(result).toEqual(testCase[2]);
       });
   });
});
Enter fullscreen mode Exit fullscreen mode

And the test result looks like this:

Add function
    ✓ should return a 3 result for adding 1 and 2 (1 ms)
    ✓ should return a 24 result for adding 3 and 21 (1 ms)
    ✓ should return a 44 result for adding 1 and 43
    ✓ should return a 557 result for adding 15 and 542
    ✓ should return a 24 result for adding 5 and 19 (1 ms)
    ✓ should return a 146 result for adding 124 and 22
Enter fullscreen mode Exit fullscreen mode

BTW Jest even encourages you to do that with its built-in API, like test.each and describe.each.

Here is (somewhat) the same example with that API:

test.each(dataProvider)('.add(%i, %i)', (a, b, expected) => {
    expect(add(a, b)).toBe(expected);
});
Enter fullscreen mode Exit fullscreen mode

Looks great, right? I created 6 test cases in just a few lines of code. So why am I saying it’s bad for you?

Searching

The scenario is typically this - a test fails, you read the output on the terminal and go searching for that specific failing test case. What you have in your hand is the description of the test case, but what you don’t know is that this description is a concatenation of strings.
You won't be able to find “should return a 3 result for adding 1 and 2” in the code because it simply does not exist. It really depends on how complex your test’s data-provider is, but this can become a real time waster trying to figure out what to search for.

Readability

So you found you test and it looks like this:

dataProvider.forEach((testCase) => {
       it(`should return ${testCase[2]} result for adding ${testCase[0]} and ${testCase[1]}`, () => {
           const result = add(testCase[0], testCase[1]);
           expect(result).toEqual(testCase[2]);
       });
});
Enter fullscreen mode Exit fullscreen mode

You gotta admit that this is not intuitive. Even with the sugar (is it really sweeter?) syntax Jest offers it takes you some time to wrap your head around all the flying variables and string concatenations to realize exactly what’s been tested.
When you do realize what’s going on, you need to isolate the case which failed by breaking the loop or modifying your data-provider, since you cannot isolate the failing test case to run alone.
One of the best “tools” I use to resolve failing tests is to isolate them completely and avoid the noise from the other tests, and here it is much harder to do.
Tests should be easy to read, easy to understand and easy to modify. It is certainly not the place to prove that a test can be written in a one-liner, or with (god forbid) a reducer.

State leakage

Running tests in loops increases the potential of tests leaking state from one another. You can sometimes find out that after you’ve isolated the test which fails, it suddenly passes with flying colors. This usually means that previous tests within that loop leaked a certain state which caused it to fail.
When you have each test as a standalone isolated unit, the potential of one test affecting the others dramatically reduces.

The cost of generic code

Let’s go back to our React rendering example and expand it a little. Say that our generic rendering function receives props in order to render the component differently for each test case, and it might also receive a state “store” with different attributes to wrap the component with.
If, for some reason, you need to change the way you want to render the component for a certain test case you will need to add another argument to the rendering generic function, and your generic function will start growing into this little monster which needs to support any permutation of your component rendering.
As with any generic code, there is a cost of maintaining it and keeping it compatible with the evolving conditions.

Wrapping up

I know.
There are cases where looping over a data-provider to create test cases, or creating “helper” functions is probably the best way for achieving a good code coverage with little overhead. However, I would like you to take a minute and understand the cost of going full DRY mode in your tests given all the reasons mentioned above.
There is a clear purpose for your tests and that is to prevent regressions and provide confidence when making future changes. Your tests should not become a burden to maintain or use.
I much prefer simple tests, where everything which is relevant to a test case can be found between its curly brackets, and I really don’t care if that code repeats itself. It reassures me that there is little chance that this test is affected somehow by any side effect I’m not aware of.

As always, if you have any thoughts or comments about what's written here, please share with the rest of us :)

Hey! If you liked what you've just read check out @mattibarzeev on Twitter 🍻

Discussion (6)

Collapse
gamma032steam profile image
Matthew Lui

I've been reading pragmatic programmer recently (the origin of the DRY principle). One thing the authors stress in the latest is edition is how what they wrote was misinterpreted. DRY is about having single sources of truth in a system. Two tests with different inputs have different intent - they do different things - so it's okay to copy-paste.

In this case I would think about how I expect the tests to be changed. Do I want to make it easy to add and remove tests? Write a loop and have an array of inputs that are passed in. Do I want the test itself to be easy to change? Keep it simple and copy-paste within reason.

Collapse
mbarzeev profile image
Matti Bar-Zeev Author

Totally! Thanks for the additional resource to back this up

Collapse
hseritt profile image
Harlin Seritt

And this is exactly what I was thinking when I wrote my comment, Matthew. Thanks for pointing it out.

Collapse
hseritt profile image
Harlin Seritt

Agreed. DRY in practice is not always what DRY is too.

Collapse
skyboyer profile image
Yevhen Kozlov

I 100% agree that for complex tests the more local helpers are used, the harder debug/isolation gets. Similarly, having loops or conditions inside of test code(not "code under test") also makes it much harder to figure out what's going wrong when test fails.

But from the other side, for data-driven tests(no interaction simulated, just "input-output" check), test.each groups test cases in very natural way that test the same but for different edge conditions. And to debug them we can and will use conditional breakpoints. My rule of thumb(by experience, very subjective) for better readability: "max 3 inputs, only primitive values in input, single output to check, all inputs are listed in template for name in natural way, as if I would say what's going on out loud".

Similarly to snapshot testing, I think this approach really shines in quite limited set of conditions. But still can be used. Using example from article, name pattern ${first}+${second}=${expected} would give better readability then separate it("1+-1=0",...); it("1+Infinity=Infinity",....);...

Collapse
jakecarpenter profile image
Jake Carpenter • Edited on

I agree your example of test theories is an example of a bad test, but only because of the way it is communicated by the test. Test theories are not inherently bad or an example of being DRY, but they do require careful communication of what you're testing because it's easy to lump different scenarios under the same set of cases.

The best way to do this is to utilize the BDD-nature of Jest. This is still a contrived example, but if you're not using CTRL+Click from the terminal to jump to your failing test then it still gives you a better description:

describe('Add function', () => {
  describe('should add two positive numbers correctly', () => {
    const theories = [
      [1, 1, 2],
      [21, 33, 54],
      [100, 88, 188],
    ]

    test.each(theories)('given %i and %i then %i', (a, b, expected) => {
      expect(add(a, b)).toBe(expected)
    })
  })
})
Enter fullscreen mode Exit fullscreen mode

I feel a lot of people dislike "nesting" tests and descriptions this way, but it's really one of the intended strengths. I was taught testing in Javascript by one of the maintainers of Jasmine who used BDD patterns very effectively.

Personally I dislike the naming templates with jest.each() and use jest-theories in my own current projects.

None of this is arguing against your broader point of being too DRY in tests though. I very much agree with that and feel the most common mistakes involve shared test setup.