My issues with Let

For a while now, we’ve recommended against using let in RSpec and minitest. We even have a blog post explaining our thoughts. Despite that, I still have trouble explaining my problems with let and before to other developers. They’re wonderful for cleaning up duplication, and can make your tests smaller. That has to be a win, right?

Maybe not.

Let’s talk about some of the pain associated with using these options.

Let’s hide some things

Constructs like let allow us to hide our code. This may seem like a great thing when you initially start using it but in the end, it can lead to a lot of pain. Here’s an example from a code base I recently worked on:

it "application_header must be empty" do
  application_header.programs(true).must_be_empty
  ApplicationHeader.count.must_equal 0
end

Without any context, this seems like a fairly reasonable test. This test alone was taking almost an entire minute to execute! What you don’t see in this test is that the let statement for application_header was referencing 18 other let statements, inserting 23 database records and running 25 additional queries. WHOA!

My mind was blown so I did a bit of digging in the Git history. When this test was originally written, it only executed 2 queries. The author did a great job of testing exactly what needed to be tested. In the time since that test was written, other developers needed other things to be tested alongside the application_header and so they added a query here or an association there. The final result was a single test that ended up taking a full minute.

Cascading creation

let!(:person) {
  create :person,
  gender: female,
  date_of_birth: date_of_birth,
  city_of_birth: city,
  state_of_birth: state,
  country_of_birth: country,
  county_of_birth: county,
  number_of_dependents: dependents,
  nickname: nickname,
  materials_under_another_name: materials,
  alt_first_name: alt_first,
  alt_middle_name: alt_middle,
  alt_last_name: alt_last,
}

let(:person_presenter) {
  PersonPresenter.new(person)
}

describe '#person_presenter' do
  specify { person_presenter('birth_location').must_include city.name }
end

Again we see a test that, on the surface, looks reasonable. The thing you don’t see is that every reference (like country) in the person block is to another let statement. Each of those let statements is at least a single database call, some of them have multiple. The end result is that this test, and all the other tests in the file, start out with a 7 second overhead.

Let’s see what we can do to speed that up:

describe '#person_presenter' do
  it "shows the birth location as the city" do
    city_name = "Raleigh"
    city = build_stubbed(:city, name: city_name)
    person = build_stubbed(:person, city_of_birth: city)

    presenter = PersonPresenter.new(person)

    presenter("birth_location").must_include city_name
  end
end

This test does not have any external references and does not include a before block. It runs in under a second. Even more importantly, it’s easy to read. You can clearly see each step that is required and each external dependency. This test is also nice because it doesn’t hit the database at all.

Is let the problem?

I don’t think that let is inherently bad but it does make it easier for your tests to grow out of control. It allows you to move the important parts of your test out of your test. It makes it unclear what is really happening and can lead to bloat. By putting your setup inline, you ensure that each test has exactly what is required to work with the test.

Don’t think of your test files as a class that needs to share information and reduce duplication. Think of each individual test as its own class and if you need to pull out shared functionality, do so carefully.