From 551a819e7613bed45404641e52c90f0368e9b309 Mon Sep 17 00:00:00 2001
From: Greg Anderson <greg.1.anderson@greenknowe.org>
Date: Thu, 8 Sep 2016 12:51:45 -0700
Subject: [PATCH 1/3] Fixes #442: Allow CollectionBuilder, Collection and
 Simulated tasks to be used by tasks (e.g. Remote\Ssh) that require a
 CommandInterface.

---
 src/Collection/Collection.php        | 33 +++++++++++++++++++++++++++-
 src/Collection/CollectionBuilder.php | 17 +++++++++++++-
 src/Config.php                       |  9 +++++++-
 src/Runner.php                       |  9 +++++---
 src/Task/Remote/Ssh.php              |  9 +++++++-
 src/Task/Simulator.php               | 25 +++++++++++++++++++--
 src/Task/Testing/PHPUnit.php         |  6 +++--
 tests/src/RoboFileFixture.php        | 11 ++++++++++
 tests/unit/RunnerTest.php            | 17 ++++++++++++++
 9 files changed, 125 insertions(+), 11 deletions(-)

diff --git a/src/Collection/Collection.php b/src/Collection/Collection.php
index cd65c33e9..19bef6c4a 100644
--- a/src/Collection/Collection.php
+++ b/src/Collection/Collection.php
@@ -8,7 +8,10 @@
 use Robo\Task\BaseTask;
 use Robo\TaskInfo;
 use Robo\Contract\WrappedTaskInterface;
+use Robo\Exception\TaskException;
 use Robo\Exception\TaskExitException;
+use Robo\Contract\CommandInterface;
+
 
 use Robo\Contract\ProgressIndicatorAwareInterface;
 use Robo\Common\ProgressIndicatorAwareTrait;
@@ -28,7 +31,7 @@
  * called. Here, taskDeleteDir is used to remove partial results
  * of an unfinished task.
  */
-class Collection extends BaseTask implements CollectionInterface
+class Collection extends BaseTask implements CollectionInterface, CommandInterface
 {
     protected $taskList = [];
     protected $rollbackStack = [];
@@ -411,6 +414,34 @@ public function progressIndicatorSteps()
         return $steps;
     }
 
+    /**
+     * A Collection of tasks can provide a command via `getCommand()`
+     * if it contains a single task, and that task implements CommandInterface.
+     * @return string
+     */
+    public function getCommand()
+    {
+        if (empty($this->taskList)) {
+            return '';
+        }
+
+        if (count($this->taskList) > 1) {
+            // TODO: We could potentially iterate over the items in the collection
+            // and concatenate the result of getCommand() from each one, and fail
+            // only if we encounter a command that is not a CommandInterface.
+            throw new TaskException($this, "getCommand() does not work on arbitrary collections of tasks.");
+        }
+
+        $taskElement = reset($this->taskList);
+        $task = $taskElement->getTask();
+        $task = ($task instanceof WrappedTaskInterface) ? $task->original() : $task;
+        if ($task instanceof CommandInterface) {
+            return $task->getCommand();
+        }
+
+        throw new TaskException($task, get_class($task) . " does not implement CommandInterface, so can't be used to provide a command");
+    }
+
     /**
      * Run our tasks, and roll back if necessary.
      */
diff --git a/src/Collection/CollectionBuilder.php b/src/Collection/CollectionBuilder.php
index 4f44dd34e..66772e77a 100644
--- a/src/Collection/CollectionBuilder.php
+++ b/src/Collection/CollectionBuilder.php
@@ -17,6 +17,8 @@
 use ReflectionClass;
 use Robo\Task\BaseTask;
 use Robo\Contract\BuilderAwareInterface;
+use Robo\Contract\CommandInterface;
+use Robo\Exception\TaskException;
 
 /**
  * Creates a collection, and adds tasks to it.  The collection builder
@@ -45,7 +47,7 @@
  * In the example above, the `taskDeleteDir` will be called if
  * ```
  */
-class CollectionBuilder extends BaseTask implements NestedCollectionInterface, WrappedTaskInterface
+class CollectionBuilder extends BaseTask implements NestedCollectionInterface, WrappedTaskInterface, CommandInterface
 {
     protected $commandFile;
     protected $collection;
@@ -354,6 +356,19 @@ protected function runTasks()
         return $this->getCollection()->run();
     }
 
