Check that string contains multiple other strings #858

Open
postatum opened this Issue Nov 2, 2016 · 12 comments

Projects

None yet

6 participants

@postatum
postatum commented Nov 2, 2016

Hello everyone!

First of all, thanks for a great library ;)

Second of all, please give me an advice - is there a nicer way to test that string contains multiple other strings than chaining .and.contains(...)?

@lucasfcosta lucasfcosta added the question label Nov 2, 2016
@lucasfcosta
Member

Hi @postatum, thanks for the issue! 😄
Currently I think there's no other way to do it. You either use and to chain assertions or create multiple different assertions. However, your suggestions seems useful and maybe we could change the include assertion behavior to allow users to pass an array of strings that should be contained within another string.

What does the rest of the team think? Is it useful for our users? Should we implement it?

@shvaikalesh
Member
shvaikalesh commented Nov 2, 2016 edited

I feel like we should have .{include,contain}.{all,any}.strings (same way we have .keys) in the Chai's core. Having them will likely save users from extra regular expressions (which are error prone because of escaping).

@lucasfcosta
Member

@shvaikalesh agreed! Great idea, I think that way we will still be able to maintain modularity and follow the same pattern throughout the whole code. +1 for that!

@shvaikalesh
Member
shvaikalesh commented Nov 2, 2016 edited

We have long-term plans to migrate Brunch's e2e tests back to mocha + chai and would benefit from this too, as other folks doing bundlers/transpilers.

@vivganes
vivganes commented Nov 9, 2016

@shvaikalesh @lucasfcosta Does this mean that we will be able to write any of the following statements when this feature is implemented?

expect(myArray).to.include.any.strings("str1","str2");
expect(myArray).to.include.all.strings("str1","str2");

This would make the code look clean when there is an error code kind of scenario that needs to be tested.

👍 for this request

@shvaikalesh
Member

Yep, that is what we are suggesting. Would love to hear what other awesome maintainers think. @keithamus @meeber @vieiralucas

@meeber
Contributor
meeber commented Nov 9, 2016

@vivganes Just to be clear, since you used myArray in your latest examples: If you're matching strings inside of an array, then you can use the existing .members assertion:

expect(myArray).to.include.members(["str1","str2"]);
expect(myArray).to.have.members(["str1","str2"]);

But if you're searching for strings within a string, then yes, we'd need the not-yet-implemented .strings assertion:

expect(myString).to.include.any.strings("str1","str2");
expect(myString).to.include.all.strings("str1","str2");
@lucasfcosta
Member
lucasfcosta commented Nov 9, 2016 edited

So since 3 of us agreed on this already I'll tag this as medium-fix and assertion-request. If anyone disagrees, please let us know.
Would you like to implement it @postatum? If you want to do so you could take a look at how the keys assertion works and implement something like that, as @shvaikalesh suggested.
If you don't feel like doing a PR for that, no problem! Just let us know so others can work on that!

Also, thanks for the awesome suggestion!

@postatum

@lucasfcosta thanks! I'll take a look this weekend and let you know how things are going.

@vieiralucas
Member
vieiralucas commented Nov 10, 2016 edited

Hello @postatum, thank you for your issue!

I'm very happy to see that you are interested in implementing it.
We are very interested in helping anyone to contribute, so if you have any problem with the code base or if you need help with anything while you are implementing this feature, just come tu us.
You can post here on the issue or you can open a PR as a WIP and ask there.

Thank you so much for willing to contribute to chai 😄

@vieiralucas
Member

Hi @postatum have you had a chance to look into this more?

@postatum
postatum commented Nov 29, 2016 edited
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment