[Process] Trow exceptions in case a Process method is supposed to be called after termination

Author: romainneutron

src/Symfony/Component/Process/Process.php

@@ -296,13 +296,11 @@ public function restart($callback = null)
      *
      * @throws RuntimeException When process timed out
      * @throws RuntimeException When process stopped after receiving signal
-     * @throws LogicException When process is not started
+     * @throws LogicException   When process is not yet started
      */
     public function wait($callback = null)
     {
-        if (!$this->isStarted()) {
-            throw new LogicException(sprintf('Process must be started before calling %s', __FUNCTION__));
-        }
+        $this->requireProcessIsStarted(__FUNCTION__);
 
         $this->updateStatus(false);
         if (null !== $callback) {
@@ -377,9 +375,7 @@ public function signal($signal)
      */
     public function getOutput()
     {
-        if (!$this->isStarted()) {
-            throw new LogicException(sprintf('Process must be started before calling %s', __FUNCTION__));
-        }
+        $this->requireProcessIsStarted(__FUNCTION__);
 
         $this->readPipes(false, defined('PHP_WINDOWS_VERSION_BUILD') ? !$this->processInformation['running'] : true);
 
@@ -392,10 +388,14 @@ public function getOutput()
      * In comparison with the getOutput method which always return the whole
      * output, this one returns the new output since the last call.
      *
+     * @throws LogicException In case the process is not started
+     *
      * @return string The process output since the last call
      */
     public function getIncrementalOutput()
     {
+        $this->requireProcessIsStarted(__FUNCTION__);
+
         $data = $this->getOutput();
 
         $latest = substr($data, $this->incrementalOutputOffset);
@@ -415,9 +415,7 @@ public function getIncrementalOutput()
      */
     public function getErrorOutput()
     {
-        if (!$this->isStarted()) {
-            throw new LogicException(sprintf('Process must be started before calling %s', __FUNCTION__));
-        }
+        $this->requireProcessIsStarted(__FUNCTION__);
 
         $this->readPipes(false, defined('PHP_WINDOWS_VERSION_BUILD') ? !$this->processInformation['running'] : true);
 
@@ -431,10 +429,14 @@ public function getErrorOutput()
      * whole error output, this one returns the new error output since the last
      * call.
      *
+     * @throws LogicException In case the process is not started
+     *
      * @return string The process error output since the last call
      */
     public function getIncrementalErrorOutput()
     {
+        $this->requireProcessIsStarted(__FUNCTION__);
+
         $data = $this->getErrorOutput();
 
         $latest = substr($data, $this->incrementalErrorOutputOffset);
@@ -446,7 +448,7 @@ public function getIncrementalErrorOutput()
     /**
      * Returns the exit code returned by the process.
      *
-     * @return integer The exit status code
+     * @return null|integer The exit status code, null if the Process is not terminated
      *
      * @throws RuntimeException In case --enable-sigchild is activated and the sigchild compatibility mode is disabled
      *
@@ -469,14 +471,18 @@ public function getExitCode()
      * This method relies on the Unix exit code status standardization
      * and might not be relevant for other operating systems.
      *
-     * @return string A string representation for the exit status code
+     * @return null|string A string representation for the exit status code, null if the Process is not terminated.
+     *
+     * @throws RuntimeException In case --enable-sigchild is activated and the sigchild compatibility mode is disabled
      *
      * @see http://tldp.org/LDP/abs/html/exitcodes.html
      * @see http://en.wikipedia.org/wiki/Unix_signal
      */
     public function getExitCodeText()
     {
-        $exitcode = $this->getExitCode();
+        if (null === $exitcode = $this->getExitCode()) {
+            return;
+        }
 
         return isset(self::$exitCodes[$exitcode]) ? self::$exitCodes[$exitcode] : 'Unknown error';
     }
@@ -501,11 +507,14 @@ public function isSuccessful()
      * @return Boolean
      *
      * @throws RuntimeException In case --enable-sigchild is activated
+     * @throws LogicException   In case the process is not terminated.
      *
      * @api
      */
     public function hasBeenSignaled()
     {
+        $this->requireProcessIsTerminated(__FUNCTION__);
+
         if ($this->isSigchildEnabled()) {
             throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved.');
         }
@@ -523,11 +532,14 @@ public function hasBeenSignaled()
      * @return integer
      *
      * @throws RuntimeException In case --enable-sigchild is activated
+     * @throws LogicException   In case the process is not terminated.
      *
      * @api
      */
     public function getTermSignal()
     {
+        $this->requireProcessIsTerminated(__FUNCTION__);
+
         if ($this->isSigchildEnabled()) {
             throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved.');
         }
@@ -544,10 +556,14 @@ public function getTermSignal()
      *
      * @return Boolean
      *
+     * @throws LogicException   In case the process is not terminated.
+     *
      * @api
      */
     public function hasBeenStopped()
     {
+        $this->requireProcessIsTerminated(__FUNCTION__);
+
         $this->updateStatus(false);
 
         return $this->processInformation['stopped'];
@@ -560,10 +576,14 @@ public function hasBeenStopped()
      *
      * @return integer
      *
+     * @throws LogicException   In case the process is not terminated.
+     *
      * @api
      */
     public function getStopSignal()
     {
+        $this->requireProcessIsTerminated(__FUNCTION__);
+
         $this->updateStatus(false);
 
         return $this->processInformation['stopsig'];
@@ -945,6 +965,10 @@ public function setEnhanceSigchildCompatibility($enhance)
      */
     public function checkTimeout()
     {
+        if ($this->status !== self::STATUS_STARTED) {
+            return;
+        }
+
         if (null !== $this->timeout && $this->timeout < microtime(true) - $this->starttime) {
             $this->stop(0);
 
@@ -1162,4 +1186,32 @@ private function doSignal($signal, $throwException)
 
         return true;
     }
+
+    /**
+     * Ensures the process is running or terminated, throws a LogicException if the process has a not started.
+     *
+     * @param $functionName The function name that was called.
+     *
+     * @throws LogicException If the process has not run.
+     */
+    private function requireProcessIsStarted($functionName)
+    {
+        if (!$this->isStarted()) {
+            throw new LogicException(sprintf('Process must be started before calling %s.', $functionName));
+        }
+    }
+
+    /**
+     * Ensures the process is terminated, throws a LogicException if the process has a status different than `terminated`.
+     *
+     * @param $functionName The function name that was called.
+     *
+     * @throws LogicException If the process is not yet terminated.
+     */
+    private function requireProcessIsTerminated($functionName)
+    {
+        if (!$this->isTerminated()) {
+            throw new LogicException(sprintf('Process must be terminated before calling %s.', $functionName));
+        }
+    }
 }

src/Symfony/Component/Process/Tests/AbstractProcessTest.php

@@ -272,6 +272,12 @@ public function testTTYCommandExitCode()
         $this->assertTrue($process->isSuccessful());
     }
 
+    public function testExitCodeTextIsNullWhenExitCodeIsNull()
+    {
+        $process = $this->getProcess('');
+        $this->assertNull($process->getExitCodeText());
+    }
+
     public function testExitCodeText()
     {
         $process = $this->getProcess('');
@@ -512,6 +518,19 @@ public function testRunProcessWithTimeout()
         $this->assertLessThan($timeout + Process::TIMEOUT_PRECISION, $duration);
     }
 
+    public function testCheckTimeoutOnNonStartedProcess()
+    {
+        $process = $this->getProcess('php -r "sleep(3);"');
+        $process->checkTimeout();
+    }
+
+    public function testCheckTimeoutOnTerminatedProcess()
+    {
+        $process = $this->getProcess('php -v');
+        $process->run();
+        $process->checkTimeout();
+    }
+
     public function testCheckTimeoutOnStartedProcess()
     {
         $timeout = 0.5;
@@ -618,33 +637,52 @@ public function testSignalProcessNotRunning()
     }
 
     /**
-     * @expectedException \Symfony\Component\Process\Exception\LogicException
-     * @expectedExceptionMessage Process must be started before calling getOutput
+     * @dataProvider provideMethodsThatNeedARunningProcess
      */
-    public function testGetOutputProcessNotStarted()
+    public function testMethodsThatNeedARunningProcess($method)
     {
         $process = $this->getProcess('php -m');
-        $process->getOutput();
+        $this->setExpectedException('Symfony\Component\Process\Exception\LogicException', sprintf('Process must be started before calling %s.', $method));
+        call_user_func(array($process, $method));
     }
 
-    /**
-     * @expectedException \Symfony\Component\Process\Exception\LogicException
-     * @expectedExceptionMessage Process must be started before calling getErrorOutput
-     */
-    public function testGetErrorOutputProcessNotStarted()
+    public function provideMethodsThatNeedARunningProcess()
     {
-        $process = $this->getProcess('php -m');
-        $process->getErrorOutput();
+        return array(
+            array('getOutput'),
+            array('getIncrementalOutput'),
+            array('getErrorOutput'),
+            array('getIncrementalErrorOutput'),
+            array('wait'),
+        );
     }
 
     /**
-     * @expectedException \Symfony\Component\Process\Exception\LogicException
-     * @expectedExceptionMessage Process must be started before calling wait
+     * @dataProvider provideMethodsThatNeedATerminatedProcess
      */
-    public function testWaitProcessWithoutTimeoutNotStarted()
+    public function testMethodsThatNeedATerminatedProcess($method)
     {
-        $process = $this->getProcess('php -m')->setTimeout(null);
-        $process->wait();
+        $process = $this->getProcess('php -r "sleep(1);"');
+        $process->start();
+        try {
+            call_user_func(array($process, $method));
+            $process->stop(0);
+            $this->fail('A LogicException must have been thrown');
+        } catch (\Exception $e) {
+            $this->assertInstanceOf('Symfony\Component\Process\Exception\LogicException', $e);
+            $this->assertEquals(sprintf('Process must be terminated before calling %s.', $method), $e->getMessage());
+        }
+        $process->stop(0);
+    }
+
+    public function provideMethodsThatNeedATerminatedProcess()
+    {
+        return array(
+            array('hasBeenSignaled'),
+            array('getTermSignal'),
+            array('hasBeenStopped'),
+            array('getStopSignal'),
+        );
     }
 
     private function verifyPosixIsEnabled()