+    public function getCommand()
+    {
+        if (!$this->collection && $this->currentTask) {
+            $task = $this->currentTask;
+            $task = ($task instanceof WrappedTaskInterface) ? $task->original() : $task;
+            if ($task instanceof CommandInterface) {
+                return $task->getCommand();
+            }
+        }
+
+        return $this->getCollection()->getCommand();
+    }
+
     public function original()
     {
         return $this->getCollection();
diff --git a/src/Config.php b/src/Config.php
index d86226087..50a553812 100644
--- a/src/Config.php
+++ b/src/Config.php
@@ -21,7 +21,7 @@ public function set($key, $value)
         return $this;
     }
 
-    public function setGlobalOptions($input)
+    public function getGlobalOptionDefaultValues()
     {
         $globalOptions =
         [
@@ -30,6 +30,13 @@ public function setGlobalOptions($input)
             self::SUPRESS_MESSAGES => false,
         ];
 
+        return $globalOptions;
+    }
+
+    public function setGlobalOptions($input)
+    {
+        $globalOptions = $this->getGlobalOptionDefaultValues();
+
         foreach ($globalOptions as $option => $default) {
             $value = $input->hasOption($option) ? $input->getOption($option) : null;
             // Unfortunately, the `?:` operator does not differentate between `0` and `null`
diff --git a/src/Runner.php b/src/Runner.php
index 635ab6bad..1ba06088b 100644
--- a/src/Runner.php
+++ b/src/Runner.php
@@ -89,8 +89,11 @@ public function execute($argv, $output = null, $appName = null, $appVersion = nu
     {
         $argv = $this->shebang($argv);
         $argv = $this->processRoboOptions($argv);
-        $app = Robo::createDefaultApplication($appName, $appVersion);
-        $commandFiles = $this->getRoboFileCommands($app, $output);
+        $app = null;
+        if ($appName && $appVersion) {
+            $app = Robo::createDefaultApplication($appName, $appVersion);
+        }
+        $commandFiles = $this->getRoboFileCommands($output);
         return $this->run($argv, $output, $app, $commandFiles);
     }
 
@@ -140,7 +143,7 @@ public function run($input = null, $output = null, $app = null, $commandFiles =
         return $statusCode;
     }
 
-    protected function getRoboFileCommands($app, $output)
+    protected function getRoboFileCommands($output)
     {
         if (!$this->loadRoboFile($output)) {
             return;
diff --git a/src/Task/Remote/Ssh.php b/src/Task/Remote/Ssh.php
index 820467461..4b55f6056 100644
--- a/src/Task/Remote/Ssh.php
+++ b/src/Task/Remote/Ssh.php
@@ -6,6 +6,7 @@
 use Robo\Contract\CommandInterface;
 use Robo\Exception\TaskException;
 use Robo\Task\BaseTask;
+use Robo\Contract\SimulatedInterface;
 
 /**
  * Runs multiple commands on a remote server.
@@ -45,7 +46,7 @@
  *                                            and stop the chain if one command fails
  * @method $this remoteDir(string $remoteWorkingDirectory) Changes to the given directory before running commands
  */
-class Ssh extends BaseTask implements CommandInterface
+class Ssh extends BaseTask implements CommandInterface, SimulatedInterface
 {
     use \Robo\Common\CommandReceiver;
     use \Robo\Common\ExecOneCommand;
@@ -177,6 +178,12 @@ public function run()
         return $this->executeCommand($command);
     }
 
+    public function simulate($context)
+    {
+        $command = $this->getCommand();
+        $this->printTaskInfo("Running {command}", ['command' => $command] + $context);
+    }
+
     protected function validateParameters()
     {
         if (empty($this->hostname)) {
diff --git a/src/Task/Simulator.php b/src/Task/Simulator.php
index e0ffcd18c..e0a7f3b5d 100644
--- a/src/Task/Simulator.php
+++ b/src/Task/Simulator.php
@@ -7,8 +7,9 @@
 use Robo\Contract\SimulatedInterface;
 use Robo\Log\RoboLogLevel;
 use Psr\Log\LogLevel;
+use Robo\Contract\CommandInterface;
 
-class Simulator extends BaseTask
+class Simulator extends BaseTask implements CommandInterface
 {
     protected $task;
     protected $constructorParameters;
@@ -63,6 +64,23 @@ public function run()
         return $result;
     }
 
+    /**
+     * Danger: reach through the simulated wrapper and pull out the command
+     * to be executed.  This is used when using a simulated task with another
+     * simulated task that runs commands, e.g. the Remote\Ssh task.  Using
+     * a simulated CommandInterface task with a non-simulated task may produce
+     * unexpected results (e.g. execution!).
+     *
+     * @return string
+     */
+    public function getCommand()
+    {
+        if (!$this->task instanceof CommandInterface) {
+            throw new TaskException($this->task, 'Simulated task that is not a CommandInterface used as a CommandInterface.');
+        }
+        return $this->task->getCommand();
+    }
+
     protected function formatParameters($action)
     {
         $parameterList = array_map([$this, 'convertParameter'], $action);
@@ -78,11 +96,14 @@ protected function convertParameter($item)
             return $this->shortenParameter(var_export($item, true));
         }
         if (is_object($item)) {
-            return $this->shortenParameter(var_export($item, true), '[' . get_class($item). 'object]');
+            return '[' . get_class($item). ' object]';
         }
         if (is_string($item)) {
             return $this->shortenParameter("'$item'");
         }
+        if (is_null($item)) {
+            return 'null';
+        }
         return $item;
     }
 
diff --git a/src/Task/Testing/PHPUnit.php b/src/Task/Testing/PHPUnit.php
index 1bfb280b4..c0c101d4d 100644
--- a/src/Task/Testing/PHPUnit.php
+++ b/src/Task/Testing/PHPUnit.php
@@ -131,7 +131,8 @@ public function files($files)
      * @param string $file path to file to test
      * @return $this
      */
-    public function file($file) {
+    public function file($file)
+    {
         return $this->files($file);
     }
 
@@ -140,7 +141,8 @@ public function file($file) {
      * @param string $dir path to directory to test
      * @return $this
      */
-    public function dir($dir) {
+    public function dir($dir)
+    {
         return $this->dir($dir);
     }
 
diff --git a/tests/src/RoboFileFixture.php b/tests/src/RoboFileFixture.php
index df8b7ebdf..94b1ea36c 100644
--- a/tests/src/RoboFileFixture.php
+++ b/tests/src/RoboFileFixture.php
@@ -88,4 +88,15 @@ public function testVerbosity()
         $this->logger->notice('This is a notice log message.');
         $this->logger->debug('This is a debug log message.');
     }
+
+    public function testDeploy()
+    {
+        $gitTask = $this->taskGitStack()
+            ->pull();
+
+        $this->taskSshExec('mysite.com')
+            ->remoteDir('/var/www/somesite')
+            ->exec($gitTask)
+            ->run();
+    }
 }
diff --git a/tests/unit/RunnerTest.php b/tests/unit/RunnerTest.php
index 3e32e59c1..0a542fa7c 100644
--- a/tests/unit/RunnerTest.php
+++ b/tests/unit/RunnerTest.php
@@ -113,6 +113,14 @@ public function testSymfonyStyle()
         $this->guy->seeInOutput('Some text in section one.');
     }
 
+    public function testDeploy()
+    {
+        $argv = ['placeholder', 'test:deploy', '--simulate'];
+        $this->runner->execute($argv, $this->guy->capturedOutputStream());
+        $this->guy->seeInOutput('[Simulator] Simulating Remote\\Ssh(\'mysite.com\', null)');
+        $this->guy->seeInOutput('[Simulator] Running ssh mysite.com \'cd "/var/www/somesite" && git pull\'');
+    }
+
     public function testRunnerTryError()
     {
         $argv = ['placeholder', 'test:error'];
@@ -122,6 +130,15 @@ public function testRunnerTryError()
         $this->assertTrue($result > 0);
     }
 
+    public function testRunnerTrySimulatedError()
+    {
+        $argv = ['placeholder', 'test:error', '--simulate'];
+        $result = $this->runner->execute($argv, $this->guy->capturedOutputStream());
+
+        $this->guy->seeInOutput('Simulating Exec');
+        $this->assertEquals(0, $result);
+    }
+
     public function testRunnerTryException()
     {
         $argv = ['placeholder', 'test:exception', '--task'];

From 7ec4e7cfd88b245bca763ba1b1d71aa2f9484626 Mon Sep 17 00:00:00 2001
From: Greg Anderson <greg.1.anderson@greenknowe.org>
Date: Fri, 9 Sep 2016 17:33:23 -0700
Subject: [PATCH 2/3] Simplify container initialization.

---
 src/Robo.php   |  9 +++++++--
 src/Runner.php | 12 ++++--------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/Robo.php b/src/Robo.php
index 2cd1c65d2..f475ca4f3 100644
--- a/src/Robo.php
+++ b/src/Robo.php
@@ -47,7 +47,7 @@ public static function unsetContainer()
     /**
      * Returns the currently active global container.
      *
-     * @return \League\Container\ContainerInterface|null
+     * @return \League\Container\ContainerInterface
      *
      * @throws \RuntimeException
      */
@@ -74,10 +74,14 @@ public static function hasContainer()
      */
     public static function createDefaultContainer($input = null, $output = null, $app)
     {
+        // Do not allow this function to be called more than once.
+        if (static::hasContainer()) {
+            return static::getContainer();
+        }
+
         // Set up our dependency injection container.
         $container = new Container();
         static::configureContainer($container, $input, $output, $app);
-        static::setContainer($container);
 
         return $container;
     }
@@ -89,6 +93,7 @@ public static function configureContainer($container, $input = null, $output = n
     {
         // Self-referential container refernce for the inflector
         $container->add('container', $container);
+        static::setContainer($container);
 
         // Create default input and output objects if they were not provided
         if (!$input) {
diff --git a/src/Runner.php b/src/Runner.php
index 1ba06088b..595fe4d53 100644
--- a/src/Runner.php
+++ b/src/Runner.php
@@ -112,19 +112,15 @@ public function run($input = null, $output = null, $app = null, $commandFiles =
         $this->setInput($input);
         $this->setOutput($output);
 
-        // If our client gave us a container, then also set it inside
-        // the static Robo class.
-        if ($this->getContainer()) {
-            Robo::setContainer($this->getContainer());
-        }
         // If we were not provided a container, then create one
-        if (!Robo::hasContainer()) {
-            Robo::createDefaultContainer($input, $output, $app);
-            $this->setContainer(Robo::getContainer());
+        if (!$this->getContainer()) {
+            $container = Robo::createDefaultContainer($input, $output, $app);
+            $this->setContainer($container);
             // Automatically register a shutdown function and
             // an error handler when we provide the container.
             $this->installRoboHandlers();
         }
+
         if (!$app) {
             $app = Robo::application();
         }

From 71d484972fa07e7b87e0a57ef9988d1be803399d Mon Sep 17 00:00:00 2001
From: Greg Anderson <greg.1.anderson@greenknowe.org>
Date: Fri, 9 Sep 2016 18:04:24 -0700
Subject: [PATCH 3/3] Refactor Collection a bit.

---
 src/Collection/Collection.php | 60 +++--------------------------------
 src/Collection/Element.php    | 21 ++++++++++++
 src/Result.php                | 33 +++++++++++++++++++
 3 files changed, 59 insertions(+), 55 deletions(-)

diff --git a/src/Collection/Collection.php b/src/Collection/Collection.php
index 19bef6c4a..db6514e78 100644
--- a/src/Collection/Collection.php
+++ b/src/Collection/Collection.php
@@ -13,7 +13,6 @@
 use Robo\Contract\CommandInterface;
 
 
-use Robo\Contract\ProgressIndicatorAwareInterface;
 use Robo\Common\ProgressIndicatorAwareTrait;
 use Robo\Contract\InflectionInterface;
 
@@ -244,16 +243,6 @@ public function hasTask($name)
         return array_key_exists($name, $this->taskList);
     }
 
-    /**
-     * Test to see if the given name is an unnamed task, or
-     * something functionally equivalent.  Any numeric index
-     * is renumbered when added to the collection.
-     */
-    public static function isUnnamedTask($name)
-    {
-        return is_numeric($name);
-    }
-
     /**
      * Find an existing named task.
      *
@@ -300,7 +289,7 @@ protected function addCollectionElementToTaskList($name, Element $taskGroup)
     {
         // If a task name is not provided, then we'll let php pick
         // the array index.
-        if (static::isUnnamedTask($name)) {
+        if (Result::isUnnamed($name)) {
             $this->taskList[] = $taskGroup;
             return $this;
         }
@@ -397,19 +386,7 @@ public function progressIndicatorSteps()
     {
         $steps = 0;
         foreach ($this->taskList as $name => $taskGroup) {
-            foreach ($taskGroup->getTaskList() as $task) {
-                if ($task instanceof WrappedTaskInterface) {
-                    $task = $task->original();
-                }
-                // If the task is a ProgressIndicatorAwareInterface, then it
-                // will advance the progress indicator a number of times.
-                if ($task instanceof ProgressIndicatorAwareInterface) {
-                    $steps += $task->progressIndicatorSteps();
-                }
-                // We also advance the progress indicator once regardless
-                // of whether it is progress-indicator aware or not.
-                $steps++;
-            }
+            $steps += $taskGroup->progressIndicatorSteps();
         }
         return $steps;
     }
@@ -482,7 +459,7 @@ private function runWithoutCompletion()
      * Run every task in a list, but only up to the first failure.
      * Return the failing result, or success if all tasks run.
      */
-    private function runTaskList($name, array $taskList, $result)
+    private function runTaskList($name, array $taskList, Result $result)
     {
         try {
             foreach ($taskList as $taskName => $task) {
@@ -496,8 +473,8 @@ private function runTaskList($name, array $taskList, $result)
                 // We accumulate our results into a field so that tasks that
                 // have a reference to the collection may examine and modify
                 // the incremental results, if they wish.
-                $key = static::isUnnamedTask($taskName) ? $name : $taskName;
-                $result = $this->accumulateResults($key, $result, $taskResult);
+                $key = Result::isUnnamed($taskName) ? $name : $taskName;
+                $result->accumulate($key, $taskResult);
             }
         } catch (TaskExitException $exitException) {
             $this->fail();
@@ -578,33 +555,6 @@ protected function setParentCollectionForTask($task, $parentCollection)
         }
     }
 
-    /**
-     * Add the results from the most recent task to the accumulated
-     * results from all tasks that have run so far, merging data
-     * as necessary.
-     */
-    public function accumulateResults($key, Result $result, Result $taskResult)
-    {
-        // If the result is not set or is not a Result, then ignore it
-        if (isset($result) && ($result instanceof Result)) {
-            // If the task is unnamed, then all of its data elements
-            // just get merged in at the top-level of the final Result object.
-            if (static::isUnnamedTask($key)) {
-                $result->merge($taskResult);
-            } elseif (isset($result[$key])) {
-                // There can only be one task with a given name; however, if
-                // there are tasks added 'before' or 'after' the named task,
-                // then the results from these will be stored under the same
-                // name unless they are given a name of their own when added.
-                $current = $result[$key];
-                $result[$key] = $taskResult->merge($current);
-            } else {
-                $result[$key] = $taskResult;
-            }
-        }
-        return $result;
-    }
-
     /**
      * Run all of the tasks in a provided list, ignoring failures.
      * This is used to roll back or complete.
diff --git a/src/Collection/Element.php b/src/Collection/Element.php
index 6a79caa29..f8295a9d6 100644
--- a/src/Collection/Element.php
+++ b/src/Collection/Element.php
@@ -3,6 +3,8 @@
 namespace Robo\Collection;
 
 use Robo\Contract\TaskInterface;
+use Robo\Contract\WrappedTaskInterface;
+use Robo\Contract\ProgressIndicatorAwareInterface;
 
 /**
  * One element in a collection.  Each element consists of a task
@@ -58,4 +60,23 @@ public function getTaskList()
     {
         return array_merge($this->getBefore(), [$this->getTask()], $this->getAfter());
     }
+
+    public function progressIndicatorSteps()
+    {
+        $steps = 0;
+        foreach ($this->getTaskList() as $task) {
+            if ($task instanceof WrappedTaskInterface) {
+                $task = $task->original();
+            }
+            // If the task is a ProgressIndicatorAwareInterface, then it
+            // will advance the progress indicator a number of times.
+            if ($task instanceof ProgressIndicatorAwareInterface) {
+                $steps += $task->progressIndicatorSteps();
+            }
+            // We also advance the progress indicator once regardless
+            // of whether it is progress-indicator aware or not.
+            $steps++;
+        }
+        return $steps;
+    }
 }
diff --git a/src/Result.php b/src/Result.php
index a4235c9d0..e424b2029 100644
--- a/src/Result.php
+++ b/src/Result.php
@@ -87,6 +87,39 @@ public function getContext()
         ];
     }
 
+    /**
+     * Add the results from the most recent task to the accumulated
+     * results from all tasks that have run so far, merging data
+     * as necessary.
+     */
+    public function accumulate($key, Result $taskResult)
+    {
+        // If the task is unnamed, then all of its data elements
+        // just get merged in at the top-level of the final Result object.
+        if (static::isUnnamed($key)) {
+            $this->merge($taskResult);
+        } elseif (isset($this[$key])) {
+            // There can only be one task with a given name; however, if
+            // there are tasks added 'before' or 'after' the named task,
+            // then the results from these will be stored under the same
+            // name unless they are given a name of their own when added.
+            $current = $this[$key];
+            $this[$key] = $taskResult->merge($current);
+        } else {
+            $this[$key] = $taskResult;
+        }
+    }
+
+    /**
+     * We assume that named values (e.g. for associative array keys)
+     * are non-numeric; numeric keys are presumed to simply be the
+     * index of an array, and therefore insignificant.
+     */
+    public static function isUnnamed($key)
+    {
+        return is_numeric($key);
+    }
+
     /**
      * @return TaskInterface
      */