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.

• Jonah: do_request seems like unnecessary indirection. I'd rather read post :create, request_body. Other than that this seems fine. I'm unclear what the question is.

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.

• jtmarmon: 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
• jtmarmon: but yes, i'm sharing :params between specs so that's why I have it in a let
• Greg Burghardt: @jtmarmon: I've updated my answer based on your comments.