Friday, October 24, 2014

A TDD story about a test that started "green"

This is a story is about the value of making sure you start with a test that's actually failing.

First a little context. I've been sharpening my TDD-fu and design-fu with a pet project: implementing the rules for Monopoly, the venerable and venerated game from Parker Brothers. (For some interesting background reading, check out the original rules and the Wiki page describing Monopoly's history.) The game's domain model and rules are fairly rich, making this exercise reasonably challenging and thought-provoking; and the rules are well defined, so the stories are easy to find and are clearly constrained. (There's little chance of me going off the rails chasing an interesting subset of the problem domain.)

Someday, ambition and time permitting, I might write a whole series of posts on this. Today I'm going to describe a particular situation and how it highlights for me the value of starting with a "red bar" when a new test is written.

This story begins with a new test named shouldNotAllowTurnsWhenGameIsOver. Its intent is to ensure that the winning player can't continue to take turns when game is over. There is a winner when all but one player is bankrupt.

I already had a test that ensured bankrupt players couldn't take a turn, and another that verified a winner was identified when all other players were bankrupt. But I didn't have any specific logic in the game implementation to ensure that the winning player couldn't take a turn after the game was over. So I was a little surprised when I ran my new test and it passed.

When I write a test I'm saying "I want this to happen when I do that". When I run that new test and it tells me I'm already getting this to happen before I've written any production code, there's something wrong.

A test that starts out green is a bad sign: first, you don't know that it can ever be red, and thus it can't be trusted to do anything useful. But equally importantly, it's a signal that either something in the production code is accidentally resulting in the test passing or the test is not valuable and ought to be scrapped or rewritten.

My initial thought was to rewrite the test to make it go red, but I couldn't figure out a way to change it that didn't rely on implementation details. So I dug into the code.

A little investigation revealed the answer. In the GameState class I keep track of which player will take a turn next in an integer field named nextPlayer. The GameState#getNextState() method figures out what the next state is when a turn is taken, and constructs an instance of the correct type:

       GameState getNextState() {
           if (game.hasWinner()) {
               return new GameOverGameState();
           return new TakingTurnsGameState(nextPlayer);

Notice the difference in the constructor calls to GameOverGameState and TakingTurnsGameState. The GameOverGameState constructor doesn't have a nextPlayer parameter, so its nextPlayer field is initialized to 0.

I fixed this by refactoring to always call the GameState constructor that takes a nextPlayer parameter. Once I did this, the test turned red and I could continue normally, implementing the code to make it green.

I also wanted to dig a little deeper. After a little root cause analysis (walking backwards through my commits) I decided I had made two mistakes earlier. The first was implementing code in a way that required me to know about (or assume) a future requirement. (I knew that once the game was over no more turns would be taken, so I assumed the value of nextPlayer didn't matter when creating GameOverGameState.) The second error was not ensuring that all fields of the GameState class were properly initialized on construction. (Don't leave your fields uninitialized, kids!)

Following a good TDD process helps find defects early and leads to a better design by making it safe and easy to refactor code. But it is very much not an automatic process. You have to pay attention and be diligent and conscientious. I was able to find and fix a bit of troublesome code today by paying attention when a new test was passing.

tl;dr, if you get a green bar first when writing a test, pay attention: there might be something smelly in your code.

No comments: