Add a test for Result::$stopOnFail. Make stop on fail throw an except…

…ion, so that it will be testable. Make stopOnFail and collections interact better - stop on fail can be used to interrupt a collection, which will still clean up on failure, and stop execution of further code.

src/Collection/Collection.php

@@ -8,6 +8,7 @@
 use Robo\Task\BaseTask;
 use Robo\TaskInfo;
 use Robo\Contract\WrappedTaskInterface;
+use Robo\Exception\TaskExitException;
 
 use Robo\Contract\ProgressIndicatorAwareInterface;
 use Robo\Common\ProgressIndicatorAwareTrait;
@@ -467,6 +468,9 @@ private function runTaskList($name, array $taskList, $result)
                 $key = static::isUnnamedTask($taskName) ? $name : $taskName;
                 $result = $this->accumulateResults($key, $result, $taskResult);
             }
+        } catch (TaskExitException $exitException) {
+            $this->fail();
+            throw $exitException;
         } catch (\Exception $e) {
             // Tasks typically should not throw, but if one does, we will
             // convert it into an error and roll back.

src/Exception/TaskExitException.php

@@ -0,0 +1,13 @@
+<?php
+namespace Robo\Exception;
+
+class TaskExitException extends \Exception
+{
+    public function __construct($class, $message, $status)
+    {
+        if (is_object($class)) {
+            $class = get_class($class);
+        }
+        parent::__construct("  in task $class \n\n  $message", $status);
+    }
+}

src/Result.php

@@ -3,6 +3,7 @@
 
 use Robo\Contract\TaskInterface;
 use Robo\Contract\LogResultInterface;
+use Robo\Exception\TaskExitException;
 
 class Result extends ResultData
 {
@@ -116,8 +117,13 @@ public function stopOnFail()
             if ($resultPrinter) {
                 $resultPrinter->printStopOnFail($this);
             }
-            exit($this->exitCode);
+            $this->exitEarly($this->getExitCode());
         }
         return $this;
     }
+
+    private function exitEarly($status)
+    {
+        throw new TaskExitException($this->getTask(), $this->getMessage(), $status);
+    }
 }

src/Runner.php

@@ -8,6 +8,7 @@
 use Consolidation\AnnotatedCommand\PassThroughArgsInput;
 use Robo\Contract\BuilderAwareInterface;
 use Robo\Common\IO;
+use Robo\Exception\TaskExitException;
 
 class Runner
 {
@@ -102,7 +103,11 @@ public function run($input = null, $output = null, $app = null)
         $this->setInput($input);
         $this->setOutput($output);
 
-        $statusCode = $app->run($input, $output);
+        try {
+            $statusCode = $app->run($input, $output);
+        } catch (TaskExitException $e) {
+            $statusCode = $e->getCode() ?: 1;
+        }
         return $statusCode;
     }
 

tests/_helpers/CodeHelper.php

@@ -24,6 +24,11 @@ public function _before(\Codeception\TestCase $test)
 
     public function _after(\Codeception\TestCase $test)
     {
+        // Ensure that $stopOnFail global static is reset, as tests
+        // that set it to true will force an exception, and therefor
+        // will not have a chance to clean this up.
+        \Robo\Result::$stopOnFail = false;
+
         \AspectMock\Test::clean();
         $consoleOutput = new ConsoleOutput();
         static::$container->add('output', $consoleOutput);

tests/src/TestRoboFile.php

@@ -43,4 +43,18 @@ public function testException($options = ['task' => false])
         }
         throw new \RuntimeException('Task failed with an exception.');
     }
+
+    public function testStopOnFail()
+    {
+        $this->stopOnFail();
+        $this->collectionBuilder()
+            ->taskExec('ls xyzzy' . date('U'))
+                ->dir('/tmp')
+            ->run();
+
+        // stopOnFail() should cause the failed task to throw an exception,
+        // so we should not get here, and instead exit the program with a
+        // non-zero status.
+        return 0;
+    }
 }

tests/unit/ResultTest.php

@@ -1,5 +1,6 @@
 <?php
 use Robo\Result;
+use Robo\Exception\TaskExitException;
 
 class ResultTest extends \Codeception\TestCase\Test {
 
@@ -35,6 +36,39 @@ public function testArrayAccess()
         $result = new Result($task, 1, 'The foo barred', ['time' => 10]);
         $this->assertEquals($result['time'], 10);
     }
+
+    public function testStopOnFail()
+    {
+        $exceptionClass = false;
+        $task = new ResultDummyTask();
+
+        Result::$stopOnFail = true;
+        $result = Result::success($task, "Something that worked");
+        try {
+            $result = Result::error($task, "Something that did not work");
+            // stopOnFail will cause Result::error() to throw an exception,
+            // so we will never get here. If we did, the assert below would fail.
+            $this->assertTrue($result->wasSuccessful());
+            $this->assertTrue(false);
+        } catch (\Exception $e) {
+            $exceptionClass = get_class($e);
+        }
+        $this->assertEquals(TaskExitException::class, $exceptionClass);
+        $this->assertTrue($result->wasSuccessful());
+
+        /*
+        // This gives an error:
+        //    Exception of class Robo\Exception\TaskExitException expected to
+        //    be thrown, but PHPUnit_Framework_Exception caught
+        // This happens whether or not the expected exception is thrown
+        $this->guy->expectException(TaskExitException::class, function() {
+            // $result = Result::error($task, "Something that did not work");
+            $result = Result::success($task, "Something that worked");
+        });
+        */
+
+        Result::$stopOnFail = false;
+    }
 }
 
 class ResultDummyTask implements \Robo\Contract\TaskInterface

tests/unit/RunnerTest.php

@@ -138,4 +138,13 @@ public function testInitCommand()
         unlink('testRoboFile');
         $this->assertContains('class RoboTestClass', $commandContents);
     }
+
+    public function testTasksStopOnFail()
+    {
+        $argv = ['placeholder', 'test:stop-on-fail'];
+        $result = $this->runner->execute($argv, Robo::output());
+
+        $this->guy->seeInOutput('[');
+        $this->assertTrue($result > 0);
+    }
 }