Enhance contains for edge cases, make nesting behave consistently #359
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
Since two Errors are never rightly equal, adds contains() support if their messages are the same. In fact, so often a message has superfluous stuff at the beginning and end, "contains" should pass even if what is specified by the test is a substring. This should make it a little easier for testing invocations that receive an error argument
Like Error, Dates are never === equal, so adds contains() support if their times are the same.
Usually someone would pass a regex to contains to test a string, but in case a regex is passed to both sides that is identical, it should match (and if the regexes aren't equivalent, it should not match)
Previously the comparison logic for traversable JS objects was separate
(presumably to avoid cycles, but possibly also because it was a
partial equality check, whereas the top-level contains() routine was a
case-by-case assessment of whether something contained something else.
This leads to confusing edge cases where something considered
"contained" when inside a {} property would not be considered contained
when passed to the top-level matcher. Whatever benefit this might have
conferred was surely more confusing than it was worth.
This only really required a change to one existing test-case, that is
when whatever is passed to contains() happens to deeply equal whatever
is actually passed, but that was never really a supported requirement,
so I'm wary of judging this a breaking change, since intuitively it
makes sense that the two things are in agreement. It could, however,
throw some folks off when they're testing that something wraps something
else in an additional array (in which case this would generate a false
positive), though, and I don't have a good answer for that other than
it's hard to divine someone's intent given how incredibly permissive
this function's parameter signature is.
Aside from handling the issue stated in #358, in which someone calling
contains() and passing a {d: Date} would expect it to fail if the
actual date differed from the passed one, this ch
Fixes #358
searls
force-pushed
the
enhance-contains
branch
from
0c1c7b8
to
4e894e7
Compare
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.
Previously the comparison logic for traversable JS objects was separate
(presumably to avoid cycles, but possibly also because it was a
partial equality check, whereas the top-level contains() routine was a
case-by-case assessment of whether something contained something else.
This leads to confusing edge cases where something considered
"contained" when inside a {} property would not be considered contained
when passed to the top-level matcher. Whatever benefit this might have
conferred was surely more confusing than it was worth.
This only really required a change to one existing test-case, that is
when whatever is passed to contains() happens to deeply equal whatever
is actually passed, but that was never really a supported requirement,
so I'm wary of judging this a breaking change, since intuitively it
makes sense that the two things are in agreement. It could, however,
throw some folks off when they're testing that something wraps something
else in an additional array (in which case this would generate a false
positive), though, and I don't have a good answer for that other than
it's hard to divine someone's intent given how incredibly permissive
this function's parameter signature is.
Fixes #358