Skip to content

Conversation

@stloyd
Copy link
Member

@stloyd stloyd commented Feb 12, 2023

Change Log

Added

Fixed

  • Fixed dependency for `amphp/socket` & `amphp/process`
  • Fixed wrong class name used in amphp `SocketServer` class
  • Don't use do real http calls in tests

Changed

Removed

Deprecated

Security


Description

There is no release higher than 2.x for both amphp/ & amphp/process:

This change also fixes broken tests:

1) Flow\ETL\Async\Tests\Integration\Server\SocketServerClientTest::test_start_tcp_server
TypeError: Cannot assign Amp\Socket\ResourceServerSocket to property Flow\ETL\Async\Amp\Server\SocketServer::$server of type ?Amp\Socket\SocketServer

/home/runner/work/flow/flow/src/adapter/etl-adapter-amphp/src/Flow/ETL/Async/Amp/Server/SocketServer.php:79
/home/runner/work/flow/flow/src/adapter/etl-adapter-amphp/tests/Flow/ETL/Async/Tests/Integration/Server/SocketServerClientTest.php:36

2) Flow\ETL\Async\Tests\Integration\Server\SocketServerClientTest::test_start_unix_domain_socket_server
TypeError: Cannot assign Amp\Socket\ResourceServerSocket to property Flow\ETL\Async\Amp\Server\SocketServer::$server of type ?Amp\Socket\SocketServer

/home/runner/work/flow/flow/src/adapter/etl-adapter-amphp/src/Flow/ETL/Async/Amp/Server/SocketServer.php:79
/home/runner/work/flow/flow/src/adapter/etl-adapter-amphp/tests/Flow/ETL/Async/Tests/Integration/Server/SocketServerClientTest.php:73

@norberttech norberttech merged commit b24c101 into flow-php:1.x Feb 12, 2023

$requests = function () use ($psr17Factory) : \Generator {
$psr18Client = new Client($psr17Factory);
$psr18Client->addResponse(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those are no longer functional tests, I would probably call then integration but since they were failing way too frequently it's better to keep them that way 👍

@stloyd stloyd deleted the amp-socket branch February 13, 2023 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants