How can this rails controller test be cleaner refactored?

Question

I tend to write quite explicit tests for my rails controllers exposing APIs with two seperate concerns:

• meeting the defined API-Response with headers headers and the response itself
• ensuring the right calls to underlaying methods/objects are made

However when the calls are not simply executable in a test the controller tends to get bloated. This is due to the fact that when the calls are stubbed out the execution order of the two test concerns is reversed:

• for the api calls:
  • first stub the call method (since we don't want this to be run)
  • make the request
  • test the response
• for the method calls
  • define the receive-expectations
  • make the request

Example Code:

require 'spec_helper'
describe ItemsController do
 render_views
 shared_examples :empty_success_response do
 it { expect(response.status).to eql(202) }
 it { expect(response.content_type).to eql('application/json')}
 it 'returns error message' do
 json = JSON.parse(response.body)
 expect(json).to be_blank
 end
 end
 context 'a single item' do
 subject { FactoryGirl.create(:item) }
 describe '#destroy' do
 let(:do_action) do
 delete :destroy, id: subject.id.to_s
 end
 describe 'response' do
 before do
 controller.stub!(:destroy_item)
 do_action
 end
 it_behaves_like :empty_success_response
 end
 it 'makes the call' do
 expect(controller).to receive(:destroy_item).once.times.with(subject.id.to_s)
 do_action
 end
 end
 # ...
 end
end

Note: The destroy_item method is just a stand in, the actual method makes more sense ;)

The code in the example works nicely, and is even fairly dry (mostly because the request is moved into a let), but feels still very chunky and bloated - especially if we consider this is just one simple actions, with a usual controller having quite a few more real logic to test.

Does anyone have an idea how to make this code more readable?

Or does it maybe make sense to split the 2 kinds of tests completely? This would however mean quite a bit of duplication since we have the same caller-code in 2 places.

Answer 1

shared examples share the let scope, so you could further dry up your code (assuming you use do_action for every action) to this:

shared_examples :empty_success_response do
 before do
 do_action
 end
 ...
end

Also, if you are using do_action, I also like to add to my scopes:

describe '#destroy' do
 let(:do_action) do
 delete :destroy, id: subject.id.to_s
 end
 after do
 do_action
 end
 ...
end

This way, you don't have to call do_action if all you do in the test is set expectations.

it 'makes the call' do
 expect(controller).to receive(:destroy_item).once.times.with(subject.id.to_s)
end

If in some tests you want do_action to run before the end of the test - that's no problem, since let runs the code only once - so it won't run again at the end of that specific test!