---
title: Let's Not
teaser: 'How using this popular RSpec technique can sometimes do more harm than good.

  '
tags: web,ruby,testing,good code
author: Joe Ferris
published_on: 2012-10-31
---

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:

```ruby
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](http://xunitpatterns.com/Obscure%20Test.html#General%20Fixture)
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](http://xunitpatterns.com/Obscure%20Test.html#Mystery%20Guest).
* It causes
  [Fragile Tests](http://xunitpatterns.com/Fragile%20Test.html#Fragile%20Fixture)
  by creating a complicated fixture that is difficult to maintain.
* It causes [Slow Tests](http://xunitpatterns.com/Slow%20Tests.html) 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](http://www.amazon.com/xUnit-Test-Patterns-Refactoring-Code/dp/0131495054/):

> 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:

```ruby
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:

```ruby
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:

```ruby
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:

```ruby
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](http://en.wikipedia.org/wiki/Factory_method_pattern):

```ruby
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.

### Next Steps & Related Reading

Detect emerging problems in your codebase with [Ruby Science][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!][ruby-science]

[ruby-science]: http://rubyscience.com?utm_source=giantrobots&amp;utm_medium=blog&amp;utm_campaign=remarket&amp;utm_term=testing
