expected exceptions annotations, mocked object calls, oh my.

Note: I have tested this in PHPUnit 3.4.1 and haven’t tried it out in 3.5.
Anyone who has worked with PHPUnit has most likely worked with expected exceptions and mock objects. The nice thing about working with expected exceptions is that we have access to a handy @expectedException annotation. I’ve gotten into the habit of using this for exceptions my fixtures should throw but also for when I’m using a mock object to verify a method call. So my tests usually expect foo_exception for fixture throws and when i’m testing method calls via a mock, they expect Exception. Therein lies my problem. Because all my custom class exceptions obviously extend the Exception class, I can get some false positives in testing.

require_once 'Zend/Loader/Autoloader.php';
$loader = Zend_Loader_Autoloader::getInstance();
require_once('foo.php');
class tmpTest extends PHPUnit_Framework_Testcase
{

    /**
     * @expectedException Exception
     */
    public function testFooBar()
    {
        $foo=new foo();
        $foo->bar();
    }

    /**
     * @expectedException Exception
     */
    public function testBarBaz()
    {
        $mock=$this->getMock('foo',array('baz'));
        $mock->expects($this->any())
         ->method('baz')
         ->will($this->throwException(new Exception('baz')));
        $mock->barbaz();
    }
}
class foo_exception extends Exception{}

class foo
{
    public function bar()
    {
        throw new foo_exception('bar');
    }

    public function baz()
    {
        echo "bwahn";
    }

    public function barbaz()
    {
        $this->bar();
        $this->baz();
    }
}

So here we have an expectation for Exception but if we look at the code, we see that the bar method throws a foo_exception and the testBarBaz test is trying to test for the baz call via a mock that throws an Exception. if we change the annotation to expect foo_exception, the test still passes. This leads me to believe the best way to isolate the behavior we wish to test is to not use annotation for these sorts of tests. Or if you want to use annotation, be sure to use a unique exception for the mock. This means, unfortunately for me, that I’ll have to go back through all my tests and ensure there’s no false positives.

Lesson learned: be careful using shortcuts (and don’t stand in the fire).

On a side note, this part of PHPUnit is why those tests will behave that way. The behavior is completely my fault but I wanted to confirm it was behaving because of how it was verifying the expected exception.

Enhanced by Zemanta

5 comments on “expected exceptions annotations, mocked object calls, oh my.Add yours →

  1. The issue you outline here is not the only problem with expecting only “Exception”, PHPUnit itself converts PHP warnings/errors to PHPUnit_Error_* exceptions, if it's configured to do so, and in that case if you expect an Exception, and your code throws a warning, you'll think all is well, even though you're not actually testing the code properly, and are actually hiding a warning that will then appear while running the code for real.

    For the above reason, and because it's just bad practice, PHPUnit is now (in github's master, will be part of 3.6) preventing you from setting the expected exception type to “Exception”, you need to be more specific.

    See https://github.com/sebastianbe

    1. I think preventing it is a little extreme. what about doing something like

      $e instanceof $this->expectedException && get_class($e)==$this->expectedException

      as a check? I'm only objecting to preventing expecting “Exception” because I only see making a special exception class for when I'm trying to test for a specific method call in the behaviour. Now if there's a better way of doing this without resorting to throwing a base Exception, then I'm definitely interested.

      1. >> Now if there's a better way of doing this without resorting to throwing a base Exception, then I'm definitely interested.

        Because it is a best practice (in many OOP languages), I generally extend Exception for each section, module, or component of my code so exceptions are specific.

        This makes testing more accurate and makes exceptions more concise. Checking exceptions based on the message is considered bad practice and it is easy to understand why once you've had to deal with 20 exceptions that are of the same type and only differ by the textual message.

        Also, as Jordi mentioned, taking the approach that everything is the base Exception can lead to some unexpected (at best) behavior.

        1. I guess I'm just being lazy. It doesn't put me out too much to make a Library_Tests_Exception class for when I'm trying to detect method calls via mock objects. Though it would be nice if PHPUnit had some sort of Exception for this kind of testing. ^^ or maybe I should rtfm and see if there is something I didn't know of. >.>

          It's the only time I really check via message, probably best to break that habit altogether.

Leave a Reply