Add type checks for asymmetric matchers #5069
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5069 +/- ##
=======================================
Coverage 61.18% 61.18%
=======================================
Files 202 202
Lines 6765 6765
Branches 4 3 -1
=======================================
Hits 4139 4139
Misses 2625 2625
Partials 1 1Continue to review full report at Codecov.
|
| @@ -218,6 +222,10 @@ class StringMatching extends AsymmetricMatcher { | |||
| } | |||
|
|
|||
| asymmetricMatch(other: string) { | |||
| if (!isA('String', other) && !isA('RegExp', other)) { | |||
There was a problem hiding this comment.
Although sample can be either string or regexp, it seems like other can only be a string.
|
@rogeliog Happy New Year! Thank you, it looks good to me now. |
| @@ -218,6 +222,10 @@ class StringMatching extends AsymmetricMatcher { | |||
| } | |||
|
|
|||
| asymmetricMatch(other: string) { | |||
| if (!isA('String', other)) { | |||
There was a problem hiding this comment.
According to our docs, stringMatching() accepts a regexp. Mind changing the type and runtime check?
There was a problem hiding this comment.
cc @rogeliog can you take a look at this?
There was a problem hiding this comment.
@thymikee The StringMatching constructor does indeed take either string or RegExp, but argument of the asymmetricMatch function takes only string as received value, true?
That is, expect.stringMatching(/^Alic/) returns true when the received value is 'Alicia' or false when the received value is 'Bob' but should report an error if the received value is a RegExp, because how to decide if a received RegExp matches an expected RegExp?
There was a problem hiding this comment.
@pedrottimark you're right, I mistaken other with sample
|
*cough*changelog*cough* |
|
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. |
Summary
This PR adds a check to fail when the types of "string" asymmetric matchers do not match cc: @pedrottimark
Fixes: #5021
Test plan