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

Cannot mock Matcher #116

Closed
Canadadry opened this issue Nov 30, 2017 · 8 comments
Closed

Cannot mock Matcher #116

Canadadry opened this issue Nov 30, 2017 · 8 comments

Comments

@Canadadry
Copy link
Contributor

I need to mock the main Matcher class which is marked as final.

In order to do it I have made a fork and create an interface for matcher.

@teklakct
Copy link
Contributor

teklakct commented Dec 7, 2017

Matcher class is marked as final for a reason. It is so simple that you should use it. I suggest you to check your code and ensure is everything is ok. In most cases your design is broken.

Can you provide example why you want to mock Matcher?

@Canadadry
Copy link
Contributor Author

Because I am building code arround the matcher and I need to test what happend when matcher failed or succeded. I could write some fake data but that sould not concerne my test. And your code is used as a service and therefore could be mock.
Anyway, I have forked it because @uuid@ is missing some implementation. So I also use it to allow mocking.

@norberttech
Copy link
Member

I think in this case we can remove final statement from Matcher

@teklakct
Copy link
Contributor

teklakct commented Dec 7, 2017

Still not sure is that a good way to use it. I use Matcher like if construct in order to check some more complex conditions.
IMO you don't need to mock Matcher but create some abstraction for it. You can create your own Matcher interface and mock it. Next create class which implements your interface and extends a coduo Matcher

BTW. Adding uuid pattern for RegexConverter looks interesting.

@Canadadry
Copy link
Contributor Author

Yes it work too. It seems to me like adding another layer of abstraction only for mocking. Not very fan of this solution, but it does not require to fork the all project, which is a plus.
I might go fir this solution if you add uuid pattern to RegexConverter.

I use this pattern in unit test of api with hateos implementation. Wich has a lot a autoreferencing link, with obviously a lot of uuid.

@teklakct
Copy link
Contributor

teklakct commented Dec 7, 2017

@norzechowicz would you remove final statement from Matcher?

@norberttech
Copy link
Member

I’m not sure, this final statement is so meaningless and creatig abstraction just to mock Matcher, sounds a bit like overengeneerig something simple. I would say, lets remove it

@Canadadry
Copy link
Contributor Author

Canadadry commented Dec 7, 2017

Before you do anything, removing the final statement is not enough to allow mocking, I had to do some other minor fix. You can have a lot at what I have on my fork. (From memory I have also hadded an interface and make the factory return the interface)

For the exact use case here how Create my mock.

 $simpleFactory = $this->getMockBuilder('Coduo\PHPMatcher\Factory\SimpleFactory')
            ->setMethods(['createMatcher'])
            ->getMock();
 $matcher = $this->getMockBuilder('Coduo\PHPMatcher\MatcherInterface')
            ->setMethods(['match','getError'])
            ->getMock();
 $simpleFactory->method('createMatcher')->willReturn($matcher);
 $matcher->method('match')->willReturn($matcherReturn);
 $matcher->method('getError')->willReturn('mock error');

Also one last thing, I have written the first version of this mock of your service montg ago, the issue only occured recently when you changed the interface.

edit : after reflexion, removing the final keywork is not necessary if you had an interface, because I mock the interface and not the matcher itself, wich is way cleaner.

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

No branches or pull requests

3 participants