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

[PHPUnit bridge] Support for PHPUnit 11.1+ #49069

Closed
Jean85 opened this issue Jan 22, 2023 · 74 comments · Fixed by #58467 · May be fixed by #49440
Closed

[PHPUnit bridge] Support for PHPUnit 11.1+ #49069

Jean85 opened this issue Jan 22, 2023 · 74 comments · Fixed by #58467 · May be fixed by #49440
Labels
Help wanted Issues and PRs which are looking for volunteers to complete them. PhpUnitBridge

Comments

@Jean85
Copy link
Contributor

Jean85 commented Jan 22, 2023

Description

PHPUnit 10 is around the corner (possibly released next month), and it contains a complete overhaul of its internals, particularly the new event system: sebastianbergmann/phpunit#4676

The new system also provides an easy way too hook into those events: https://github.com/sebastianbergmann/phpunit/blob/2a89c3c62c4d94fa92b72cb3693ca4fa057e4517/src/Event/Facade.php#L40
Among the events, there are even multiple dedicated to deprecations:

This maybe warrants a discussion here on how to tackle this new release, and how to leverage all this new features.

Example

No response

@Jean85
Copy link
Contributor Author

Jean85 commented Jan 23, 2023

I should add that those events are triggered through an error handler, and that having the bridge installed actively breaks it.

Maybe we should disable (and later disband) that feature under PHPUnit 10+?

@stof
Copy link
Member

stof commented Jan 24, 2023

Well, I don't think PHPUnit 10 contains the existing deprecation-related features of the bridge (at least not all of them).

However, we should look at rebuilding them on top of the PHPUnit events instead of replacing the error handler with our own for PHPUnit 10+.

@sebastianbergmann
Copy link
Contributor

I discussed PHPUnit 10's event system with @nicolas-grekas at SymfonyCon in November. I think the way forward for Symfony is to no longer register its own error handler for the bridge, let PHPUnit's own error handle emit events for deprecations etc., and then have the bridge subscribe to the events it is interested in. If you want the bridge to produce different output for deprecations than what PHPUnit offers out of the box.

@dozer111
Copy link

dozer111 commented Feb 8, 2023

Do u plan to fix it in 6.3?

@arderyp
Copy link
Contributor

arderyp commented Feb 9, 2023

now that PHPUnit 10 is out, I'm giving this a shot. I experience the same error noted here: symfony/recipes#1173

@nicolas-grekas notes here that this should resolve the problem.

It looks like that was merged, so I'm wondering what the next step is here.

@Jean85
Copy link
Contributor Author

Jean85 commented Feb 9, 2023

Next step would be adding the PHPUnit 10 support to the deprecation handler; this probably means a nearly total rewrite of it, to be done side-by-side to the old one, since the versioning of the bridge is tied to the framework.

@arderyp
Copy link
Contributor

arderyp commented Feb 9, 2023

Sounds pretty labor intensive.

Would you say this is a priority of the team, or consumers shouldn't expect to see 10 support for quite some time?

@stof
Copy link
Member

stof commented Feb 14, 2023

