Rspec controller tests

Question

I wanted to keep my controller tests DRY, readable, and make writing new tests very simple so that it encourages good practices like one assertion per test (if you have to redo a lot of work over and over, chances are you're just gonna shove more assertions into the same test to avoid the extra work).

Here's the strategy I'm using:

RSpec.describe FireController do
    let(:do_request) { post :create, request_body }
    let(:request_body) { {} }

    describe '#create' do
       context 'when we have firewood' do
           let(:request_body) { { firewood: true } }
           it 'creates fire' do
               do_request
               expect(response.body).to eql('fire!!!')
           end
       end
    end
end

I would like some opinions on potential downsides to a strategy like this.

Answer 1

Usually let is used to create data or functionality that is the same for each individual spec. In this example, I don't see much difference between your code and:

it 'creates fire' do
  params = { firewood: true }
  post :create, params
  expect(response.body).to eql('fire!!!')
end

If you need to pass { firewood: true } to multiple specs, then using let to define the controller params makes sense. Furthermore, a before_each is more declarative for executing behavior. Use let to mock up data, but use before_each to execute the same behavior or methods for each spec:

RSpec.describe FireController do
  describe '#create' do
    context 'when we have firewood' do
      let :params, { { firewood: true } }

      before_each do
        post :create, params
      end

      it 'creates fire' do
        expect(response.body).to eql('fire!!!')
      end

      it 'is really really on fire' do
        expect(...).to ...
      end

      it 'does something else as well' do
        expect(...).to ...
      end
    end
  end
end

This way each spec is literally reduced to a single expect.

---

@jtmarmon said:

the reason I didn't run the request in before_each is because some specs need to check if a method get called. those expectations need to be set before the request is made

In this case I would either:

1. Just call post :create, params in each spec, or
2. Group those specs inside additional contexts that contain their own before_eachs setting up the method call expectations

You aren't gaining clarity, or really reducing code by encapsulating post :create, params with a let. Every time I see a let I think it is setting up data, rather than executing behavior.