Request : Issue with the 'TestCase' class alias in bootstrap file

[El-Sam]

Hi, first of all, thanks for this library, it's of a great help.

I'm using PHPUnit 6 for my unit tests.

In the bootstrap.php file in PHP_CodeSniffer/tests , a class alias is created, this alias causes Multiple definitions exist for class TestCase issue in my IDE (PHPStorm).

This breaks a lot of features that speed up my testing like code completion. So, I think you should remove this alias or handle it in another way that you find convenient.

[gsherwood]

I might need a bit more information. Is this just an issue with your IDE thinking that multiple class definitions exist when they actually don't at runtime? If so, isn't this an issue with your IDE and not an issue with PHPCS?

The reason the class alias is there is so that the unit tests will run with both old and new versions of PHPUnit, so it can't just be removed. Do you have a suggestion for another way to achieve PHPUnit backwards compatibility?

[orlangur]

Caused by a hacky https://github.com/squizlabs/PHP_CodeSniffer/pull/1384/files.

+if (class_exists('PHPUnit_Framework_TestCase') === true && class_exists('PHPUnit\Framework\TestCase') === false) {
 +    class_alias('PHPUnit_Framework_TestCase', 'PHPUnit\Framework\TestCase');
 +}

So, PhpStorm ignores this condition?

https://stackoverflow.com/a/23079082/8640867 doesn't help? Even if PHPUnit_Framework_TestCase does not exist which is the case for PHPUnit 6 PhpStorm is complaining?

[orlangur]

The reason the class alias is there is so that the unit tests will run with both old and new versions of PHPUnit, so it can't just be removed. Do you have a suggestion for another way to achieve PHPUnit backwards compatibility?

@gsherwood sorry if I missed a previous explanation somewhere, but why PHP_CodeSniffer needs to explicitly support unsupported PHPUnit versions?

I remember you explained reasoning behind supporting unsupported PHP versions but PHPUnit is just a tool to test PHP_CodeSniffer itself, it was tested just fine without such a hack https://travis-ci.org/squizlabs/PHP_CodeSniffer/jobs/254679990. Why not just let Composer to install whatever PHPUnit version is suitable for the particular platform?

[gsherwood]

PHPUnit is just a tool to test PHP_CodeSniffer itself

No, it's not. Developers of custom coding standards want to be able to write unit tests for their sniffs, which I (of course) encourage. If they are already using newer versions of PHPUnit for their projects because they don't need to support older PHP versions, the ability for the PHPUnit tests to be run on newer versions of PHPUnit would be a great benefit to them.

[jrfnl]

I missed a previous explanation somewhere, but why PHP_CodeSniffer needs to explicitly support unsupported PHPUnit versions?

The explanation is quite simple:

• PHP Codesniffer 3.x has a minimum PHP requirement of PHP 5.4.
• The PHPUnit versions which are still actively supported are:
  • PHPUnit 6.x which has a minimum PHP requirement of PHP 7.0
  • PHPUnit 5.7 which has a minimum PHP requirement of PHP 5.6. Support for PHPUnit 5.7 will end February 2018.

To be able to run the PHPCS unit tests on PHP 5.4 up to nightly, it is therefore necessary that the PHPCS unit tests are compatible with PHPUnit 4.x, 5.x and 6.x.

Using the Composer based interim solution would lock PHPCS into a PHPUnit version which will lose support by February 2018 (and an unsupported version for PHP 5.4/5.5) and is therefore not a sustainable solution.

[orlangur]

@gsherwood thanks for the explanation, I was scratching my head trying to understand the use case, as with necessity to support PHPUnit 4 modern PHPUnit 6 features cannot be used anyway. The fact some projects may drop some PHP versions didn't came to my mind.

@jrfnl yeah, this is quite clear knowing the fact PHP 5.4 requirement is unavoidable, just that I don't think all three PHPUnit versions have to be supported for internal testsuite, it's enough to have it runnable for some PHPUnit version on every platform.

[gmponos]

Hello.. this case happens with other libraries as well. I think it happened for me with twig.

What I do is having this the tests folder marked as excludes. It is also mentioned at stackoverflow at the link posted above.

Btw @gsherwood is there any reason why the tests are included when the lib is installed with composer? I didn't see the tests folder in .gitattributes file.

[El-Sam]

as @orlangur mentionned, it's this lines of code that are missing with the PHPStorm :

// Compatibility for PHPUnit < 6 and PHPUnit 6+.
if (class_exists('PHPUnit_Framework_TestSuite') === true && class_exists('PHPUnit\Framework\TestSuite') === false) {
    class_alias('PHPUnit_Framework_TestSuite', 'PHPUnit\Framework\TestSuite');
}

if (class_exists('PHPUnit_Framework_TestCase') === true && class_exists('PHPUnit\Framework\TestCase') === false) {
    class_alias('PHPUnit_Framework_TestCase', 'PHPUnit\Framework\TestCase');
}

I support @gmponos suggestion to add your tests folder to the .gitattributes so it would not be fetched with the lib.
I don't need your tests in my production.

[kubawerlos]

@El-Sam it was removed from .gitattributes not long ago after discussion.

What about bumping minimum PHPUnit version to 4.8? This one has forward compatibility classes, so we wouldn't need the lines in bootstrap.php that are messing with PHPStorm.

Or am I missing something?

[michalbundyra]

@El-Sam
In PHPUnit 4.8 we have only couple forward compatibility classes:
https://github.com/sebastianbergmann/phpunit/tree/4.8/src/ForwardCompatibility
and it's not enough for PHP_CodeSniffer:
https://github.com/squizlabs/PHP_CodeSniffer/blob/master/tests/bootstrap.php#L31-L46

So there is no PHPUnit\TextUI\TestRunner and PHPUnit\Framework\TestResult...

[orlangur]

At least it would allow to slightly reduce the compatibility layer, unless somebody is sitting on some PHPUnit 4.1 and needs this constraint unchanged. If such improvement useful or not is more a matter of long-term strategy.

[El-Sam]

Do you mean that I should use PHPUnit v4.8 instead of v6? if that's the case, I can't, my whole team uses v6 and I can't just switch to v4.8.

I'm sorry if I'm missing something.

[orlangur]

No-no, I mean some projects with custom standards may still need PHPUnit 4.lo (an opposite case to #1671 (comment) when they want newest PHPUnit).

[El-Sam]

Ah ok, thanks for explaining.

So, I guess adding the tests folder to .getattributes will not happen.

Any other solutions? all the PHPStorm tricks mentioned above and on StackOverflow end up disabling features in the IDE that are essentials for coding.