Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make compatible with stream interface #72

Closed
wants to merge 2 commits into from

Conversation

dbu
Copy link
Contributor

@dbu dbu commented Mar 3, 2024

the remaining failures will be fixed with php-http/client-integration-tests#59

@dbu dbu force-pushed the compatibility-stream-interface branch from f4b9555 to e2d3906 Compare March 5, 2024 13:00
@andrew-demb
Copy link
Contributor

Can we help to proceed this PR?

@dbu
Copy link
Contributor Author

dbu commented Jun 5, 2024

thanks for asking. if you have some time to look if you can reproduce the test failures locally, and ideally can find what causes them, it would help for sure. you could do a merge request against this branch if its a small change, or create a new merge request against 2.x with your changes, whatever is easier for you.

@andrew-demb
Copy link
Contributor

About HTTP client integration feature tests - looks like this library was never support such features:

  • auto set content length
  • gzip
  • deflate
  • redirect
  • chunked

So this test failures from CI are valid.

image

@andrew-demb
Copy link
Contributor

I cannot get stable responses for the latest version of php-http/client-integration-tests, because target domain changed from https://httpbin.org in favor of https://httpbingo.org.

https://httpbingo.org always returns 402 Payment Required status code for runned tests from my machine.

// var_dump for response
// \Http\Client\Tests\HttpFeatureTest::testGet()
class Nyholm\Psr7\Response#310 (6) {
  private $reasonPhrase =>
  string(16) "Payment Required"
  private $statusCode =>
  int(402)
  private $headers =>
  array(5) {
    'date' =>
    array(1) {
      [0] =>
      string(29) "Thu, 13 Jun 2024 22:09:46 GMT"
    }
    'server' =>
    array(1) {
      [0] =>
      string(26) "Fly/04517508a (2024-06-12)"
    }
    'via' =>
    array(1) {
      [0] =>
      string(10) "1.1 fly.io"
    }
    'fly-request-id' =>
    array(1) {
      [0] =>
      string(30) "01J09TCMXJ67ZKCR8068JP6VQ4-waw"
    }
    'content-length' =>
    array(1) {
      [0] =>
      string(1) "0"
    }
  }
  private $headerNames =>
  array(5) {
    'date' =>
    string(4) "date"
    'server' =>
    string(6) "server"
    'via' =>
    string(3) "via"
    'fly-request-id' =>
    string(14) "fly-request-id"
    'content-length' =>
    string(14) "content-length"
  }
  private $protocol =>
  string(3) "1.1"
  private $stream =>
  class Http\Client\Socket\Stream#344 (5) {
    private $socket =>
    resource(158) of type (stream)
    private $isDetached =>
    bool(false)
    private $size =>
    int(0)
    private $readed =>
    int(0)
    private $request =>
    class GuzzleHttp\Psr7\Request#307 (7) {
      private $method =>
      string(3) "GET"
      private $requestTarget =>
      NULL
      private $uri =>
      class GuzzleHttp\Psr7\Uri#300 (8) {
        ...
      }
      private $headers =>
      array(2) {
        ...
      }
      private $headerNames =>
      array(2) {
        ...
      }
      private $protocol =>
      string(3) "1.1"
      private $stream =>
      class GuzzleHttp\Psr7\Stream#339 (7) {
        ...
      }
    }
  }
}
$ curl --request GET 'https://httpbingo.org/get' -H 'User-Agent:' -i
HTTP/2 402 
date: Thu, 13 Jun 2024 22:13:52 GMT
content-length: 0
server: Fly/04517508a (2024-06-12)
via: 2 fly.io
fly-request-id: 01J09TM5APY6FCJZW1X1RHTXC6-waw

As I can see, only when I include header User-Agent in the request - I will reveive stable responses according to the used endpoint.

@andrew-demb
Copy link
Contributor

This test failures with lowest deps:

1) Http\Client\Socket\Tests\SocketClientFeatureTest::testGet
LogicException: You cannot use "Http\Message\MessageFactory\GuzzleMessageFactory" as the "php-http/message-factory" package is not installed. Try running "composer require php-http/message-factory". Note that this package is deprecated, use "psr/http-factory" instead in /home/runner/work/socket-client/socket-client/vendor/php-http/message/src/MessageFactory/GuzzleMessageFactory.php:10

2) Http\Client\Socket\Tests\SocketHttpAdapterTest::testSendRequest
LogicException: You cannot use "Http\Message\MessageFactory\GuzzleMessageFactory" as the "php-http/message-factory" package is not installed. Try running "composer require php-http/message-factory". Note that this package is deprecated, use "psr/http-factory" instead in /home/runner/work/socket-client/socket-client/vendor/php-http/message/src/MessageFactory/GuzzleMessageFactory.php:10

Can be fixed either by:

@andrew-demb
Copy link
Contributor

@dbu
Summary.

I suggest to do three things:

  1. rollback a Replace httpbin.org by httpbingo.org to fix postmanlabs/httpbin issues client-integration-tests#56 and release a 3.1.1 (change httpbingo.org to httpbin.org)
  2. change dependency constraint of php-http/client-integration-tests from ^3.0 to ^3.1
  3. explicitly skip testing unsupported features in integration tests
===================================================================
diff --git a/composer.json b/composer.json
--- a/composer.json	(revision 1bb68e85b6be6567cc1ea145f8a2b145d9108cfe)
+++ b/composer.json	(date 1718317478552)
@@ -17,7 +17,7 @@
     },
     "require-dev": {
         "friendsofphp/php-cs-fixer": "^3.51",
-        "php-http/client-integration-tests": "^3.0",
+        "php-http/client-integration-tests": "^3.1",
         "php-http/message": "^1.16",
         "php-http/client-common": "^2.7",
         "phpunit/phpunit": "^8.5.23 || ~9.5"
===================================================================
diff --git a/tests/SocketClientFeatureTest.php b/tests/SocketClientFeatureTest.php
--- a/tests/SocketClientFeatureTest.php	(revision 1bb68e85b6be6567cc1ea145f8a2b145d9108cfe)
+++ b/tests/SocketClientFeatureTest.php	(date 1718317770046)
@@ -12,4 +12,29 @@
     {
         return new SocketHttpClient();
     }
+
+    public function testAutoSetContentLength(): void
+    {
+        $this->markTestSkipped('Feature is unsupported');
+    }
+
+    public function testGzip(): void
+    {
+        $this->markTestSkipped('Feature is unsupported');
+    }
+
+    public function testDeflate(): void
+    {
+        $this->markTestSkipped('Feature is unsupported');
+    }
+
+    public function testChunked(): void
+    {
+        $this->markTestSkipped('Feature is unsupported');
+    }
+
+    public function testRedirect(): void
+    {
+        $this->markTestSkipped('Feature is unsupported');
+    }
 }

image

@andrew-demb
Copy link
Contributor

thanks for asking. if you have some time to look if you can reproduce the test failures locally, and ideally can find what causes them, it would help for sure. you could do a merge request against this branch if its a small change, or create a new merge request against 2.x with your changes, whatever is easier for you.

Here it is #75

@dbu
Copy link
Contributor Author

dbu commented Sep 1, 2024

continued in #75, thanks a lot @andrew-demb

@dbu dbu closed this Sep 1, 2024
@dbu dbu deleted the compatibility-stream-interface branch September 1, 2024 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants