Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List screen tests for searching / paging / sorting #77

Open
amnesia7 opened this issue Jul 2, 2015 · 4 comments
Open

List screen tests for searching / paging / sorting #77

amnesia7 opened this issue Jul 2, 2015 · 4 comments

Comments

@amnesia7
Copy link

amnesia7 commented Jul 2, 2015

I've looked through the specs in the repo but there doesn't appear to be anything along the lines of listing results that are searchable, sortable and offer paging.

I was wondering how best to set out / separate the specs for this and where the crossover may be between model specs for .search() and maybe controller specs for .page() and .sort() and any feature scenarios to check how they all hang together.

I thought it best to mention it as an issue on here rather than on stackoverflow in case it highlighted an omission from the repo.

Thanks

@eliotsykes
Copy link
Owner

Hey @amnesia7 , thanks for the request, this is a good one. I won't have time in the near future to build something out for this, however I can tell you how I'd approach it - and of course this is just my way and that there are plenty of good alternative approaches

  • Feature specs that cover:
    • User searches for blank (invalid) search term
    • User searches and gets back results:
      • No results
      • 1 page of results
      • Multiple pages of results
    • Sorting
  • Model specs that test the following in more depth:
    • search
    • sort
    • page

It might feel like there's a little overlap between parts of the model and feature specs and that's OK.

As far as controller spec goes, I'd consider writing one if I felt there was code that had to be in the controller and wasn't tested by the above.

@amnesia7
Copy link
Author

amnesia7 commented Jul 5, 2015

Thanks @eliotsykes

How would you expand on that if the search had multiple fields, eg select a category, or date filters?

I'm currently rewriting the test suite for my app because it was taking ages (using, more or less, just integration tests that were rendering views).

I've done all the model specs which are pretty extensive and account for all the possible uses of each of the scopes etc (including .search() and .sort()). I hadn't done any for paging in the model specs because I wasn't sure whether that would come under controller specs since it is called by the controller and isn't actually one of my scopes but one of kaminari's. Or it the intention of your way for paging mean that paging is tested via the model and then the feature tests make sure that it cna be performed with pagination links etc to get to the next page of results?!

I'm trying to figure out what I should ideally be doing in the way of controller and feature specs.

@amnesia7
Copy link
Author

amnesia7 commented Jul 5, 2015

Thinking about this again, you're right; .page() is a class method and as such would be tested along with the others in the model spec rather than in the controller specs.

@amnesia7
Copy link
Author

amnesia7 commented Jul 5, 2015

Is this the kind of model specs you'd expect for page?

  describe '.page(params[:page])' do
    context 'when no enquiries' do
      specify { expect(Enquiry.page(1)).to be_empty }
    end

    context 'with enquiries' do
      let!(:enquiries) { create_list(:enquiry, 21) }
      let(:first_page) { Enquiry.page(1) }
      let(:second_page) { Enquiry.page(2) }

      context 'when page is nil' do
        it 'should return enquiries for page 1' do
          expect(Enquiry.page(nil)).to eq(first_page)
        end
      end

      context 'when page is blank' do
        it 'should return enquiries for page 1' do
          expect(Enquiry.page('')).to eq(first_page)
        end
      end

      context 'when page is greater than number available' do
        specify { expect(Enquiry.page(3)).to be_empty }
      end

      context 'on page 1' do
        it 'should return enquiries for page 1' do
          expect(Enquiry.page(1)).to eq(first_page)
        end

        it 'should not return enquiries for page 2' do
          second_page.each do |enquiry|
            expect(Enquiry.page(1)).not_to include(enquiry)
          end
        end
      end

      context 'on page 2' do
        it 'should return enquiries for page 2' do
          expect(Enquiry.page(2)).to eq(second_page)
        end

        it 'should not return enquiries for page 1' do
          first_page.each do |enquiry|
            expect(Enquiry.page(2)).not_to include(enquiry)
          end
        end
      end
    end
  end

If not, what would you change/suggest?

Would you include specs that count the results as well?

I'm not quite sure how I could guarantee creating a page's worth of enquiries for testing page 1 and then adding another that would only be shown on page 2 without being able to guarantee the ordering that page would use.
Or would you just expect them to be in ascending order of created_at / id so create 20 to test page 1 and then create 1 more which would definitely go on the second page?

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants