tearDown not executed when test fails with error or exception

[richard-ejem]

I suppose this is a bug - this tidy method should be executed no matter if the test succeeds or fails. Actually tearDown works if the test fails on assertion, but not on error/exception.

//Edit: it works on exception, but not on any runtime error.

Please confirm it is a bug, I can fix and pullrequest it.

[milo]

I guess it's already fixed fc6d855

[MartinMystikJonas]

TearDown is not executed when PHP stops on Fatal error I think. For all other errors and exceptions it works.

@milo: Maybe add tearDown to shutdown_handler?

[richard-ejem]

Confirming it does not work on errors, even E_NOTICE.
try this:

class ErroredTest extends Tester\TestCase
{
    public static $tornDown = FALSE;
    protected function tearDown() { self::$tornDown = TRUE; }
    public function testPublic() { ++$a; }
}

register_shutdown_function(function() {
    Assert::true(ErroredTest::$tornDown);
    exit(0); //to reset the exit code back from 255 after E_NOTICE killed the script
});

$test = new ErroredTest;
$test->run();

Registering teardown to shutdown function seems legit.

[milo]

Test will fail on unexpected error. There is no need to run tearDown().

[richard-ejem]

It will skip tearDown on unexpected error, but not on unexpected exception, isn't that just inconsistent?

tearDown is supposed to "tidy after the test", i.e. restore test database state, delete temporary files etc. This should be done regardless ot the test result, including termination on error.

[MartinMystikJonas]

I agree witch @richard-ejem. Not executing tearDown can lead to unexpected bahaviour of following tests.

[milo]

Tests must be independent. The only reason for executing tearDown() when test method fails, used to be @throw annotation. After 710c4b5 it can be removed.

[richard-ejem]

@milo what should the tearDown() method be used for by your opinion?

As i wrote: I think it should tidy database, temp files etc., which should happen regardless of test result.

So, If you say that it is not needed when test ends with error, I assume that you use tearDown for something different, that makes sense when the test succeeds, but not when it fails?

[milo]

I'm using it for unlocking and additional assertions. All preparations should be done in setUp(), it prepares environment for testing.

On test fail, I need to preserve environment as is as much as possible.

[richard-ejem]

OK, that's quite valid reason. Thanks, it's clear to me now that tearDown() makes sense only for succeeding tests to cleanup before following test methods.
And I just figured out that in case I really need to cleanup some mess regardless of test result (mentioned temp files etc.), I can use TestCase::__destruct, which executes even when failing with notice :)

Therefore I think the correct solution is to not execute teardown even when the test fails on exception, am I correct? If yes, I'll change the pull request to do that.

[milo]

I already opened #257, let's wait for David's opinion on this.

[richard-ejem]

OK, we may close it there, thanks ;)
(and I'm happy that I've found a valid usage of a destructor :D )