Merge pull request #131 from clue-labs/timeout-tests

Apply maximum test timeouts for integration tests

Author: WyriHaximus

composer.json

@@ -13,7 +13,9 @@
         "ringcentral/psr7": "^1.2"
     },
     "require-dev": {
-        "phpunit/phpunit": "^6.4 || ^5.7 || ^4.8.35"
+        "clue/block-react": "^1.2",
+        "phpunit/phpunit": "^6.4 || ^5.7 || ^4.8.35",
+        "react/promise-stream": "^1.1"
     },
     "autoload": {
         "psr-4": {

tests/FunctionalIntegrationTest.php

@@ -2,14 +2,37 @@
 
 namespace React\Tests\HttpClient;
 
+use Clue\React\Block;
 use React\EventLoop\Factory;
 use React\HttpClient\Client;
 use React\HttpClient\Response;
+use React\Promise\Deferred;
+use React\Promise\Stream;
 use React\Socket\Server;
 use React\Socket\ConnectionInterface;
 
 class FunctionalIntegrationTest extends TestCase
 {
+    /**
+     * Test timeout to use for local tests.
+     *
+     * In practice this would be near 0.001s, but let's leave some time in case
+     * the local system is currently busy.
+     *
+     * @var float
+     */
+    const TIMEOUT_LOCAL = 1.0;
+
+    /**
+     * Test timeout to use for remote (internet) tests.
+     *
+     * In pratice this should be below 1s, but this relies on infrastructure
+     * outside our control, so consider this a maximum to avoid running for hours.
+     *
+     * @var float
+     */
+    const TIMEOUT_REMOTE = 10.0;
+
     public function testRequestToLocalhostEmitsSingleRemoteConnection()
     {
         $loop = Factory::create();
@@ -24,9 +47,11 @@ public function testRequestToLocalhostEmitsSingleRemoteConnection()
 
         $client = new Client($loop);
         $request = $client->request('GET', 'http://localhost:' . $port);
+
+        $promise = Stream\first($request, 'close');
         $request->end();
 
-        $loop->run();
+        Block\await($promise, $loop, self::TIMEOUT_LOCAL);
     }
 
     public function testRequestLegacyHttpServerWithOnlyLineFeedReturnsSuccessfulResponse()
@@ -47,9 +72,10 @@ public function testRequestLegacyHttpServerWithOnlyLineFeedReturnsSuccessfulResp
             $response->on('data', $once);
         });
 
+        $promise = Stream\first($request, 'close');
         $request->end();
 
-        $loop->run();
+        Block\await($promise, $loop, self::TIMEOUT_LOCAL);
     }
 
     /** @group internet */
@@ -65,9 +91,10 @@ public function testSuccessfulResponseEmitsEnd()
             $response->on('end', $once);
         });
 
+        $promise = Stream\first($request, 'close');
         $request->end();
 
-        $loop->run();
+        Block\await($promise, $loop, self::TIMEOUT_REMOTE);
     }
 
     /** @group internet */
@@ -79,19 +106,17 @@ public function testPostDataReturnsData()
         $data = str_repeat('.', 33000);
         $request = $client->request('POST', 'https://' . (mt_rand(0, 1) === 0 ? 'eu.' : '') . 'httpbin.org/post', array('Content-Length' => strlen($data)));
 
-        $buffer = '';
-        $request->on('response', function (Response $response) use (&$buffer) {
-            $response->on('data', function ($chunk) use (&$buffer) {
-                $buffer .= $chunk;
-            });
+        $deferred = new Deferred();
+        $request->on('response', function (Response $response) use ($deferred) {
+            $deferred->resolve(Stream\buffer($response));
         });
 
         $request->on('error', 'printf');
         $request->on('error', $this->expectCallableNever());
 
         $request->end($data);
 
-        $loop->run();
+        $buffer = Block\await($deferred->promise(), $loop, self::TIMEOUT_REMOTE);
 
         $this->assertNotEquals('', $buffer);
 
@@ -110,19 +135,17 @@ public function testPostJsonReturnsData()
         $data = json_encode(array('numbers' => range(1, 50)));
         $request = $client->request('POST', 'https://httpbin.org/post', array('Content-Length' => strlen($data), 'Content-Type' => 'application/json'));
 
-        $buffer = '';
-        $request->on('response', function (Response $response) use (&$buffer) {
-            $response->on('data', function ($chunk) use (&$buffer) {
-                $buffer .= $chunk;
-            });
+        $deferred = new Deferred();
+        $request->on('response', function (Response $response) use ($deferred) {
+            $deferred->resolve(Stream\buffer($response));
         });
 
         $request->on('error', 'printf');
         $request->on('error', $this->expectCallableNever());
 
         $request->end($data);
 
-        $loop->run();
+        $buffer = Block\await($deferred->promise(), $loop, self::TIMEOUT_REMOTE);
 
         $this->assertNotEquals('', $buffer);
 
@@ -142,7 +165,5 @@ public function testCancelPendingConnectionEmitsClose()
         $request->on('close', $this->expectCallableOnce());
         $request->end();
         $request->close();
-
-        $loop->run();
     }
 }