Allow installation in combination with PHPUnit < 9.1#38
Conversation
|
Rather than defining a custom autoloader, can't we use a conditional class definition in the ProphecyTrait.php file itself, which keeps things compatible with the Composer autoloader (and also with other autoloaders than might be used, for instance if someone builds a phar) ? for the bootstrap file, please keep it out of that PR (btw, I'm not convinced about the need for such |
This commit: * Adds a custom autoloader which will either load the ProphecyTrait or will load an empty trait which will fall through to using the PHPUnit native functionality for PHPUnit 4.x - 9.0. * Widens the version requirements for PHP and PHPUnit to allow installing of this package on PHP 5.4 - current in combination with PHPUnit 4.8.36/5.7.21 - current. * Updates the `autoload` section in the `composer.json` to load the custom autoloader instead of using PSR4 autoloading. * Updates the README with information about the fact that the package no longer restricts installation to PHPUnit 9.
This commit:
* Adds an additional test class which make sure that the `ProphecyTrait` functionality can be safely used and falls through to the PHPUnit native functionality for PHPUnit < 9.1.
* Prevents the _real_ unit tests from loading on PHP < 7.3 via an annotation in the `phpunit.xml.dist` file.
If those files would be loaded on older PHP versions, the test run would break on the parse errors due to modern PHP syntax being used.
* Prevents the _real_ unit tests from running on PHPUnit < 9.1.
These tests only need to be run on PHPUnit >= 9.1 as that is the only time when the polyfills will actually be used.
... to: * Run the availability tests against all PHP versions on which the package can be installed with `prefer-stable` (`highest`). * Run the availability tests, as well as the _real_ tests against `lowest` and `stable` for PHPUnit 9.1+ on PHP 7.3 - 8.1. The test selection is done automatically via the `phpunit.xml.dist` file as adjusted in the previous commit. As for the running of the _real_ tests against the `lowest` supported PHPUnit 9 version, this requires some tweaking to the workflow script. As PHPUnit didn't always have an explicit requirement for PHP, running with `prefer-lowest` on PHP 7.3 - 8.1 with the current PHPUnit version constraints would install PHPUnit 4.8. So, for the `prefer-lowest` runs, we need to make sure that `lowest` is correctly interpreted as "lowest" within the PHPUnit 9.1+ range.
Unfortunately, no. For this PR, I've used the same PHP and PHPUnit version requirements as for the PHPUnit Polyfills package (and the The (real, non-empty) It would still allow to widen the requirement, but in a far more limited manner ( As for removing those type declarations: that would make the Trait incompatible with PHPUnit 9.x and only allow for it to be used with PHPUnit 10.x in which the PHPUnit native
No problem, I put it in a separate commit anyway for that reason, so I will just remove that commit from the PR. |
eed9de4 to
5cbdeb0
Compare
|
@stof Just checking: did my answer above clarify the "why this way?" of the setup of this PR sufficiently ? |
|
Thanks for the review @stof ! I've left some questions/responses to verify my understanding of your feedback. |
|
@stof I believe I've addressed all the feedback with the new commits. Hope this complies with how you want it. |
|
Oh and one side-note: while the availability test does run and pass on PHPUnit 4.x, it is marked as risky now:
|
Includes renaming the file in which the _real_ trait lives to `ProphecyTraitReal.php`.
... but use a fixture instead.
0cfd781 to
0ff4d97
Compare
|
(Repushed to get rid of some tabs which had sneaked in. Naughty tabs!) |
|
The test probably needs to define the |
It did indeed 🎉 . Note: I've used the |
|
@stof Anything I can do to help move this forward ? |
|
@stof Just checking if there is anything on my side this PR is waiting for ? |
|
Just checking in again: anything I can do to move this PR forward ? |
|
@stof We would love to see this merged, as it's very difficult for us to retain our prophecy tests across multiple (pre-7.2) versions of PHP! |
|
@stof What is this waiting for... ? |
|
Only 6 more days and then this PR has a birthday.... is there any hope of getting this merged and shall I just close this and call it a (wasted) day ? |
@jrfnl I've approved this PR, but I don't have any permission to merge it. It's sad to admit, that nowadays people tend to use all the latest PHP and PHPUnit versions, and creating a library, that can work across older PHP/PHPUnit versions isn't as popular. People doing so are considered outcasts or worse. In any case, let's wait a little longer because it costs nothing to keep an open PR, which someday might be even merged. |
I hear you and I fully encourage that, but for those who are trying to upgrade to get there, it is still very helpful if they can at least run their tests on both old vs new. In the end, this PR doesn't actually enable the functionality of this repo for old PHP/PHPUnit versions, it just allows for it to be installed in combination with old PHP/PHPUnit versions, which makes those kind of migrations a lot easier. |
|
This PR will go to kindergarden soon... Enough. @stof Next time you state that you would welcome a PR for something, it might be nice to actually act on it. Now you've just wasted my time. |
Add custom autoloader/fall-through to PHPUnit native functionality
This commit:
autoloadsection in thecomposer.jsonto load the custom autoloader instead of using PSR4 autoloading.Add availability tests for the fall-through functionality
This commit:
ProphecyTraitfunctionality can be safely used and falls through to the PHPUnit native functionality for PHPUnit < 9.1.phpunit.xml.distfile.If those files would be loaded on older PHP versions, the test run would break on the parse errors due to modern PHP syntax being used.
These tests only need to be run on PHPUnit >= 9.1 as that is the only time when the polyfills will actually be used.
GH Actions: adjust the test workflow
... to:
prefer-stable(highest).lowestandstablefor PHPUnit 9.1+ on PHP 7.3 - 8.1.The test selection is done automatically via the
phpunit.xml.distfile as adjusted in the previous commit.As for the running of the real tests against the
lowestsupported PHPUnit 9 version, this requires some tweaking to the workflow script.As PHPUnit didn't always have an explicit requirement for PHP, running with
prefer-loweston PHP 7.3 - 8.1 with the current PHPUnit version constraints would install PHPUnit 4.8.So, for the
prefer-lowestruns, we need to make sure thatlowestis correctly interpreted as "lowest" within the PHPUnit 9.1+ range.[Optional] Tests: use custom bootstrap file(Removed)