Skip to content
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

PHPUnit 6 compatiblity #93

Closed
aik099 opened this issue Jul 26, 2017 · 22 comments
Closed

PHPUnit 6 compatiblity #93

aik099 opened this issue Jul 26, 2017 · 22 comments
Assignees

Comments

@aik099
Copy link
Member

aik099 commented Jul 26, 2017

PHPUnit 6 release changed PHPUnit_Framework_TestCase class into \PHPUnit\Framework\TestCase and therefore any extends PHPUnit_Framework_TestCase in this project now fails with this error:

PHP Fatal error:  Class 'PHPUnit_Framework_TestCase' not found in /var/www/html/vendor/aik099/phpunit-mink/library/aik099/PHPUnit/BrowserTestCase.php on line 33

Maybe https://github.com/minkphp/phpunit-mink/blob/master/library/aik099/PHPUnit/AbstractPHPUnitCompatibilityTestCase.php can be updated to not only support PHPUnit 5 and 4, but also PHPUnit 6.

@aik099
Copy link
Member Author

aik099 commented Jul 26, 2017

Hopefully all public API of test case classes remained the same and extending them from different parent class would do the trick.

@robertfausk
Copy link
Contributor

robertfausk commented Jul 27, 2017

I'll give it a try today. Should not be that hard.

@aik099
Copy link
Member Author

aik099 commented Jul 27, 2017

You can also try changing the .travis.yml so that PHPUnit supplied by Travis CI could be used. This would automatically test that each of PHPUnit versions (4.x, 5.x, 6.x) are still working. Right now we're forced to use PHPUnit 4.x on Travis CI if I'm not mistaken.

@robertfausk
Copy link
Contributor

Yep. Right now it's PHPUNIT 4.8 for all PHP versions.

According to https://docs.travis-ci.com/user/languages/php/#Choosing-PHP-versions-to-test-against travis uses installed version found in $COMPOSER_BIN_DIR/phpunit

I would suggest do add a composer update phpunit/phpunit in before_script section and also test PHP 7.1.

# .travis.yml
...
php:
    - 7.0
    - 7.1    # added
    - hhvm
...
install:
    - composer update phpunit/phpunit    # added
    - composer require satooshi/php-coveralls:^1.0 --dev

This would result in following matrix:

PHP PHPUNIT
5.3 4.8
5.4 4.8
5.5 4.8
5.6 5.7
7.0 6.2
7.1 6.2

With this setup the .travis.yml wouldn't grow that much. What do you think?

Btw: Why is 7.0 marked as allow_failures? Do you mind if it's not marked as allow_failures anymore for 7.0 and 7.1?

@aik099
Copy link
Member Author

aik099 commented Jul 29, 2017

That would 2 different PRs:

  1. use PHPUnit for used PHP version
  2. add support for PHP 7.1 and move out PHP 7.0 from allow_failures

Btw: Why is 7.0 marked as allow_failures? Do you mind if it's not marked as allow_failures anymore for 7.0 and 7.1?

Because at the time .travis.yml was created PHP 7 wasn't even released or contained many bugs.

@robertfausk
Copy link
Contributor

I think there will be 3 PRs:

  1. use PHPUnit for used PHP version
  2. PHPUnit 6 compatiblity (currently there is only PHPUnit ~4|~5 in composer.json)
  3. add support for PHP 7.1 and move out PHP 7.0 from allow_failures

@robertfausk
Copy link
Contributor

Hmmm. I currently stuck with following problem:
The library code extends PHPUnit classes.
The test code extends PHPUnit classes.

@aik099 Do you know if it's possible to execute tests with e.g. PHPUnit 4.8 while library code uses a different PHPUnit version e.g. PHPUnit 6.2?
If this is not possible then also test code has to be adopted to work with all PHPUnit versions (4, 5 and 6)


  • Additionally mockery currently doesn't support PHPUnit 6 yet PHPUnit 6 support mockery/mockery#728 (e.g. TestListener of mockery doesn't work)
  • There are many type hint conflicts in methods and class constructors which need to be solved

@aik099 What do you think about a new major release for PHPUnit 6 support? This would result in cleaner code (less if else constructs).

@aik099
Copy link
Member Author

aik099 commented Jul 29, 2017

Hmmm. I currently stuck with following problem:
The library code extends PHPUnit classes.
The test code extends PHPUnit classes.

Then we need to:

  1. update AbstractPHPUnitCompatibilityTestCase class to support PHPUnit 6 (hopefully other method signatures match)
  2. add analog of AbstractPHPUnitCompatibilityTestCase but for test suite only with same functionality.

... Do you know if it's possible to execute tests with e.g. PHPUnit 4.8 while library code uses a different PHPUnit version e.g. PHPUnit 6.2?

I guess no, because at time of PHPUnit 4.x development it wasn't aware that different class names will be used in PHPUnit 6.x.

There are many type hint conflicts in methods and class constructors which need to be solved

