39

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 Answers7

68

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."),
         );
     }
    
starball
  • 20,030
  • 7
  • 43
  • 238
SDC
  • 14,192
  • 2
  • 35
  • 48
34

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);
}
Jonathan Butler
  • 341
  • 3
  • 3
8

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;
    }
Serhii Smirnov
  • 1,338
  • 1
  • 16
  • 24
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);
Glitch Desire
  • 14,632
  • 7
  • 43
  • 55
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

localheinz
  • 9,179
  • 2
  • 33
  • 44
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

hek2mgl
  • 152,036
  • 28
  • 249
  • 266
  • 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. –  Jan 28 '13 at 12:25
  • Also after fixing the typo? – hek2mgl Jan 28 '13 at 12:26
  • Ok. Will prepare a test locally – hek2mgl Jan 28 '13 at 12:27
  • Here is a link of already prepared: https://www.dropbox.com/s/601oxl2lu4afurl/PHPygmentizator.tar.gz So you don't have to write it again. –  Jan 28 '13 at 12:29
  • sorry, I don't recognized the last comment. will download and test with your code – hek2mgl Jan 28 '13 at 12:40
  • Hmmm,I'm checking it this whole time and just can't find the mistake, I'm really curious about this. Except for "number of assertions" not changing at all when commenting/uncommenting those last 3 lines of test, it really doesn't matter what I type down there, I changed exception message to something random and it still didn't show me test failure. –  Jan 28 '13 at 12:45
  • What PHP/PHPUnit version are you using? I got a syntax error on line 60 of the test. If I outcomment this line the test runs fine. I'm using PHP 5.3.10-1ubuntu3.5 and phpunit 3.7 – hek2mgl Jan 28 '13 at 12:45
  • PHPUnit 3.7.13 by Sebastian Bergmann. PHP 5.4.10 (cli) (built: Dec 19 2012 13:05:53) –  Jan 28 '13 at 12:47
  • is the `[]` after `()` allowed in PHP 5.4? (would really appreciate this) – hek2mgl Jan 28 '13 at 12:47
  • Looks like it is, I'm using it without any errors, not even a notice. When I change that line to expect something else it throws me a failure as it should. So I guess it does. –  Jan 28 '13 at 12:49
  • This is really strange, What PHPUnit version to you use? I want to try pulling it via composer and testing with it. –  Jan 28 '13 at 12:51
  • PHPUnit 3.7.10 by Sebastian Bergmann. (And PHP 5.3 this might be important too) – hek2mgl Jan 28 '13 at 12:51
  • Hmmm, it's a minor version difference, it shouldn't affect this simple stuff. I'll check with it and do an issue report if the problem fixes. Yeah but PHP version shouldn't cause different behavior in a library if library supports both versions. –  Jan 28 '13 at 12:53
  • It works exactly the same on 3.7.10, I'll do an issue report and see what they say. –  Jan 28 '13 at 13:00
  • Will do, and will mention it in report. –  Jan 28 '13 at 13:07
  • 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 Jan 28 '13 at 13:30
  • @hek2mgl - I am busy writing up an answer as we speak :-) – SDC Jan 28 '13 at 13:36
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');
Remote
  • 11
  • 2