Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$this->getStatus() returns STATUS_PASSED in tearDown() after unexpected Exception #3031

Closed
rafaelbeckel opened this issue Mar 1, 2018 · 7 comments

Comments

@rafaelbeckel
Copy link
Contributor

Q A
PHPUnit version 7.0.2
PHP version 7.1.14
Installation Method Composer
    public function test1() {
        $this->assertEquals(1,1);
    }

    public function test2() {
        $this->assertEquals(1,2);
    }

    public function test3() {
        $this->expectException(Exception::class);
        throw new Exception;
    }

    public function test4() {
        throw new Exception; // <= HERE
    }

    public function test5() { } 

    public function tearDown() {
        echo " " . $this->getName() . " - " . $this->getStatus() . "\n";
    }

Results in:

. test1 - 0
F test2 - 3
. test3 - 0
E test4 - 0 # Should't it be 4?

# For reference
STATUS_PASSED     = 0;
STATUS_SKIPPED    = 1;
STATUS_INCOMPLETE = 2;
STATUS_FAILURE    = 3;
STATUS_ERROR      = 4;
STATUS_RISKY      = 5;
STATUS_WARNING    = 6;
$ composer info | sort
#...
phpunit/php-code-coverage          6.0.1              Library that provides collection, processing, and rendering functionality for PHP code coverage information.
phpunit/php-file-iterator          1.4.5              FilterIterator implementation that filters files based on a list of suffixes.
phpunit/php-text-template          1.2.1              Simple template engine.
phpunit/php-timer                  2.0.0              Utility class for timing
phpunit/php-token-stream           3.0.0              Wrapper around PHP's tokenizer extension.
phpunit/phpunit                    7.0.2              The PHP Unit Testing framework.
phpunit/phpunit-mock-objects       6.0.1              Mock Object library for PHPUnit
#...
@rafaelbeckel
Copy link
Contributor Author

I did not run the testcases for phpunit itself yet, but this seems to fix it:

// phpunit/src/Framework/TestCase.php
750        } catch (Throwable $_e) {
751            $e = $_e;
752            $this->status        = BaseTestRunner::STATUS_ERROR;
753            $this->statusMessage = $e->getMessage();
754        }

@rafaelbeckel
Copy link
Contributor Author

I can reproduce the error in dev-master as well.

I noticed there is a test called testExceptionInTest() in TestCaseTest.php. It tests exactly what this issue is about and pass. If I add the line: $this->assertEquals(BaseTestRunner::STATUS_ERROR, $test->getStatus()); it'll also pass.

The status is set after tearDown() is executed. If I add the same line in test/_files/ExceptionInTest.php in its tearDown() method, then the test fails.

@rafaelbeckel
Copy link
Contributor Author

rafaelbeckel commented Mar 2, 2018

I created a new file in _files/ to isolate this case and created a new test in TestCaseTest for it.

Then, I solved the issue by adding the lines from lines 750 to 754 as I commented before.

I'll create a pull request soon.

rafaelbeckel added a commit to rafaelbeckel/phpunit that referenced this issue Mar 2, 2018
rafaelbeckel added a commit to rafaelbeckel/phpunit that referenced this issue Mar 2, 2018
@CDJojn
Copy link

CDJojn commented Mar 23, 2018

Hi, i'm almost sure, that problem is connected with this change - #2940
We reproduced this same issue in current 5.7.27 version.

I suggest to leave setting of $this->status and $this->statusMessage on new place and add back on original place. It should be working solution. (Works for me)

We need to check that tests were passed or failed in tearDown() function by calling function $this->hasFailed() and this is not currently working. In case exception was thrown in test status is not correctly set.

@sebastianbergmann
Copy link
Owner

@CDJojn Why do you need to know in tearDown() whether or not a test was successful? This makes no sense to me as that method is only meant to be used for cleanup.

@CDJojn
Copy link

CDJojn commented Apr 9, 2018

@sebastianbergmann Hello Sebastian, we don't use PHPUnit project only for unit tests, but also for functional tests which are bound on different external services and our internal CI processes. During tearDown() we do cleanup and we need to know if test was failed from any reason (own exceptions, failed assertions, technical failure), in this case we need to contact internal/external APIs to log this test was failed + save texts logs, screenshots etc...

@kolsys
Copy link

kolsys commented Dec 18, 2018

Same issue in 6.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants