9

I'm testing the index action for my ProjectsController.

I'm using the will_paginate gem, and am trying to write an RSpec test that ensures the paginate method is called on the current user's projects when they projects_path.

The result I'm getting, however, isn't what I expected and I can't figure out why.

result

Failure/Error: expect(user.projects).to receive(:paginate)
       (#<ActiveRecord::Associations::CollectionProxy::ActiveRecord_Associations_CollectionProxy_Project:0x00000004719ef0>).paginate(any args)
           expected: 1 time with any arguments
           received: 0 times with any arguments
     # ./spec/requests/project_pages_spec.rb:82:in `block (3 levels) in <top (required)>'

projects#index

def index
    if params[:search]
      @projects = current_user.projects.search(params[:search]).paginate(:page => params[:page], :per_page => 13)
    else
      @projects = current_user.projects.paginate(:page => params[:page], :per_page => 13)
    end 
end 

index test

describe "index" do

  describe "pagination" do

    let(:user) { FactoryGirl.create(:user) }
    let(:client) { FactoryGirl.create(:client) }

    before do
      capybara_sign_in(user)
      @project = Project.create(name: "Blog", fee: 550, client_id: client.id)
    end 

    it "should call the paginate method" do
      expect(user.projects).to receive(:paginate)
      visit projects_path
    end
  end
end

Note that I haven't finished writing the tests, so please omit any comments re: drying up the code etc.

Thanks for the help guys/gals! :)

bitfizzy
  • 157
  • 1
  • 1
  • 8

2 Answers2

7

The reason it is failing is that user in your spec file is not the same as current_user in your controller. So while current_user is receiving paginate your spec's user never is. Couple of ways you could solve it.

You could do this:

expect_any_instance_of(User).to receive(:paginate)

Drawback here is that you're testing for any instance, not specifically your current_user instance. Whether that's a problem or not is up to you :)

The other way would be to stub current_user:

controller.stub(:current_user).and_return(user)
expect(user.projects).to receive(:paginate)

Upside is you're testing exactly the same user. Downside is stubbing current_user might introduce other issues.

Philip Hallstrom
  • 19,673
  • 2
  • 42
  • 46
1

Part of the pain you are feeling is that you are testing implementation instead of behavior. In general, the behavior of a web request can be described in a few words: it accepts input parameters and returns a response. To test the behavior, test the contents of the response.

In your case, the app will behave one way when params[:search] is present, and another way when it is missing. So write one test with params[:search] set to an appropriate value, and one with it missing, and test that the response is what is expected in each case. You don't need to check the entire response, just enough to verify that the right data is returned.

To make the tests return the right responses, use a different set of data for each test. The model will return the expected rows, and voilà!, passing tests.

Now this may seem like more work than mocking and stubbing, and it will take more time to run, but it is the right way to test a request. You want to check the whole stack, not just the method call in the controller. The resulting tests will be true integration tests, and will be less brittle since they are not coupled to the implementation.

zetetic
  • 47,184
  • 10
  • 111
  • 119
  • Thanks so much for the answer! I'll definitely rework the tests to focus on behaviour rather than implementation. The only reason I didn't give your answer the green tick of approval is that it didn't directly solve my technical problem. But still, thanks for the tip. Will put it into practice ;) – bitfizzy Aug 27 '15 at 10:38
  • 1
    Op didn't say they wanted "true integration tests". I know integration tests show the bigger-picture functionality, but I'm a little weary of people explaining RSpec's shortcomings with criticism of their testing philosophy or whatever. Opinionated defaults are one thing, but if I want to test that a method was called N times, then by god I'm going to do that, just try and stop me. – max pleaner Jan 12 '17 at 21:53
  • @maxple This is not an "RSpec shortcoming". RSpec allows you to mock and stub to your heart's content. If anything, the problem lies in the difficulty of writing tests for Rails controllers. I rarely write controller tests for that very reason -- they can't really be tested as units in isolation due to the "magic" of Rails. If you are going to write full-stack tests for Rails (and you should), controller tests are not all that valuable IMHO. – zetetic Jan 12 '17 at 22:03
  • @zetetic I was kinda using you as a straw man. But if you look at RSpec's github issues, there are many instances of them saying stuff like "we don't allow that because it's not a good design pattern", yet people over and over visit those threads and argue with them. – max pleaner Jan 12 '17 at 22:05
  • @maxple I think all open-source projects encounter that problem at some point. The maintainers have a vision for the project, and they promote that vision, and some of the users push for the project to move in another direction. I generally side with the maintainers in these sorts of arguments, since they are in the best position to know how to use the product. In the specific case of RSpec, I think since most of its usage is for Rails testing, many of the complaints are unfair. Outside of Rails, RSpec is a breeze. In Rails, it often seems hard to use, despite years of work tailoring it ... – zetetic Jan 12 '17 at 22:32
  • ... specifically for use in Rails apps. Testing A/R models as units is easy. Testing your own PORO code is easy. Controllers and views are a PITA. That is a Rails thing, not an RSpec thing. – zetetic Jan 12 '17 at 22:35
  • My qualms aren't rails-specific. It's mainly `expect_any_instance_of` and calling `expect().to receive` in a loop that I had trouble with. Didn't mean to take it out on you, but i just spent like 2 hours debugging this (thankfully figured it out). – max pleaner Jan 12 '17 at 22:52
  • @maxple don't worry, I don't take it personally. The fact that people are criticizing RSpec is good in a way --- that means they are using it! – zetetic Jan 13 '17 at 00:37