Testing: One expectation per test

In a previous entry I discussed why I prefer One Assertion Per Test. In that entry I give a few state based examples, but didn't address the topic of mocks. This entry is going to focus on how you can use mocks while maintaining focus on maintainability.

Let's start with an example.

require 'test/unit'
require 'rubygems'
require 'mocha'

module Expectations
  class SuiteRunner
    attr_accessor :suite
  
    def initialize
      self.suite = Expectations::Suite.new
    end
  
    def self.suite_eval(&block)
      self.new.suite.instance_eval &block
    end
  end
end

class ObjectTests < Test::Unit::TestCase
  def test_suite_eval_evals_the_block_in_the_context_of_the_suite
    suite = mock
    suite.expects(:instance_eval)
    runner = mock
    runner.expects(:suite).returns(suite)
    Expectations::SuiteRunner.expects(:new).returns runner
    Expectations::SuiteRunner.suite_eval {}
  end
end

The SuiteRunner class is fairly straightforward, it's simply delegating the block on to the instance_eval method on the suite attribute, which is an instance of the Suite class. Even though the class isn't doing anything very interesting the test isn't completely simple. Due to the chain of method calls you need to set up 2 mocks and a total of 3 expectations. While the test isn't unmaintainable, if the description (the method name) isn't kept up to date you could easily lose the intent of the test.

A step in the right direction toward making this test more readable is to introduce a few stubs. However, before we decide what to stub we need to decide what this test is trying to verify. Given the description it seems that eval'ing in the context of the suite is what is being tested, thus the suite mock should probably remain. Since the suite mock is the focus of the test, the rest of the mocks are probably better expressed as stubs. The test can be written more concisely now that we've chosen to use stubs.

class ObjectTests < Test::Unit::TestCase
  def test_suite_eval_evals_the_block_in_the_context_of_the_suite
    suite = mock
    suite.expects(:instance_eval)
    Expectations::SuiteRunner.stubs(:new).returns stub(:suite => suite)
    Expectations::SuiteRunner.suite_eval {}
  end
end

With one mock and one expectation the test expresses it's purpose. Code should not only express how it works, but also why it's been written in a particular way. The new test not only executes, but it conveys the intent of the test (even if the description becomes stale). Converting that test was easy, but tests that have behavior expectations and state based assertions can be a bit more complicated to clean up.

require 'test/unit'
require 'rubygems'
require 'mocha'

class ReservationService
  # implementation
end
class MaidService
  # implementation
end
class VipService
  # implementation
end

class HotelRoom
  attr_accessor :booked
  def book_for(customer)
    reservation = ReservationService.reserve_for(customer)
    self.booked = true
    MaidService.notify(reservation)
    VipService.notify(reservation) if reservation.for_vip?
    reservation.confirmation_number
  end
end

class HotelRoomTests < Test::Unit::TestCase
  def test_book_for_returns_confirmation_number
    customer = mock
    room = HotelRoom.new
    reservation = mock
    reservation.expects(:for_vip?).returns true
    reservation.expects(:confirmation_number).returns 1979
    ReservationService.expects(:reserve_for).with(customer).returns(reservation)
    MaidService.expects(:notify).with(reservation)
    VipService.expects(:notify).with(reservation)
    assert_equal 1979, room.book_for(customer)
    assert_equal true, room.booked
  end
end
# >> Loaded suite -
# >> Started
# >> .
# >> Finished in 0.001121 seconds.
# >> 
# >> 1 tests, 7 assertions, 0 failures, 0 errors

There's a lot going on in this test. Test::Unit even reports that 7 assertions have been met. Unfortunately, with that much going on in a test, it's hard to tell what the original intent of the test was. Even worse, it would be hard to change anything in the book_for method without breaking this test. The test is completely tied to the implementation and if that implementation changes you'll break one or more of the assertions within the test. What really bothers me about this test is the ability to break "one or more" assertions. If you change the reserve_for method to also take the hotel as an argument the test will immediately stop executing with the following error.

# >> Loaded suite -
# >> Started
# >> F
# >> Finished in 0.001728 seconds.
# >> 
# >>   1) Failure:
# >> test_book_for_returns_confirmation_number(HotelRoomTests)
# >>     [(eval):1:in `reserve_for'
# >>      -:18:in `book_for'
# >>      -:36:in `test_book_for_returns_confirmation_number']:
# >> #<Mock:0x50f2a8>.reserve_for(#<Mock:0x50fa50>, #<HotelRoom:0x50fadc>) - expected calls: 0, actual calls: 1
# >> Similar expectations:
# >> #<Mock:0x50f2a8>.reserve_for(#<Mock:0x50fa50>)
# >> 
# >> 1 tests, 0 assertions, 1 failures, 0 errors

