Allow installation in combination with PHPUnit < 9.1

[jrfnl]

Add custom autoloader/fall-through to PHPUnit native functionality

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.

Add availability tests for the fall-through functionality

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.

GH Actions: adjust the test workflow

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

[Optional] Tests: use custom bootstrap file (Removed)

[stof]

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 PATH_TO_PROPHECY variable. There are already ways to use a local clone of prophecy, either by configurating a composer path repository or just by adding replacing the folder inside vendor by a symlink).

[jrfnl]

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) ?

Unfortunately, no.

For this PR, I've used the same PHP and PHPUnit version requirements as for the PHPUnit Polyfills package (and the rdohms/phpunit-arraysubset-asserts package for that matter): "php": "^5.4 || ^7.0 || ^8.0" and "phpunit/phpunit": "^4.8.36 || ^5.7.21 || ^6.0 || ^7.0 || ^8.0 || ^9.1".
This allows for these packages to be used as companion add-ons.

The (real, non-empty) ProphecyTrait.php file contains nullable types and return types, which means loading that file would result in parse errors on PHP < 7.1, even when the real/empty trait would be created conditionally.

It would still allow to widen the requirement, but in a far more limited manner ("php": "^7.1 || ^8.0" and "phpunit/phpunit": "^7.0 || ^8.0 || ^9.1") than currently implemented.

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 prophesize() method has been removed, leaving people on PHPUnit 9.x to still receive the deprecation warnings.

for the bootstrap file, please keep it out of that PR (btw, I'm not convinced about the need for such PATH_TO_PROPHECY variable. There are already ways to use a local clone of prophecy, either by configurating a composer path repository or just by adding replacing the folder inside vendor by a symlink).

No problem, I put it in a separate commit anyway for that reason, so I will just remove that commit from the PR.

[jrfnl]

@stof Just checking: did my answer above clarify the "why this way?" of the setup of this PR sufficiently ?

[jrfnl]

Thanks for the review @stof ! I've left some questions/responses to verify my understanding of your feedback.

[jrfnl]

@stof I believe I've addressed all the feedback with the new commits. Hope this complies with how you want it.
Let me know if you want me to squash the new commits into the original ones for a cleaner history.

[jrfnl]

Oh and one side-note: while the availability test does run and pass on PHPUnit 4.x, it is marked as risky now:

1. Prophecy\PhpUnit\Tests\Availability\AvailabilityTest::testSuccessfullyCallingProphesizeMethod
   Test code or tested code did not (only) close its own output buffers

[jrfnl]

(Repushed to get rid of some tabs which had sneaked in. Naughty tabs!)

[stof]

The test probably needs to define the PHPUNIT_TESTSUITE constant, as it runs a testcase as subject under test. I think that will remove that warning.

[jrfnl]

The test probably needs to define the PHPUNIT_TESTSUITE constant, as it runs a testcase as subject under test. I think that will remove that warning.

It did indeed 🎉 . Note: I've used the @before annotation with an arbitrary method name to prevent having to add the void return type.

[jrfnl]

@stof Anything I can do to help move this forward ?

[jrfnl]

@stof Just checking if there is anything on my side this PR is waiting for ?

[jrfnl]

Just checking in again: anything I can do to move this PR forward ?

[bshaffer]

@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!

[jrfnl]

@stof What is this waiting for... ?

[jrfnl]

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 ?

[aik099]

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.

[jrfnl]

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.

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.

[jrfnl]

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.