From 5801c6083ea4fbb4e24fe61378fc993a6a7d3935 Mon Sep 17 00:00:00 2001 From: "steve.brazier" Date: Thu, 12 Dec 2013 14:15:44 +0000 Subject: [PATCH 1/6] move command execution code out of builder class --- PHPCI/Builder.php | 32 +++++-------- PHPCI/Helper/CommandExecutor.php | 77 ++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 22 deletions(-) create mode 100644 PHPCI/Helper/CommandExecutor.php diff --git a/PHPCI/Builder.php b/PHPCI/Builder.php index 08fd3bed..c53fd670 100644 --- a/PHPCI/Builder.php +++ b/PHPCI/Builder.php @@ -9,6 +9,7 @@ namespace PHPCI; +use PHPCI\Helper\CommandExecutor; use PHPCI\Helper\MailerFactory; use PHPCI\Model\Build; use b8\Store; @@ -96,6 +97,11 @@ class Builder implements LoggerAwareInterface, BuildLogger */ protected $pluginExecutor; + /** + * @var Helper\CommandExecutor + */ + protected $commandExecutor; + /** * Set up the builder. * @param \PHPCI\Model\Build $build @@ -109,6 +115,8 @@ class Builder implements LoggerAwareInterface, BuildLogger $this->build = $build; $this->store = Store\Factory::getStore('Build'); $this->pluginExecutor = new Plugin\Util\Executor($this->buildPluginFactory($build), $this); + + $this->commandExecutor = new CommandExecutor($this, $this->quiet, $this->verbose); } /** @@ -209,27 +217,7 @@ class Builder implements LoggerAwareInterface, BuildLogger */ public function executeCommand() { - $command = call_user_func_array('sprintf', func_get_args()); - - if (!$this->quiet) { - $this->log('Executing: ' . $command); - } - - $status = 0; - exec($command, $this->lastOutput, $status); - - if (!empty($this->lastOutput) && ($this->verbose || $status != 0)) { - $this->log($this->lastOutput); - } - - - $rtn = false; - - if ($status == 0) { - $rtn = true; - } - - return $rtn; + return $this->commandExecutor->executeCommand(func_get_args()); } /** @@ -237,7 +225,7 @@ class Builder implements LoggerAwareInterface, BuildLogger */ public function getLastOutput() { - return implode(PHP_EOL, $this->lastOutput); + return $this->commandExecutor->getLastOutput(); } /** diff --git a/PHPCI/Helper/CommandExecutor.php b/PHPCI/Helper/CommandExecutor.php new file mode 100644 index 00000000..0515f83c --- /dev/null +++ b/PHPCI/Helper/CommandExecutor.php @@ -0,0 +1,77 @@ +logger = $logger; + $this->quiet = $quiet; + $this->verbose = $verbose; + + $this->lastOutput = array(); + } + + /** + * Executes shell commands. + * @param array $args + * @return bool Indicates success + */ + public function executeCommand($args = array()) + { + $command = call_user_func_array('sprintf', $args); + + if ($this->quiet) { + $this->logger->log('Executing: ' . $command); + } + + $status = 0; + exec($command, $this->lastOutput, $status); + + if (!empty($this->lastOutput) && ($this->verbose|| $status != 0)) { + $this->logger->log($this->lastOutput); + } + + $rtn = false; + + if ($status == 0) { + $rtn = true; + } + + return $rtn; + } + + /** + * Returns the output from the last command run. + */ + public function getLastOutput() + { + return implode(PHP_EOL, $this->lastOutput); + } +} From d26568b962bd0d9903550ba9e23926e1ad73e948 Mon Sep 17 00:00:00 2001 From: "steve.brazier" Date: Thu, 12 Dec 2013 14:16:14 +0000 Subject: [PATCH 2/6] fix issue #218 by blanking last output before executing new command. --- PHPCI/Helper/CommandExecutor.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PHPCI/Helper/CommandExecutor.php b/PHPCI/Helper/CommandExecutor.php index 0515f83c..b7fc3494 100644 --- a/PHPCI/Helper/CommandExecutor.php +++ b/PHPCI/Helper/CommandExecutor.php @@ -45,6 +45,8 @@ class CommandExecutor */ public function executeCommand($args = array()) { + $this->lastOutput = array(); + $command = call_user_func_array('sprintf', $args); if ($this->quiet) { From 7a5aa5814f5cf3e415a9d3828b3c615be000b6f6 Mon Sep 17 00:00:00 2001 From: "steve.brazier" Date: Thu, 12 Dec 2013 14:37:47 +0000 Subject: [PATCH 3/6] add tests for CommandExecutor. --- Tests/PHPCI/Helper/CommandExecutorTest.php | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Tests/PHPCI/Helper/CommandExecutorTest.php diff --git a/Tests/PHPCI/Helper/CommandExecutorTest.php b/Tests/PHPCI/Helper/CommandExecutorTest.php new file mode 100644 index 00000000..217844b1 --- /dev/null +++ b/Tests/PHPCI/Helper/CommandExecutorTest.php @@ -0,0 +1,36 @@ +prophesize('\PHPCI\BuildLogger'); + $this->testedExecutor = new CommandExecutor($mockBuildLogger->reveal()); + } + + public function testGetLastOutput_ReturnsOutputOfCommand() + { + $this->testedExecutor->executeCommand(array('echo "%s"', 'Hello World')); + $output = $this->testedExecutor->getLastOutput(); + $this->assertEquals("Hello World", $output); + } + + public function testGetLastOutput_ForgetsPreviousCommandOutput() + { + $this->testedExecutor->executeCommand(array('echo "%s"', 'Hello World')); + $this->testedExecutor->executeCommand(array('echo "%s"', 'Hello Tester')); + $output = $this->testedExecutor->getLastOutput(); + $this->assertEquals("Hello Tester", $output); + } +} \ No newline at end of file From 13072406220e808a9e497b5642cf4f1684ad79f8 Mon Sep 17 00:00:00 2001 From: "steve.brazier" Date: Thu, 12 Dec 2013 16:04:07 +0000 Subject: [PATCH 4/6] add return value tests for CommandExecutor. --- Tests/PHPCI/Helper/CommandExecutorTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Tests/PHPCI/Helper/CommandExecutorTest.php b/Tests/PHPCI/Helper/CommandExecutorTest.php index 217844b1..cfdcc6cf 100644 --- a/Tests/PHPCI/Helper/CommandExecutorTest.php +++ b/Tests/PHPCI/Helper/CommandExecutorTest.php @@ -33,4 +33,16 @@ class CommandExecutorTest extends ProphecyTestCase $output = $this->testedExecutor->getLastOutput(); $this->assertEquals("Hello Tester", $output); } + + public function testExecuteCommand_ReturnsTrueForValidCommands() + { + $returnValue = $this->testedExecutor->executeCommand(array('echo "%s"', 'Hello World')); + $this->assertTrue($returnValue); + } + + public function testExecuteCommand_ReturnsFalseForInvalidCommands() + { + $returnValue = $this->testedExecutor->executeCommand(array('eerfdcvcho "%s"', 'Hello World')); + $this->assertFalse($returnValue); + } } \ No newline at end of file From 8d8714746c3cde960fe999fcb8786e3b9bb12755 Mon Sep 17 00:00:00 2001 From: "steve.brazier" Date: Thu, 12 Dec 2013 16:17:11 +0000 Subject: [PATCH 5/6] move findBinary from Builder to CommandExecutor. --- PHPCI/Builder.php | 45 ++++++---------------- PHPCI/Helper/CommandExecutor.php | 44 ++++++++++++++++++++- Tests/PHPCI/Helper/CommandExecutorTest.php | 9 ++++- 3 files changed, 62 insertions(+), 36 deletions(-) diff --git a/PHPCI/Builder.php b/PHPCI/Builder.php index c53fd670..c1203b13 100644 --- a/PHPCI/Builder.php +++ b/PHPCI/Builder.php @@ -116,7 +116,7 @@ class Builder implements LoggerAwareInterface, BuildLogger $this->store = Store\Factory::getStore('Build'); $this->pluginExecutor = new Plugin\Util\Executor($this->buildPluginFactory($build), $this); - $this->commandExecutor = new CommandExecutor($this, $this->quiet, $this->verbose); + $this->commandExecutor = new CommandExecutor($this, PHPCI_DIR, $this->quiet, $this->verbose); } /** @@ -228,6 +228,16 @@ class Builder implements LoggerAwareInterface, BuildLogger return $this->commandExecutor->getLastOutput(); } + /** + * Find a binary required by a plugin. + * @param $binary + * @return null|string + */ + public function findBinary($binary) + { + return $this->commandExecutor->findBinary($binary); + } + /** * Add an entry to the build log. * @param string|string[] $message @@ -363,39 +373,6 @@ class Builder implements LoggerAwareInterface, BuildLogger return true; } - /** - * Find a binary required by a plugin. - * @param $binary - * @return null|string - */ - public function findBinary($binary) - { - if (is_string($binary)) { - $binary = array($binary); - } - - foreach ($binary as $bin) { - // Check project root directory: - if (is_file(PHPCI_DIR . $bin)) { - return PHPCI_DIR . $bin; - } - - // Check Composer bin dir: - if (is_file(PHPCI_DIR . 'vendor/bin/' . $bin)) { - return PHPCI_DIR . 'vendor/bin/' . $bin; - } - - // Use "which" - $which = trim(shell_exec('which ' . $bin)); - - if (!empty($which)) { - return $which; - } - } - - return null; - } - /** * Sets a logger instance on the object * diff --git a/PHPCI/Helper/CommandExecutor.php b/PHPCI/Helper/CommandExecutor.php index b7fc3494..bce980e7 100644 --- a/PHPCI/Helper/CommandExecutor.php +++ b/PHPCI/Helper/CommandExecutor.php @@ -24,18 +24,27 @@ class CommandExecutor protected $lastOutput; + /** + * The path which findBinary will look in. + * @var string + */ + protected $rootDir; + /** * @param BuildLogger $logger + * @param $rootDir * @param bool $quiet * @param bool $verbose */ - public function __construct(BuildLogger $logger, &$quiet = false, &$verbose = false) + public function __construct(BuildLogger $logger, $rootDir, &$quiet = false, &$verbose = false) { $this->logger = $logger; $this->quiet = $quiet; $this->verbose = $verbose; $this->lastOutput = array(); + + $this->rootDir = $rootDir; } /** @@ -76,4 +85,37 @@ class CommandExecutor { return implode(PHP_EOL, $this->lastOutput); } + + /** + * Find a binary required by a plugin. + * @param $binary + * @return null|string + */ + public function findBinary($binary) + { + if (is_string($binary)) { + $binary = array($binary); + } + + foreach ($binary as $bin) { + // Check project root directory: + if (is_file($this->rootDir . $bin)) { + return $this->rootDir . $bin; + } + + // Check Composer bin dir: + if (is_file($this->rootDir . 'vendor/bin/' . $bin)) { + return $this->rootDir . 'vendor/bin/' . $bin; + } + + // Use "which" + $which = trim(shell_exec('which ' . $bin)); + + if (!empty($which)) { + return $which; + } + } + + return null; + } } diff --git a/Tests/PHPCI/Helper/CommandExecutorTest.php b/Tests/PHPCI/Helper/CommandExecutorTest.php index cfdcc6cf..c50fc0be 100644 --- a/Tests/PHPCI/Helper/CommandExecutorTest.php +++ b/Tests/PHPCI/Helper/CommandExecutorTest.php @@ -16,7 +16,7 @@ class CommandExecutorTest extends ProphecyTestCase { parent::setUp(); $mockBuildLogger = $this->prophesize('\PHPCI\BuildLogger'); - $this->testedExecutor = new CommandExecutor($mockBuildLogger->reveal()); + $this->testedExecutor = new CommandExecutor($mockBuildLogger->reveal(), __DIR__ . "/"); } public function testGetLastOutput_ReturnsOutputOfCommand() @@ -45,4 +45,11 @@ class CommandExecutorTest extends ProphecyTestCase $returnValue = $this->testedExecutor->executeCommand(array('eerfdcvcho "%s"', 'Hello World')); $this->assertFalse($returnValue); } + + public function testFindBinary_ReturnsPathInSpecifiedRoot() + { + $thisFileName = "CommandExecutorTest.php"; + $returnValue = $this->testedExecutor->findBinary($thisFileName); + $this->assertEquals(__DIR__ . "/" . $thisFileName, $returnValue); + } } \ No newline at end of file From 3a3cc98f86765fcfb93df7574aff02a2a208d939 Mon Sep 17 00:00:00 2001 From: meadsteve Date: Fri, 13 Dec 2013 14:41:02 +0000 Subject: [PATCH 6/6] rename CommandExecutor::executeCommand() so that it's not confused with the Builder::executeCommand() --- PHPCI/Builder.php | 2 +- PHPCI/Helper/CommandExecutor.php | 12 +++++++++++- Tests/PHPCI/Helper/CommandExecutorTest.php | 10 +++++----- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/PHPCI/Builder.php b/PHPCI/Builder.php index c1203b13..7b36d55b 100644 --- a/PHPCI/Builder.php +++ b/PHPCI/Builder.php @@ -217,7 +217,7 @@ class Builder implements LoggerAwareInterface, BuildLogger */ public function executeCommand() { - return $this->commandExecutor->executeCommand(func_get_args()); + return $this->commandExecutor->buildAndExecuteCommand(func_get_args()); } /** diff --git a/PHPCI/Helper/CommandExecutor.php b/PHPCI/Helper/CommandExecutor.php index bce980e7..43813924 100644 --- a/PHPCI/Helper/CommandExecutor.php +++ b/PHPCI/Helper/CommandExecutor.php @@ -47,12 +47,22 @@ class CommandExecutor $this->rootDir = $rootDir; } + /** + * Executes shell commands. Accepts multiple arguments the first + * is the template and everything else is inserted in. c.f. sprintf + * @return bool Indicates success + */ + public function executeCommand() + { + return $this->buildAndExecuteCommand(func_get_args()); + } + /** * Executes shell commands. * @param array $args * @return bool Indicates success */ - public function executeCommand($args = array()) + public function buildAndExecuteCommand($args = array()) { $this->lastOutput = array(); diff --git a/Tests/PHPCI/Helper/CommandExecutorTest.php b/Tests/PHPCI/Helper/CommandExecutorTest.php index c50fc0be..008443e9 100644 --- a/Tests/PHPCI/Helper/CommandExecutorTest.php +++ b/Tests/PHPCI/Helper/CommandExecutorTest.php @@ -21,28 +21,28 @@ class CommandExecutorTest extends ProphecyTestCase public function testGetLastOutput_ReturnsOutputOfCommand() { - $this->testedExecutor->executeCommand(array('echo "%s"', 'Hello World')); + $this->testedExecutor->buildAndExecuteCommand(array('echo "%s"', 'Hello World')); $output = $this->testedExecutor->getLastOutput(); $this->assertEquals("Hello World", $output); } public function testGetLastOutput_ForgetsPreviousCommandOutput() { - $this->testedExecutor->executeCommand(array('echo "%s"', 'Hello World')); - $this->testedExecutor->executeCommand(array('echo "%s"', 'Hello Tester')); + $this->testedExecutor->buildAndExecuteCommand(array('echo "%s"', 'Hello World')); + $this->testedExecutor->buildAndExecuteCommand(array('echo "%s"', 'Hello Tester')); $output = $this->testedExecutor->getLastOutput(); $this->assertEquals("Hello Tester", $output); } public function testExecuteCommand_ReturnsTrueForValidCommands() { - $returnValue = $this->testedExecutor->executeCommand(array('echo "%s"', 'Hello World')); + $returnValue = $this->testedExecutor->buildAndExecuteCommand(array('echo "%s"', 'Hello World')); $this->assertTrue($returnValue); } public function testExecuteCommand_ReturnsFalseForInvalidCommands() { - $returnValue = $this->testedExecutor->executeCommand(array('eerfdcvcho "%s"', 'Hello World')); + $returnValue = $this->testedExecutor->buildAndExecuteCommand(array('eerfdcvcho "%s"', 'Hello World')); $this->assertFalse($returnValue); }