Merge pull request #56 from clue-labs/loop

Fix forward compatibility with upcoming EventLoop releases

Author: jsor

composer.json

@@ -6,7 +6,7 @@
     "require": {
         "php": ">=5.3.0",
         "evenement/evenement": "^3.0 || ^2.0 || ^1.0",
-        "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3",
+        "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5",
         "react/stream": "^1.0 || ^0.7.6"
     },
     "require-dev": {

src/Process.php

@@ -4,7 +4,6 @@
 
 use Evenement\EventEmitter;
 use React\EventLoop\LoopInterface;
-use React\EventLoop\Timer\TimerInterface;
 use React\Stream\ReadableResourceStream;
 use React\Stream\WritableResourceStream;
 
@@ -116,7 +115,7 @@ public function start(LoopInterface $loop, $interval = 0.1)
                 return;
             }
 
-            $loop->addPeriodicTimer($interval, function (TimerInterface $timer) use ($that, $loop) {
+            $loop->addPeriodicTimer($interval, function ($timer) use ($that, $loop) {
                 if (!$that->isRunning()) {
                     $that->close();
                     $loop->cancelTimer($timer);

tests/AbstractProcessTest.php

@@ -2,10 +2,9 @@
 
 namespace React\Tests\ChildProcess;
 
-use React\ChildProcess\Process;
-use React\EventLoop\Timer\Timer;
 use PHPUnit\Framework\ExpectationFailedException;
 use PHPUnit\Framework\TestCase;
+use React\ChildProcess\Process;
 use SebastianBergmann\Environment\Runtime;
 
 abstract class AbstractProcessTest extends TestCase
@@ -152,15 +151,13 @@ public function testProcessWithDefaultCwdAndEnv()
         $cmd = $this->getPhpBinary() . ' -r ' . escapeshellarg('echo getcwd(), PHP_EOL, count($_SERVER), PHP_EOL;');
 
         $loop = $this->createLoop();
+
         $process = new Process($cmd);
+        $process->start($loop);
 
         $output = '';
-
-        $loop->addTimer(0.001, function(Timer $timer) use ($process, &$output) {
-            $process->start($timer->getLoop());
-            $process->stdout->on('data', function () use (&$output) {
-                $output .= func_get_arg(0);
-            });
+        $process->stdout->on('data', function () use (&$output) {
+            $output .= func_get_arg(0);
         });
 
         $loop->run();
@@ -180,15 +177,13 @@ public function testProcessWithCwd()
         $cmd = $this->getPhpBinary() . ' -r ' . escapeshellarg('echo getcwd(), PHP_EOL;');
 
         $loop = $this->createLoop();
+
         $process = new Process($cmd, '/');
+        $process->start($loop);
 
         $output = '';
-
-        $loop->addTimer(0.001, function(Timer $timer) use ($process, &$output) {
-            $process->start($timer->getLoop());
-            $process->stdout->on('data', function () use (&$output) {
-                $output .= func_get_arg(0);
-            });
+        $process->stdout->on('data', function () use (&$output) {
+            $output .= func_get_arg(0);
         });
 
         $loop->run();
@@ -205,15 +200,13 @@ public function testProcessWithEnv()
         $cmd = $this->getPhpBinary() . ' -r ' . escapeshellarg('echo getenv("foo"), PHP_EOL;');
 
         $loop = $this->createLoop();
+
         $process = new Process($cmd, null, array('foo' => 'bar'));
+        $process->start($loop);
 
         $output = '';
-
-        $loop->addTimer(0.001, function(Timer $timer) use ($process, &$output) {
-            $process->start($timer->getLoop());
-            $process->stdout->on('data', function () use (&$output) {
-                $output .= func_get_arg(0);
-            });
+        $process->stdout->on('data', function () use (&$output) {
+            $output .= func_get_arg(0);
         });
 
         $loop->run();
@@ -236,9 +229,7 @@ public function testStartAndAllowProcessToExitSuccessfullyUsingEventLoop()
             $termSignal = func_get_arg(1);
         });
 
-        $loop->addTimer(0.001, function(Timer $timer) use ($process) {
-            $process->start($timer->getLoop());
-        });
+        $process->start($loop);
 
         $loop->run();
 
@@ -257,15 +248,13 @@ public function testStartInvalidProcess()
         $cmd = tempnam(sys_get_temp_dir(), 'react');
 
         $loop = $this->createLoop();
+
         $process = new Process($cmd);
+        $process->start($loop);
 
         $output = '';
-
-        $loop->addTimer(0.001, function(Timer $timer) use ($process, &$output) {
-            $process->start($timer->getLoop());
-            $process->stderr->on('data', function () use (&$output) {
-                $output .= func_get_arg(0);
-            });
+        $process->stderr->on('data', function () use (&$output) {
+            $output .= func_get_arg(0);
         });
 
         $loop->run();
@@ -338,10 +327,8 @@ public function testTerminateWithDefaultTermSignalUsingEventLoop()
             $termSignal = func_get_arg(1);
         });
 
-        $loop->addTimer(0.001, function(Timer $timer) use ($process) {
-            $process->start($timer->getLoop());
-            $process->terminate();
-        });
+        $process->start($loop);
+        $process->terminate();
 
         $loop->run();
 
@@ -379,22 +366,20 @@ public function testTerminateWithStopAndContinueSignalsUsingEventLoop()
         });
 
         $that = $this;
-        $loop->addTimer(0.001, function(Timer $timer) use ($process, $that) {
-            $process->start($timer->getLoop());
-            $process->terminate(SIGSTOP);
-
-            $that->assertSoon(function() use ($process, $that) {
-                $that->assertTrue($process->isStopped());
-                $that->assertTrue($process->isRunning());
-                $that->assertEquals(SIGSTOP, $process->getStopSignal());
-            });
-
-            $process->terminate(SIGCONT);
-
-            $that->assertSoon(function() use ($process, $that) {
-                $that->assertFalse($process->isStopped());
-                $that->assertEquals(SIGSTOP, $process->getStopSignal());
-            });
+        $process->start($loop);
+        $process->terminate(SIGSTOP);
+
+        $that->assertSoon(function () use ($process, $that) {
+            $that->assertTrue($process->isStopped());
+            $that->assertTrue($process->isRunning());
+            $that->assertEquals(SIGSTOP, $process->getStopSignal());
+        });
+
+        $process->terminate(SIGCONT);
+
+        $that->assertSoon(function () use ($process, $that) {
+            $that->assertFalse($process->isStopped());
+            $that->assertEquals(SIGSTOP, $process->getStopSignal());
         });
 
         $loop->run();
@@ -444,7 +429,12 @@ function ($output) use (&$stdErr) {
             }
         );
 
-        $loop->tick();
+        // tick loop once
+        $loop->addTimer(0, function () use ($loop) {
+            $loop->stop();
+        });
+        $loop->run();
+
         sleep(1); // comment this line out and it works fine
 
         $loop->run();

tests/ExtEventLoopProcessTest.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace React\Tests\ChildProcess;
+
+use React\EventLoop\ExtEventLoop;
+
+class ExtEventLoopProcessTest extends AbstractProcessTest
+{
+    public function createLoop()
+    {
+        if (!extension_loaded('event')) {
+            $this->markTestSkipped('ext-event is not installed.');
+        }
+        if (!class_exists('React\EventLoop\ExtEventLoop')) {
+            $this->markTestSkipped('ext-event not supported by this legacy react/event-loop version');
+        }
+
+        return new ExtEventLoop();
+    }
+}

tests/ExtLibevLoopProcessTest.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace React\Tests\ChildProcess;
+
+use React\EventLoop\ExtLibevLoop;
+use React\EventLoop\LibEvLoop;
+
+class ExtLibevLoopProcessTest extends AbstractProcessTest
+{
+    public function createLoop()
+    {
+        if (!class_exists('libev\EventLoop')) {
+            $this->markTestSkipped('ext-libev is not installed.');
+        }
+
+        return class_exists('React\EventLoop\ExtLibevLoop') ? new ExtLibevLoop() : new LibEvLoop();
+    }
+}

tests/ExtLibeventLoopProcessTest.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace React\Tests\ChildProcess;
+
+use React\EventLoop\ExtLibeventLoop;
+use React\EventLoop\LibEventLoop;
+
+class ExtLibeventLoopProcessTest extends AbstractProcessTest
+{
+    public function createLoop()
+    {
+        if (!function_exists('event_base_new')) {
+            $this->markTestSkipped('ext-libevent is not installed.');
+        }
+
+        return class_exists('React\EventLoop\ExtLibeventLoop') ? new ExtLibeventLoop() : new LibEventLoop();
+    }
+}