-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Add custom equality matcher to support ES 2015 iterables #923
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
Conversation
1ed5764 to
e95adf5
Compare
e95adf5 to
6be523c
Compare
| */ | ||
| 'use strict'; | ||
|
|
||
| describe('compare iterables correctly', () => { |
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.
Just call it iterators? :)
|
Looks good overall but I added some code-style recommendations. |
packages/jest-jasmine2/src/index.js
Outdated
| let nextRight = rightIterator.next(); | ||
|
|
||
| result = true; | ||
| while (result && !nextLeft.done) { |
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.
actually, instead of using a while loop, can't you just use a for of loop? That way you'll avoid doing the manual iterator advancement for at least one of the iterators.
for (const value of a) {
if (!matches) {
return false;
}
bIterator.next();
}148c682 to
6dc8831
Compare
…me constructor, fixes tests, refactor code following comments
6dc8831 to
79093d7
Compare
packages/jest-jasmine2/src/index.js
Outdated
| !jasmine.matchersUtil.equals( | ||
| aValue, | ||
| nextB.value, | ||
| jestCustomEqualityTesters |
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.
this is needed for the recursive check
|
Looks great! Just a few minor comments – I recommend using the simplest possible solution to most things. You are unlikely to need some abstractions ever, so they just stay in the code until the end of time. It's better to pick the simple solution and extend it when needed :) |
|
@facebook-github-bot import |
|
Thanks for importing. If you are an FB employee go to Phabricator to review. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Allows jest to check for equalities of Sets/Maps and Array-like by adding a custom equality tester for iterators.
Jasmine run custom equality before the normal equality checker, so I also add a test to ensure it didn't break normal Arrays equality (as they implement
Symbol.iterator).