Fixed error that finished processes are not cleaned correctly.

Removed processSuccessCallback and processFailCallback: $process->isSuccessful() can be used in processFinishCallback.

Author: BluePsyduck

README.md

@@ -41,11 +41,6 @@ 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.
-* **processSuccessCallback:** Triggered when a processes has finished with an exit code of 0.
-* **processFailCallback:** Triggered when a processes has failed with an exit code other than 0.
-
-_Note:_ Each process will trigger either the `processSuccessCallback` or the `processFailCallback` depending of its exit 
-code. The `processFinishCallback` will always be triggered afterwards, ignoring the exit code.
 
 Each callback gets the process instance which triggered the event passed as only parameter. Here is an example of 
 setting a `processStartCallback`:

src/ProcessManager.php

@@ -50,18 +50,6 @@ class ProcessManager
      */
     protected $processFinishCallback;
 
-    /**
-     * The callback for when a process has successfully finished with exit code 0.
-     * @var callable|null
-     */
-    protected $processSuccessCallback;
-
-    /**
-     * The callback for when a process has failed to finish with exit code 0.
-     * @var callable|null
-     */
-    protected $processFailCallback;
-
     /**
      * ProcessManager constructor.
      * @param int $numberOfParallelProcesses The number of processes to run in parallel.
@@ -95,28 +83,6 @@ public function setProcessFinishCallback(?callable $processFinishCallback)
         return $this;
     }
 
-    /**
-     * Sets the callback for when a process has failed to finish with exit code 0.
-     * @param callable|null $processSuccessCallback The callback, accepting a Process as only argument.
-     * @return $this
-     */
-    public function setProcessSuccessCallback(?callable $processSuccessCallback)
-    {
-        $this->processSuccessCallback = $processSuccessCallback;
-        return $this;
-    }
-
-    /**
-     * Sets the callback for when a process has successfully finished with exit code 0.
-     * @param callable|null $processFailCallback The callback, accepting a Process as only argument.
-     * @return $this
-     */
-    public function setProcessFailCallback(?callable $processFailCallback)
-    {
-        $this->processFailCallback = $processFailCallback;
-        return $this;
-    }
-
     /**
      * Invokes the callback if it is an callable.
      * @param callable|null $callback
@@ -173,26 +139,23 @@ protected function canExecuteNextPendingRequest(): bool
      */
     protected function checkRunningProcesses(): void
     {
-        foreach ($this->runningProcesses as $process) {
-            $this->checkRunningProcess($process);
+        foreach ($this->runningProcesses as $pid => $process) {
+            $this->checkRunningProcess($pid, $process);
         }
     }
 
     /**
      * Checks the process whether it has finished.
+     * @param int $pid
      * @param Process $process
      */
-    protected function checkRunningProcess(Process $process): void
+    protected function checkRunningProcess(int $pid, Process $process): void
     {
         $process->checkTimeout();
         if (!$process->isRunning()) {
-            $this->invokeCallback(
-                $process->isSuccessful() ? $this->processSuccessCallback : $this->processFailCallback,
-                $process
-            );
             $this->invokeCallback($this->processFinishCallback, $process);
 
-            unset($this->runningProcesses[$process->getPid()]);
+            unset($this->runningProcesses[$pid]);
             $this->executeNextPendingProcess();
         }
     }

test/src/ProcessManagerTest.php

@@ -67,36 +67,6 @@ public function testSetProcessFinishCallback(): void
         $this->assertSame($callback, $this->extractProperty($manager, 'processFinishCallback'));
     }
 
