Fix TestRunner compatibility to PhpUnit 8 #30085
Conversation
3e4a969 to
0e36059
Compare
0e36059 to
353ba54
Compare
|
Isn't it a better solution to check phpunit version here: symfony/src/Symfony/Bridge/PhpUnit/TextUI/TestRunner.php Lines 22 to 26 in e8c3f9e and then provide the class with or without final? (if it is even possible like that) |
|
@kunicmarko20 could you please have a look if that would be possible? Can you find any other alternative? |
|
@nicolas-grekas is it maybe possible to remove the TestRunner and move this logic to the Test Command? |
|
@nicolas-grekas @alexander-schranz How can we move forward here? |
|
Will have a look at it, at the EU-FOSSA Hackathon. |
|
Could this help? https://github.com/dg/bypass-finals |
|
@ostrolucky think we don't want to remove all finals. Maybe there is some suggestion by @sebastianbergmann how we could automatically register the SymfonyListener programmatically now. Did create an issue at the phpunit repository sebastianbergmann/phpunit#3594 #EU-FOSSA |
|
As I already wrote in sebastianbergmann/phpunit#3594 (comment) (why do we have to discuss the same issue in different places?), "automatically registereing" is not possible, and intentionally so: the test runner should not be extended through inheritance but by hooking into certain events. You can find the available hooks here. What you should do is implement a class that implements one or more of these I can only advise against "hacking around" or "extending" the PHPUnit test runner and then, again and again, update your workarounds when internal implementation details of PHPUnit change. Please do not rely on private implementation details. In #30085 (review) @nicolas-grekas wrote
Yes, please: let me know if you are missing an extension point. While working on https://github.com/Roave/no-leaks @Ocramius ran into limitations of the current hooks provided and we plan to work on this in the future. On a related note, the whole idea of phpunit-bridge is a mystery to me. I do not know what problem(s) you are trying to solve with it. It would be great if somebody could explain that at some point to me. It might be that there are solutions in there for problems that could be solved differently, either inside PHPUnit or in Symfony. I am not conviced that the best medium for this explanation would be a ticket or email. This should, ideally, be a real conversation. Maybe somebody from Symfony can come to a PHPUnit code sprint (there is one in Würzburg this month) or I can come to a Symfony code sprint. |
|
Meeting in person would be great for sure! I'm pretty busy this spring so not sure I can join the next sprint but let's tell each other if we see any opportunity! The bridge creates an alternative "phpunit binary" that provides some extra features out of the box. With inheritance, it was possible to wire a different behavior by default, but now I'm not sure we can (I didn't have a deeper look). Which features? some extra annotations ( The extra features related to annotations work with a line in config files, but the extra behaviors are shipped out of the box when using the bridge. That's why there is this specific bootstrapping extra layer, which is closed now, with |
878d241 to
737ef18
Compare
349ac1e to
343f4d4
Compare
|
Analysed more the current state and I think I could fix it by moving the logic from the TestRunner to the Command. #EUFOSSA |
|
if it works, perfect :) |
|
Can we also remove the |
|
@xabbuh It's still used in the |
343f4d4 to
3bf78a0
Compare
|
@xabbuh done and move the v5 testrunner logic also to the commandforv5 |
| return new TestRunnerForV5($this->arguments['loader']); | ||
| $listener = new SymfonyTestsListenerForV5(); | ||
|
|
||
| $this->arguments['listeners'] = isset($this->arguments['listeners']) ? $this->arguments['listeners'] : array(); |
There was a problem hiding this comment.
@nicolas-grekas but here the long array syntax is correct or not?
3bf78a0 to
a0c66a3
Compare
|
Thank you @alexander-schranz. |
This PR was squashed before being merged into the 3.4 branch (closes #30085). Discussion ---------- Fix TestRunner compatibility to PhpUnit 8 | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | related to: #30055 | License | MIT | Doc PR | - Modify the installed phpunit version to be compatibility with the symfony custom TestRunner. This is sure not the best way but maybe currently the fastest way to support PhpUnit 8. The hack should be removed as soon as there is another way to implement a custom Runner. Commits ------- a0c66a3 Fix TestRunner compatibility to PhpUnit 8
Modify the installed phpunit version to be compatibility with the symfony custom TestRunner. This is sure not the best way but maybe currently the fastest way to support PhpUnit 8. The hack should be removed as soon as there is another way to implement a custom Runner.