While I was reading the Ruby Pickaxe book I came across an example of Unit Testing which – in my opinion – is testing implementation rather than behavior. This particular example was used to explain Duck Typing in Ruby, but I think it illustrates that it’s pretty easy to write tests which end up testing the wrong thing.
A Simple Example
In this example we have a simple Customer class.
Let’s go ahead and write a test for the append_name_to_file method. (I’m not a fan of doing testing this way – we really should write the test first, but let’s ignore that for a moment)
One way to test this would be to create an actual temporary file and pass that to the Customer object.
If I run the test I get the familiar success output. Hurrah!
That’s an ugly test
Ok, so this test isn’t all that pretty. For one thing, there’s quite a lot of work going on just in order to get the test to run and to make sure the file we create is removed at the end. Even worse, when the test fails the final line of the test will not get executed and the temporary file is never removed! Using my knowledge of Exceptions I can fix that easily enough:
I’ve taken care of the file being left behind when the test fails, but my test code is still pretty ugly. The author of the Pickaxe book now argues that we don’t really need to pass a file to the append_name_to_file method – we simply need to pass something which acts like a file. How about using a simple array?
The test passes, and the code is much simpler. Are we done?
The Implementation changes
Not quite. Let’s say I’m working on the Customer class and I decide that I don’t really this C++ syntax – I would prefer to do a simple formatted string.
Notice that I didn’t change the behavior, but when I run the test…
Uh-oh. The problem stems from the fact that we were testing the underlying implementation details, not the behavior. I didn’t check that the Customer object can write to a file, I tested that the Customer object uses the « method to write 3 different strings to the file object I pass to it.
One way to fix this would be to stop using an array and pass in a string. Let’s try that.
Once again our test passes, and the code is even simpler. Are we done? Well, let’s see – another developer is now working on the code and he/she is of the strong opinion that the only correct way to write to a file is to use the File#write method.
And we’re back to square one. Once again, I didn’t change the behavior of the method, but changing the implementation has broken the test. This is an example of fragile unit tests – tests which break when the code under test is still functionally correct. This is often a symptom of poorly designed tests.
What’s the Answer?
So what’s the answer? Should we go back to using a temporary file? (Using a temporary file worked for all three implementations) Not really. I think in this case the problem is actually poor design and we’re struggling to test the code as a result. From an object-orientated perspective it doesn’t really make sense for a Customer object to have an append_name_to_file method. Any time you see a method taking an object and then doing something to it, it’s usually a code smell.
Let’s try and refactor this code. Instead of the Customer writing something to a file, I think it makes more sense for a Customer to have a method that returns their full name.
Now it’s straightforward to write a test.
That’s only ONE solution
Ok, what should we do when we really do need to test interaction with a file?
Unfortunately, there is no silver bullet or golden rule here. If there was, this wouldn’t be an issue. We could possibly mock the behavior or maybe we really do need to use a temporary file. The only golden rule (in my opinion) is that we need to make sure we’re testing the right thing – behavior, not implementation. This leads to better, more maintainable tests – which will pay massive dividends in the long run.