Problem with formatObjects() and erroneous getters

[richard-ejem]

Encountered using Nette framework, where deprecated getters trigger E_USER_DEPRECATED error, and nette/tester that has no custom global error handler transforming errors into exceptions like PHPUnit:

mockery/library/Mockery.php

Line 629 in 8c66b8c

} catch (\Exception $e) {

when the extracted getter triggers an error, it is normally transformed to PHPUnit_Framework_Error when using PHPUnit. However in nette/tester, the error is not handled and fails the test.

It is a serious wtf moment, as Mockery calls formatObject often internally and there is no clue why it is calling some unrelated getters.

Making this hidden dependency on PHPUnit's error handling behavior is really confusing.

What do you think would be an appropriate fix? Set simple custom error handler before calling the getter? Or somehow detect if PHPUnit's handler is used or not?

[robertbasic]

@richard-ejem mind providing a small example repo that shows the behaviour you see? It would make figuring out what's wrong so much easier for us. Thanks!

[richard-ejem]

@robertbasic Here you have a minimum working example, comparing the same behaviour inside PhpUnit vs. Nette Tester: https://github.com/richard-ejem/mockery-bug-example

[robertbasic]

Found the pull request #149 which added the try/catch block that now catches these deprecations errors from PHPUnit. Now need to figure out why was it added.

[robertbasic]

And here's the commit 7956cd3 that added the calling of all the get* methods on a mocked object...

I have no idea why we need this?

[richard-ejem]

@robertbasic I think that the try/catch block is not wrong in there. It is more likely not enough - it needs to handle PHP errors too, not only catch exceptions.
You definitely do not want your test to fail on a deprecated getter which you actually never call, but Mockery tries to get a value from it to pretty-print an object.
So, the succeeding test behaviour in the PHPUnit version of my example is what would I expect, I consider the failing test with Nette Tester to be wrong behavior.

[robertbasic]

@richard-ejem I'm now thinking that Mockery shouldn't be doing random calls to any get* or is* method on the mocked object, as it is now.

[richard-ejem]

@robertbasic I agree with that as long as many other side effects may be triggered. That is probably impossible without dropping support of such pretty-printing of objects with getter-retrieved values, the question is if anyone needs it.

[davedevelopment]

I have a branch somewhere that was going to use the phpunit object dumping to replace all of that formatObject nonsense, but I can't find it right now. Too many bloody branches :)

I'd say to rip out the method calling as a starter for ten until I can't sort my stuff out.

[robertbasic]

The simplest fail that triggers the pretty printing is

        $dt1 = new DateTime('now', new DateTimeZone('UTC'));
        $dt2 = new DateTime('now', new DateTimeZone('UTC'));
        $mock = \Mockery::mock('SomeClass');
        $mock->shouldReceive('foo')->with($dt1);
        $mock->foo($dt2);

but for objects I'd rather just show an error message saying something like "You can't compare two separate instances of a class expecting them to be the same"

[richard-ejem]

@davedevelopment that would be nice to use another existing solution, but please keep in mind that not everybody uses PHPUnit (I personally don't like it because it is unnecessarily complex in compare with Nette Tester) and adding whole PHPUnit as a dependency or even some of its few big repositories it is splitted into just for dumping objects is not so nice.

[robertbasic]

I created #732 that removes this pretty printing, add opened a new issue #733 to add a nicer way of pretty printing back in.