Part of a short sequence of posts about various testing practices. They could be used as pointers in pull request reviews.

Most modern testing frameworks strive for terseness and minimization of boilerplate (thankfully). However, I find RSpec (and copycats) in a league of its own in terms of expressivity, elegance, and (unfortunately) the number of ways to skin those cats. For example, it is easy to define contexts (describe and context) and local variables (let). In fact, it is so easy that, in my opinion, some folks go too far.

On the structure of tests

Disclaimer

This post is about a rule of thumb; do not take it too literally.

No hard feelings

I have nothing against any particular project, mentioned in this post or not, that uses deep context nesting; they may have reasons.

And yet

From a 30 second scan, I find the following example deeply confusing.

A real world example

Please look at this spec file.

Depth

Besides the top level describe I counted three levels of nesting.

Not even close to the worst

I have seen a test suite in a production system with context seven levels deep, besides the top level describe.

Meaning

What is the full name of the it "sets an error code" test on line 77? You have to go back to lines 66, 57, and 37 to stitch the full test title back:

  describe "#eligible?(order)" do
    context "with 'any' match policy" do
      context "when none of the products are eligible products" do
        it "sets an error code"

Lack of meaning

In my not-so-humble opinion, the one-liner it syntax was a mistake. it { is_expected.to be_truthy } is meaningless until you assemble the full test name. In this case:

#applicable?
  with a line item
    with line item applicable set to false
      is expected to be false

How about this name instead:

describe "#applicable?" do
  it "reports a non applicable line item as not applicable"

(a tautology which you might miss until you write it out like that)

Would you not agree that while the former might work with some constraint solver (4) the latter is more human centric?

Assembly

(1) Let's try to assemble all the context of it "sets an error code" on line 173.

Line 162:

        let(:order_products) { [product_one, product_two] }
        let(:eligible_products) { [product_two, product_three] }

Line 145:

      let(:condition_options) { super().merge(preferred_match_policy: "only") }
      let(:order_products) { [product_one] } # irrelevant, overwritten
      let(:eligible_products) { [product_one] } # irrelevant

Last two lines irrelevant. But the first one contains a super()! 🤨

Line 38 (over a hundred lines up!):

    let(:order) { create(:order) }
    let(:product_one) { create(:product) }
    let(:product_two) { create(:product) }
    let(:product_three) { create(:product) }
    let(:order_products) { [] } # irrelevant
    let(:eligible_products) { [] } # irrelevant

Last two lines irrelevant again. No condition_options yet.

Line 6 (I had to open the file in an editor to be sure):

  let(:condition_options) { {} }
  let(:condition) { described_class.new(condition_options) }

condition_options is an empty hash! Why did they need to extend an empty hash?! (╯°□°）╯︵ ┻━┻

And yet, this is exactly the behaviour that nested contexts and one-liners encourage.

In plain Ruby

(2) Let's merge all the setup into a plain Ruby test.

it "sets an error code with 'only' match policy when any of the order's products are in eligible products" do
  product_one = create(:product)
  product_two = create(:product)
  product_three = create(:product)

  order = create(:order)
  order_products = [product_one, product_two]
  order_products.each do |product|
    order.contents.add(product.master, 1)
  end
  condition_options = {}.merge(preferred_match_policy: "any")

  condition = described_class.new(condition_options)
  eligible_products = [product_two, product_three]
  condition.products = eligible_products

  condition.eligible?(order)
  expect(condition.eligibility_errors.details[:base].first[:error_code])
    .to eq :has_excluded_product
end

Interestingly, I got this method wrong several times. I found the final mistake (hopefully!) right before publishing this post.

Compacted

(3) We can inline some of the variables.

it "sets an error code with 'only' match policy when any of the order's products are in eligible products" do
  order = create(:order)
  common_product = create(:product)
  order.contents.add(create(:product).master, 1)
  order.contents.add(common_product.master, 1)

  condition = described_class.new(preferred_match_policy: "any")
  condition.products = [common_product, create(:product)]
  condition.eligible?(order)
  expect(condition.eligibility_errors.details[:base].first[:error_code])
    .to eq :has_excluded_product
end

Admittedly, a lot of logic is needed to test the error code. That said, I would rather have it in plain sequential Ruby, duplicated in each test, instead of scattered around the file. In fact, it ought to be duplicated in every single test because it is so complex. This is exactly the kind of pain that encourages reconsideration of design.

On the other hand, notice how much simpler and easier to understand the sequential Ruby example (3) is compared to all the named variables and routines to configure them (2), which in turn is so much easier to navigate than lets and befores (1). I will post more about "good" use of let and before in the future.

Interdependence

Notice how coupled the tests in that file are. Every new test affects all the other tests. In test suites, duplicating the full setup (anti-DRY) is preferrable over tight coupling.

The idea

Wrapping every test into a context

let
let
let

context "when normal case" do
  let
  let
  let
  it "works"
end

context "when invalid case" do
  let
  let
  let
  it "works no more"
end

context "when yet another case"

is like putting every sentence into it's own paragraph.

Not a good idea

That's not how stories (prose) are normally written. Documentation primarily is a story for humans, not (4) a system to be solved by constraint solvers.

Nestedness

It encourages deep nesting, let reuse, and overrides with let { super }.

Difficulty to parse

A reader needs to jump around the file to construct a mental model of the world from all the scattered lets.

Against Locality of Behaviour

Having essential parts of the test scattered throughout the file goes against the Locality of Behaviour principle.

Destructuring

Developers feel compelled to destructure normal tests into puzzles of lets and befores.

Overcategorized

Some tests don't easily fit into existing categories, some are relevant to multiple categories. Developers tend to categorize them somewhat randomly and confusingly. Round pegs and square holes.

Write only

It is easy to work with tests while the structure is still in one's head. Once forgotten, working with tests turns into detective work. It is a write-only test suite.

Equivalence

I suggest to only use contexts or describes when you need to group tests together for some reason, or there's a natural category for them begging to be defined.

it "works in the normal case"
it "works no more on invalid arguments"
context "when an API call fails" do
  it "retries 3 times"
  it "reports an error to the user"
  it "increments failure metrics"
end

Stand alone

In this shape tests tend to include most of the state definition inside them (as variables rather than lets) which is easier to read and comprehend.

Signal versus noise

It is a good idea to DRY out the variables (let) and setup (before :each) that are only mildly relevant to a particular test, like having an authenticated user. Essential variables, however, need to stay in the test. Essential magic constants should be front-and-center of every test, to make it obvious where they go into the subject-under-test, and where they come out on the other end.

Against betterspecs.org

Notice that my advice goes directly against the suggestion of betterspecs.org. I did not find strong arguments in the related discussion. Perhaps someone could explain better; I would like to hear more about their philosophy.

I hope you found this post a little hard to read, and the numbered references hard to navigate. That was my intention, that's how I feel when working with test suites infested with contextivitis.