Address a number of issues with array-like "to satisfy". #550
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@alexjeffburke could I ask you to add the tests as the first commit and then the fix as a separate commit. It will be useful to see how it fails before this change. |
|
@sunesimonsen yes of course, apologies I didn't do that up front - very much stumbled into this. |
|
@alexjeffburke no problem, it will just make it a bit easier to review. |
While trying to write a test for the expect context being passed to an invocation of expect.it when satisfying array-like subjects some other issues around key checking became apparent. Add regression tests that thoroughly exercise these cases.
Note that the regression tests added in the previous commit do not yet pass due to other key handling issues in the assertion.
This commit fixes key checking issues when satisfying array-like. First, when a non-numerical property comparison is forced by it being supplied on the RHS this should work. Secondly, in the case of a non-numerical array-like there was an issue calculating how many outstanding keys there were when enumerability differences came into the picture. Fixes regression tests added in f44439d.
alexjeffburke
force-pushed
the
fix/arrayLikeToSatisfyAndProperties
branch
from
6facb54
to
d7e5f5f
Compare
|
@sunesimonsen I've split this into three commits - the second commit is only the fix for the context which I hope helps highlight the need for the other changes. |
papandreou
approved these changes
Dec 31, 2018
|
I'll merge this into v11 and do a new prerelease. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
While trying to write tests for the expect context being passed to
an invocation of expect.it when satisfying array-like subjects some
other issues around key checking became apparent.
First, when a non-numerical property comparison is forced by it
being supplied on the RHS this should work. Secondly, in the case
of a non-numerical array-like there was an issue calculating how
many outstanding keys there were when enumerability differences
came into the picture. Both of the added test cases exercise this
thoroughly.
Note that this commit also fixes the context issue.