Permalink
Browse files

merged branch proofek/fix-process-exception (PR #4111)

Commits
-------

1c03a16 [Process] Fixed ProcessFailedException not populating exception message due to a missing sprintf parameter

Discussion
----------

[Process] Fixed ProcessFailedException not populating exception message ...

...due to a missing sprintf parameter

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: http://travis-ci.org/#!/proofek/symfony/builds/1172817
Fixes the following tickets: -
Todo: -

Found the issue when started using Cilex with Process and raised an exception using ProcessFailedException. Not sure whether this will get into the main release, as I couldn't find that on 2.0 branch, so I am guessing it's quite a recent addition to Process component.
  • Loading branch information...
2 parents 52e1fa9 + 1c03a16 commit 4e1e48809cbe0d23dde3ba8df7872a6f93fd3ef8 @fabpot fabpot committed Apr 25, 2012
@@ -28,7 +28,12 @@ public function __construct(Process $process)
throw new \InvalidArgumentException('Expected a failed process, but the given process was successful.');
}
- parent::__construct(sprintf('The command "%s" failed.'."\n\nOutput:\n================\n".$process->getOutput()."\n\nError Output:\n================\n".$process->getErrorOutput()));
+ parent::__construct(
+ sprintf(
+ 'The command "%s" failed.'."\n\nOutput:\n================\n".$process->getOutput()."\n\nError Output:\n================\n".$process->getErrorOutput(),
+ $process->getCommandLine()
+ )
+ );
$this->process = $process;
}
@@ -0,0 +1,75 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Process\Tests;
+
+use Symfony\Component\Process\Process,
+ Symfony\Component\Process\Exception\ProcessFailedException;
+
+/**
+ * @author Sebastian Marek <proofek@gmail.com>
+ */
+class ProcessFailedExceptionTest extends \PHPUnit_Framework_TestCase
+{
+ /**
+ * tests ProcessFailedException throws exception if the process was successful
+ */
+ public function testProcessFailedExceptionThrowsException()
+ {
+ $process = $this->getMock(
+ 'Symfony\Component\Process\Process',
+ array('isSuccessful'),
+ array('php')
+ );
+ $process->expects($this->once())
+ ->method('isSuccessful')
+ ->will($this->returnValue(true));
+
+ $this->setExpectedException(
+ '\InvalidArgumentException',
+ 'Expected a failed process, but the given process was successful.'
+ );
+ $exception = new ProcessFailedException($process);
+ }
+
+ /**
+ * tests ProcessFailedException uses information from process output
+ * to generate exception message
+ */
+ public function testProcessFailedExceptionPopulatesInformationFromProcessOutput()
+ {
+ $cmd = 'php';
+ $output = "Command output";
+ $errorOutput = "FATAL: Unexpected error";
+
+ $process = $this->getMock(
+ 'Symfony\Component\Process\Process',
+ array('isSuccessful', 'getOutput', 'getErrorOutput'),
+ array($cmd)
+ );
+ $process->expects($this->once())
+ ->method('isSuccessful')
+ ->will($this->returnValue(false));
+ $process->expects($this->once())
+ ->method('getOutput')
+ ->will($this->returnValue($output));
+ $process->expects($this->once())
+ ->method('getErrorOutput')
+ ->will($this->returnValue($errorOutput));
+
+ $exception = new ProcessFailedException($process);
+
+ $this->assertEquals(
+ "The command \"$cmd\" failed.\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}",
+ $exception->getMessage()
+ );
+ }
+}

0 comments on commit 4e1e488

Please sign in to comment.