Let's Not

RSpec is an excellent test framework with a large community and an active team of maintainers. It sports a powerful DSL that can make testing certain things much easier and more pleasant.

However, there are a few features of RSpec’s DSL that are frequently overused, leading to an increase in test maintenance and a decrease in test readability.

Let’s look at an example from factory_bot’s test suite and see how we can improve it by favoring plain Ruby methods over DSL constructs:

describe FactoryBot::EvaluatorClassDefiner do
  let(:simple_attribute) {
    stub("simple attribute", name: :simple, to_proc: -> { 1 })
  }
  let(:relative_attribute) {
    stub("relative attribute", name: :relative, to_proc: -> { simple + 1 })
  }
  let(:attribute_that_raises_a_second_time) {
    stub(
      "attribute that would raise without a cache",
      name: :raises_without_proper_cache,
      to_proc: -> { raise "failed" if @run; @run = true; nil }
    )
  }
  let(:attributes) {
    [
      simple_attribute,
      relative_attribute,
      attribute_that_raises_a_second_time
    ]
  }
  let(:class_definer) {
    FactoryBot::EvaluatorClassDefiner.new(
      attributes,
      FactoryBot::Evaluator
    )
  }
  let(:evaluator) {
    class_definer.evaluator_class.new(
      stub("build strategy", add_observer: true)
    )
  }

  it "adds each attribute to the evaluator" do
    evaluator.simple.should eq 1
  end

  it "evaluates the block in the context of the evaluator" do
    evaluator.relative.should eq 2
  end

  # More tests
end

A General Fixture is declared at the top. The fixture is then reused and augmented by each test to create the necessary setup. The examples (the it blocks) don’t declare any test setup; instead, they reference relevant portions of the existing fixture.

This approach causes a number of issues:

It obscures each test by introducing a Mystery Guest.
It causes Fragile Tests by creating a complicated fixture that is difficult to maintain.
It causes Slow Tests by creating more data than is necessary in each test.

Will our mystery guest please leave

Addressing the Mystery Guest issue solves the largest concern: readability. A mystery guest causes obscure tests. Gerard Meszaros defines the Mystery Guest in his xUnit Patterns:

The test reader is not able to see the cause and effect between fixture and verification logic because part of it is done outside the Test Method.

Here are some examples from this test suite:

it "adds each attribute to the evaluator" do
  evaluator.simple.should eq 1
end

it "evaluates the block in the context of the evaluator" do
  evaluator.relative.should eq 2
end

Without context, the reader has no idea what’s happening in this test, and the example description can’t really help. By parsing out the large fixture above, the reader can determine what’s going on, but correlating the fixture and test is slow and error-prone.

Let’s start by in-lining the fixture for this example:

it "evaluates the block in the context of the evaluator" do
  simple_attribute =
    stub("simple attribute",   name: :simple, to_proc: -> { 1 })
  relative_attribute =
    stub("relative attribute", name: :relative, to_proc: -> { simple + 1 })
  attribute_that_raises_a_second_time =
    stub("attribute that would raise without a cache",
          name: :raises_without_proper_cache,
          to_proc: -> { raise "failed" if @run; @run = true; nil })

  attributes = [
    simple_attribute,
    relative_attribute,
    attribute_that_raises_a_second_time
  ]
  class_definer = FactoryBot::EvaluatorClassDefiner.new(
    attributes,
    FactoryBot::Evaluator
  )
  evaluator = class_definer.evaluator_class.new(
    stub(
      "build strategy",
      add_observer: true
    )
  )

  evaluator.simple.should eq 1
end

The test continues to pass. Looking through the expected result, we can see that some data isn’t actually used in this scenario. Let’s remove it:

it "adds each attribute to the evaluator" do
  simple_attribute =
    stub("simple attribute",   name: :simple, to_proc: -> { 1 })

  attributes =
    [simple_attribute]
  class_definer = FactoryBot::EvaluatorClassDefiner.new(
    attributes,
    FactoryBot::Evaluator
  )
  evaluator = class_definer.evaluator_class.new(
    stub("build strategy", add_observer: true)
  )

  evaluator.simple.should eq 1
end

Now let’s in-line the fixture and remove unrelated data for the second example:

it "evaluates the block in the context of the evaluator" do
  simple_attribute =
    stub("simple attribute",   name: :simple, to_proc: -> { 1 })
  relative_attribute =
    stub("relative attribute", name: :relative, to_proc: -> { simple + 1 })

  attributes =
    [simple_attribute, relative_attribute]
  class_definer = FactoryBot::EvaluatorClassDefiner.new(
    attributes,
    FactoryBot::Evaluator
  )
  evaluator = class_definer.evaluator_class.new(
    stub("build strategy", add_observer: true)
  )

  evaluator.relative.should eq 2
end

Now that we’ve in-lined these two fixtures, there’s obviously a lot of duplicated setup logic. Let’s extract all that to a few factory methods:

it "adds each attribute to the evaluator" do
  attribute = stub_attribute(:attribute) { 1 }
  evaluator = define_evaluator(attributes: [attribute])

  evaluator.attribute.should eq 1
end

it "evaluates the block in the context of the evaluator" do
  dependency_attribute = stub_attribute(:dependency) { 1 }
  attribute = stub_attribute(:attribute) { dependency + 1 }
  evaluator = define_evaluator(attributes: [dependency_attribute, attribute])

  expect(evaluator.attribute).to eq 2
end

def define_evaluator(arguments = {})
  evaluator_class = define_evaluator_class(arguments)
  evaluator_class.new(FactoryBot::Strategy::Null)
end

def define_evaluator_class(arguments = {})
  evaluator_class_definer = FactoryBot::EvaluatorClassDefiner.new(
    arguments[:attributes] || [],
    arguments[:parent_class] || FactoryBot::Evaluator
  )
  evaluator_class_definer.evaluator_class
end

def stub_attribute(name = :attribute, &value)
  value ||= -> {}
  stub(name.to_s, name: name.to_sym, to_proc: value)
end

Once we convert the remaining examples, we can delete the let statements that created the general fixture.

And the winner is…everyone

These converted examples are greatly improved:

They’re easier to read, because all the actors referenced from the verification step are declared in the setup step within the it block.
They’re less brittle, because each example only specifies the information it needs.
They’re faster, because each example is running with a smaller data set.

It turns out that removing the Mystery Guests also solved our other complaints with these tests.

An added benefit is that the factory methods we created are easier to reuse throughout the test suite, whereas let statements are too specific to the examples for each example group. In time, this approach will make the entire test suite easier to maintain.

Until you need to break out the big guns like shared examples, avoid DSL constructs like subject, let, its, and before. Stick to your old friends: variables, methods, and classes.

Detect emerging problems in your codebase with Ruby Science. We’ll deliver solutions for fixing them, and demonstrate techniques for building a Ruby on Rails application that will be fun to work on for years to come.

Grab a free sample of Ruby Science today!

Will our mystery guest please leave

And the winner is…everyone

Next Steps & Related Reading

Sign up to receive a weekly recap from thoughtbot