Fix message for catching wrong exception in raises() #1252
Conversation
tests/utils/tools.php
Outdated
| @@ -607,7 +607,7 @@ function raises($function, $type, $infunction = null) { | |||
| } | |||
| printf("OK: Got %s\n", severityToString($e->getSeverity())); | |||
| } else { | |||
| printf("ALMOST: Got %s - expected %s\n", get_class($e), $exceptionname); | |||
| printf("ALMOST: Got %s - expected %s\n", get_class($e), ErrorException::class); | |||
There was a problem hiding this comment.
Actually, this is not a complete fix. This else condition is also reached if the severity (e.g. notice, warning) is incorrect. I'll revise that in a bit.
There was a problem hiding this comment.
Yes. In this case, I think we can change the catch in L593 to only catch ErrorException, so we only have to handle the case where we've caught messages with a different severity.
We should also wrap the restore_error_handler call in a finally block for the try to properly restore the error handler regardless of outcome.
There was a problem hiding this comment.
Added finally with restore_error_handler. I also improved output for converting severity to a string and refactored both raises() and throws() to reduce conditional nesting.
jmikola
changed the title
jmikola
changed the title
jmikola
force-pushed
the
raises-error-message
branch
from
04ee9bf to
7cbf854
Compare
jmikola
force-pushed
the
raises-error-message
branch
from
7cbf854 to
9e15119
Compare
The original implementation of raises() in c6a793d apparently referenced an undeclared $exceptionname variable several times. That was later addressed 804f383 but that commit neglected to fix this reference.
This change avoids a possible message where the actual exception is printed twice:
Given the error handler implementation here, we should always except an ErrorException.