-
Notifications
You must be signed in to change notification settings - Fork 43
Description
I'm on PHP 8.2 and tried running the phpunit tests.
I get a bunch of deprecated followed by two errors related to the phpunit tests that interact with the node server.js integration.
fXmlRpc\Exception\TransportException: Transport error occurred: Server error: `POST http://127.0.0.1:9091/` resulted in a `500 Internal Server Error` response
Http\Client\Exception\HttpException: Server error: `POST http://127.0.0.1:9091/` resulted in a `500 Internal Server Error` response
GuzzleHttp\Exception\ServerException: Server error: `POST http://127.0.0.1:9091/` resulted in a `500 Internal Server Error` response
They can all be traced back to this file:
/fxmlrpc/tests/fXmlRpc/Integration/NodeCallClientIntegrationTest.php:61
/** @dataProvider getClientsOnly */
public function testServerReturnsInvalidResult(Client $client)
{
$this->executeSystemFailureTest($client);
}
Basically what's happening is port 9091 is supposed to behave like an api returning a 500 Internal Server Error, that's all fine, then the phpunit tests are supposed to handle it properly. Guzzle6 and onwards changed the behavior of Guzzle to be http_errors: true by default. So if the status code is 4xx or 5xx, then it will throw an exception. The tests aren't catching these, and we see 2 errors in phpunit preventing the tests from passing.
When constructing the Client, you can simply pass in ['http_errors' => false] and the tests will pass.
This would be done in AbstractIntegreationTest.php.
Before:
new \Http\Adapter\Guzzle7\Client(new \GuzzleHttp\Client()
After:
new \Http\Adapter\Guzzle7\Client(new \GuzzleHttp\Client(['http_errors'=>false])
But this got me thinking, maybe the package should catch these exceptions:
https://docs.guzzlephp.org/en/stable/quickstart.html#exceptions