41

Why doesn't PHPUnit do last exception assertion in this code?

public function testConfigOverriding()
{
    $this->dependencyContainer = new DependencyContainer(__DIR__ . "/../../Resources/valid_json.json");
    $this->assertEquals('overriden', $this->dependencyContainer->getConfig('shell_commander')['pygmentize_command']);

    $unexisting = "unexisting_file";
    $this->setExpectedException('Exception', "Configuration file at path \"$unexisting\" doesn't exist.");
    $this->dependencyContainer = new DependencyContainer($unexisting);

    $invalid = __DIR . "/../../Resources/invalid_json.json";
    $this->setExpectedException('Exception', "Configuration JSON file provided is not valid.");
    $this->dependencyContainer = new DependencyContainer($invalid);
}

So basically: it tests whether "unexsisting_file" exception was thrown, but completely ignores "invalid json" test. Do I need to make separate tests for each exception thrown?

7 Answers 7

70

Even with setExpectedException, your test is still regular PHP code, and follows PHP's normal rules. If an exception is thrown, program flow immediately jumps out of the current context until it reaches a try/catch block.

In PHPUnit, when you use setExpectedException, it tells PHPUnit's core that when it should be expecting an exception from the code that's about to run. It therefore waits for it with a try/catch block and passes the test if the catch is called with the type of exception it is expecting.

However, within your test method, the normal PHP rules still apply -- when the exception happens, that's the end of the current code block. Nothing more in that method will be executed, unless you have your own try/catch block within the test method.

So therefore, in order to test multiple exceptions, you have a few options:

  1. Add your own try/catch to the test method, so that you can carry on with further tests within that method after the first exception.

  2. Split the tests into separate methods, so that each exception is in its own test.

  3. This particular example looks like a good case to use PHPUnit's dataProvider mechanism, because you're basically testing the same functionality with two sets of data. The dataProvider feature allows you to define a separate function that contains an array of input data for each set of values you want to test. These values are then passed one set at a time into the test method. Your code would look something like this:

     /**
      * @dataProvider providerConfigOverriding
      */
     public function testConfigOverriding($filename, $expectedExceptionText) {
         $this->dependencyContainer = new DependencyContainer(__DIR__ . "/../../Resources/valid_json.json");
         $this->assertEquals('overriden', $this->dependencyContainer->getConfig('shell_commander')['pygmentize_command']);
    
         $this->setExpectedException('Exception', $expectedExceptionText);
         $this->dependencyContainer = new DependencyContainer($filename);
     }
    
     public function providerConfigOverriding() {
         return array(
             array('unexisting_file', 'Configuration file at path "unexisting_file" doesn\'t exist.'),
             array(__DIR__ . "/../../Resources/invalid_json.json", "Configuration JSON file provided is not valid."),
         );
     }
    
0
40

I found the easiest way of continuing the test after an exception was to implement the try/finally block within the test. This essentially allows the execution of the test to continue regardless of any exception being thrown.

This was my implementation:

$this->expectException(InvalidOperationException::class);

try {
    $report = $service->executeReport($reportId, $jobId);
} finally {
    $this->assertEquals($report->getStatus(), StatusMapper::STATUS_ABORTED);
}
0
9

If you need to perform additional assertions after exception has been thrown, just use this template:

    //You can use annotations instead of this method
    $this->expectException(FooException::class);

    try {
        $testable->setFoo($bar);
    } catch (FooException $exception) {
        //Asserting that $testable->foo stays unchanged
        $this->assertEquals($foo, $testable->getFoo());
        //re-throwing exception
        throw $exception;
    }
4

For anyone looking to do what's in the question title, this is the cleanest thing I've come up with.

$exception_thrown = false

try {
    ... stuff that should throw exception ...
} catch (SomeTypeOfException $e) {
    $exception_thrown = true;
}

$this->assertSame(true, $exception_thrown);
0
2

Building on top of @SDC's answer, I recommend the following

  • split the tests even further
  • avoid using instance properties to reference the system under test

Splitting the tests further

There's a problem with multiple assertions in a single test if the assertions are not related to the same behaviour: you cannot name the test properly, you might even end up using and within the test method name. If that happens, split the tests into separate tests

Avoid using instance properties for the SUT

When I started writing tests, I felt that there's an opportunity to reduce code duplication when arranging the system under test (SUT) in setUp, and then referencing the SUT via the corresponding instance properties in the individual tests.

