Skip to content

[PhpUnitBridge] do not patch the TestCase class with PHPUnit 10+ #58571

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

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Oct 15, 2024

Q A
Branch? 7.2
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT

The PHPUnit\TextUI\Command class was removed in PHPUnit 10. Since the sole purpose of the class extended the base Command class is to register a the Symfony test listener we can completely stop patching it as support for the listener-based event system was also removed in PHPUnit 10.

@alexandre-daubois
Copy link
Member

alexandre-daubois commented Oct 15, 2024

Would it be possible to have a little description in the PR to understand why this change is required please? Would be useful if we need to come back to this someday 😄

@xabbuh xabbuh force-pushed the phpunit-wrapper-compat branch from 41cf723 to 9aea66e Compare October 15, 2024 14:24
@xabbuh xabbuh added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Oct 15, 2024
@xabbuh xabbuh force-pushed the phpunit-wrapper-compat branch 3 times, most recently from 4bda026 to 370f45d Compare October 22, 2024 13:20
The PHPUnit\TextUI\Command class was removed in PHPUnit 10. Since the sole
purpose of the class extended the base Command class is to register a the
Symfony test listener we can completely stop patching it as support for the
listener-based event system was also removed in PHPUnit 10.
@xabbuh xabbuh force-pushed the phpunit-wrapper-compat branch from 370f45d to eb42716 Compare October 22, 2024 13:24
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Easier to review without whitespaces

@@ -109,6 +109,11 @@
$PHPUNIT_VERSION = $MAX_PHPUNIT_VERSION;
}

if (version_compare($PHPUNIT_VERSION, '10.0', '>=') && version_compare($PHPUNIT_VERSION, '11.0', '<')) {
fwrite(STDERR, 'This script does not work with PHPUnit 10.'.\PHP_EOL);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fwrite(STDERR, 'This script does not work with PHPUnit 10.'.\PHP_EOL);
fwrite(STDERR, 'The simple-phpunit script does not work with PHPUnit 10.'.\PHP_EOL);

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about this because when you install the bridge the recipe will create a bin/phpunit file which one will run. So no pointer about a simple-phpunit script. My guess was that this is also the reason why the check on line 16 also talks about "This script".

@nicolas-grekas
Copy link
Member

Thank you @xabbuh.

@nicolas-grekas nicolas-grekas merged commit c221b5e into symfony:7.2 Oct 22, 2024
9 of 11 checks passed
@xabbuh xabbuh deleted the phpunit-wrapper-compat branch October 22, 2024 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug PhpUnitBridge ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" Status: Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants