RSpec test for pagination

This is a Rspec test for pagination in Rails project. I'm not sure if I should write the test in spec/requests or spec/controllers.

And there must be a lot of thing that I had better to do. Which part of code should I refactor?

spec/requests/companies_spec.r

describe "Companies" do
  describe "GET /companies" do
    before(:all) { 50.times { FactoryGirl.create(:company) }}
    describe "index" do
      context "with 50 companies" do
        it "has not second page" do
          visit root_path
          expect(page).to have_no_xpath("//*[@class='pagination']//a[text()='2']")
        end
      end
      context "with 51 companies" do
        before{ FactoryGirl.create(:company) }
        it "has second page" do
          visit root_path
          find("//*[@class='pagination']//a[text()='2']").click
          expect(page.status_code).to eq(200)
        end
      end
    end
  end
end

There are many ways to test this. Mostly, though, it'd be nice to avoid having to create 50+ records, since it slows down your tests.

If you use a request spec, though, it's probably best to create 50+ records, since it's a high-level test, so you'll want to be close to the "real" usage scenario.

But you can cheat a little in other places. For instance, if you have the records-per-page number defined in a way that's configurable, you can set it to something lower in you pagination test (or you can set it globally for the test environment). For instance, if the per-page is set to 2, you only need to create 3 records to test pagination. That'll be a lot faster than creating 51 records.

If you're spec'ing the view itself, you can simply define the instance variables that'll trigger pagination links, and not bother with the actual records. Or you can use FactoryGirl.build_list to merely build the records and assign them to a view-accessible variable, without actually storing them in the database - again, faster.

You can also look into mocking and stubbing to avoid actually creating the records.

For your current code, You can do a couple of things, like:

describe "Companies" do
  describe "GET /companies", order: :defined do
    before(:all) { FactoryGirl.create_list :company, PER_PAGE }

    context "with few records" do
      it "does not paginate records" do
        visit "/companies"
        expect(page).to have_no_xpath("//*[@class='pagination']//a[text()='2']")
      end
    end

    context "with many records" do
      it "paginates records" do
        FactoryGirl.create :company

        visit "/companies"
        expect(page).to have_xpath("//*[@class='pagination']//a[text()='2']")
        find("//*[@class='pagination']//a[text()='2']").click
        expect(page.status_code).to eq(200)
      end
    end
  end
end

Changes I've made:

• Using FactoryGirl.create_list to create a number of records at once.

• Using a PER_PAGE constant, just in case it isn't 50. This could also be an ENV var, an instance variable, or simply hard-coded. But naming it helps document the code.

• Using order: :defined to force the examples to be run in the order they're defined. This avoids the specs randomly failing because the 2nd test has been run before the first one.

• I've change the visit path to /companies because that's what the spec is about. You used visit root_path, which no doubt worked fine, but the spec is about visiting /companies, so I find it nicer to keep it consistent.

You might also want to check that the correct records actually show up on the page. I.e. attempt to find the name of the 51st company within the rendered page, when you've gone to the 2nd page's path.

Lastly, you may want to add some specs for how the system should behave if you go say page 4, but there aren't enough records to show anything.

But again, I'd probably start with view/controller specs, before moving on to high-level request specs. Request specs are great because they test everything pretty close to actual usage. But that also makes them more complex, so the more you can check at a lower level, the better.

StackExchange Code Review Q#77180, answer score: 8