Well, this indeed requires work. Whether this will be done in time for 6.3 depends of when someone works on it (however, it would be great to have it in 6.3 rather than waiting for 6.4 if possible, given that PHPUnit 10 is stable).
Help is welcome on that topic (but that's indeed not an easy task).

@derrabus derrabus added the Help wanted Issues and PRs which are looking for volunteers to complete them. label Feb 14, 2023
@Jean85
Copy link
Contributor Author

Jean85 commented Feb 18, 2023

Can we at least consider a quick patch were we completely disable the error handler if we detect PHPUnit 10? As of now, the simple presence of the symfony/phpunit-bridge is messing up my CI on facile-it/paraunit#192, see:

I'm even setting SYMFONY_DEPRECATIONS_HELPER to disabled but it seems to not solve the issue. I'll try to set it in real env variables now, but IMO we should disable the whole flow under PHPUnit 10.

[EDIT] Using a real env var fixed it: https://github.com/facile-it/paraunit/actions/runs/4213058997/jobs/7312518640
So it's clear that we need that patch.

@arderyp
Copy link
Contributor

arderyp commented Feb 19, 2023

@Jean85 I like your idea! I'd rather be able to stay on a neutered phpunit-bridge and let it stay silent until its ready rather than switching my project over to raw phpunit/phpunit temporarily and then remembering to switch back

@arderyp
Copy link
Contributor

arderyp commented Feb 20, 2023

@Jean85 are you saying this can be worked around temporarily by setting SYMFONY_DEPRECATIONS_HELPER=false at the core os/linux level, but setting the same var in .env or phpuni.xml won't work?

@Jean85
Copy link
Contributor Author

Jean85 commented Feb 20, 2023

Yes, exactly.

The bootstrap of the bridge is loaded through the autoloader, so long before DotEnv or PHPUnit config can kick in. They can still influence the bridge behavior, but the error handler is already registered (and causing the issue) at that point.

@arderyp
Copy link
Contributor

arderyp commented Feb 21, 2023

@Jean85 I am unable to reproduce your workaround.

I removed phpunit/phpunit (I was explicitly including ^10.0). Then I did composer require --dev symfony/test-pack. It installed bin/phpunit and it also installed added phpunit/phpunit:^9 to my composer.json. So then I had to require phpunit/phpunit:^10.0 --with-all-dependencies to get phpunit back up to 10.

when I run ./bin/phpunit

$ ./bin/phpunit --version
PHP Fatal error:  Uncaught Error: Class "PHPUnit\TextUI\Command" not found in /home/arderyp/app/bin/phpunit:11
Stack trace:
#0 {main}
  thrown in /home/arderyp/app/bin/phpunit on line 1

Similar issues when I run ./vendor/bin/phpunit I get errors.

PHP Fatal error:  Uncaught Error: Class "PHPUnit\TextUI\Command" not found in /home/arderyp/app/vendor/symfony/phpunit-bridge/Legacy/CommandForV9.php:25
Stack trace:
#0 /home/arderyp/app/vendor/bin/.phpunit/phpunit-10.0-0/vendor/composer/ClassLoader.php(571): include()
#1 /home/arderyp/app/vendor/bin/.phpunit/phpunit-10.0-0/vendor/composer/ClassLoader.php(428): Composer\Autoload\includeFile()
#2 [internal function]: Composer\Autoload\ClassLoader->loadClass()
#3 /home/arderyp/app/vendor/symfony/phpunit-bridge/TextUI/Command.php(17): class_alias()
#4 /home/arderyp/app/vendor/bin/.phpunit/phpunit-10.0-0/vendor/composer/ClassLoader.php(571): include('...')
#5 /home/arderyp/app/vendor/bin/.phpunit/phpunit-10.0-0/vendor/composer/ClassLoader.php(428): Composer\Autoload\includeFile()
#6 /home/arderyp/app/vendor/bin/.phpunit/phpunit-10.0-0/phpunit(22): Composer\Autoload\ClassLoader->loadClass()
#7 /home/arderyp/app/vendor/symfony/phpunit-bridge/bin/simple-phpunit.php(441): include('...')
#8 /home/arderyp/app/vendor/symfony/phpunit-bridge/bin/simple-phpunit(13): require('...')
#9 /home/arderyp/app/vendor/bin/phpunit(120): include('...')
#10 {main}
  thrown in /home/arderyp/app/vendor/symfony/phpunit-bridge/Legacy/CommandForV9.php on line 25

Defining hard env var SYMFONY_DEPRECATIONS_HELPER=disabled doesn't change the ouput. The error is occuring because both executables are looking for this PHPUnit 9 class/method, which doesn't exist in PHPUnit 10.

I'm wondering how you managed to get phpunit 10 working with symfony/test-pack at all. Or are you just explicitly calling /vendor/phpunit/phpunit/phpunit to run your tests?

$ ./vendor/phpunit/phpunit/phpunit --version
PHPUnit 10.0.11 by Sebastian Bergmann and contributors.

@Jean85
Copy link
Contributor Author

Jean85 commented Feb 21, 2023

Yeah I'm not using the test pack, just plain PHPUnit + the bridge. I was using it just for the deprecation helper (and the clock mock).

@arderyp
Copy link
Contributor

arderyp commented Feb 21, 2023

hmmm, okay. Maybe I will try that. I was using phpunit-bridge previously, but it looks like the symfony official documentation has been updated to instruct us to install symfony/test-pack instead of symfony/phpunit-bridge, which is the only reason I was attempting to install that. test-pack does install phpunit-bridge

@arderyp
Copy link
Contributor

arderyp commented Mar 9, 2023

@stof @nicolas-grekas @dunglas i can work on this and submit a PR if you all are interested and have time to review my work. I've done so for symfony panther, though the work still isn't merged: symfony/panther#589

The change log there could be reduced if we get this package phpunit 10 compliant first, as the work there currently requires a workaround the lack of compliance here.

Are you all interested?

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Mar 9, 2023

There are different tasks on the topic. One of them is ensuring test suites can run on phpunit 10. We've already merged some changes on this topic from @OskarStark and @alexandre-daubois mostly. The first step is running deprecation-free on 9.6.
Then, there is figuring out what to do with the bridge itself? I must admit I'm 🙈 for now but sure, any help would be appreciated!

@arderyp
Copy link
Contributor

arderyp commented Mar 9, 2023

Thanks for the info @nicolas-grekas. Would it be helpful for me to start work or hold off until those other items are addressed first?

Part of the question is whether we are merely adding support for 10, or also dropping support for some older versions(s) too

@alexandre-daubois
Copy link
Contributor

I think we're almost done with deprecations brought by PHPUnit 9.6 on the Symfony code base 🙂 Once that's done, I'll have a look at the bridge if I can also do something to help. @arderyp I think that if you have ideas on how to bring support of PHPUnit 10, you should definitely go ahead! 😄

@arderyp
Copy link
Contributor

arderyp commented Mar 9, 2023

Thanks for the insight @alexandre-daubois. Is there a specific branch I should be working off of?

@arderyp
Copy link
Contributor

arderyp commented Mar 10, 2023

@alexandre-daubois @nicolas-grekas will/should 5.4 support phpunit 10? If I work on this, should I be working off the 5.4 branch? I'd certainly like 5.4 to support it.

@vinceAmstoutz
Copy link

To anyone requesting blog posts: as you can see, we don't know yet how a compatible bridge will look like, so there's nothing to blog about.

As soon as you know 😉 Many people are currently confused about the situation, hence my suggestion.

@vinceAmstoutz
Copy link

An official post would be most welcome. I'm confused about the bridge, I'd probably be happy just using PHPUnit itself, but so much of the testing documentation for Symfony uses the bridge that it's hard to separate. Thanks for the link to the slideshow!

Yes, it's clear that it's not always very clear what role bridge, the test pack and the library play in relation to each other.

@mondrake
Copy link
Contributor

Hi, thanks for the update.

In latest PHPUnit 11 I still do not find an alternative to the deprecation ignore file. We use it a lot in Drupal. I saw that the @group legacy annotations can be replaced by the #[IgnoreDeprecations] attribute, but nothing on the file.

Is that planned if you know?

FTR, I will close now the MR #50371

@stof
Copy link
Member

stof commented Feb 28, 2024

PHPUnit also has a concept of baseline (in 10.4+): https://docs.phpunit.de/en/11.0/error-handling.html#ignoring-previously-reported-issues

@mondrake
Copy link
Contributor

Yes I saw that, @stof, thanks. I was inquiring on the ignore file since in Drupal we did not implement the baseline, and it will be rather hard to do in our testing framework.

@stof
Copy link
Member

stof commented Feb 28, 2024

The plan for the new version of the bridge is to drop all features related to the deprecations, by using the features of PHPUnit. If the current PHPUnit baseline feature is not usable for you, I would suggest discussing that with the PHPUnit team.

@soyuka
Copy link
Contributor

soyuka commented Jul 2, 2024

I guess phpunit 11.2 now supports direct/indirect (https://docs.phpunit.de/en/11.2/error-handling.html#limiting-issues-to-your-code) and a baseline therefore we don't need the phpunit bridge anymore this should probably be closed in favor of #53812.

@arderyp
Copy link
Contributor

arderyp commented Jul 2, 2024

So will the test-pack drop the bridge component? That would be nice

fabpot added a commit that referenced this issue Jul 25, 2024
…` (derrabus)

This PR was merged into the 7.2 branch.

Discussion
----------

[PhpUnitBridge] Add `ExpectUserDeprecationMessageTrait`

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | Part of #49069, replaces #54538
| License       | MIT

PHPUnit 11 introduces a method `expectUserDeprecationMessage()` which lets us define which deprecation messages we expect the tested code to raise. This new method can replace our own `expectDeprecation()` method once we upgrade to PHPUnit 11.

This PR introduces a `ExpectUserDeprecationMessageTrait` that polyfills this method for older PHPUnit versions. This allowed me to run all tests that I've migrated to `expectUserDeprecationMessage()` with PHPUnit 11.

Commits
-------

2485e15 [PhpUnitBridge] Add ExpectUserDeprecationMessageTrait
@upyx
Copy link
Contributor

upyx commented Aug 6, 2024

So will the test-pack drop the bridge component? That would be nice

Bridge does many things, and PHPUnit isn't the only part of TestPack. If you'd like to install PHPUnit only, you can do it. You do not have to install the whole TestPack.

If I remember right, the original main goal of Bridge was compatibility between tests written atop different PHPUnit versions. Imagine you have a lot of legacy tests in PHPunit (6.x, 7.x, etc.). It would take a while to update them, and there is no guarantee that a bug wouldn't be added. So we need a way to run old tests alongside new ones.

@derrabus
Copy link
Member

derrabus commented Aug 6, 2024

That is correct. You don't have to install PHPUnit through the bridge if you don't want to. Use PHPUnit 11 and be happy with it. Symfony's own test suite is stuck on PHPUnit 9.6 at the moment, but don't let this stop you.

@stof
Copy link
Member

stof commented Aug 6, 2024

Also note that the PHPUnit bridge is not only about the deprecation reporting layer. Once we work on using the PHPUnit native deprecation reporting system on 11.2+, we should provide a version of the bridge supporting its other features on PHPunit 11

@soyuka
Copy link
Contributor

soyuka commented Aug 8, 2024

Also note that the PHPUnit bridge is not only about the deprecation reporting layer. Once we work on using the PHPUnit native deprecation reporting system on 11.2+, we should provide a version of the bridge supporting its other features on PHPunit 11

Let's see what can be done! Mocks should already work with phpunit 11, is there a repository where we could move these and avoid the phpunit 9 dependency lock?

For other features I think that most are not useful to Symfony developers but let's see each documented feature.

  1. Sets by default a consistent locale (C) for your tests (if you create locale-sensitive tests, use PHPUnit's setLocale() method);

setLocale is deprecated but anyways is not useful, just use the native setlocale if you really need this (sebastianbergmann/phpunit#5200 (comment))

  1. Auto-register class_exists to load Doctrine annotations (when used);

Not needed anymore, use attributes.

  1. It displays the whole list of deprecated features used in the application;

--display-deprecations

  1. Displays the stack trace of a deprecation on-demand;

--display-deprecations outputs the stack trace

  1. Provides a ClockMock, DnsMock and ClassExistsMock classes for tests sensitive to time, network or class existence;

Any idea where to move these?

  1. Provides a modified version of PHPUnit that allows:
    a. separating the dependencies of your app from those of phpunit to prevent any unwanted constraints to apply;

Not sure why this is needed by Symfony users this should probably stay internal.

b. running tests in parallel when a test suite is split in several phpunit.xml files;

If you need this you will probably prefer ParaTest or --process-isolation

c. recording and replaying skipped tests;

I never needed this but there's no equivalent right now as far as I know.

  1. It allows to create tests that are compatible with multiple PHPUnit versions (because it provides polyfills for missing methods, namespaced aliases for non-namespaced classes, etc.).

Same as above I think that this is mostly for Symfony tests and it should probably stay internal.

@nikophil
Copy link
Contributor

nikophil commented Aug 8, 2024

I don't think that --display-deprecations outputs the stack trace

@derrabus
Copy link
Member

derrabus commented Aug 8, 2024

is there a repository where we could move these and avoid the phpunit 9 dependency lock?

There is no dependency lock. You can install the PhpUnitBridge together with PHPUnit 11. Or am I missing something?

@AnjaLiebermann
Copy link

Is there any hope of symfony/phpunit-bridge supporting the newer phpunit versions?
ClockMock is a fantastic feature, but with any of my projects, which doesn't need it I threw the bridge away and use phpunit 11.3 directly.

@derrabus
Copy link
Member

ClockMock is a fantastic feature

If you want to work on making ClockMock compatible with PHPUnit 11, please do so. It's not actively being worked on, as far as I know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help wanted Issues and PRs which are looking for volunteers to complete them. PhpUnitBridge
Projects
None yet