RFC: Refactoring to support PHPUnit 10

[cspray]

PHPUnit 10 has recently been released and it includes improvements to the testing framework along with a new Event system. Unfortunately, it also brings along a breaking change that causes this library to no longer work. Specifically, the PHPUnit\Framework\TestCase::setName method has been marked as final and can no longer be overridden. Fortunately, I believe the event system could simplify what we'd have to do.

A tl;dr is that we create an Extension that invokes EventLoop::getSuspension()->suspend() in an appropriate event. The first thought is the PHPUnit\Event\Events\Test\Finished event.

If this sounds like a good place to start I can work on a new branch for PHPUnit 10 support in the next week or two.

[trowski]

Thanks for looking into this. When I saw that several methods were marked as final I knew this library was going to require some refactoring. Using the event system sounds like a logical replacement, so please do start a branch. 😊

[cspray]

I looked into the PHPUnit 10 Extension system and it is not suitable for our needs moving to PHPUnit 10. All Exceptions thrown in an Extension are caught by PHPUnit and silently eaten. I put together a small spike showing an exception getting thrown in the EventLoop during a test case and tests continuing to run with no problem.

https://github.com/cspray/amp-phpunit10-example

Have some other ideas on how to accomplish this. Will post more updates soon(ish).

[dinamic]

@cspray do you have an update on this?

[cspray]

@dinamic Unfortunately, I don't. Had a bunch of personal stuff happen in 2023 and I had very little time to devote to Amp related work. The only thing I remember off-hand is that the Extension system is not suitable. I vaguely recall there being some other method that we could potentially hook into, perhaps in some non-API compliant way (i.e. using Reflection on some PHPUnit internals) but that's only a vague recollection.

Over the last couple weeks I started working on an Amp-based app and had a need to actually use a Future in my design. Not having this test case has made testing that functionality a lot more difficult. A part of me believes that having the EventLoop running goes beyond the typical PHPUnit TestCase and a new, purpose-built testing framework would be more appropriate. I had this idea back when Amp was v2 and did quite a bit of work on https://github.com/labrador-kennel/async-unit to accomplish this. I'm inclined to do the work to make this compatible with Amp v3 versus trying to make, and continue making, a highly-opinionated library do something it isn't really intended to do.

[cspray]

Would also like to point out #22 where a solution appears to have been found that will keep this working for PHPUnit 10. Given the changes appears to be relatively low risk.

[cspray]

I did some more looking into this. I do believe that #22 is the appropriate way to support PHPUnit 10. I implemented these changes and used them in a fairly comprehensive test suite and everything worked as I expected it to. I decided to see what these changes would look like with PHPUnit 11 and that failed. In 11.0 PHPUnit\Framework\TestCase::runTest is marked final (https://github.com/sebastianbergmann/phpunit/blob/ece3536c22fc5113906a42e7e82de00baaef36d0/src/Framework/TestCase.php#L1177). The other run* methods also appeared to have been marked final. Overriding this method is a pretty fundamental aspect of how this library works and it isn't immediately obvious how we'd support PHPUnit 11.

I contend this is more evidence that at some point PHPUnit will no longer be a suitable tool for this type of testing. Once PHPUnit 10 is no longer suitable for use I suspect we'll have to start thinking about alternative solutions or come up with something far more clever than our current implementation.

For the purpose of this issue however, I would tie the merging of #22 to closing this as I see that the proper solution.