Are Your Unit Tests Too DRY?

While tackling some long-overdue refactoring of a particularly tangled part of our current project, Dmitri and I were faced with the task of updating some long and unreadable unit tests that we had just caused to fail.

We had made a classic mistake earlier in the project and neglected technical issues like dependency management in a rush to be productive and show our customer some quick progress. We ended up with poorly structured and highly-interdependent classes. This made writing simple tests impossible. Trying to test a single piece of functionality often required 20 to 30 lines of setup code. As I mentioned last time, this resulted in a downward spiral: poorly structured code made it hard to write good tests, and a lack of good tests made it hard to refactor the code.

So here we were. We had struggled for several hours to decipher some tangled code and find a good starting point for the refactoring (what Michael Feathers calls a seam), we had made a small change, and we'd run the unit tests. Now we were staring at a list of 4 failing tests, and not only did we not know why they were failing, we had no idea what they were trying to test and how they were testing it.

One of the problems with the tests was interesting. Normally, I am a huge proponent of DRY programming. If there is no good reason for duplicating data or logic, don't duplicate it -- and there is hardly ever a good reason. However, we had just stumbled upon one of the good reasons.

Unit tests should function as documentation. You should be able to read a unit test in 10-15 seconds and know what requirement it is testing, and how client code should interact with the code that implements that requirement. If our tests had had this property, we would have saved ourselves the hours of trying to figure out what the code was doing, and the hour or so it took us to figure out what the tests were doing, why they were failing, and how to fix them.

If you feel the urge to DRY up your tests, stop for a minute and question whether you are working at the best point of leverage available to you.

If DRYing up the tests seems like the only way to set up tests in under 30 lines of code, your real problem is that the code you are testing is poorly isolated. This is probably your problem if you find yourself writing methods named setupTypicalUserAccount(), setupOtherThingyForTest(), etc.

If DRYing up the tests seems like the only way to make the tests readable, the code under test probably doesn't have a useful or meaningful interface. For example, you may be tempted to create a factory method in your test that calls the three or four methods necessary to put your target object into a valid state. Put the factory method in the target class instead. In general, if you can remove duplication in tests by making the interface of the code under test more humane, do it. If code is difficult for your tests to use, it will be difficult for other client code to use too.

Of course, soemtimes DRYing up tests is a good idea. If you want to replace repetetive lists of assertions with your own assertion methods, go for it. There's nothing wrong with an assertUserAccountIsValid(Account account) method that contains half a dozen assertions. There are lots of times when DRYing up your tests really is a good idea. Just make sure you've got a good reason and are not running on instinct.

Think of your tests the same way you'd think of a user manual or other documentation. You wouldn't want to repeat yourself on every second page, but you also wouldn't want to eliminate duplication entirely from a user manual. You'd drive your users nuts. In the case of a user manual, readability trumps maintainability, and the same goes for your tests. (Of course, you want both readability and maintainability if you can get it.)

If you found it impossible to write a clear, concise user manual, you might start worrying about the user interface of the program itself. If you find it impossible to write clear, concise tests, start worrying about programming interface of the program itself. That's probably your best point of leverage.