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

Sorting tests #418

Merged
merged 3 commits into from
Nov 2, 2018
Merged

Sorting tests #418

merged 3 commits into from
Nov 2, 2018

Conversation

kbrock
Copy link
Collaborator

@kbrock kbrock commented Nov 2, 2018

extracted from #415
Introduce sorting tests (with some skipped) so we can state the problems and see them getting fixed as we merge PRs

- a consistent ordering (id) makes test_sort_by_ancestry more consistent

arrange was originally using sort_by_ancestry
sort(ancestry) acts differently in different operating systems due to
the locale used for sorting unicode strings.
Now that the sort is only sorting the id, the edge cases are no longer
necessary.

- ordering the original records has been moved from ruby to sql

- extracted test helper build methods into a separate method
Change the tests so they don't match a simple id ordering
introduce a few that fail, so the fixing methods can call them out
@coveralls
Copy link

coveralls commented Nov 2, 2018

Coverage Status

Coverage remained the same at 96.515% when pulling 08328a2 on kbrock:sorting_tests into 34fa0e9 on stefankroes:master.

@kbrock kbrock merged commit f56ad4a into stefankroes:master Nov 2, 2018
@kbrock kbrock deleted the sorting_tests branch November 2, 2018 16:32
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

Successfully merging this pull request may close these issues.

2 participants