Skip to content

[PHPUnit Bridge]Implement poor man's error handler #21795

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[PHPUnit Bridge]Implement poor man's error handler #21795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public static function register($mode = 0)
$trace = debug_backtrace(true);
$group = 'other';

$isWeak = DeprecationErrorHandler::MODE_WEAK === $mode || (DeprecationErrorHandler::MODE_WEAK_VENDORS === $mode && $isVendor = $inVendors($file));
$isWeak = DeprecationErrorHandler::MODE_WEAK === $mode || (self::MODE_WEAK_VENDORS === $mode && $isVendor = $inVendors($file));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is a no-go. The PHPUnit Bridge is still compatible with PHP 5.3 even in master (our own testsuite uses the newest bridge even in LTS branches)

Copy link
Contributor Author

@greg0ire greg0ire Feb 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof: this is to demonstrate the error handling, and will be removed from the PR before merging. Please read the commit messages :P

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any way to showcase it in a test case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would imply isolating it in a public method. Or maybe… in a private method, and then do a bit of black magic to make it public with reflection. I'll see what I can do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm… that won't be so easy since we're inside a closure, which is itself inside another closure. But the condition to run this piece of code is just to change the error handler. Let's try that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why focus on this specific way to trigger a failure? Aren't there "userland" side to showcase the new behavior in e.g. a phpt test case?

Copy link
Contributor Author

@greg0ire greg0ire Feb 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No there aren't AFAIK. I think this only happens when there is a programming error in the deprecation error handler. Should I think otherwise? I only saw it in action when (failing at) writing my other PR on this error handler, so maybe I'm missing other possible ways to trigger this piece of code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? Then we don't care sorry...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh. Please reconsider… these few lines can save literal hours to anyone try to contribute to this error handler. I know the audience is small, but it exists.


$i = count($trace);
while (1 < $i && (!isset($trace[--$i]['class']) || ('ReflectionMethod' === $trace[$i]['class'] || 0 === strpos($trace[$i]['class'], 'PHPUnit_') || 0 === strpos($trace[$i]['class'], 'PHPUnit\\')))) {
Expand Down Expand Up @@ -196,7 +196,7 @@ public static function register($mode = 0)
$colorize = function ($str) { return $str; };
}
if ($currErrorHandler !== $deprecationHandler) {
echo "\n", $colorize('THE ERROR HANDLER HAS CHANGED!', true), "\n";
echo "\n", $colorize("THE ERROR HANDLER HAS CHANGED!\n".var_export(error_get_last(), true), true), "\n";
}

$cmp = function ($a, $b) {
Expand Down
43 changes: 43 additions & 0 deletions src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/failure.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
--TEST--
Test DeprecationErrorHandler failure
--FILE--
<?php
namespace Symfony\Bridge\PhpUnit;
function error_get_last()
{
return array(
'type' => E_ERROR,
'message' => 'failed to load flux capacitor',
'file' => '/wherever/the/deprecation_handler_is',
'line' => 42
);
}

putenv('SYMFONY_DEPRECATIONS_HELPER');
putenv('ANSICON');
putenv('ConEmuANSI');
putenv('TERM');

$vendor = __DIR__;
while (!file_exists($vendor.'/vendor')) {
$vendor = dirname($vendor);
}
define('PHPUNIT_COMPOSER_INSTALL', $vendor.'/vendor/autoload.php');
require PHPUNIT_COMPOSER_INSTALL;
require_once __DIR__.'/../../bootstrap.php';

trigger_error('test failure', E_USER_DEPRECATED);
set_error_handler('var_dump');

--EXPECTF--
THE ERROR HANDLER HAS CHANGED!
array (
'type' => 1,
'message' => 'failed to load flux capacitor',
'file' => '/wherever/the/deprecation_handler_is',
'line' => 42,
)

Other deprecation notices (1)

test failure: 1x