Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Iterable equality within object #8359
Iterable equality within object #8359
Changes from 13 commits
9bc6a08
86312c9
7e13ba4
8ec8ab5
53408bd
87c4e62
1e5d3b7
84f8bda
8068054
3cd2b4e
c4d40b9
2c3a862
416f5b3
e50e95e
5a46db1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Do we need to add
[iterableEquality]
as optionalcustomTesters
argument toequals
call in case any of these property values have iterators? All the calls inmatchers.ts
include it. I am asking, not telling.This illustrates a limitation that we recently noticed: it is not (yet) possible to for a custom tester to know about sibling testers (for example,
toStrictEqual
matcher needs more testers thantoEqual
does, anditerableEquality
has no way to provide them in this recursive call).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.
@pedrottimark is there an extra test case you're thinking of we should add?
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 so sorry for the very late follow up.
I'm not familiar with the code base but I see the point and the limitation.
IMO
customTesters
should be kept like this so each custom tester should test only one thing, so my changes initerableEquality
method will be reverted and will include non-enumerable members again inkeys
method injasmineUtils.ts
I'm not sure of the consequences of this change but will try it out.