[TASK] Add more unit tests for class TaskManager

- Create class TaskFactory. The new class is responsible for the real creation of the task. First step to depedency injection container

src/Domain/Service/TaskFactory.php

@@ -0,0 +1,49 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\Surf\Domain\Service;
+
+/*
+ * This file is part of TYPO3 Surf.
+ *
+ * For the full copyright and license information, please view the LICENSE.txt
+ * file that was distributed with this source code.
+ */
+
+use TYPO3\Surf\Domain\Model\Task;
+use TYPO3\Surf\Exception as SurfException;
+
+/**
+ * @final
+ */
+class TaskFactory
+{
+    /**
+     * Create a task instance from the given task name
+     *
+     * @return ShellCommandServiceAwareInterface|Task
+     */
+    public function createTaskInstance(string $taskName)
+    {
+        $taskClassName = $this->mapTaskNameToTaskClass($taskName);
+        $task = new $taskClassName();
+
+        if (!$task instanceof Task) {
+            throw new SurfException(sprintf('The task %s is not a subclass of %s but of class %s', $taskName, Task::class, get_class($task)), 1451210811);
+        }
+
+        if ($task instanceof ShellCommandServiceAwareInterface) {
+            $task->setShellCommandService(new ShellCommandService());
+        }
+        return $task;
+    }
+
+    private function mapTaskNameToTaskClass(string $taskName): string
+    {
+        if (!class_exists($taskName)) {
+            throw new SurfException(sprintf('No task found for identifier "%s". Make sure this is a valid class name or a defined task with valid base class name!', $taskName), 1451210811);
+        }
+
+        return $taskName;
+    }
+}

src/Domain/Service/TaskManager.php

@@ -11,10 +11,10 @@
 use TYPO3\Surf\Domain\Model\Application;
 use TYPO3\Surf\Domain\Model\Deployment;
 use TYPO3\Surf\Domain\Model\Node;
