-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
PHPUnit 8 support for the bridge: SymfonyTestsListenerForV7 implements TestListener that is deprecated, we need to use TestHook instead #33331
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
Comments
TestHook
instead
I will give it a try :) |
First hurdle: it's unclear to me what the migration path is for symfony/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php Lines 112 to 179 in c1bca29
BeforeFirstTestHook , which seems to be the best candidate for this, does not have access to the current suite.
Its only method is not even called here: https://github.com/sebastianbergmann/phpunit/blob/154de22df1fda1d67758ab1c417603df7b290809/src/Runner/Hook/TestListenerAdapter.php#L130-L132, but directly in the test runner: https://github.com/sebastianbergmann/phpunit/blob/41c75847e0a7e7df2e0eba6b6edf4d1b99da3103/src/TextUI/TestRunner.php#L535-L539 |
I created this PR in order to clarify things: sebastianbergmann/phpunit#3800 |
Ok so it looks like we should also stop relying on test suites entirely, and find another way to get all tests. |
So why do we rely on test suites?
symfony/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php Lines 117 to 124 in b9ba0c5
symfony/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php Lines 138 to 149 in b9ba0c5
Or by reading the file, and replacing the suite's tests with the tests found in the file: symfony/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php Lines 168 to 178 in b9ba0c5
The last piece of code using test suites is that one: symfony/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php Lines 151 to 167 in b9ba0c5
But I don't understand it… phpunit test suites can have test suites in their test suites? Can somebody explain? EDIT: I got an explanation from Nicolas Grekas: apparently, the sub-test suite can be used when there are data providers. The use case of this code block is to inject ClockMock functions in the namespace before it is used. |
Sebastian Bergmann wrote:
Yet test suite related methods are not marked as deprecated yet, so I think I can ignore this for the moment and just work on following the deprecations for the other methods. EDIT: I noticed I need to do the something similar but simpler for The main discussion happens here: sebastianbergmann/phpunit#3390 |
ping @epdenouden |
For reference, the doc has some reminders about the features: https://symfony.com/doc/current/components/phpunit_bridge.html |
Not sure if somebody made a PR already, but the solution is to use extensions (possible from phpunit >= 7.1) instead of listeners. The CHANGELOG should reflect that the phpunit.xml.dist should be changed to use the new feature. The code would be something like this:
And the phpunit.xml.dist would be to remove the listener and instead add an extension:
Note that CoverageExtension is just one of the phpunit-bridge extensions. The same system applies to other existing extensions. I could write the code, but don't want to interfere with existing work on this. |
@rbaarsma your solution will not work for the more complicated |
@greg0ire after some more examination and attempted solution, I come to the same conclusion as you already said before. The My conclusion is that the current hook implementation is not enough to migrate these classes. Perhaps we should contact @sebastianbergmann to discuss what his idea is with the current hook implementation and how we are supposed to use the minimum input to cover the use-cases of symfony, such as access to the annotations. testResultObject, status, and failures. Also the SymfonyTestListener uses a |
Hello everyone! At the SymfonyCon in Amsterdam last weekend we decided on refactoring both in PHPUnit and the Symfony test collection, which will remove the need for a bridge altogether. PHPUnit will get new features that will bring much of the bridge functionality to its core, whereas any missing Symfony-specific requirements will be handled with a proper PHPUnit extension. The upcoming event-based logging system in PHPUnit allows for much better logging and analysis of -you guessed it!- events happening during testing. Thanks to the work by @localheinz and @theseer we will be able to report many new events in much more detail. @sebastianbergmann and I discussed this with @greg0ire and @nicolas-grekas last Saturday and I will write up a list of what is needed where and how to approach this. Also, we propose a communal bonfire during a conference next year to burn all the legacy code. |
Nevermind, that's actually the test listener of the polyfills package. I should've read the error message more carefully. 🙈 |
Thank you for this issue. |
Hello? This issue is about to be closed if nobody replies. |
Hey, I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen! |
SymfonyTestsListenerForV7
implementsTestListener
that is deprecated, we need to useTestHook
instead/cc @greg0ire maybe?
The text was updated successfully, but these errors were encountered: