phpspec / prophecy

Highly opinionated mocking framework for PHP 5.3+
MIT License
8.53k stars 242 forks source link

UnexpectedCallException extends RuntimeException which may give false positives if \RuntimeException is expected to be thrown #394

Open mieszkomalawski opened 6 years ago

mieszkomalawski commented 6 years ago

Consider this situation:

TestCase expects \RuntimeException to be thrown (we are testing some case where tested class throws \RuntimeException - expected behavior)

TestCase also expects that some mock will be called eg. logging service will be called before throwing exception

Mock expectation faile (eg. method is called with wrong args on logging service ), but test passes because it received UnexpectedCallException which extends \RuntimeException so the first expectation is met.

Possible solution: mock expectations should be verified before exception make to the part where phpunit verifies exception expectations

jakzal commented 6 years ago

The process is not under prophecy's control. Mock verification is triggered in PHPUnit's TestCase.

Seldaek commented 5 years ago

Alternative solution here would be for prophecy to only extend \Exception to avoid possible clashes. That would fix the problem. I just spent 10minutes trying to figure out why my code was dying mid-test until I realized the problem and removed the expectedException annotation to find "Prophecy\Exception\Call\UnexpectedCallException: Unexpected method call". Easy to fix now.. but makes me not so confident about this test's robustness going forward if any new call can trigger weird unrelated errors due to the exception mismatch. I can try/catch myself and assert the exact exception class as a workaround, but still would be nice to see this fixed here.