(feat): expect value to contain oneOf #1242
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1242 +/- ##
==========================================
+ Coverage 94.6% 94.61% +<.01%
==========================================
Files 33 33
Lines 1705 1708 +3
Branches 415 416 +1
==========================================
+ Hits 1613 1616 +3
Misses 92 92
Continue to review full report at Codecov.
|
chai.js
Outdated
| @@ -3496,16 +3496,27 @@ module.exports = function (chai, _) { | |||
| if (msg) flag(this, 'message', msg); | |||
There was a problem hiding this comment.
I think the docs above need to specify that this assertion supports the contains flag since it ultimately changes its behaviour. It would also be good to add an example.
|
Hi @voliva, this is just a friendly ping to see if you need help with this PR. I'd love to get it merged in. Thanks again for the work 😊 |
|
I totally forgot 😱
I'll take a look today, thanks for the reminder!
…On Tue, 25 Jun 2019, 09:09 Lucas Fernandes da Costa, < ***@***.***> wrote:
Hi @voliva <https://github.com/voliva>, this is just a friendly ping to
see if you need help with this PR.
I'd love to get it merged in. Thanks again for the work 😊
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1242?email_source=notifications&email_token=ABI5533HNNGTN5AH2FL3X6TP4HADBA5CNFSM4G5NGNIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYPIMQY#issuecomment-505316931>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABI5537ICBW7ET6G4SSTI2LP4HADBANCNFSM4G5NGNIA>
.
|
|
Does that look right? @lucasfcosta |
There was a problem hiding this comment.
There's only one minor typo I've caught, but I'm happy to merge this in once that's addressed.
I'll have some time to have a second look at this early in the morning tomorrow (UK time). If it's fixed by then and we get another review I'll press the merge button 😄
Thanks!
lib/chai/core/assertions.js
Outdated
| this.assert( | ||
| if(contains) { | ||
| this.assert( | ||
| list.some(posibility => expected.indexOf(posibility) > -1) |
There was a problem hiding this comment.
There seems to be a typo here. It should be possibility instead of posibility.
There was a problem hiding this comment.
Good catch, I've just amended the relevant commit.
There was a problem hiding this comment.
Thank you so much @voliva for your contribution, everything is looking good to me except for two very small code style issues.
Once that gets fixed, I'll be happy to merge this.
chai.js
Outdated
| new Assertion(list, flagMsg, ssfi, true).to.be.an('array'); | ||
|
|
||
| this.assert( | ||
| if(contains) { |
There was a problem hiding this comment.
Sorry about this nitpicking but can you add an extra space here?
if (contains) {
Just to keep style consistency.
lib/chai/core/assertions.js
Outdated
| new Assertion(list, flagMsg, ssfi, true).to.be.an('array'); | ||
|
|
||
| this.assert( | ||
| if(contains) { |
There was a problem hiding this comment.
Sorry about this nitpicking but can you add an extra space here?
if (contains) {
Just to keep style consistency.
|
Hey @vieiralucas thanks for your feedback! I've addressed your comments |
Previously, doing an assertion such as
Would reject with the message
expected [ ... ] to be one of ['cat', 'dog', 'bird'], asoneOfdoesn't check forcontainsflag.This PR adds this additional check to
oneOf, so now it can be chained withcontain,contains,includeandincludes.