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.

Sign up for GitHub

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

Jump to bottom

Fix investment pagination tests #3405

Merged
merged 2 commits into from
Apr 3, 2019
Merged

Fix investment pagination tests #3405

merged 2 commits into from
Apr 3, 2019

Conversation

javierm
Copy link
Member

@javierm javierm commented Mar 25, 2019 •
edited
Loading

Objectives

  • Fix tests which were accidentally passing because of typos
  • Generate the right amount or records we need to test budget investment pagination

@javierm javierm force-pushed the remove_obsolete_test branch 3 times, most recently from 12d0422 to 81683a8 Compare March 27, 2019 13:32
@javierm javierm changed the title Remove slow and obsolete test Update slow and obsolete tests Mar 27, 2019
@javierm javierm changed the title Update slow and obsolete tests Update slow and obsolete pagination tests Mar 27, 2019
@javierm
Use investments per page instead of Kaminari's
5a529e5 
This is the actual number of investments per page in the index action.

Also note one test was generating 100 extra records, which made the test
take more than 40 seconds (on my machine).
@javierm javierm changed the title Update slow and obsolete pagination tests Fix investment pagination tests Mar 28, 2019
@javierm
Fix typos in investment order tests
f6da20d 
There was a typo: `new_order = eq(all(` instead of `new_order = all(`,
which was causing the tests to pass.

However, the final expectation should test that we keep the same order
in the same session, and we were accidentally testing the opposite.

We're also adding an extra check to verify there are investments on the
page, since in some cases we were accessing pages with no investments,
and so these tests were always passing.
@javierm javierm merged commit 844ab5f into master Apr 3, 2019
@javierm javierm deleted the remove_obsolete_test branch April 3, 2019 19:13
@javierm javierm mentioned this pull request Apr 3, 2019
Merged
@voodoorai2000 voodoorai2000 added this to Release 1.0.0-beta in Roadmap Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@voodoorai2000 voodoorai2000 voodoorai2000 approved these changes

Assignees
No one assigned
Labels
Budgets
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@javierm @voodoorai2000