Unit-testing a controller in Ruby on Rails

Question

I intend this to be a general question on writing an effective set of test cases for a controller action.

I include the following ingredients:

• Ruby on Rails
• RSpec: A testing framework. I considered doing a vanilla Rails question, but I personally use RSpec, and I feel many other folks do too. The principles that come out of this discussion should be portable to other testing frameworks, though.
• Will Paginate: I include this to provide an example of code whose implementation is blackboxed to the controller. This is an extreme example of just using the model methods like @programs = Program.all. I chose to go this route to incorporate an additional factor for discussion, and to demonstrate that the same principles apply whether using external application code (e.g., model code) or an external plugin.

There seems to be a lack, given my humble Google Fu, of style guides for RSpec testing at this level, so it is my hope that, on top of me improving my code, this can become a useful guide for my fellow travelers.

For example purposes, let's say I currently have in my controller the following:

class ProgramssController < ApplicationController
  def index
    @programs = Program.paginate :page => params[:page], :per_page => params[:per_page] || 30
  end
end

Sidebar: For those unfamiliar with will_paginate, it tacks onto a ActiveRecord Relation (all, first, count, to_a are other examples of such methods) and delivers a paginated result set of class WillPaginate::Collection, which basically behaves like an array with a few helpful member methods.

What are the effective tests I should run in this situation? Using RSpec, this is what I've conceived at the moment:

describe ProgramsController do
  def mock_program(stubs={})
    @mock_program ||= mock_unique_program(stubs)
  end

  def mock_unique_program(stubs={})
    mock_model(Program).as_null_object.tap do |program|
      program.stub(stubs) unless stubs.empty?
    end
  end

  describe "GET index" do
    it "assigns @programs" do
      Program.stub(:paginate) { [mock_program] }
      get :index
      response.should be_success
      assigns(:programs).should == [mock_program]
    end

    it "defaults to showing 30 results per page" do
      Program.should_receive(:paginate).with(:page => nil, :per_page => 30) do
        [mock_program]
      end
      get :index
      response.should be_success
      assigns(:programs).should == [mock_program]
    end

    it "passes on the page number to will_paginate" do
      Program.should_receive(:paginate).with(:page => '3', :per_page => 30) do
        [mock_program]
      end
      get :index, 'page' => '3'
      response.should be_success
      assigns(:programs).should == [mock_program]
    end

    it "passes on the per_page to will_paginate" do
      Program.should_receive(:paginate).with(:page => nil, :per_page => '15') do
        [mock_program]
      end
      get :index, 'per_page' => '15'
      response.should be_success
      assigns(:programs).should == [mock_program]
    end
  end
end

I wrote this with the following principles in mind:

• Don't test non-controller code: I don't delve into the actual workings of will_paginate at all and abstract away from its results.
• Test all controller code: The controller does four things: assigns @programs, passes on the page parameter to the model, passes on the per_page parameter to the model, and defaults the per_page parameter to 30, and nothing else. Each of these things are tested.
• No false positives: If you take away the method body of index, all of the test will fail.
• Mock when possible: There are no database accesses (other ApplicationController logic notwithstanding)

My concerns, and hence the reason I post it here:

• Am I being too pedantic? I understand that TDD is rooted in rigorous testing. Indeed, if this controller acts as a part of a wider application, and say, it stopped passing on page, the resulting behaviour would be undesirable. Nevertheless, is it appropriate to test such elementary code with such rigor?
• Am I testing things I shouldn't be? Am I not testing things I should? It think I've done an okay job at this, if anything veering on the side of testing too much.
• Are the will_paginate mocks appropriate? You see, for instance, with the final three tests, I return an array containing only one program, whereas will_paginate is quite liable to return more. While actually delving this behaviour by adding, say 31 records, may (?) violate modular code and testing, I am a bit uneasy returning an unrealistic or restrictive mock result.
• Can the test code be simplified? This seems quite long to test one line of controller code. While I fully appreciate the awesomeness of TDD, this seems like it would bog me down. While writing these test cases were not too intensive on the brain and basically amounted to a fun vim drill, I feel that, all things considered, writing this set of tests may cost more time than it saves, even in the long run.
• Is this a good set of tests? A general question.

Answer 1

You forgot to test what view should be rendered. If you use this, your specs will be much cleaner. The last four should be standard matchers. See this as an example.

describe ProgramsController do
  let(:programs) { [mock_model(Program)] }    

  describe "GET index" do
    it { should paginate(Program).with_default_per_page(30) }

    describe "after pagination" do
      before(:each) do
        Program.stub(:paginate).and_return(programs)    
        get :index
      end

      it { should assign_to(:programs).with(programs) }
      it { should respond_with(:success) }
      it { should render_template(:index) }
      it { should_not set_the_flash } 
    end
  end
end

Answer 2

You've coded by your principles pretty reasonably.

My only real suggestion is to have just one assertion per test, including any #should_receive or #should= statements. The intention of most of your tests seems to be to test one single unit of functionality, so there is no need to repeat assertions, particularly when they are dependent. It will help focus more clearly on the aim of the individual tests, reducing their volume and heightening the readability.

As far as RSpec style, there are a few idioms I have seen more commonly used:

• using #and_return in place of returning values from blocks
• using mock_model
• doing setup in before blocks

Overall, I would likely have written it like this:

describe ProgramsController do
  before(:each) do
    @mock_programs = [mock_model(Program)]
    Program.stub(:paginate).and_return(@mock_programs)
  end

  describe "GET index" do
    it "succeeds" do
      get :index

      response.should be_success
    end

    it "renders the index template" do
      get :index

      response.should render_template("programs/index")
    end

    it "assigns @programs" do
      get :index

      assigns(:programs).should == [mock_program]
    end

    it "defaults to showing 30 results per page" do
      Program.should_receive(:paginate).with(:page => nil, :per_page => 30).and_return(@mock_programs)

      get :index
    end

    it "passes on the page number to will_paginate" do
      Program.should_receive(:paginate).with(:page => '3', :per_page => 30).and_return(@mock_programs)

      get :index, 'page' => '3'
    end

    it "passes on the per_page to will_paginate" do
      Program.should_receive(:paginate).with(:page => nil, :per_page => '15').and_return(@mock_programs)

      get :index, 'per_page' => '15'
    end
  end
end

I think, for the limited scope which you've provided, this is a good set of tests. You may be able to skip the test for basic success since all your other tests are dependent on a successful action. I personally can't speak too deeply about testing will_paginate, since I do much the same when testing the use of it from the controller. If it's tested well in the model that uses it, you've covered the basic behavior.

Answer 3

just something that i have considered how about hiding the paginate stuff behind a helper method and then stub the helper method this way you can swap out the pagination implementation with anytime you want and then just test the helper method is delegating the params to will_paginate

class ProgramssController < ApplicationController
  def index
    @programs = paginate_progams(params)
  end
end

now you stub the paginate_programs method and then you can spec up the helper method paginate

Answer 4

I'm a few years out of RoR, but worked for a company that was heavy on testing an we used RSpec.

In a case like this we would have tested some additional points, mainly about the content of the response. A few lines like (syntax may be totally wrong, as said I'm out of this):

test the amount of records @programs.should.have(30).records

then there was a command to test if certain records are in the result @programs.should.contain(...) this requires fixtures of course.

The idea behind this was to make sure we could replace a certain plugin against a new one and be sure it doesn't break anything or contains bugs. (For example the acts_as_taggable plugin had a bug that made it impossible to delete tags, thing was running into an infinite loop) And of course to avoid programmers to make changes while testing in the browser or similar. With will_paginate a typical bug would be to set the pagination to 10 to see several pages in the browser (because there are only 20 records in the fixtures...) and forget to set this value back to 30. And if it's not the programmer, then maybe the guy working on the HTML/CSS. We made sure every of those details were tested

More tests would make sure that the right file would be used for rendering. something like response.should.render_template(name)

This seems to be a lot of work. But on the positive side: When my first web site for a customer went online, after six weeks of programming and my humble 3 months of knowing about Ruby on Rails, it had no bugs at all and stood 1.000 hits/minute without a single line of code to be changed.