Skip to content
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

[PhpUnitBridge] fixed PHPUnit 8.3 compatibility: method handleError was renamed to __invoke #32933

Merged

Conversation

karser
Copy link
Contributor

@karser karser commented Aug 4, 2019

Q A
Branch? 4.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #32879
License MIT

The PHPUnit method handleError was renamed to __invoke in v8.3.

So we should check in Symfony DeprecationErrorHandler if method handleError exists, otherwise call __invoke

It works with phpunit v8.2.5 and 8.3.2.
The PHPUnit handler is called when I trigger some error, e.g iconv('fdsfs', 'fsdfds', '');

@nicolas-grekas nicolas-grekas changed the title Fixed PHPUnit 8.3 incompatibility: method handleError was renamed to __invoke (from stacktrace) [PhpUnitBridge] fixed PHPUnit 8.3 compatibility: method handleError was renamed to __invoke Aug 5, 2019
@nicolas-grekas nicolas-grekas changed the base branch from 4.3 to 3.4 August 5, 2019 10:01
@nicolas-grekas nicolas-grekas force-pushed the phpunit83-errorhandler-fix-stacktrace branch 2 times, most recently from a505eab to b253eb6 Compare August 5, 2019 10:05
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(I rebased this PR for 3.4 and made some fixes)

@nicolas-grekas nicolas-grekas force-pushed the phpunit83-errorhandler-fix-stacktrace branch from b253eb6 to 0c9539f Compare August 5, 2019 10:06
@karser
Copy link
Contributor Author

karser commented Aug 5, 2019

I see. Anyways I'm glad I could help you with the research part and learnt some error handling stuff. Still have one question though, why do you return a function that returns false in getPhpUnitErrorHandler? isn't just false not enough in the set_error_handler callback?

@nicolas-grekas
Copy link
Member

why do you return a function that returns false in getPhpUnitErrorHandler? isn't just false not enough in the set_error_handler callback?

the return value is used with call_user_func, returning false here would then break.

@karser
Copy link
Contributor Author

karser commented Aug 5, 2019

Should I apply fabbot patch?
image

@nicolas-grekas
Copy link
Member

Nope, fabbot doesn't work for the bridge.

@nicolas-grekas
Copy link
Member

Thank you @karser.

@nicolas-grekas nicolas-grekas merged commit 0c9539f into symfony:3.4 Aug 5, 2019
nicolas-grekas added a commit that referenced this pull request Aug 5, 2019
…ndleError was renamed to __invoke (karser)

This PR was merged into the 3.4 branch.

Discussion
----------

[PhpUnitBridge] fixed PHPUnit 8.3 compatibility: method handleError was renamed to __invoke

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #32879   <!-- #-prefixed issue number(s), if any -->
| License       | MIT

The PHPUnit method  [handleError](https://github.com/sebastianbergmann/phpunit/blob/8.2.5/src/Util/ErrorHandler.php#L38) was renamed to [__invoke](https://github.com/sebastianbergmann/phpunit/blob/8.3/src/Util/ErrorHandler.php#L71) in v8.3.

So we should check in Symfony [DeprecationErrorHandler](https://github.com/symfony/symfony/blob/v4.3.3/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php) if method `handleError` exists, otherwise call `__invoke`

It works with phpunit v8.2.5 and 8.3.2.
The PHPUnit handler is called when I trigger some error, e.g `iconv('fdsfs', 'fsdfds', '');`

Commits
-------

0c9539f [PhpUnitBridge] fixed PHPUnit 8.3 compatibility: method handleError was renamed to __invoke
@@ -44,6 +48,7 @@ public static function register($mode = 0)
}

$UtilPrefix = class_exists('PHPUnit_Util_ErrorHandler') ? 'PHPUnit_Util_' : 'PHPUnit\Util\\';
self::$isAtLeastPhpUnit83 = method_exists('PHPUnit\Util\ErrorHandler', '__invoke');
Copy link
Member

Choose a reason for hiding this comment

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

ErrorHandler::class

Copy link
Member

Choose a reason for hiding this comment

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

nvm. Didn't notice that version were 3.4

public static function getPhpUnitErrorHandler()
{
if (!self::$isAtLeastPhpUnit83) {
return (class_exists('PHPUnit_Util_ErrorHandler', false) ? 'PHPUnit_Util_' : 'PHPUnit\Util\\').'ErrorHandler::handleError';
Copy link
Member

Choose a reason for hiding this comment

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

return [ErrorHandler::class, 'handleError'] since #32940

Copy link
Member

Choose a reason for hiding this comment

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

nvm. Didn't notice that version were 3.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants