Fix error_reporting() with PHP >= 8
#1378
Conversation
| Scenario: A suppressed error will not cause a spec failure | ||
| Given the spec file "spec/Runner/ErrorSuppression2/ErrorControlSpec.php" contains: | ||
| """ | ||
| <?php | ||
|
|
||
| namespace spec\Runner\ErrorSuppression2; | ||
|
|
||
| use PhpSpec\ObjectBehavior; | ||
|
|
||
| class ErrorControlSpec extends ObjectBehavior | ||
| { | ||
| function it_returns_string() | ||
| { | ||
| $this->suppressing()->shouldBe('it works!'); | ||
| } | ||
| } | ||
|
|
||
| """ | ||
| And the class file "src/Runner/ErrorSuppression2/ErrorControl.php" contains: | ||
| """ | ||
| <?php | ||
|
|
||
| namespace Runner\ErrorSuppression2; | ||
|
|
||
| class ErrorControl | ||
| { | ||
| public function suppressing(): string | ||
| { | ||
| @trigger_error('Nope!', E_USER_WARNING); | ||
|
|
||
| return 'it works!'; | ||
| } | ||
| } | ||
|
|
||
| """ | ||
| When I run phpspec | ||
| Then the suite should pass |
There was a problem hiding this comment.
this Behat scenario will fail in PHP 8 without the code change, but not in PHP 7
| } | ||
|
|
||
| function it_throws_error_exception_when_message_not_match() | ||
| { | ||
| $oldLevel = error_reporting(E_ALL); |
There was a problem hiding this comment.
adding these in all the specs removes the hidden dependency on the value set in php.ini and clarifies the purpose of the spec a bit more
|
hey @ciaranmcnulty, this is ready for review on my side. Let me know if there's anything else you'd like to see on here |
|
This PR is indeed what we should do. Thanks for this! |
| * | ||
| * @link https://www.php.net/manual/en/language.operators.errorcontrol.php | ||
| */ | ||
| function it_returns_false_when_error_suppressed_in_php_8() |
There was a problem hiding this comment.
No need to have _in_php_8 in the name, because the same behavior should be present for other versions.
There was a problem hiding this comment.
I think it's useful to see the distinction because I'm making error reporting return this code, which is specific to php 8:
error_reporting(E_ERROR | E_PARSE | E_CORE_ERROR | E_COMPILE_ERROR | E_USER_ERROR | E_RECOVERABLE_ERROR)
The spec above it uses error_reporting(0), which is the behaviour when suppressing errors in php < 8
There was a problem hiding this comment.
@kelunik Are you ok with this?
There was a problem hiding this comment.
Sure, it was just a suggestion, because from the outside, there shouldn't be a difference, regardless of the error level set with error_reporting.
|
@ciaranmcnulty hey, could you give this a review please? Cheers |
|
Hi all, It would be nice to have some feedback here. We had to add a specific condition in our tests in order to prevent failures with PHP 8. Is there anything I can do to help, feel free to let me know, I guess @AlexandruGG share the same feelings. Thanks! |
|
Thanks for this @AlexandruGG, it looks great |
|
@ciaranmcnulty thank you for reviewing and getting this merged! |
This PR improves on #1376 with a Behat feature and clearer specs.
Solves #1375.
The code change is based on:
In a nutshell this is the main issue:
Warning Prior to PHP 8.0.0, the value of the severity passed to the custom error handler was always 0 if the diagnostic was suppressed. This is no longer the case as of PHP 8.0.0.The code change allows use of the error control operator
@in PHP 8 without causing PHPSpec to fail.