Testing: High Implementation Specification Smell

At the end of my last blog entry, I touched briefly on the testing smell: High Implementation Specification. I got a fair amount of good feedback and based on several requests I decided it was worth creating a blog entry focused entirely on the smell.

Well written tests specify as little implementation detail as possible.

Developers new to mocking and stubbing tend to overuse the capabilities provided by mocking frameworks. I consider each call to a method of a mocking framework to be 1 implementation specification point. While using mocha I count a point for each of: stub, stub_everything, stubs, mock, expects, with, and returns. Any test that has an implementation specification score of 5 or more has a high implementation specification smell.

High Implementation Specification can stem from misusing mocking frameworks in various ways such as verifying parameters while stubbing methods, returning values that are never used, and specifying every interaction instead of using an object that will respond to any message.

Most mock frameworks provide the ability to verify the parameters sent to a mocked or stubbed method. When setting up a mock expectation, specifying the parameters can be useful for verifying interaction; however, when stubbing methods that are not the focus of the test, adding the additional parameter verification makes the test unnecessarily brittle.

The following code provides an example where the MainService notify method stub could verify the parameters, but the test is focused on testing the ReservationService. Given that the test is verifying the ReservationService there's no need for it to return a value and there's no need to verify the parameters sent to MaidService.notify. If the test did specify the parameters send to MaidService it would fail if those parameters changed. Since the test is verifying ReservationService, it would be disappointing if it failed due to a change in how MadService is called.

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

class ReservationService
  # implementation
end
class MaidService
  # implementation
end

class HotelRoom
  attr_accessor :booked
  def book_for(customer)
    reservation = ReservationService.reserve_for(customer, self)
    MaidService.notify(reservation)
  end
end

class HotelRoomTests < Test::Unit::TestCase
  def test_book_for_reserves_via_ReservationService
    room = HotelRoom.new
    ReservationService.expects(:reserve_for).with(:customer, room)
    MaidService.stubs(:notify)
    room.book_for(:customer)
  end
end

Returning a value from a mocked method can be necessary, but should be avoided in general for two reasons: Returning a value signifies to someone reading a test that the value is necessary for some reason. Returning a value that is later verified with an assertion can cause false positives.

In the above example, the ReservationService does not provide a return value because that value isn't used anywhere within the test. If you read the above test and saw that a return value was specified you would probably want to spend time finding where and why it was used. Worse, if the above test returned a value and then later asserted equality the test would pass, but the implementation could change the return value in the future and the test would not break.

At times a mock or stub must be exercised in various ways within one test due to how the original method was written. However, specifying all the interactions within one test leads to a test that breaks with every implementation change. A better solution is to utilize an object that can respond to anything and only specify the behavior that the test focuses on.

The following example uses a stub_everything object to ensure that the location and capacity setting methods can be called without being specified, but it also expects that the name is set correctly. Using a stub_everything also ensures that if more attributes are set in the future the test will continue to pass without any intervention.

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


class HotelTests < Test::Unit::TestCase
  def test_name_is_set_from_options
    hotel = stub_everything
    hotel.expects(:name=).with "Marriott"
    Hotel.stubs(:new).returns hotel
    Hotel.parse("Marriott||")
  end
end

class Hotel
  def self.parse(attributes)
    hotel = self.new
    hotel.name, hotel.location, hotel.capacity = attributes.split("|")
    hotel
  end
end