Fix TestRunner compatibility to PhpUnit 8 #30085

alexander-schranz · 2019-02-05T23:59:52Z

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.

nicolas-grekas

Before merging, we should be sure there is really no alternatives - we should also report the issue to Sebastian so that he knows some extensibility point is now missing. Could you please have a look?

src/Symfony/Bridge/PhpUnit/bin/simple-phpunit

kunicmarko20 · 2019-02-07T09:29:58Z

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

    
           if (false) { 
        
               class TestRunner 
        
               { 
        
               } 
        
           }

and then provide the class with or without final? (if it is even possible like that)

nicolas-grekas · 2019-02-20T09:00:47Z

@kunicmarko20 could you please have a look if that would be possible? Can you find any other alternative?

alexander-schranz · 2019-02-20T09:11:41Z

@nicolas-grekas is it maybe possible to remove the TestRunner and move this logic to the Test Command?

fabpot · 2019-04-01T18:37:06Z

@nicolas-grekas @alexander-schranz How can we move forward here?

alexander-schranz · 2019-04-02T08:41:14Z

Will have a look at it, at the EU-FOSSA Hackathon.

ostrolucky · 2019-04-04T10:41:32Z

Could this help? https://github.com/dg/bypass-finals

alexander-schranz · 2019-04-06T08:58:22Z

@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

sebastianbergmann · 2019-04-06T09:22:17Z

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 Hook interfaces. Then have PHPUnit load that class through its configuration file. PHPUnit will then register the hooks.

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

we should also report the issue to Sebastian so that he knows some extensibility point is now missing

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.

nicolas-grekas · 2019-04-06T10:08:11Z

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 (@expectedDeprecation, @time-sensitive, etc) and some extra behaviors: the deprecations report that is displayed after running the tests, or a record/replay feature for skipped tests, maybe some more.

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 final.

alexander-schranz · 2019-04-07T13:20:18Z

Analysed more the current state and I think I could fix it by moving the logic from the TestRunner to the Command.

#EUFOSSA

src/Symfony/Bridge/PhpUnit/Legacy/CommandForV6.php

nicolas-grekas · 2019-04-07T13:32:00Z

if it works, perfect :)
wasn't TestRunnerForV6 reference somewhere?

xabbuh · 2019-04-07T13:46:05Z

Can we also remove the src/Symfony/Bridge/PhpUnit/TextUI/TestRunner.php file?

derrabus · 2019-04-07T13:54:04Z

@xabbuh It's still used in the CommandForV5, isn't it?

alexander-schranz · 2019-04-07T13:57:09Z

@xabbuh done and move the v5 testrunner logic also to the commandforv5

alexander-schranz · 2019-04-07T14:42:54Z

src/Symfony/Bridge/PhpUnit/Legacy/CommandForV5.php

-        return new TestRunnerForV5($this->arguments['loader']);
+        $listener = new SymfonyTestsListenerForV5();
+
+        $this->arguments['listeners'] = isset($this->arguments['listeners']) ? $this->arguments['listeners'] : array();


@nicolas-grekas but here the long array syntax is correct or not?

nicolas-grekas

cool, thanks for digging!

nicolas-grekas · 2019-04-08T07:56:49Z

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

carsonbot added Status: Needs Review Bug labels Feb 5, 2019

alexander-schranz mentioned this pull request Feb 6, 2019

Check that we are not using features deprecated in PHPUnit 8 #30055

Closed

alexander-schranz force-pushed the feature/phpunit-8-runner-compatibility branch 5 times, most recently from 3e4a969 to 0e36059 Compare February 6, 2019 00:15

alexander-schranz changed the title ~~Fix PhpUnit 8 runner compatibility~~ Fix test runner compatibility to PhpUnit 8 Feb 6, 2019

alexander-schranz changed the title ~~Fix test runner compatibility to PhpUnit 8~~ Fix TestRunner compatibility to PhpUnit 8 Feb 6, 2019

alexander-schranz force-pushed the feature/phpunit-8-runner-compatibility branch from 0e36059 to 353ba54 Compare February 6, 2019 00:32

nicolas-grekas added this to the 3.4 milestone Feb 7, 2019

nicolas-grekas reviewed Feb 7, 2019

View reviewed changes

src/Symfony/Bridge/PhpUnit/bin/simple-phpunit Outdated Show resolved Hide resolved