-use TYPO3\Surf\Exception as SurfException;
+use TYPO3\Surf\Domain\Model\Task;
 
 /**
- * A task manager
+ * @final
  */
 class TaskManager
 {
@@ -25,19 +25,21 @@ class TaskManager
     protected $taskHistory = [];
 
     /**
-     * Execute a task
-     *
-     * @param string $taskName
-     * @param string $stage
-     * @param array $options Local task options
-     * @param string $definedTaskName
+     * @var TaskFactory
      */
-    public function execute($taskName, Node $node, Application $application, Deployment $deployment, $stage, array $options = [], $definedTaskName = '')
+    private $taskFactory;
+
+    public function __construct(TaskFactory $taskFactory = null)
+    {
+        $this->taskFactory = $taskFactory ?? new TaskFactory();
+    }
+
+    public function execute(string $taskName, Node $node, Application $application, Deployment $deployment, $stage, array $options = [], string $definedTaskName = ''): void
     {
         $definedTaskName = $definedTaskName ?: $taskName;
         $deployment->getLogger()->info($node->getName() . ' (' . $application->getName() . ') ' . $definedTaskName);
 
-        $task = $this->createTaskInstance($taskName);
+        $task = $this->taskFactory->createTaskInstance($taskName);
 
         $globalOptions = $this->overrideOptions($definedTaskName, $deployment, $node, $application, $options);
 
@@ -60,7 +62,7 @@ public function execute($taskName, Node $node, Application $application, Deploym
     /**
      * Rollback all tasks stored in the task history in reverse order
      */
-    public function rollback()
+    public function rollback(): void
     {
         foreach (array_reverse($this->taskHistory) as $historicTask) {
             $historicTask['deployment']->getLogger()->info('Rolling back ' . get_class($historicTask['task']));
@@ -71,10 +73,7 @@ public function rollback()
         $this->reset();
     }
 
-    /**
-     * Reset the task history
-     */
-    public function reset()
+    public function reset(): void
     {
         $this->taskHistory = [];
     }
@@ -92,12 +91,8 @@ public function reset()
      * Global options for a task should be prefixed with the task name to prevent naming
      * issues between different tasks. For example passing a special option to the
      * GitCheckoutTask could be expressed like GitCheckoutTask::class . '[sha1]' => '1234...'.
-     *
-     * @param string $taskName
-     * @param array $taskOptions
-     * @return array
      */
-    protected function overrideOptions($taskName, Deployment $deployment, Node $node, Application $application, array $taskOptions)
+    protected function overrideOptions(string $taskName, Deployment $deployment, Node $node, Application $application, array $taskOptions): array
     {
         $globalOptions = array_merge(
             $deployment->getOptions(),
@@ -117,36 +112,4 @@ protected function overrideOptions($taskName, Deployment $deployment, Node $node
             $taskOptions
         );
     }
-
-    /**
-     * Create a task instance from the given task name
-     *
-     * @param string $taskName
-     *
-     * @return mixed
-     */
-    protected function createTaskInstance($taskName)
-    {
-        $taskClassName = $this->mapTaskNameToTaskClass($taskName);
-        $task = new $taskClassName();
-        if ($task instanceof ShellCommandServiceAwareInterface) {
-            $task->setShellCommandService(new ShellCommandService());
-        }
-        return $task;
-    }
-
-    /**
-     * Map the task name to the proper task class
-     *
-     * @param string $taskName
-     * @return string
-     * @throws SurfException
-     */
-    protected function mapTaskNameToTaskClass($taskName)
-    {
-        if (class_exists($taskName)) {
-            return $taskName;
-        }
-        throw new SurfException(sprintf('No task found for identifier "%s". Make sure this is a valid class name or a defined task with valid base class name!', $taskName), 1451210811);
-    }
 }

tests/Unit/Domain/Service/TaskFactoryTest.php

@@ -0,0 +1,86 @@
+<?php
+
+namespace TYPO3\Surf\Tests\Unit\Domain\Service;
+
+/*
+ * This file is part of TYPO3 Surf.
+ *
+ * For the full copyright and license information, please view the LICENSE.txt
+ * file that was distributed with this source code.
+ */
+
+use PHPUnit\Framework\TestCase;
+use TYPO3\Surf\Domain\Model\Application;
+use TYPO3\Surf\Domain\Model\Deployment;
+use TYPO3\Surf\Domain\Model\Node;
+use TYPO3\Surf\Domain\Model\Task;
+use TYPO3\Surf\Domain\Service\ShellCommandService;
+use TYPO3\Surf\Domain\Service\ShellCommandServiceAwareInterface;
+use TYPO3\Surf\Domain\Service\TaskFactory;
+use TYPO3\Surf\Exception as SurfException;
+
+class TaskFactoryTest extends TestCase
+{
+    /**
+     * @var TaskFactory
+     */
+    protected $subject;
+
+    protected function setUp()
+    {
+        $this->subject = new TaskFactory();
+    }
+
+    /**
+     * @test
+     */
+    public function createTaskInstance(): void
+    {
+        $task = new class extends Task {
+            public function execute(Node $node, Application $application, Deployment $deployment, array $options = [])
+            {
+            }
+        };
+
+        $this->assertEquals($task, $this->subject->createTaskInstance(get_class($task)));
+    }
+
+    /**
+     * @test
+     */
+    public function createTaskInstanceImplementingShellCommandServiceAwareInterface(): void
+    {
+        $task = new class extends Task implements ShellCommandServiceAwareInterface {
+            public function execute(Node $node, Application $application, Deployment $deployment, array $options = [])
+            {
+            }
+
+            public function setShellCommandService(ShellCommandService $shellCommandService)
+            {
+            }
+        };
+
+        $this->assertEquals($task, $this->subject->createTaskInstance(get_class($task)));
+    }
+
+    /**
+     * @test
+     */
+    public function createTaskInstanceThrowsExceptionClassDoesNotExist(): void
+    {
+        $this->expectException(SurfException::class);
+        $this->subject->createTaskInstance('SomeFooBarBaz');
+    }
+
+    /**
+     * @test
+     */
+    public function createTaskInstanceThrowsExceptionClassIsNotOfCorrectSubclass(): void
+    {
+        $task = new class {
+        };
+
+        $this->expectException(SurfException::class);
+        $this->subject->createTaskInstance(get_class($task));
+    }
+}