Don’t assert return types

Last Friday, during a mob session we were testing a data object that represented a room. (We were doing an Advent of Code puzzle.) Below is the full text of the class, but you don’t need to understand what it does. It’s an immutable object that’s constructed via a factory method.

Room = Data.define(:encrypted_name, :sector_id, :checksum) do
  def self.from(room_code)
    /
      (?<encrypted_name>.*)-
      (?<sector_id>\d+)
      \[(?<checksum>\w+)\]
    /x =~ room_code

    new(encrypted_name, Integer(sector_id), checksum)
  end

  def decoy?
    checksum != encrypted_name
      .delete("-")
      .chars
      .tally
      .sort_by { |a, b| [-b, a] }
      .map(&:first)
      .take(checksum.size)
      .join
  end
end

As we began to write tests to help us flesh out this class, someone suggested that we test that Room.from returns a Room object. Ruby is duck-typed, so I’d been taught early on that you don’t right tests like expect(subject).to be_a Room. Let’s explore why that is.

Tests describe how to use an object

When done well, tests can serve as documentation describing how consumers of the object should interact with it. It’s not the primary goal of testing, but it’s worth considering when writing tests. What does a type assertion on the return value of our factory method tell us about the method? Not much, especially if our factory method never returns any other kind of object.

Instead, the return value of the factory method should be tested for the behaviours it exposes. If we change the structure of the code later so that it returns some other kind of object, we want our tests to check that this new object works the way our consumers will expect it to. We don’t really care about its class.

Test in terms of activities

Tests should be written in terms of the things that you do with the object, not in terms of individual methods. This contributes both to tests as documentation, but also gives better test feedback. Once tests are committed to a codebase, they only provide value with the feedback they give how and when they fail. By testing in terms of activities, we tell the person reading the test failure in the future exactly what behaviour of the object is no longer working.

This object is constructed from a string and exposes information about the data that string encodes. The activities are reading the sector ID and testing whether the room is a decoy. We can write tests in terms of these activities, which depend on constructing the object through the factory method.

If our factory method returns something other than an instance of Room, we only care if it doesn’t expose the right interface to us. An additional type check in our tests would also fail in that case, but it will fail on intentional changes to the return type that don’t break the behaviour we actually care about.

This is an example of the kind of test that puts people off testing. It doesn’t make the code easier to change; it makes it harder. It’s just another thing to update if we want to change the structure of the code.

Tests are a design tool

In London-style TDD, tests are a design tool. The incremental Red-Green-Refactor process surfaces coupling issues and promotes cohesive objects. Test smells can lead us towards better designs.

Unfortunately, all a type assertion does is cement a piece of the design in place. It captures an implementation detail and creates resistance if we want to change it. Sometimes tests like this can be useful as stepping stones when you don’t know what comes next, but they can be removed once the next test is written.

The Result

Instead of trying to break down the class and instance methods into a series of separate describe blocks and testing them individually, you can write the tests more holistically.

Rspec.describe Room do
  subject(:room) { Room.from(room_code) }

  let(:room_code) { "aaaaa-bbb-z-y-x-123[abxyz]" }

  it "exposes the sector_id" do
    expect(room.sector_id).to eq 123
  end

  it { is_expected.not_to be_decoy }

  context "when checksum isn't the five most common letters from most to least" do
    let(:room_code) { "totally-real-room-200[decoy]" }

    it { is_expected.to be_decoy }
  end
end

These tests make it relatively clear that you construct a Room from a “room code” and that it exposes the “sector ID”. A room is a “decoy” if the the checksum isn’t the five most common letters from most to least. When any of them fail, they point you to the behaviour that broke without any extra tests clogging up the output.

A bad test is worse than no test at all

Testing is a skill, something that the most popular empirical study into TDD ignored. Poorly written tests that over-specify the functionality of objects make systems harder to change, leading to frustration with the practice of testing and TDD itself.

If an organization or project wants to be see the value of testing in the long term, they need to ensure that test suites are kept lean, that tests are structured to aid developers in understanding them, and that failures provide useful feedback.

Too many organizations suffer from poorly written tests suites, making “updating tests” into a chore that developers slog their way through after making the change they wanted; a mandatory rite that’s required to get changes merged. This only exacerbates the issue, leading to test suites that are deeply coupled to implementations and provide very little value to the developers who are forced to drag them forward.