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

added constraint matcher #543

Merged
merged 3 commits into from
Feb 9, 2016

Conversation

yjv
Copy link

@yjv yjv commented Feb 1, 2016

adds a matcher you can pass any phpunit constrain to. makes it alot easier to use any phpunit constraint in arg matching

@aik099
Copy link

aik099 commented Feb 1, 2016

Why not use Hamcrest matchers instead?

@yjv
Copy link
Author

yjv commented Feb 1, 2016

this will make it so in projects that arent using hamcrest also have this option since they will almost definitely have phpunit

@davedevelopment
Copy link
Collaborator

Happy to have this in! Two things:

If we're going to be creating a separate class for it, I think it needs to be explicitly named PHPUnitConstraint or something. Alternatively, this could be done with a couple of lines in Mockery\Expectation::_matchArg, much like we do for Hamcrest matchers.

Needs an integration test or two as well, especially given the exception throwing nature of the matchers.

@aik099
Copy link

aik099 commented Feb 1, 2016

Ah, so it's more like a wrapper to other library matchers, then a matcher of it's own?

Maybe in that case you should be changing code, where we're integrating with Hamcrest instead: https://github.com/padraic/mockery/blob/cc22d9bcd9fe9b34fc73587ed9bf2895b228138b/library/Mockery/Expectation.php#L334 ?

@yjv
Copy link
Author

yjv commented Feb 8, 2016

@davedevelopment ok ill rename it. @aik099 i think i prefer implementing it as a matcher and leaving the expectation director more generic.

thanks!

davedevelopment added a commit that referenced this pull request Feb 9, 2016
@davedevelopment davedevelopment merged commit 36b9bfb into mockery:master Feb 9, 2016
@davedevelopment
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants