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!