Rspec controller tests

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.

• 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. – Jonah Nov 18 '15 at 5:39

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.

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