ro0NL mentioned this pull request Feb 25, 2019

ichrusciel/api-test-case not compatible with phpunit bridge #30373

Closed

xabbuh added the PhpUnitBridge label Mar 30, 2019

stof mentioned this pull request Apr 2, 2019

May not inherit from final class #30834

Closed

stof added the ⭐️ EU-FOSSA Hackathon label Apr 2, 2019

alexander-schranz force-pushed the feature/phpunit-8-runner-compatibility branch from 878d241 to 737ef18 Compare April 7, 2019 12:52

alexander-schranz requested review from dunglas, lyrixx and sroze as code owners April 7, 2019 12:52

alexander-schranz requested a review from xabbuh as a code owner April 7, 2019 12:52

alexander-schranz force-pushed the feature/phpunit-8-runner-compatibility branch 2 times, most recently from 349ac1e to 343f4d4 Compare April 7, 2019 13:01

nicolas-grekas reviewed Apr 7, 2019

View reviewed changes

src/Symfony/Bridge/PhpUnit/Legacy/CommandForV6.php Outdated Show resolved Hide resolved

alexander-schranz force-pushed the feature/phpunit-8-runner-compatibility branch from 343f4d4 to 3bf78a0 Compare April 7, 2019 13:54

alexander-schranz commented Apr 7, 2019

View reviewed changes

nicolas-grekas approved these changes Apr 8, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Apr 8, 2019

Fix TestRunner compatibility to PhpUnit 8

a0c66a3

nicolas-grekas force-pushed the feature/phpunit-8-runner-compatibility branch from 3bf78a0 to a0c66a3 Compare April 8, 2019 07:56

nicolas-grekas merged commit a0c66a3 into symfony:3.4 Apr 8, 2019

alexander-schranz deleted the feature/phpunit-8-runner-compatibility branch April 8, 2019 10:29

This was referenced Apr 16, 2019

Release v3.4.25 #31123

Merged

Release v4.2.6 #31125

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix TestRunner compatibility to PhpUnit 8 #30085

Fix TestRunner compatibility to PhpUnit 8 #30085

alexander-schranz commented Feb 5, 2019 •

edited

nicolas-grekas left a comment

kunicmarko20 commented Feb 7, 2019 •

edited

nicolas-grekas commented Feb 20, 2019

alexander-schranz commented Feb 20, 2019

fabpot commented Apr 1, 2019

alexander-schranz commented Apr 2, 2019

ostrolucky commented Apr 4, 2019

alexander-schranz commented Apr 6, 2019

sebastianbergmann commented Apr 6, 2019

nicolas-grekas commented Apr 6, 2019 •

edited

alexander-schranz commented Apr 7, 2019 •

edited

nicolas-grekas commented Apr 7, 2019

xabbuh commented Apr 7, 2019

derrabus commented Apr 7, 2019

alexander-schranz commented Apr 7, 2019

alexander-schranz Apr 7, 2019

nicolas-grekas left a comment

nicolas-grekas commented Apr 8, 2019

Fix TestRunner compatibility to PhpUnit 8 #30085

Fix TestRunner compatibility to PhpUnit 8 #30085

Conversation

alexander-schranz commented Feb 5, 2019 • edited

nicolas-grekas left a comment

Choose a reason for hiding this comment

kunicmarko20 commented Feb 7, 2019 • edited

nicolas-grekas commented Feb 20, 2019

alexander-schranz commented Feb 20, 2019

fabpot commented Apr 1, 2019

alexander-schranz commented Apr 2, 2019

ostrolucky commented Apr 4, 2019

alexander-schranz commented Apr 6, 2019

sebastianbergmann commented Apr 6, 2019

nicolas-grekas commented Apr 6, 2019 • edited

alexander-schranz commented Apr 7, 2019 • edited

nicolas-grekas commented Apr 7, 2019

xabbuh commented Apr 7, 2019

derrabus commented Apr 7, 2019

alexander-schranz commented Apr 7, 2019

alexander-schranz Apr 7, 2019

Choose a reason for hiding this comment

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas commented Apr 8, 2019

alexander-schranz commented Feb 5, 2019 •

edited

kunicmarko20 commented Feb 7, 2019 •

edited

nicolas-grekas commented Apr 6, 2019 •

edited

alexander-schranz commented Apr 7, 2019 •

edited