Knowing that an assertion failed is good, knowing that other assertions might also fail when you fix the first problem is not so good. The problem with this test is that it's verifying 7 different things. This can most likely be resolved by writing several different tests that all focus on one thing. (There are several tests that can be written, here are 5 examples)

require 'test/unit'
require 'rubygems'
require 'mocha'

class ReservationService
  # implementation
end
class MaidService
  # implementation
end
class VipService
  # implementation
end

class HotelRoom
  attr_accessor :booked
  def book_for(customer)
    reservation = ReservationService.reserve_for(customer, self)
    self.booked = true
    MaidService.notify(reservation)
    VipService.notify(reservation) if reservation.for_vip?
    reservation.confirmation_number
  end
end

class HotelRoomTests < Test::Unit::TestCase
  def test_book_for_reserves_via_ReservationService
    room = HotelRoom.new
    ReservationService.expects(:reserve_for).with(:customer, room).returns(stub_everything)
    MaidService.stubs(:notify)
    room.book_for(:customer)
  end
  
  def test_book_for_notifys_MaidService
    reservation = stub_everything
    MaidService.expects(:notify).with(reservation)
    ReservationService.stubs(:reserve_for).returns(reservation)
    HotelRoom.new.book_for(:customer)
  end
  
  def test_book_for_notifys_VipService_if_reservation_if_for_vip
    reservation = stub_everything(:for_vip? => true)
    VipService.expects(:notify).with(reservation)
    MaidService.stubs(:notify)
    ReservationService.stubs(:reserve_for).returns(reservation)
    HotelRoom.new.book_for(:customer)
  end
  
  def test_book_for_sets_booked_to_true
    room = HotelRoom.new
    MaidService.stubs(:notify)
    ReservationService.stubs(:reserve_for).returns(stub_everything)
    room.book_for(:customer)
    assert_equal true, room.booked
  end
  
  def test_book_for_returns_confirmation_number
    reservation = stub_everything
    reservation.stubs(:confirmation_number).returns 1979
    ReservationService.stubs(:reserve_for).returns(reservation)
    MaidService.stubs(:notify)
    assert_equal 1979, HotelRoom.new.book_for(:customer)
  end
end
# >> Loaded suite -
# >> Started
# >> .....
# >> Finished in 0.002766 seconds.
# >> 
# >> 5 tests, 5 assertions, 0 failures, 0 errors

The above tests take more lines of code than the original, but they are far more maintainable. You can change the implementation of the book_for method in various ways and only break the tests that are relevant to the change. The tests have become more robust. They've also become more readable because they express what their focus is. The tests that have a mock with an expectation are written to test a behavior that is expected. The tests that have an assertion are written to statefully verify that the object has been set as expected. When the implementation does inevitably change the tests can communicate on their own what the original author intended.

Writing tests in this way is actually quite easy if you follow a few simple suggestions.

• If your test has an assertion, do not add any mock expectations (instead use stubs for any methods that need to be changed for the test).
• If you add a mock expectation you probably don't need it to return a meaningful value (since you won't be verifying that value).
• If you already have a mock expectation you should use the stubs method for any other method where you don't want to use the implementation.
• Do not use .with when using a stub unless you need to. If you are stubbing, you don't need to verify the arguments.
• When returning stubs, prefer stub_everything so that additional calls to the stub will not cause unnecessary exceptions to be raised.

The above suggestions focus on creating tests that assert (or expect) one thing at a time and specify as little other implementation as possible. Every expectation, .with, and .returns statement is specifying implementation. The less implementation you express in a test, the more that test becomes.

I consider each call to stub (or stub_everything), stubs, mock, expects, with, and returns to be 1 implementation specification point. Any test that has an implementation specification score of 5 or more has the high implementation specification smell. As with all smells, everything might be okay, but I'm going to take the extra time to see if anything can be broken up by moving responsibilities or creating smaller methods.

By limiting your tests to one assertion or one expectation you create tests that express their intent. By creating tests that specify as little implementation as possible you reduce the amount of noise taking away from the intent of the test. An additional consequence of creating tests that focus on intent is that they specify less and are therefore more robust.