Make FeatureTestListener PHPUnit 6 compatible #21
Conversation
|
Ping @xabbuh |
src/FeatureTestListener.php
Outdated
| @@ -2,13 +2,16 @@ | |||
|
|
|||
| namespace Http\Client\Tests; | |||
|
|
|||
| class FeatureTestListener extends \PHPUnit_TextUI_ResultPrinter | |||
| use PHPUnit\Framework\TestCase; | |||
| use PHPUnit\TextUI\ResultPrinter; | |||
There was a problem hiding this comment.
This class was not present in PHPUnit 4.x and 5.x, was it?
There was a problem hiding this comment.
There was a problem hiding this comment.
Yeah, but its name was \PHPUnit_TextUI_ResultPrinter back then. I think we could register a class alias in case the namespaced class does not exist (the class body doesn't seem to be different).
There was a problem hiding this comment.
I'm a little bit confused: I followed the same naming pattern as you did. I guess there is a legacy layer which resolves the new class names to the old ones in case the new one does not exist. You raised the minimum version of PHPUnit in the previous PR to 4.8.35 (for which version the tests are passing).
So I am not sure I understand what the problem is. 😞
There was a problem hiding this comment.
The classes that are part of the forward compatibility layer are explicitly listed here: https://github.com/sebastianbergmann/phpunit/tree/4.8/src/ForwardCompatibility Thus, for every other class we will have to use class aliases.
There was a problem hiding this comment.
Okay, got it. I think this would better fit into a bootstrap file which is always autoloaded by composer. What do you think? Or just place it here? Should do some checks, if the alias already exists?
src/FeatureTestListener.php
Outdated
| { | ||
| public function write($buffer) | ||
| { | ||
| } | ||
|
|
||
| public function startTest(\PHPUnit_Framework_Test $test) | ||
| public function startTest(TestCase $test) |
There was a problem hiding this comment.
TestCase is one implementation of Test. Thus this change will violate Liskov's substitution principle.
There was a problem hiding this comment.
Ah, didn't spot it was Test, not TestCase
|
Will the |
|
AFAIK yes, but only with a certain version (where the compatibility layer is added). |
|
Okey. Good |
|
Wait a sec, I missed this comment: |
|
We need to add some class aliases |
What's in this PR?
Fixes the rest of #20