Closed Tobion closed 5 years ago
HHVM test still fail but at least we see the failures now instead of a syntax error.
I've found the problem. HHVM is not compatible with Generator::send
, see https://github.com/facebook/hhvm/issues/6807
This is good to go. It won't get better.
Can second that finding, we see similar failures on the AWS SDK for PHP based on that HHVM issue.
I'm not sure what this change accomplishes. Guzzle coroutines are still incompatible with HHVM, and the HHVM team has announced that PHP compatibility is no longer a goal of the HHVM project.
This library does not work with HHVM, and I don't see that changing without a major version bump.
It accomplishes to see hhvm errors. I can also remove hhvm from travis config completely if you prefer that.
HHVM should probably stay in travis, at least to show the incompatibilities for those that explore it since there's no FAQ that provides this detail.
As for the fix itself, it's swapping out:
Fatal error: syntax error, unexpected T_YIELD in /home/travis/build/guzzle/promises/tests/functionsTest.php on line 536
for actual errors like:
1) GuzzleHttp\Promise\Tests\EachPromiseTest::testMutexPreventsGeneratorRecursion Exception: Generator is already finished
I'm also in the not sure camp but lean towards cleaning this up since, while it doesn't provide anything functional, it more clearly defines that facebook/hhvm/#6807 is the real reason for this problem.
Is this going to get merged?
It's not possible to fix hhvm tests and most project dropped hhvm support, even composer https://github.com/composer/composer/issues/8127. So I removed hhvm and php 5.5 which is EOL since a while. This way I could also update phpunit.
Trying to fix tests on hhvm and cleaning inconsistent use-statements