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

run tearDown() if TestCase fails with PHP error [closes #254] #255

Closed
wants to merge 1 commit into from

Conversation

@richard-ejem
Copy link
Contributor

richard-ejem commented Sep 16, 2015

No description provided.

@richard-ejem richard-ejem force-pushed the richard-ejem:teardown branch 2 times, most recently from 0f84e3f to 016d854 Sep 18, 2015
@@ -107,8 +107,12 @@ public function runTest($method, array $args = NULL)

foreach ($data as $params) {
try {
$me = $this; // PHP 5.3 compatibility
$lastHandler = set_error_handler(function() use ($me, &$lastHandler) {
$me->failsafeTearDown();

This comment has been minimized.

Copy link
@milo

milo Sep 26, 2015

Member

Restore handler here to prevent recursion.

This comment has been minimized.

Copy link
@richard-ejem

richard-ejem Sep 27, 2015

Author Contributor

done ;)

@@ -174,6 +179,10 @@ protected function tearDown()
{
}

/** @internal */
public function failsafeTearDown() {
$this->tearDown();

This comment has been minimized.

Copy link
@milo

milo Sep 26, 2015

Member

What about error in tearDown(). How to handle it?

This comment has been minimized.

Copy link
@richard-ejem

richard-ejem Sep 27, 2015

Author Contributor

@milo well, if tearDown() fails, there is nothing more we can do (point of this pull request is to execute tearDown if setUp or test* fails).

$me = $this; // PHP 5.3 compatibility
$lastHandler = set_error_handler(function() use ($me, &$lastHandler) {
$me->failsafeTearDown();
return ($lastHandler === NULL) ? FALSE : $lastHandler();

This comment has been minimized.

Copy link
@milo

milo Sep 26, 2015

Member

Missing error handler arguments.

This comment has been minimized.

Copy link
@richard-ejem

richard-ejem Sep 27, 2015

Author Contributor

thanks, fixed now

@dg dg force-pushed the nette:master branch from 2dbfc68 to eb4ab0f Oct 1, 2015
@@ -174,6 +180,10 @@ protected function tearDown()
{
}

/** @internal */
public function failsafeTearDown() {

This comment has been minimized.

Copy link
@milo

milo Oct 3, 2015

Member

Name it as _tearDown() and fix coding style. Double empty line between methods, one empty line after the last one, curly bracket on separated line.

restore_error_handler();
$me->failsafeTearDown();
return ($lastHandler === NULL) ? FALSE : call_user_func_array($lastHandler, func_get_args());
});
$this->setUp();

This comment has been minimized.

Copy link
@milo

milo Oct 3, 2015

Member

Move setUp() call before error handler, exceptions aren't catched too.

restore_error_handler();
$me->failsafeTearDown();
return ($lastHandler === NULL) ? FALSE : call_user_func_array($lastHandler, func_get_args());
});

This comment has been minimized.

Copy link
@milo

milo Oct 3, 2015

Member

Fix coding style. Space before (), space after &.

This comment has been minimized.

Copy link
@pavelkouril

pavelkouril Oct 4, 2015

Also, shouldn't it be function ()?

@milo

This comment has been minimized.

Copy link
Member

milo commented Oct 3, 2015

What about

    /** @internal */
    public function _errorHandler()
    {
        restore_error_handler();
        @$this->tearDown(); // ignore, error from testMethod() has higher precedence

        return $this->previousHandler
            ? call_user_func_array($this->previousHandler, func_get_args())
            : FALSE;
    }
milo added a commit to milo/tester that referenced this pull request Oct 9, 2015
milo added a commit to milo/tester that referenced this pull request Oct 9, 2015
milo added a commit to milo/tester that referenced this pull request Oct 9, 2015
@richard-ejem

This comment has been minimized.

Copy link
Contributor Author

richard-ejem commented Oct 9, 2015

@milo thanks for suggestion, I like your _errorHandler alternative :) fixed in my code

@richard-ejem richard-ejem force-pushed the richard-ejem:teardown branch from 92524fa to 6fcfe03 Oct 9, 2015
@richard-ejem richard-ejem force-pushed the richard-ejem:teardown branch from 6fcfe03 to 91cc74a Oct 9, 2015
milo added a commit to milo/tester that referenced this pull request Oct 12, 2015
@milo milo mentioned this pull request Oct 12, 2015
@milo

This comment has been minimized.

Copy link
Member

milo commented Oct 13, 2015

Solved by d25b672.

@milo milo closed this Oct 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.