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

Test exact array contents where possible #3731

Merged
merged 7 commits into from
Sep 29, 2019
Merged

Test exact array contents where possible #3731

merged 7 commits into from
Sep 29, 2019

Conversation

javierm
Copy link
Member

@javierm javierm commented Sep 26, 2019

References

Objectives

  • Make tests checking the contents of an array easier to read
  • Make these tests more consistent with the rest of our tests
  • Reduce the number of unused variable warnings given by the Ruby interpreter

@javierm javierm added the Specs label Sep 26, 2019
@javierm javierm self-assigned this Sep 26, 2019
@javierm javierm added this to Reviewing in Roadmap via automation Sep 26, 2019
@javierm javierm force-pushed the array_specs branch 2 times, most recently from e4ad68d to 66d0fb9 Compare September 26, 2019 19:15
houndci-bot
houndci-bot reviewed Sep 26, 2019
View reviewed changes
spec/models/debate_spec.rb Show resolved Hide resolved
spec/models/debate_spec.rb Show resolved Hide resolved
@javierm javierm force-pushed the array_specs branch 2 times, most recently from 81da7d7 to 144f290 Compare September 27, 2019 18:13
@javierm javierm force-pushed the array_specs branch 4 times, most recently from f7fec4c to 0bfe3b4 Compare September 28, 2019 03:38
@javierm javierm force-pushed the array_specs branch 5 times, most recently from ba73617 to 90195da Compare September 29, 2019 13:54
@javierm javierm force-pushed the remove_obsolete_code branch 2 times, most recently from 82aa8db to f426b25 Compare September 29, 2019 20:53
@javierm
Simplify testing arrays exact contents
a04a289 
Instead of testing the contents of each element, we can test the whole
array at once.
@javierm
Use eq to match arrays with 1 element
f69a033 
It's shorter, and easy to read.
@javierm javierm changed the base branch from remove_obsolete_code to master September 29, 2019 21:25
@javierm
Simplify test for heading names
44ffcfb 
We were using `to_sentence` to check the order, while it's easier to
just check the exact contents of the array.

Furthermore, using `to_sentence` checked the order of the array, so it
was a potentially flaky spec since the method doesn't specify an order
and PostgreSQL doesn't guarantee the order of the records.
@javierm
Only create records on tests using them
9f00883 
The test or the draft phase legislation filter was using an entirely
different set of records, but was still creating the records used in the
open and past filter as well.

This made it hard to test the filter, since it returned records from
both sets.

Grouping the past and open filters together guarantees their records
won't be created in the phase draft test, and so we can test the exact
contents of the array.
@javierm
Check exact array contents instead of inclusion
fd13257 
We're using `eq` and `match_array` in most places, but there were a few
places where we were still checking each element is included in the
array. This is a bit dangerous, because the array could have duplicate
elements, and we wouldn't detect them with `include`.
@javierm
Use be_empty instead of not_to include
35143ac 
Using `not_to include` does not test for other elements which could be
present in the array.
@javierm
Simplify testing records exclusion
c2d7420 
When we already have tests checking which records are included, in the
tests testing records are not included we can just generate records
which will not be included and test against an empty array.

We were already using this approach in some tests. This way we also
avoid useless assignments.
@javierm javierm merged commit 3b5cbc2 into master Sep 29, 2019
Roadmap automation moved this from Reviewing to Release 1.1.0 Sep 29, 2019
@javierm javierm deleted the array_specs branch November 7, 2019 23:42
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
@javierm
Merge pull request consuldemocracy#3731 from consul/array_specs
aee260f 
Test exact array contents where possible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

Assignees

@javierm javierm

Labels
Specs
Projects
No open projects
Roadmap
  
Release 1.1.0
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@javierm @houndci-bot