-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
docs: clarify the difference between toIncludeAllMembers and toIncludeSameMembers #647
Conversation
|
Thanks for the PR! I can certainly understand why these two matchers would be confused. I had to read it a few times before I understood myself. But I'm not sure the removal of the word "same" makes it any clearer. I think this makes it clearest expect([1, 2, 2]).toIncludeAllMembers([2, 1]); // passes
expect([1, 2, 2]).toIncludeSameMembers([2, 1]); // fails
I'm for improving the documentation, but I think probably a more extensive edit is needed to improve on what we have. |
@keeganwitt I'll update the docs, thank you! For This test case passes, and I'm not sure why. Could it be a bug? expect([1, 2, 2]).toIncludeAllPartialMembers([999]); |
Co-authored-by: Keegan Witt <keeganwitt@gmail.com>
|
||
<TestFile name="toIncludeSameMembers"> | ||
{`test('passes when arrays match in a different order', () => { | ||
expect([1, 2, 3]).toIncludeSameMembers([3, 1, 2]); | ||
expect([{ foo: 'bar' }, { baz: 'qux' }]).toIncludeSameMembers([{ baz: 'qux' }, { foo: 'bar' }]); | ||
expect([{ foo: 'bar' }, { baz: 'qux' }, { fred: 'thud' }]).not.toIncludeSameMembers([{ baz: 'qux' }, { foo: 'bar' }]); | ||
expect([1, 2, 2]).not.toIncludeSameMembers([2, 1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change this?
@@ -62,13 +62,13 @@ Use `.toIncludeAnyMembers` when checking if an `Array` contains any of the membe | |||
|
|||
### .toIncludeSameMembers([members]) | |||
|
|||
Use `.toIncludeSameMembers` when checking if two arrays contain equal values, in any order. | |||
Use `.toIncludeSameMembers` when checking if two arrays contain members in exact match, in any order. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is clearer. The matcher is using the equals
function, which is why the original was worded the way it was.
It's because the matcher is expecting to compare objects. That's what is meant by "partial" is that some of the keys in the object match. I wonder if this matcher should be failing if the type isn't an object 🤔 I think this is at least somewhere we can improve the documentation. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #647 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 73 73
Lines 674 674
Branches 290 290
=========================================
Hits 674 674 ☔ View full report in Codecov by Sentry. |
What
This PR updates the description of
toIncludeAllPartialMembers
by removing the'same'
keyword.Adds
not
test example to the docs.Also, updates the description in the types, and adds a test code corresponding to the docs.
Why
It confuses the reader with
toIncludeSameMembers
andtoIncludeAllMembers
Notes
Fixes #609
Housekeeping