Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(expectations): add toMatch #191

Merged
merged 3 commits into from
Sep 21, 2020
Merged

feat(expectations): add toMatch #191

merged 3 commits into from
Sep 21, 2020

Conversation

owenvoke
Copy link
Member

Q A
Bug fix? no
New feature? yes
Fixed tickets #...

I wasn't sure about the naming of this, whether it should be toMatchRegEx, toMatchRegExp, or toMatchRegularExpression. (The toBe* prefix doesn't really work with any of them).

@olivernybroe
Copy link
Member

What if we just call it toMatch instead?

Then let me do the following

expect('Hello World')->toMatch('/^hello wo.*$/i');
expect('Hello World')->toMatch('Hello World'); 

So a regex and a string is supported for matching.

@owenvoke
Copy link
Member Author

@olivernybroe, that makes sense too. 👍🏻 Although, I wonder if people might get confused between toBe and toMatch. 🤔 @nunomaduro, what're your thoughts on this?

@olivernybroe
Copy link
Member

@owenvoke Hmm true. Could we extend toBe to just support regex maybe?

@nunomaduro
Copy link
Member

So, to give context. Normally, we mimic the API of Jest. In this case, Jest don't have this feature. So let's take a moment to actually make sure we don't make a mistake here.

JavaScript uses match : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/match.

I think we can use toMatchRegex for now, and change it later if we think people will be confortable with an toMatch.

@olivernybroe
Copy link
Member

@nunomaduro According to jest they use toMatch in their expect api
Which can take a string and a regex or am I misunderstanding something here? 👍

https://jestjs.io/docs/en/expect#tomatchregexporstring

@nunomaduro
Copy link
Member

@owenvoke @olivernybroe If Jest uses toMatch, let's use toMatch too. 👍

@owenvoke
Copy link
Member Author

Pushed an update. 👍🏻

@owenvoke owenvoke changed the title feat(expectations): add toMatchRegEx feat(expectations): add toMatch Sep 21, 2020
@nunomaduro
Copy link
Member

Fell free to merge, tag a new version, update the docs, and make a tweet about it @owenvoke .

@owenvoke
Copy link
Member Author

@nunomaduro, I noticed you merged the docs for toMatchConstraint. Do you want me to merge #190 as well and include that in v0.3.6?

@nunomaduro
Copy link
Member

@owenvoke Yes.

@owenvoke owenvoke merged commit 2b138ad into pestphp:master Sep 21, 2020
@owenvoke owenvoke deleted the feature/assert-regex branch September 21, 2020 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants