Rspec expect block with multiple changes

Testing
1

I was writing an rspec test with multiple assertions on the same action, and I came across this accepted answer on Stack Overflow: ruby - Is it possible for RSpec to expect change in two tables? - Stack Overflow. To quote SO:

This should be two tests. RSpec best practices call for one assertion per test.

describe "#bar" do
  subject { lambda { Foo.bar } }

  it { should change { Counter.count }.by 1 }
  it { should change { AnotherCounter.count }.by 1 }
end

In my discussions with my mentor @greg, we discussed having simple it blocks that do the setup, rather than more using let, subject, etc. To quote the thoughtbot best practice guide:

Avoid its, let, let!, specify, before, and subject in RSpec.

Two possible alternatives are:

describe "#bar" do
  it "should change counter counts" do
    counter_count_before_bar = Counter.count
    another_counter_count_before_bar = AnotherCounter.count
    Foo.bar
    expect(Counter.count).to eq(counter_count_before_bar + 1)
    expect(AnotherCounter.count).to eq(another_counter_count_before_bar + 1)
  end
end

and

describe "#bar" do
  it "should change counter" do
    expect { Foo.bar }.to_change { Counter.count }.by 1
  end

 it "should change counter" do
    expect { Foo.bar }.to_change { AnotherCounter.count }.by 1
 end
end

The second solution of separate it blocks is possibly better for simple cases. However, suppose that setting up the tests and calling Foo.bar is fairly detailed involves lots of queries, and that what’s expect from calling Foo.bar is that both things happen at once. Can we argue that the simpler case of one it block is best?

And is the slightly more verbose syntax better of avoiding subject?

2

From my experiences, I don’t always do one assertion per test, I do the same logic assertions per test. For example, creating a user, then we assert first name and last name were assigned correctly, in this example I do both assertions in the same test.

In your example, it’s hard to say because I don’t see the context detail, but lets assume both counters suppose to increment together after we invoke the action, then here what I go with it:

describe "#bar" do
  it "should change counter counts" do
    Foo.bar
    expect(Counter.count).to eq(1)
    expect(AnotherCounter.count).to eq(1)
  end
end

I don’t think we need to verify with previous count values because we don’t have any data in the setup, but if we do have, then we already know many records in the setup, and we should also know what values we put in assertions.

Anyways, if there is a condition to increment just one, but not the other, then I absolutely put it in different tests.

3

I’ve found that in practice, due to potential expensive setup, there’s very little benefit to having one assertion per it block. You can see how that can slow down your tests, if setup is costly.

Like @samnang said, if the assertions are testing the same logic then it should be fine to put it in the same block. When writing an it block, ask yourself: “If the logic that I’m testing is wrong would both of these assertions fail?” If the answer is “Yes”, then having them in separate it blocks doesn’t add any benefit.

I feel like “One assertion per test” is a blanket statement, and it doesn’t need to be followed rigorously. The benefits of “one assertion per test” is that you’ll know all the specs that failed in a single run. As opposed to, fixing one spec running the tests again to see what fails next. However, I also see people set fail_fast flag to true which nullifies this benefit.

4

I do think it’s fine to write specs like these (and do so myself) where it makes sense. If setup is fairly cheap, or/and it reads much better, or the logic being tested is different. I would encourage to encapsulate setup and exercise steps into a single call if possible:

it "should change counter" do
  expect { setup_and_do_something }.to_change { Counter.count }.by 1
end

it "should change counter" do
  expect { setup_and_do_something }.to_change { AnotherCounter.count }.by 1
end

I might even write these specs as one liners to read more naturally (a la shoulda-matchers):

subject { -> { setup_and_do_something } }
it { should change(Counter, :count).by(1) }
it { should change(AnotherCounter, :count).by(1) }

Notice the usage of subject here. The goal is to avoid the [Mystery Guest](Obscure Test at XUnitPatterns.com Guest) anti-pattern, and since subject is right next to execution, it doesn’t seem to violate that.

These things are subjective to how they look, but our guidances of Avoid its, let, let!, specify, before, and subject in RSpec. in in reaction to reduce Mystery Guest appearances. Related blog post

5

I think the problem is to look at this as one assertion as of your testing framework. I believe the idea is to have one logical assertion, meaning you should only be testing one thing.

Thus, I think the problem does not lie on statement itself, but on the way it is interpreted.