Just wrapped up a pretty hefty refactor, involving plenty of testing responsibility shuffling. Having encountered a variety of testing styles, I can tell you now that I largely agree with Joe Ferris on thoughtbot. When you set the bar extra high for pulling repetition out in your test suite, it is an order of magnitude easier to change. And I can point to (1) a good chunk of experience in both the DRY and non-DRY camp, (2) the opinions of the majority of my current and former colleagues, and (3) my own research around the net.

There are absolutely times when DRY tools should be used, but following the actual code path that the spec is testing is work enough. Why force yourself to jump around in the spec too? Developers constantly seek perfection, which is where the DRY allure comes from. But I'm going to attempt to convince you to put that on the backburner when it comes to tests.

This article is geared toward small-to-medium size applications. At enterprise scale, testing is a whole different beast and quite probably its own department.

Ease of reading & refactoring

I'll turn your attention to this excerpt from the xUnit Patterns website by Gerard Meszaros:

When either the fixture setup and/or the result verification part of a test depends on information that is not visible within the test and the test reader finds it difficult to understand the behavior that is being verified without first having to find and inspect the external information, we have a Mystery Guest on our hands.

The idea of the "mystery guest" tells us that a test is not easy to read when we have to hunt for a piece of it. My experience aligns with this conclusion. For instance, what if you came across this in a review?

include_context 'creates a car', 'hubcaps' do
  before do
    expect(VehicleSettings).to receive(:get).with(:wheel_size) { :r15 }
    expect(VehicleSettings).to receive(:get).with(:hubcap_type) { :standard }
  end
  let(:expected_value) { 42 }
end

You have to find the shared_context and follow not only what's passed, but also what method expectations are being overridden in the passed block, and how expected_value is used. When we have to jump to references in several places, in and out of the current file, our brain gears turn extra hard to fully understand the example. It makes much more sense to centralize dependencies of the test as much as possible.

Ease of executing

You might think extracting multiple examples could make sense sometimes. For example, testing a success and a failure case for a handful of conditions of data.

I strongly suggest you do not. At the end of a dev cycle, we run a test that covers the corresponding changes. Whether it's your tooling or a direct command, the spec runs on a line number. If that is a reference to multiple specs, you'll run all of those at once. That distracting and confusing when working against a single point of failure. If half of your DRYed examples fail, and half succeed, you'll wind up with noise in your console output.

Working against yourself

Have you ever seen an example such as:

it 'does something' do
  subject.reload
  subject.update!(another_attribute: 'foo')

  expect(subject.something_type).to eq 'red'
end

These kinds of last-minute alterations are commonly the result of overly DRY tests in that rather than tweaking clever code, sometimes developers make the change just before the assertion.

Before we're even asserting, we're altering something we created ourselves. As a developer worth your salt, in an environment such as your test suite, which stands between you and shipping all of your code, how do you justify consuming a bunch of resources, then immediately consuming a tad more to change the outcome of the first effort? You can't possibly tell me that's not smelly.

Aim to create your test data properly the first time. Avoid extra work. We want our test suites to be as efficient as possible.

Better performance & runaway factories

It's also far too easy to implicitly do extra work.

Take this:

describe 'properties of a car' do
  let(:car) { create(:car) }

  it 'has wheels' do
    expect(car.wheels).to be_present
  end

  it 'has an engine' do
    expect(car.engine).to be_present
  end

  it 'has doors' do
    expect(car.doors).to be_present
  end
end

Idiomatic RSpec at first glance, but suppose that the :car factory pulls in lots of dependencies. The way this is written, that expensive call will happen three times. Contrast with:

describe 'properties of a car' do
  let(:car) { create(:car) }

  it 'has wheels, an engine, and doors' do
    expect(car.wheels).to be_present
    expect(car.engine).to be_present
    expect(car.doors).to be_present
  end
end

This only builds the test data once (and is less LOC!). Anyone who has spent considerable time with factories knows they require continuous optimzation efforts to keep efficient. By their nature, they become heavy and bloated as you add more associations and callbacks. For this article's point, however, we can reduce calls to heavier factories via consolidating examples where they don't need fresh new data every time.

For your test suite, even a slight performance win is worth it because those savings will likely compound over many usages.

Avoiding duplicated testing

As somewhat of a sidenote, remember to ask why there is duplication in the first place. We can't go overboard just in the name of "better safe than sorry" -- your test suite shouldn't be any larger than necessary. Each component of your code is either tested or not tested.

If you have a few large blocks of repeated code, probe through to form an understanding of what exactly is covered. Often, the DRY question is moot and you can compartmentalize your testing to trim off a lot of needless overhead. This commonly arises between controllers and models. If your model is tested on its own, the controller shouldn't re-test it. It needs to do nothing but verify the right methods of that class are called.

Documentation

An added bonus to more verbose specs is that they can serve as documentation. Remember that the word "spec" stands for specification. Your colleagues can easily understand your tests to determine cause and effect, but can only do so if you utilized RSpec's remarkably human-readable DSL. Following a code path through too many levels of abstraction is about as useful as reading the actual implementation.

Summary

Of course, extract larger groups of commonalities. Just keep asking yourself if you're creating the right abstraction, and if another developer could quickly assimilate the relevant test inputs.

Same factory called several times with the exact same data? Pull it out to a let block. Controller HTTP verb method called several times with almost the same data? Leave it be. Logging in a test user? Extract away. Several almost identical assertions? Don't DRY.

Bad:

describe 'token verification' do
  shared_examples 'invalid token' do |expected_error|
    it 'responds with the expected error' do
      response_json = JSON.parse(response.body)

      expect(response).to be_forbidden
      expect(response_json.fetch("error").to eq expected_error
    end
  end

  subject { get :test }

  context 'missing header' do
    include_examples 'invalid token', 'Authorization header not found'
  end

  context 'bad header' do
    before { request.headers.add('Authorization', 'badvalue') }
    include_examples 'invalid token', 'Incorrect authorization token'
  end
end

Good:

describe 'token verification' do
  let(:response_json) { JSON.parse(response.body) }

  it 'requires an authorization header' do
    get :test
    expect(response).to be_forbidden
    expect(response_json.fetch("error").to eq 'Authorization header not found'
  end

  it 'returns an error with a bad authorization token' do
    request.headers.add('Authorization', 'badvalue')

    get :test
    expect(response).to be_forbidden
    expect(response_json.fetch("error").to eq 'Incorrect authorization token'
  end
end

I'll leave you with a quote from Sandi Metz (source):

DRYing out ‘concepts’ exposes sameness and enables reuse; DRYing ‘incidental duplication’ lies about sameness and impedes change.

Factory data is DRYed concepts. Lots of thought goes into the design of FactoryBot's API, as well as your application's definitions (hopefully). This creates the correct abstraction.

However, the actual application of factory data and RSpec DSL is mostly not a concept that's worth wrapping up. It's too often just incidental duplication. Resist that DRY urge, or you'll just make life harder for yourself.

More blog posts