Skip to content

catch general Throwable for before test execution code#6929

Open
applyACS wants to merge 1 commit intoCodeception:mainfrom
applyACS:patch-catch-throwable
Open

catch general Throwable for before test execution code#6929
applyACS wants to merge 1 commit intoCodeception:mainfrom
applyACS:patch-catch-throwable

Conversation

@applyACS
Copy link

We had an error in a seeder and this was not caught by codeception. No test was running but codeception return 0 so it looked like everything was fine.
This is a fix this that case.

@burned42
Copy link
Contributor

I just tried throwing an \Error in the try block (because that would be caught with your change, but not without) and codeception exited with a code 255 and the output contained a stack trace for Uncaught Error. No test was executed, but codeception also didn't exit with code 0 for me.

Applying your change makes codeception exit with a code of 1 and it does list all the tests as failing.

So the change looks ok to me, but I couldn't reproduce your issue with codeception exiting with code 0 in this case.

@applyACS
Copy link
Author

my error was: Call to a member function foo() on null
this was in one of the observers
maybe that help reproducing it

@burned42
Copy link
Contributor

I tried it with

$foo = null;
$foo->foo();

in the try block and got

PHP Fatal error: Uncaught Error: Call to a member function foo() on null

but still got exit code 255.

I don't think this is a blocker for this PR though.

@TavoNiievez
Copy link
Member

I'm okay with this change, but I'm not sure it's the only place where it should be made.

@applyACS
Copy link
Author

Not sure if it's the only place. It's the place I've had a problem with.
Is this going to be approved?

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

Successfully merging this pull request may close these issues.

4 participants