Our Blog

Ongoing observations by End Point people

Tests are contracts, not blank checks

By Steven Jenkins · September 14, 2009

Tags: rails

Recently, I wrote up a new class and some tests to go along with it, and I was lazy and sloppy. My class had a fairly simple implementation (mostly a set of accessors, plus a to_s method). It looked something like this:

class Soldier
  attr_accessor :name, :rank, :serial_number
  def initialize(name,rank,serial_number)
    @name = name
    @rank = rank
    @serial_number = serial_number
  end

  def to_s
    "#{name}, #{rank}, #{serial_number}"
  end
end

I had been trying to determine the essential attributes of the class (e.g., what are the minimal elements of this class? should I have a base class, then sub-class it for the various differences, or should I have only a single class that contains everything I need?)

As a result of the speculative nature of the development, my tests only included a few of the attributes.

What's wrong with that?

On the surface, there is nothing technically wrong with skipping accessor tests: after all, testing each accessor individually is really testing Ruby, not the code I wrote. Another excuse I made is that testing each individually is very non-DRY - the testing code itself has lots of duplication.

The problem is that the set of tests should be considered a contract between the class writer and the outside world. By not including the correct and complete list of accessors, I left out important information; it's a check, already signed by the class developer, but with the amount left blank.

I've seen some code solve the non-DRY-ness problem like the following:

class Soldier
  Attributes = [:name, :rank, :serial_number]
  Attributes.each {|attr| attr_accessor attr}
  ...

then testing code of:

  Attributes.each do |attr|
    it "should have an accessor for #{attr}" do
      ...

That let's the testing code be nice and compact; simply load in the class, then iterate over the Attributes to verify that the accessors are present.

From a tests are contracts standpoint, this approach is terrible, though, perhaps even worse than the original, incomplete set of tests I had written. All the reader of the tests learns is that there is an array of attributes; the reader has to go look at the implementation itself to see what those attributes are.

Better is to use an anonymous array in the test, duplicating the attribute list; i.e.,

  [:name,:rank,:serial_number].each do |attr|
    it "should have an accessor for #{attr}" do
      ...
    end
  end

That seems to be a good balance between keeping tests as contacts yet keeping them DRY.

Comments

Archive