Complex Logic in Unit Tests

There is a rule somewhere in the land of best practices that says you should avoid putting logic in Unit tests. For example, if you were making a suite of unit tests that had some common logic then you wouldn't create a method to reuse code because that would lead to the test becoming harder to read and understand.

The idea is that unit tests should read like a behavioural specification. The test should be so simple and clear that it tells the reader exactly how the function should behave without having to poke around inside different functions or crank a for loop mentally to guess what happens.

Basically,

1. Don't worry so much about Don't Repeat Yourself because when you put things into variables, constants or functions you make people hunt down these pieces of code and it makes the test harder to read.
2. Don't use fancy flow control like loops if you can possible avoid it because it means the user has to work out what the flow control is doing and that hurts readability.

I've tried this approach for several years and I can tell you that's it's all crap. The ultimate result is an unmaintainable mess.

I feel I've heard these unit test arguments before. Somewhere in our codebase there's a pile of older C/C++ code. It contains functions that are very long; pages and pages of source code per function. I remember this coding style being considered best practice by some, not just for performance but because it made the code easier to read precisely for the same reasons that are now being used for unit tests. In retrospect, it doesn't make anything easier to read and it makes code really hard to maintain.

Code is code. The reasoning behind Don't Repeat Yourself doesn't magically get suspended because you're writing a unit test. Whenever you're writing code you're constantly building abstractions that help you to solve the problem you're working on. Things like Don't Repeat Yourself allow you to find and leverage these cases of code reuse into tiny frameworks. Unit tests are no exception to this. If your code is maintainable and well factored it will be intrinsically readable because you'll have 5 well chosen lines of code instead of a page of boilerplate.

Here's an article about not putting logic in tests from Google's testing blog. In this article the example the author chooses is the following:

@Test public void shouldNavigateToPhotosPage() {
  String baseUrl = "http://plus.google.com/";
  Navigator nav = new Navigator(baseUrl);
  nav.goToPhotosPage();
  assertEquals(baseUrl + "/u/0/photos", nav.getCurrentUrl());
}

In this test there is an error. Can you spot it? Ok, let's write the test without the code reuse:

@Test public void shouldNavigateToPhotosPage() {
  Navigator nav = new Navigator("http://plus.google.com/");
  nav.goToPhotosPage();
  assertEquals("http://plus.google.com//u/0/photos", nav.getCurrentUrl()); // Oops!
}

The error is now obvious. Hurrah for no logic in tests!

However, what if you didn't spot it? What if the error was "photso" instead of a double slash? I don't know about you, but there are a bunch of words I always type wrong. Things like spelling height as "hieght" or "heigth". What if the URL was http://www.heigth.com/? In that case I would be testing a typo... but only sometimes.... If you're not reusing then you're duplicating so this error is going to be in a bunch of places and you have to find and fix them all.

For that matter, aren't there libraries to append a path on a base url already? Modifying example #1 is easier than example #2 because in the first case we know that the base URL is common text because it's explicitly marked and check by the compiler right there in the code. In the second example, how do we know what the text in assertEquals() is supposed to represent without doing a tedious, manual character by character comparison?

If logic in unit tests has you concerned about making errors then don't worry. You'll make plenty of errors no matter how you do things. In one case it's a single error in one part of the code has to be code reviewed once and fixed once. If you repeat yourself it's like the 1970s and code is duplicated all over the place with slight differences it has to be fixed many times and code reviewed until your eyes bleed. If logic in unit tests really bothers you, make your complex logic into re-usable functions and unit tests that. Yes, it's a little "meta" but at least you're being honest about the complexity instead of copying around madness without worry.

To reiterate, best practices like "Don't Repeat Yourself" came into existing to make reading and maintaining all code easier. I can tell you from personal experience that ignoring it results in difficult to maintain tests. 

What you really need to do is make what you're doing in your test as clear as possible - don't hide your function's inputs and outputs. I can't go into detail here but here's what the test would look like if I wrote it:

@Test public void shouldNavigateToPhotosPage() {
  mNav.goToPhotosPage(); // mNav is a member defined in the fixture
  assertEquals(BASE_URL.withPath("/u/0/photos").toUrlString(), 
               mNav.getCurrentUrl());
}

In the example above I've created a test fixture (not displayed). An mNav member variable is built in the test fixture's setUp(). The mNav is always initialized with a base URL. I've made a BASE_URL object using an existing library. In this library BASE_URL is immutable but has several methods that return a mutated (also immutable) object.

The net result is you can see that calling goToPhotosPage() should go to the "/u/0/photos" path.

If mNav had a getBaseUrl() and it was tested elsewhere then you might use that instead of BASE_URL constant since it's more explicit about what the expectation is.

Don't worry about logic in unit tests - that is a red herring -worry about making what you're testing clear to the reader.