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.

class Customer
  def initialize(first_name, last_name)
    @first_name, @last_name = first_name, last_name
  end
  
  def append_name_to_file(file)
    file << @first_name << " " << @last_name
  end 
end

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.

class TestCustomer < Test::Unit::TestCase
  def test_customer_writes_full_name_to_file
    customer = Customer.new("Mark","Twain")
    File.open("Output.txt","w") do |file|
      customer.append_name_to_file(file)
    end
    File.open("Output.txt") do |file|
      assert_equal(file.read, "Mark Twain")
    end
    File.delete("Output.txt")
  end
end

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:

class TestCustomer < Test::Unit::TestCase
  def test_customer_writes_full_name_to_file
    begin
      customer = Customer.new("Mark","Twain")
      File.open("Output.txt","w") do |file|
        customer.append_name_to_file(file)
      end
      File.open("Output.txt") do |file|
        assert_equal(file.read, "Mark Twain")
      end
    ensure
      File.delete("Output.txt")
    end
  end
end

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?

class TestCustomer < Test::Unit::TestCase
  def test_customer_writes_full_name_to_file
    customer = Customer.new("Charles", "Dickens")
    fake_file = []
    customer.append_name_to_file(fake_file) 
    assert_equal(["Charles", " ", "Dickens"], fake_file)
  end 
end

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.

def append_name_to_file(file)
  file << "#{@first_name} #{@last_name}"
end 

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.

class TestCustomer < Test::Unit::TestCase
  def test_customer_writes_full_name_to_file
    customer = Customer.new("Jane", "Austen")
    fake_file = ""
    customer.append_name_to_file(fake_file) 
    assert_equal("Jane Austen", fake_file)
  end 
end

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.

def append_name_to_file(file)
  file.write "#{@first_name} #{@last_name}"
end 

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.

class Customer
  def initialize(first_name, last_name)
    @first_name, @last_name = first_name, last_name
  end
  
  def full_name
    "#{@first_name} #{@last_name}"
  end 
end

Now it’s straightforward to write a test.

class TestCustomer < Test::Unit::TestCase
  def test_customer_has_full_name
    customer = Customer.new("John", "Milton")
    assert_equal("John Milton", customer.full_name)
  end
end

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.

Happy coding.

Share on: Twitter, Facebook, Google+ or use the Permalink