-    /**
-     * Tests the setProcessSuccessCallback method.
-     * @covers ::setProcessSuccessCallback
-     * @throws ReflectionException
-     */
-    public function testSetProcessSuccessCallback(): void
-    {
-        $callback = 'strval';
-
-        $manager = new ProcessManager();
-        $result = $manager->setProcessSuccessCallback($callback);
-        $this->assertSame($manager, $result);
-        $this->assertSame($callback, $this->extractProperty($manager, 'processSuccessCallback'));
-    }
-    
-    /**
-     * Tests the setProcessFailCallback method.
-     * @covers ::setProcessFailCallback
-     * @throws ReflectionException
-     */
-    public function testSetProcessFailCallback(): void
-    {
-        $callback = 'strval';
-
-        $manager = new ProcessManager();
-        $result = $manager->setProcessFailCallback($callback);
-        $this->assertSame($manager, $result);
-        $this->assertSame($callback, $this->extractProperty($manager, 'processFailCallback'));
-    }
-
     /**
      * Provides the data for the invokeCallback test.
      * @return array
@@ -109,7 +79,6 @@ public function provideInvokeCallback(): array
         ];
     }
 
-
     /**
      * Tests the invokeCallback method.
      * @param bool $withCallback
@@ -292,10 +261,10 @@ public function testCheckRunningProcesses(): void
         $manager->expects($this->exactly(2))
                 ->method('checkRunningProcess')
                 ->withConsecutive(
-                    [$process1],
-                    [$process2]
+                    [42, $process1],
+                    [1337, $process2]
                 );
-        $this->injectProperty($manager, 'runningProcesses', [$process1, $process2]);
+        $this->injectProperty($manager, 'runningProcesses', [42 => $process1, 1337 => $process2]);
 
         $this->invokeMethod($manager, 'checkRunningProcesses');
     }
@@ -307,39 +276,33 @@ public function testCheckRunningProcesses(): void
     public function provideCheckRunningProcess(): array
     {
         return [
-            [true, null, false],
-            [false, true, true],
-            [false, false, true],
+            [true, false],
+            [false, true],
+            [false, true],
         ];
     }
 
     /**
      * Tests the checkRunningProcess method.
      * @param bool $resultIsRunning
-     * @param bool|null $resultIsSuccessful
      * @param bool $expectFinish
      * @throws ReflectionException
      * @covers ::checkRunningProcess
      * @dataProvider provideCheckRunningProcess
      */
-    public function testCheckRunningProcess(bool $resultIsRunning, ?bool $resultIsSuccessful, bool $expectFinish): void
+    public function testCheckRunningProcess(bool $resultIsRunning, bool $expectFinish): void
     {
+        $pid = 42;
         /* @var Process|MockObject $process */
         $process = $this->getMockBuilder(Process::class)
-                        ->setMethods(['checkTimeout', 'isRunning', 'isSuccessful', 'getPid'])
+                        ->setMethods(['checkTimeout', 'isRunning'])
                         ->disableOriginalConstructor()
                         ->getMock();
         $process->expects($this->once())
                 ->method('checkTimeout');
         $process->expects($this->once())
                 ->method('isRunning')
                 ->willReturn($resultIsRunning);
-        $process->expects($resultIsSuccessful === null ? $this->never() : $this->once())
-                ->method('isSuccessful')
-                ->willReturn($resultIsSuccessful);
-        $process->expects($expectFinish ? $this->once() : $this->never())
-                ->method('getPid')
-                ->willReturn(42);
 
         /* @var Process $process2 */
         $process2 = $this->createMock(Process::class);
@@ -351,20 +314,15 @@ public function testCheckRunningProcess(bool $resultIsRunning, ?bool $resultIsSu
                         ->setMethods(['invokeCallback', 'executeNextPendingProcess'])
                         ->disableOriginalConstructor()
                         ->getMock();
-        $manager->expects($expectFinish ? $this->exactly(2) : $this->never())
+        $manager->expects($expectFinish ? $this->once() : $this->never())
                 ->method('invokeCallback')
-                ->withConsecutive(
-                    [(bool) $resultIsSuccessful ? 'intval' : 'floatval', $process],
-                    ['strval', $process]
-                );
+                ->with('strval', $process);
         $manager->expects($expectFinish ? $this->once() : $this->never())
                 ->method('executeNextPendingProcess');
-        $manager->setProcessFinishCallback('strval')
-                ->setProcessSuccessCallback('intval')
-                ->setProcessFailCallback('floatval');
+        $manager->setProcessFinishCallback('strval');
         $this->injectProperty($manager, 'runningProcesses', $runningProcesses);
 
-        $this->invokeMethod($manager, 'checkRunningProcess', $process);
+        $this->invokeMethod($manager, 'checkRunningProcess', $pid, $process);
         $this->assertEquals($expectedRunningProcesses, $this->extractProperty($manager, 'runningProcesses'));
     }