Any examples?

... What do you think about a new major release for PHPUnit 6 support? This would result in cleaner code (less if else constructs).

You mean drop support for PHPUnit 4.x and 5.x? If that is so, then I'm against that. I don't want anyone to force upgrade to PHPUnit 6 to get latest features of this library.

@robertfausk
Copy link
Contributor

Any examples?

if ( $test instanceof \PHPUnit_Framework_TestSuite_DataProvider ) {

could become

if ( $test instanceof \PHPUnit_Framework_TestSuite_DataProvider || $test instanceof \PHPUnit\Framework\DataProviderTestSuite  ) {

or something like

if ( $test instanceof PHPUnitCompatibilityClassNames::DATA_PROVIDER ) {
# in PHPUnitCompatibilityClassNames the const DATA_PROVIDER will be set by PHPUnit version

You mean drop support for PHPUnit 4.x and 5.x? If that is so, then I'm against that. I don't want anyone to force upgrade to PHPUnit 6 to get latest features of this library.

Another possibility would be to maintain two branches. One with PHPUnit 4.x and 5.x and the other one with PHPUnit 6.x. But I fear it's to much overhead?!

@aik099
Copy link
Member Author

aik099 commented Jul 29, 2017

\PHPUnit_Framework_TestResult $test_result,

I see no way to make type hints compatible with both PHPUnit 6.x and below.

Another possibility would be to maintain two branches. One with PHPUnit 4.x and 5.x and the other one with PHPUnit 6.x. But I fear it's to much overhead?!

That would be too much indeed.

@aik099
Copy link
Member Author

aik099 commented Jul 29, 2017

Maybe we can create polyfill the way, that we create PHPUnit 6.x class by extending from PHPUnit 5.x/5.x class. Then in code we use PHPUnit 6 classes in all places. Include polyfills in test suite bootstrap file.

Not sure how PhpStorm test runner would react to such trick though.

@stof
Copy link
Member

stof commented Jul 29, 2017

for the testsuite, your don't need anything special: just use the namespaced classes (and bump the min PHPUnit version to 4.8.35 as older 4.8 releases don't have them)

@aik099
Copy link
Member Author

aik099 commented Jul 29, 2017

I thought, that namespaced classes were only added in PHPUnit 6.x.

@stof
Copy link
Member

stof commented Jul 29, 2017

Well, the namespace version of a few classes (the one used when writing tests rather than when extending PHPUnit) have been shipped in 4.8.35 and in 5.4.3+, to allow users to migrate their testsuites progressively (before 6.0, these classes are extending the kind of polyfills you described above btw).
For internal classes, there is no forward-compatibility layer.

@aik099
Copy link
Member Author

aik099 commented Jul 29, 2017

Nice. Then @robertfausk could make use of them.

@robertfausk
Copy link
Contributor

I will give it a try after #94 has finished build and is merged.
This will result in another PR with bump PHPUnit version to 4.8.35+ or 5.4.3+.


\PHPUnit_Framework_TestResult $test_result,

I see no way to make type hints compatible with both PHPUnit 6.x and below.

Same solution like AbstractPHPUnitCompatibilityTestCase should do the trick.

@aik099
Copy link
Member Author

aik099 commented Oct 9, 2018

So what's the state of this?

Considering how quick PHPUnit is doing it's releases, that change method signatures/visibility, that we're using I'm proposing to:

  1. create 2.x branch from current master and tag all further 2.x releases, that needs PHPUnit 4.x, 5.x from there
  2. in the master branch remove PHPUnit 4.x, 5.x compatibility layer (or transform it to support PHPUnit 6.x, 7.x) and update code to work with PHPUnit 6.x, 7.x

@aik099 aik099 self-assigned this Oct 9, 2018
@zann1x
Copy link

zann1x commented Oct 1, 2019

So now that another year passed... can we expect any progress on this issue in the near future?

@aik099
Copy link
Member Author

aik099 commented Oct 1, 2019

If somebody is using this, then a PR is welcome. My personal choice is avoid using new PHPUnit version, since IMO they don't bring anything new except BC breaks. The only things developer need are assert... methods and they're the same (maybe some minor new ones are added).

@zann1x
Copy link

zann1x commented Oct 2, 2019

Well partly true. Some PHP frameworks do require a PHPUnit version of 6 or higher to work. That makes this library unusable with them.

@aik099
Copy link
Member Author

aik099 commented Oct 2, 2019

We're also having same problem in Mink library itself.

Maybe a new project like minkphp/phpunit-bc-layer would solve the problem. Idea is to have single project, that would compensate for BC breaks and we'll extend our TestSuite/TestCase/DataProvider classes from it instead.

@aik099
Copy link
Member Author

aik099 commented Mar 13, 2024

The PHPUnit-Mink v2.3.0 release added support for all PHPUnit versions up to 9.x.

@aik099 aik099 closed this as completed Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants