Merge pull request #3 from BluePsyduck/issue-2_timeout

#1: Catch timeout exceptions

Author: BluePsyduck

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## 1.2.0 - 2018-12-03
+
+### Added
+
+- New callback `processTimeoutCallback`.
+
+### Fixed
+
+- Process manager failed to continue executing processes when one of them timed out. 
+
 ## 1.1.0 - 2018-10-07
 
 ### Added

README.md

@@ -27,8 +27,8 @@ $processManager = new ProcessManager($numberOfParallelProcesses, $pollInterval,
 // Add some processes
 // Processes get executed automatically once they are added to the manager. 
 // If the limit of parallel processes is reached, they are placed in a queue and wait for a process to finish.
-$processManager->addProcess(new Process('ls -l'));
-$processManager->addProcess(new Process('ls -l'));
+$processManager->addProcess(Process::fromShellCommandline('ls -l'));
+$processManager->addProcess(Process::fromShellCommandline('ls -l'));
 
 // Wait for all processes to finish
 $processManager->waitForAllProcesses();
@@ -42,6 +42,8 @@ The process manager allows for some callbacks to be specified, which get called
 
 * **processStartCallback:** Triggered before a process is started.
 * **processFinishCallback:** Triggered when a process has finished.
+* **processTimeoutCallback:** Triggered when a process timed out. Note that the _processFinishCallback_ will be 
+  triggered afterwards as well.
 
 Each callback gets the process instance which triggered the event passed as only parameter. Here is an example of 
 setting a `processStartCallback`:
@@ -57,6 +59,6 @@ $processManager->setProcessStartCallback(function (Process $process): void {
     echo 'Starting process: ' . $process->getCommandLine();
 });
 
-$processManager->addProcess(new Process('ls -l'));
+$processManager->addProcess(Process::fromShellCommandline('ls -l'));
 $processManager->waitForAllProcesses();
 ```

src/ProcessManager.php

@@ -4,6 +4,7 @@
 
 namespace BluePsyduck\SymfonyProcessManager;
 
+use Symfony\Component\Process\Exception\ProcessTimedOutException;
 use Symfony\Component\Process\Process;
 
 /**
@@ -56,6 +57,12 @@ class ProcessManager implements ProcessManagerInterface
      */
     protected $processFinishCallback;
 
+    /**
+     * The callback for when a process timed out.
+     * @var callable|null
+     */
+    protected $processTimeoutCallback;
+
     /**
      * ProcessManager constructor.
      * @param int $numberOfParallelProcesses The number of processes to run in parallel.
@@ -128,6 +135,17 @@ public function setProcessFinishCallback(?callable $processFinishCallback)
         return $this;
     }
 
+    /**
+     * Sets the callback for when a process timed out.
+     * @param callable|null $processTimeoutCallback
+     * @return $this
+     */
+    public function setProcessTimeoutCallback(?callable $processTimeoutCallback)
+    {
+        $this->processTimeoutCallback = $processTimeoutCallback;
+        return $this;
+    }
+
     /**
      * Invokes the callback if it is an callable.
      * @param callable|null $callback
@@ -205,7 +223,7 @@ protected function checkRunningProcesses(): void
      */
     protected function checkRunningProcess(?int $pid, Process $process): void
     {
-        $process->checkTimeout();
+        $this->checkProcessTimeout($process);
         if (!$process->isRunning()) {
             $this->invokeCallback($this->processFinishCallback, $process);
 
@@ -216,6 +234,19 @@ protected function checkRunningProcess(?int $pid, Process $process): void
         }
     }
 
+    /**
+     * Checks whether the process already timed out.
+     * @param Process $process
+     */
+    protected function checkProcessTimeout(Process $process): void
+    {
+        try {
+            $process->checkTimeout();
+        } catch (ProcessTimedOutException $e) {
+            $this->invokeCallback($this->processTimeoutCallback, $process);
+        }
+    }
+
     /**
      * Waits for all processes to be finished.
      * @return $this

src/ProcessManagerInterface.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace BluePsyduck\SymfonyProcessManager;
 
 use Symfony\Component\Process\Process;

test/src/ProcessManagerTest.php

@@ -9,6 +9,7 @@
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use ReflectionException;
+use Symfony\Component\Process\Exception\ProcessTimedOutException;
 use Symfony\Component\Process\Process;
 
 /**
@@ -123,6 +124,21 @@ public function testSetProcessFinishCallback(): void
         $this->assertSame($callback, $this->extractProperty($manager, 'processFinishCallback'));
     }
 
+    /**
+     * Tests the setProcessTimeoutCallback method.
+     * @covers ::setProcessTimeoutCallback
+     * @throws ReflectionException
+     */
+    public function testSetProcessTimeoutCallback(): void
+    {
+        $callback = 'strval';
+
+        $manager = new ProcessManager();
+        $result = $manager->setProcessTimeoutCallback($callback);
+        $this->assertSame($manager, $result);
+        $this->assertSame($callback, $this->extractProperty($manager, 'processTimeoutCallback'));
+    }
+
     /**
      * Provides the data for the invokeCallback test.
      * @return array
@@ -376,11 +392,9 @@ public function testCheckRunningProcess(
     ): void {
         /* @var Process|MockObject $process */
         $process = $this->getMockBuilder(Process::class)
-                        ->setMethods(['checkTimeout', 'isRunning'])
+                        ->setMethods(['isRunning'])
                         ->disableOriginalConstructor()
                         ->getMock();
-        $process->expects($this->once())
-                ->method('checkTimeout');
         $process->expects($this->once())
                 ->method('isRunning')
                 ->willReturn($resultIsRunning);
@@ -392,9 +406,13 @@ public function testCheckRunningProcess(
 
         /* @var ProcessManager|MockObject $manager */
         $manager = $this->getMockBuilder(ProcessManager::class)
-                        ->setMethods(['invokeCallback', 'executeNextPendingProcess'])
+                        ->setMethods(['checkProcessTimeout', 'invokeCallback', 'executeNextPendingProcess'])
                         ->disableOriginalConstructor()
                         ->getMock();
+        $manager->expects($this->once())
+                ->method('checkProcessTimeout')
+                ->with($process)
+                ->willReturn(false);
         $manager->expects($expectFinish ? $this->once() : $this->never())
                 ->method('invokeCallback')
                 ->with('strval', $process);
@@ -407,6 +425,56 @@ public function testCheckRunningProcess(
         $this->assertEquals($expectedRunningProcesses, $this->extractProperty($manager, 'runningProcesses'));
     }
 
+    /**
+     * Provides the data for the checkProcessTimeout test.
+     * @return array
+     */
+    public function provideCheckProcessTimeout(): array
+    {
+        return [
+            [false, false],
+            [true, true],
+        ];
+    }
+
+    /**
+     * Tests the checkProcessTimeout method.
+     * @param bool $throwException
+     * @param bool $expectInvoke
+     * @throws ReflectionException
+     * @covers ::checkProcessTimeout
+     * @dataProvider provideCheckProcessTimeout
+     */
+    public function testCheckProcessTimeout(bool $throwException, bool $expectInvoke): void
+    {
+        /* @var Process|MockObject $process */
+        $process = $this->getMockBuilder(Process::class)
+                        ->setMethods(['checkTimeout'])
+                        ->disableOriginalConstructor()
+                        ->getMock();
+
+        if ($throwException) {
+            $process->expects($this->once())
+                    ->method('checkTimeout')
+                    ->willThrowException($this->createMock(ProcessTimedOutException::class));
+        } else {
+            $process->expects($this->once())
+                    ->method('checkTimeout');
+        }
+
+        /* @var ProcessManager|MockObject $manager */
+        $manager = $this->getMockBuilder(ProcessManager::class)
+                        ->setMethods(['invokeCallback'])
+                        ->disableOriginalConstructor()
+                        ->getMock();
+        $manager->expects($expectInvoke ? $this->once() : $this->never())
+                ->method('invokeCallback')
+                ->with('strval', $process);
+        $manager->setProcessTimeoutCallback('strval');
+
+        $this->invokeMethod($manager, 'checkProcessTimeout', $process);
+    }
+
     /**
      * Tests the waitForAllProcesses method.
      * @covers ::waitForAllProcesses