-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: add ExceptionHandlingTrait and Exceptions::(un)register #8380
Conversation
https://github.com/codeigniter4/CodeIgniter4/actions/runs/7348347348/job/20006326903?pr=8380 |
The weird thing is that this error comes out when running |
$ vendor/bin/phpunit tests/system/Debug/ExceptionsTest.php
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.
Runtime: PHP 8.2.13
Configuration: /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/phpunit.xml
......... 9 / 9 (100%)
Time: 00:00.501, Memory: 16.00 MB
OK (9 tests, 37 assertions) $ vendor/bin/phpunit tests/system/Debug/
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.
Runtime: PHP 8.2.13
Configuration: /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/phpunit.xml
..................E.................... 39 / 39 (100%)
Time: 00:05.049, Memory: 20.00 MB
There was 1 error:
1) CodeIgniter\Debug\ExceptionsTest::testUnregisteringUsesPhpUnitsOwnHandler
PHPUnit\Framework\Exception: ERROR:
PHP Fatal error: Uncaught TypeError: CodeIgniter\CLI\CLI::write(): Argument #1 ($text) must be of type string, null given, called in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/app/Views/errors/cli/error_404.php on line 6 and defined in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/system/CLI/CLI.php:454
Stack trace:
#0 /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/app/Views/errors/cli/error_404.php(6): CodeIgniter\CLI\CLI::write(NULL)
#1 Standard input code(701): require_once('/Users/kenji/wo...')
#2 {main}
thrown in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/system/CLI/CLI.php on line 454
ERRORS!
Tests: 39, Assertions: 228, Errors: 1. $ vendor/bin/phpunit --group=Others
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.
Runtime: PHP 8.2.13
Configuration: /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/phpunit.xml
......................................................................................................... 105 / 5112 ( 2%)
...
........................................................................ 5112 / 5112 (100%)
...
Time: 01:27.991, Memory: 126.50 MB
There was 1 error:
1) CodeIgniter\Debug\ExceptionsTest::testUnregisteringUsesPhpUnitsOwnHandler
PHPUnit\Framework\Exception: PHP Fatal error: Uncaught Error: Attempt to modify property "aliases" on null in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/tests/_support/Config/Filters.php:22
Stack trace:
#0 Standard input code(1144): require_once()
#1 {main}
thrown in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/tests/_support/Config/Filters.php on line 22
ERRORS!
Tests: 5112, Assertions: 8649, Errors: 1. |
5857200
to
3d2b763
Compare
|
||
if (Exceptions::isUsingPhpUnitErrorHandlingInSeparateProcess()) { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't still understand what's happening.
Can you explain what is the exact cause of the errors?
This hack is very bad, because the view file should not know about PHPUnit process isolation.
Can't we avoid to do like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this provide us over a simple environment check in register()
that would short circuit during testing?
@paulbalandan The following patch makes But I don't understand:
--- a/system/Debug/Exceptions.php
+++ b/system/Debug/Exceptions.php
@@ -109,7 +109,7 @@ class Exceptions
public function initialize()
{
set_exception_handler([$this, 'exceptionHandler']);
- set_error_handler([$this, 'errorHandler']);
+ //set_error_handler([$this, 'errorHandler']);
register_shutdown_function([$this, 'shutdownHandler']);
}
|
Description
This PR introduces a way to unregister the error and exception handlers originally registered in
Services::exceptions()->initialize()
by calling theunregister()
method. Moreover, theinitialize()
method is deprecated in favor of theregister()
method simply for consistency with the unregister method.Motivation
PHPUnit 10 registers its own error handler. This may cause issues if the SUT is registering its own error handler (such as the issues encountered in #8069 ).
To solve this, we have 2 possible solutions:
#[WithoutErrorHandler]
attribute on each test case. This is problematic as this attribute is on aTARGET_METHOD
so it needs to be applied to each test method.This uses the 2nd approach by introducing the
ExceptionHandlingTrait
intended to be used in tests. The trait just provides shortcut access to the register/unregister methods.This PR is intended to fix the failures in #8069 .
Checklist: