Testing anti-pattern: merged setup data

by Jason Swett,

In a single test file, there’s often overlap among the setup data needed for the tests in the file. For whatever reasons, perhaps in an effort to improve performance or avoid duplication, test writers often merge the setup code and bring it to the top of the file so it’s available to all test cases.

Let’s take a look at a test that contains apparently duplicative setup data. This test has two test cases, each of which needs one build and at least one job.

RSpec.describe Build, type: :model do
  describe "#start!" do
    let!(:job) { create(:job) }
    let!(:build) { job.build }
  ...

  describe "#status" do
    let!(:build) { create(:build) }
    let!(:job_1) { create(:job, build: build, order_index: 1) }
    let!(:job_2) { create(:job, build: build, order_index: 2) }
  ...
...

There’s obviously some duplication among this setup data. We’re creating two builds and three jobs, but we only really need a total of one build and two jobs. If we wanted to, we could be more “economical” with our setup data by combining it and placing it at the top of the file so that it can be used by all tests, like so:

RSpec.describe Build, type: :model do
  let!(:build) { create(:build) }
  let!(:job_1) { create(:job, build: build, order_index: 1) }
  let!(:job_2) { create(:job, build: build, order_index: 2) }

  describe "#start!" do
  ...

  describe "#status" do
  ...
...

Now our setup code is superficially a little less “wasteful” but we’ve created a couple subtle problems that make our test harder to understand and change.

Misleading details

When we create our jobs, we give each of them an order_index. This matters for the #status test but is totally immaterial to the #start! test. As the author of this test, I happen to know which details matter for which tests, but someone reading this test for the first time would have no easy way of knowing when an order_index is needed and when it’s not.

The only safe assumption is that every detail is needed for every test. If we alter the global setup data somehow, it’s possible that we’ll cause a silent defect. We could cause a test to keep passing but to lose its validity and start showing us a false positive. When data is included in the global setup beyond what’s needed for every test in the file, it creates an unnecessary risk that makes the test harder to change than it needs to be.

Shoehorned data

I’ll now reveal a bit more of the describe "#start!" test case.

describe "#start!" do
  let!(:job) { create(:job) }
  let!(:build) { job.build }

  before do
    fake_job_machine_request = double("JobMachineRequest")

    # The following line, which references "job",
    # is the line to pay attention to
    allow(job).to receive(:job_machine_request).and_return(fake_job_machine_request)
  end
end

In the above code, which shows the original version of the #start! test before we merged all the setup data, the setup and usage of job were straightforward. We created a job and then we used it.

But now that we’ve merged all our setup code, we only have job_1 and job_2 available to us and no plain old job_2 anymore. This makes things awkward for the #start! test where we only need one job to work with. Here are two possible options, both undesirable.

before do
  # Referring to job_1 misleadingly implies that the
  # fact that it's job 1 and not job 2 is significant,
  # which it's not, and also that we might do
  # something with job 2, which we won't
  allow(job_1).to receive(:job_machine_request).and_return(fake_job_machine_request)

  # This is almost better but not really
  job = job_1
  allow(job).to receive(:job_machine_request).and_return(fake_job_machine_request)
end

Another way we can deal with this issue is simply to add a third job to our setup called job:

RSpec.describe Build, type: :model do
  let!(:build) { create(:build) }
  let!(:job) { create(:job) }
  let!(:job_1) { create(:job, build: build, order_index: 1) }
  let!(:job_2) { create(:job, build: build, order_index: 2) }

  describe "#start!" do
  ...

  describe "#status" do
  ...
...

This takes away our problem of having to shoehorn job_1 into standing in for job, and it also takes away our misleading details problem, but it creates a new problem.

It’s unclear what setup data belongs to which test(s)

The other problem with combining the setup data at the top of the test is that it’s not clear which values are needed for which test. This creates a symptom very similar to the one creates by the misleading details problem: we can’t easily know what kinds of changes to the setup data are safe and which are risky.

It’s unclear what the state of the system is and how it will affect any particular test

I once worked on a project where a huge setup script ran before the test suite. The test environment would get some arbitrary number of users, superusers, accounts, customers and almost every other kind of entity imaginable.

The presence of all this data made it extremely hard to understand what the state of the system was and how any existing data might interfere with any particular test. Although it’s sometimes worth it to make compromises, it’s generally much easier if every test starts with a fully clean slate.

What’s the solution?

Instead of merging all of a test file’s setup data at the top, it’s better to give each test only the absolute minimum data that it needs. This usually means giving each test case its own individual setup data. Often it even means creating duplication—although duplication in tests is usually not real duplication. The performance cost is also usually negligible or nonexistent, since even if global setup data only appears once, it actually gets run once per test run anyway, and nothing is saved by combining it. Any cost in duplication or performance is usually overwhelmingly offset by the benefit of not merging setup data: it makes your tests easier to understand and work with.

Leave a Reply

Your email address will not be published. Required fields are marked *