Join GitHub today
$this->at($index); bug? #674
I'm fairly sure this does not work as intended.
With a test that does:
This works exactly as intended/documented. "Returns a matcher that matches when the method it is evaluated for is invoked at the given $index."
However, if another method is called on the mock. For instance:
The test now fails with an unhelpful error "Mocked method does not exist". It does exist and it works perfectly until the call to $mock->test().
Either the documentation needs to be updated to say that $index is the running count of the total number of methods called on the mock or the code needs updating to count on a per method basis.
As it stands, the test will fail based on implementation changes to the code being tested in which it shouldn't fail the test. I don't see why my second test should fail. It's still being called with the right parameters in the correct order. The fact that $mock->test() has been called is irrelevant.
As a bare minimum the returned error "mocked method does not exist" needs to be updated. It's both incorrect and misleading.
This is a working fix.
Hi, thanks for the report.
This has been discussed various times and there should be a couple of closed bugs with longer discussions here and on mock-object.
Yeah, surely the wording on http://www.phpunit.de/manual/current/en/test-doubles.html#test-doubles.mock-objects.tables.matchers could be improved or a note could be added. I'm open to suggestions.
The issue with the current
There is no question that the
If anything we could add another matcher that works on a per method basis. I'm not sure if thats worth it, it surely would make certain scenarios easier to test and since the order of calls is sometimes important it might be worth considering it.
If order of all method calls really matters and should be tested (thats what
referenced this issue
Sep 30, 2012
For the doc part I've opened: sebastianbergmann/phpunit-documentation#73 and for the rest all i can say in the end "Sorry. It was designed that way, it's not nice but we can't change it now due to BC".
Hope thats fine with you and if you have a suggestion/PR for another matcher feel free to add it :)
That's understandable but as I understand it, there's no way to test whether a method has been called multiple times without using at() unless the method returns a value.
Using exactly() requires that the method is called with the same parameters. Essentially I want to test that a method has been called multiple times with specific parameters. (e.g. in my example, a 1 then a 2. The order doesn't actually matter)
Either the documentation doesn't make it clear how this can be achieved or I can find no way to do it.
After looking into it further, I believe the best way to achieve this would be to add a logicalEach() or similar:
Which would check that the method 'foo' is called with each of the supplied constraints.
At the moment this can be achieved with logicalOr()
However, it passes if foo is called with
I had a very quick try at implementing this:
Which, in theory, should work. However I don't know enough about the internals of PHPUnit to know how it works. In this example above with a call to $mock->foo(1); then $mock->foo(2);, PHPUnit_Framework_Constraint_Each::evaluate() is called five times. Twice with
Firstly: Do you think the addition of logicalEach() or something with a more appropriate name is the right approach? ->at() isn't ideal in a lot of cases because it exposes implementation details to the test.
Secondly: If so, why is evaluate called five times? I was expecting to see it called twice. Once for each call to $mock->foo()
Thanks for your time!
Oh. Quite nice idea!
I was thinking about something like
Your implementation doesn't depend on the order of arguments but just makes sure each one would get passed once.
Maybe another idea/name could also be
For the implementation part:
I'm not fluent in the mock object lib myself but I've tried it myself and i also get 5 calls to the object. No immediate clue why though and digging through the backtraces and figuring out the code and how it's resolved will take some time.
In general i like your proposal even so I'm not sure if:
or rather just strictly
I think the second one or tests become far too vague. Perhaps rather than logicalEach it should be named logicalSequence or your suggestion of logicalOnConsecutiveCalls which also implies the parameters are called in a sequence.
Implementation wise, I've tracked down two of the extra calls. In PHPUnit_Framework_MockObject_Matcher_Parameters::matches() it calls $this->verify() which is what is calling PHPUnit_Framework_Constraint_Each::evaluate() . I can't see the reasoning behind calling $this->verify() in matches() though.
I haven't been able to track down the final call to PHPUnit_Framework_Constraint_Each::evaluate(), however.
Here's an incredibly crude workaround. It's the same as before but keeps track of the number of calls to evaluate(). While this ensures compatibility with all existing code and is entirely self-contained, it's rather brittle. If evaluate() gets called additional/fewer times it'll stop working as intended. That said... it does work:
Some test code.
As I said it's very crude but it does work. I'll play around with how it can work with multiple parameters.
Edit: The example above works as you'd specified in your first less strict example
Whereas this doesn't:
I'm not quite sure how I can change it to allow for the stricter variant you mentioned because to do that, I believe each logicalEach() would need access to the other logicalEach().
If you or anyone else can point me in the right direction that'd be great! I'd be interested to see others opinions on this idea as well.
I implemented a atMethod($method, $index)
its a little verbose, because you would write:
Should i send a pull request to MockObjects?
Incredibly sorry to ping all of the honorable participants, but I still don't understand how to achieve this, 5 years later. If it was down to returning values every time, then maybe I could do with