This is tempting, but after a while, when you start extracting collaborators from the SUT, you will have the need to set up test doubles. In the beginning this might still work for you, but then you will start setting up test doubles differently in different tests, and all the duplication that was aimed to avoid earlier comes back at you: you will end up setting up both the test doubles, and arranging the SUT in your test again.

When I encounter this in code reviews, I like to reference

and I recommend reading it.

The important point is, you want to make it easy to write and maintain tests. Maintaining tests (or any code, if you will) means mostly making the code easily readable. If you read a bit of code, let's say, a class method, you want to easily understand what is about, and ideally, the method should do what you would expect it to do from its class name. If you are testing different behaviours, make it obvious by creating different test methods.

This also has the advantage that if you run your tests with

$ phpunit --testdox

you end up with a nice list of expected behaviours, see

Example based on your Question

Note The comments I provide in this example are only to illustrate the idea of further splitting the tests, in actual code I would not have them.

/**
 * The name of this method suggests a behaviour we expect from the
 * constructor of DependencyContainer
 */
public function testCanOverrideShellCommanderConfiguration()
{
    $container = new DependencyContainer(__DIR__ . '/../../Resources/valid_json.json');

    $this->assertEquals(
        'overriden', 
        $container->getConfig('shell_commander')['pygmentize_command']
    );
}

/**
 * While the idea of using a data provider is good, splitting the test
 * further makes sense for the following reasons
 *
 * - running tests with --testdox option as lined out above
 * - modifying the behaviour independently 
 *     Currently, a generic Exception is thrown, but you might 
 *     consider using a more specific exception from the SPL library, 
 *     (see http://php.net/manual/en/spl.exceptions.php), 
 *     or creating your own NonExistentConfigurationException class, 
 *     and then a data provider might not make sense anymore)
 */
public function testConstructorRejectsNonExistentConfigurationFile()
{
    $path = 'unexisting_file';

    $this->setExpectedException(\Exception::class, sprintf(
        'Configuration file at path "%s" doesn\'t exist.',
        $path
    ));

    new DependencyContainer($path);
}

public function testConstructorRejectsInvalidConfigurationFile()
{
    $path = __DIR__ . '/../../Resources/invalid_json.json';

    $this->setExpectedException(
        \Exception::class, 
        'Configuration JSON file provided is not valid.'
    );

    new DependencyContainer($path);
}

Note I would also recommend to take a look at

0

First, there is a typo. Replace

__DIR

with

__DIR__

:)


Thanks to @SDC's comment, I realized that you'll indeed need spearate test methods for each exception ( if you are using the expectedException feature of PHPUnit ). The third assertion of your code is just not being executed. If you need to test multiple Exception in one test method I would recommend to write your own try catch statements in the test method.

Thanks again @SDC

17
  • Indeed, good catch, but if that code were executed I should get an error in the output. It just doesn't run anything below first expected exception.
    – user2742648
    Commented Jan 28, 2013 at 12:25
  • Also after fixing the typo?
    – hek2mgl
    Commented Jan 28, 2013 at 12:26
  • Ok. Will prepare a test locally
    – hek2mgl
    Commented Jan 28, 2013 at 12:27
  • Here is a link of already prepared: dropbox.com/s/601oxl2lu4afurl/PHPygmentizator.tar.gz So you don't have to write it again.
    – user2742648
    Commented Jan 28, 2013 at 12:29
  • 1
    Sorry, but in the example given here the second exception will never be tested. The test will pass because the first exception is thrown when expected, but at that point the test method will stop because it has thrown an exception. It may be expected, but it isn't caught inside the test method; it's caught further up inside PHPUnit, so the rest of the test method will never be run. You can prove this by not expecting the second exception. The test will still pass because throwExceptionB isn't called. You need either two test methods or a try/catch in the test method.
    – SDC
    Commented Jan 28, 2013 at 13:30
0

I have this in a trait

    public function assertWeGotException($callback,$expectedExceptionClass = null){
        $gotException = false;
        try {
            $callback();
        }catch (\Exception $e){
            $gotException = true;
            if (!empty($expectedExceptionClass)){
                $exceptionGottenClass = get_class($e);
                $this->assertTextEquals($expectedExceptionClass,$exceptionGottenClass,'We got a different Exception');
            }
        }
        $this->assertTrue($gotException,'We got no exception');
    }

And I call it like this:

    $this->assertWeGotException(function (){
        // Code that throws any exception here
        throw new \Exception();
    });

    $this->assertWeGotException(function (){
        // Code that throws a RuntimeException here
        throw new \RuntimeException();
    },'RuntimeException');