Editor’s Note: Today we have a guest post from Marko Anastasov. Marko is a developer and cofounder of Semaphore, a continuous integration and deployment service, and one of Code Climate’s CI partners.
The act of writing a unit test is more an act of design than of verification. - Bob Martin
A still common misconception is that test-driven development (TDD) is about testing; that by adhering to TDD you can minimize the probability of going astray and forgetting to write tests by mandating that is the first thing we need to do. While I’d pick a solution that’s designed for mere mortals over one that assumes we are superhuman any day, the case here is a bit different. TDD is designed to make us think about our code before writing it, using automated tests as a vehicle — which is, by the way, so much better than firing up the debugger to make sure that every code connected to a certain feature is working as expected. The goal of TDD is better software design. Tests are a byproduct.
Through the act of writing a test first, we ponder on the interface of the object under test, as well as of other objects that we need but that do not yet exist. We work in small, controllable increments. We do not stop the first time the test passes. We then go back to the implementation and refactor the code to keep it clean, confident that we can change it any way we like because we have a test suite to tell us if the code is still correct.
Anyone who’s been doing this has found their code design skills challenged and sharpened. Questions like agh maybe that private code shouldn’t be private or is this class now doing too much are constantly flying through your mind.
The red-green-refactor cycle may come to a halt when you find yourself in a situation where you don’t know how to write a test for some piece of code, or you do, but it feels like a lot of hard work. Pain in testing often reveals a problem in code design, or simply that you’ve come across a piece of code that was not written with the TDD approach. Some smells in test code are frequent enough to be called an anti-pattern and can identify an opportunity to refactor, both test and application code.
Take, for example, a complex test setup in a Rails controller spec.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46
The controller action is technically not very long:
1 2 3 4 5 6 7 8 9 10 11 12 13
Notice how the extensive spec setup code basically led the developers to forget to write expectations that
Venue.enabled.find is called, or
LeaderboardDecorator.new is given a correct argument, for example. It is not clear if the assigned
@leaderboard comes from the assigned venue at all.
Trapped in the MVC paradigm, the developers (myself included) were adding up some deep business logic in the controller, making it hard to write a good spec and thus maintain both of them. The difficulty comes from the fact that even a one-line Rails controller method does many things:
1 2 3
That method is:
- extracting parameters;
- calling an application-specific method;
- assigning a variable to be used in the view template; and
- rendering a response template.
Adding code that reaches deep inside the database and business rules can only turn a controller method into a mess.
The controller above includes one
if statement with four conditions. A full spec, then, should include 15 combinations just for this one part of code. Of course they were not written. But things could be different, if this code was outside the controller.
Let’s try to imagine what a better version of the controller spec would look like, and what interfaces it would prefer to work with in order to carry its’ job of processing the incoming request and preparing a response.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39
Where did all the other code go? We’re simplifying the find logic by extending the model:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
if statements can be simplified as follows:
if logged_in?- variations on this can be decided in the view template;
if @leaderboard.present?- obsolete, the view can decide what to do if it is not;
- The rest can be moved to the decorator class under a new, more descriptive method.
1 2 3 4 5 6 7 8 9 10 11 12
This new method will help the view decide whether or not to render
@leaderboard.patron_stats, which we do not need to change:
1 2 3 4 5 6 7
The resulting controller method is now fairly simple:
1 2 3 4
The next time we work with this code, we might be annoyed that controller needs to know what is the right argument to give to a
LeaderboardDecorator. We could introduce a new decorator for venues that will have a method that returns a decorated leaderboard. The implementation of that step is left as an exercise for the reader. ;)
For further reading, check out Marko’s series about antipatterns in testing Rails applications on the Semaphore blog.