improve ParallelExecutor::generateOptions to manage all types of InputOption (#1792)

src/Console/Input/Argument.php

@@ -0,0 +1,21 @@
+<?php declare(strict_types=1);
+/* (c) Anton Medvedev <anton@medv.io>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Deployer\Console\Input;
+
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+
+final class Argument
+{
+    public static function toString(
+        InputInterface $input,
+        InputArgument $argument
+    ): string {
+        return $input->getArgument($argument->getName());
+    }
+}

src/Console/Input/Option.php

@@ -0,0 +1,80 @@
+<?php declare(strict_types=1);
+/* (c) Anton Medvedev <anton@medv.io>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Deployer\Console\Input;
+
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+
+final class Option
+{
+    public static function toString(
+        InputInterface $input,
+        InputOption $option
+    ): string {
+        $name = $option->getName();
+
+        if (!$option->acceptValue()) {
+            return \sprintf(
+                '--%s',
+                $name
+            );
+        }
+
+        if (!$option->isArray()) {
+            return self::generatePartialOption(
+                $option,
+                $name,
+                $input->getOption($name)
+            );
+        }
+
+        /** @var string[] $outputs */
+        $outputs = [];
+        foreach ($input->getOption($name) as $value) {
+            $value = self::generatePartialOption(
+                $option,
+                $name,
+                $value
+            );
+
+            if ($value === '') {
+                continue;
+            }
+
+            $outputs[] = $value;
+        }
+
+        return \implode(' ', $outputs);
+    }
+
+    /**
+     * @param null|string $value
+     */
+    private static function generatePartialOption(
+        InputOption $option,
+        string $name,
+        $value
+    ): string {
+        if (\null !== $value && \strlen($value) !== 0) {
+            return \sprintf(
+                '--%s=%s',
+                $name,
+                $value
+            );
+        }
+
+        if ($option->isValueOptional()) {
+            return \sprintf(
+                '--%s',
+                $name
+            );
+        }
+
+        return '';
+    }
+}

src/Console/Output/VerbosityString.php

@@ -1,4 +1,4 @@
-<?php
+<?php declare(strict_types=1);
 /* (c) Anton Medvedev <anton@medv.io>
  *
  * For the full copyright and license information, please view the LICENSE
@@ -16,44 +16,29 @@ class VerbosityString
      */
     private $output;
 
-    /**
-     * @param OutputInterface $output
-     */
     public function __construct(OutputInterface $output)
     {
         $this->output = $output;
     }
 
-    /**
-     * @return string
-     */
-    public function __toString()
+    public function __toString(): string
     {
         switch ($this->output->getVerbosity()) {
-            case OutputInterface::VERBOSITY_NORMAL:
-                $verbosity = '';
-                break;
-
             case OutputInterface::VERBOSITY_VERBOSE:
-                $verbosity = '-v';
-                break;
+                return '-v';
 
             case OutputInterface::VERBOSITY_VERY_VERBOSE:
-                $verbosity = '-vv';
-                break;
+                return '-vv';
 
             case OutputInterface::VERBOSITY_DEBUG:
-                $verbosity = '-vvv';
-                break;
+                return '-vvv';
 
             case OutputInterface::VERBOSITY_QUIET:
-                $verbosity = '-q';
-                break;
+                return '-q';
 
+            case OutputInterface::VERBOSITY_NORMAL:
             default:
-                $verbosity = '';
+                return '';
         }
-
-        return $verbosity;
     }
 }

src/Executor/ParallelExecutor.php

@@ -8,6 +8,8 @@
 namespace Deployer\Executor;
 
 use Deployer\Console\Application;
+use Deployer\Console\Input\Argument;
+use Deployer\Console\Input\Option;
 use Deployer\Console\Output\Informer;
 use Deployer\Console\Output\VerbosityString;
 use Deployer\Exception\Exception;
@@ -18,6 +20,7 @@ use Deployer\Host\Storage;
 use Deployer\Task\Context;
 use Deployer\Task\Task;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Process\Process;
 
@@ -61,7 +64,7 @@ class ParallelExecutor implements ExecutorInterface
     public function run(array $tasks, array $hosts)
     {
         $localhost = new Localhost();
-        $limit = (int)$this->input->getOption('limit') ?: count($hosts);
+        $limit = (int) $this->input->getOption('limit') ?: count($hosts);
 
         // We need contexts here for usage inside `on` function. Pass input/output to callback of it.
         // This allows to use code like this in parallel mode:
@@ -154,7 +157,7 @@ class ParallelExecutor implements ExecutorInterface
      */
     protected function getProcess(Host $host, Task $task): Process
     {
-        $dep = PHP_BINARY . ' ' . DEPLOYER_BIN;
+        $dep = PHP_BINARY.' '.DEPLOYER_BIN;
         $options = $this->generateOptions();
         $arguments = $this->generateArguments();
         $hostname = $host->getHostname();
@@ -181,6 +184,7 @@ class ParallelExecutor implements ExecutorInterface
      * Start all of the processes.
      *
      * @param Process[] $processes
+     *
      * @return void
      */
     protected function startProcesses(array $processes)
@@ -202,6 +206,7 @@ class ParallelExecutor implements ExecutorInterface
                 return true;
             }
         }
+
         return false;
     }
 
@@ -209,6 +214,7 @@ class ParallelExecutor implements ExecutorInterface
      * Gather the output from all of the processes.
      *
      * @param Process[] $processes
+     *
      * @return void
      */
     protected function gatherOutput(array $processes, callable $callback)
@@ -247,31 +253,23 @@ class ParallelExecutor implements ExecutorInterface
      */
     private function generateOptions(): string
     {
-        $input = (string)(new VerbosityString($this->output));
+        /** @var string[] $inputs */
+        $inputs = [
+            (string) (new VerbosityString($this->output)),
+        ];
 
+        $userDefinition = $this->console->getUserDefinition();
         // Get user arguments
-        foreach ($this->console->getUserDefinition()->getArguments() as $argument) {
-            $value = $this->input->getArgument($argument->getName());
-            if (strlen($value) !== 0) {
-                $input .= " $value";
-            }
+        foreach ($userDefinition->getArguments() as $argument) {
+            $inputs[] = Argument::toString($this->input, $argument);
         }
 
         // Get user options
-        foreach ($this->console->getUserDefinition()->getOptions() as $option) {
-            $name = $option->getName();
-            $value = $this->input->getOption($name);
-
-            if (null !== $value && strlen($value) !== 0) {
-                $input .= " --{$name}";
-
-                if ($option->acceptValue()) {
-                    $input .= " {$value}";
-                }
-            }
+        foreach ($userDefinition->getOptions() as $option) {
+            $inputs[] = Option::toString($this->input, $option);
         }
 
-        return $input;
+        return implode(' ', $inputs);
     }
 
     private function generateArguments(): string

test/src/Console/Input/ArgumentTest.php

@@ -0,0 +1,59 @@
+<?php declare(strict_types=1);
+/* (c) Anton Medvedev <anton@medv.io>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Deployer\Console\Input;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+
+final class ArgumentTest extends TestCase
+{
+    public function toStringProvider(): \Generator
+    {
+        foreach ([
+                     ['fooBar', 'fooBar'],
+                     ['0', '0'],
+                     ['1', '1'],
+                     ['foo\-%&Bar', 'foo\-%&Bar'],
+                     ['\'', '\''],
+                     ['ù+ì', 'ù+ì'],
+                 ] as list($expectedValue, $inputValue)) {
+            $input = $this->createMock(InputInterface::class);
+            $input->expects($this->once())
+                ->method('getArgument')
+                ->willReturn($inputValue);
+
+            $argument = $this->createMock(InputArgument::class);
+            $argument->expects($this->once())
+                ->method('getName')
+                ->willReturn('argumentName');
+
+            yield [
+                $expectedValue,
+                $input,
+                $argument,
+            ];
+        }
+    }
+
+    /**
+     * @dataProvider toStringProvider
+     *
+     * @return void
+     */
+    public function testToString(
+        string $expectedValue,
+        InputInterface $input,
+        InputArgument $argument
+    ) {
+        $this->assertEquals(
+            $expectedValue,
+            Argument::toString($input, $argument)
+        );
+    }
+}

test/src/Console/Input/OptionTest.php

@@ -0,0 +1,212 @@
+<?php declare(strict_types=1);
+/* (c) Anton Medvedev <anton@medv.io>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Deployer\Console\Input;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+
+final class OptionTest extends TestCase
+{
+    public function toStringProvider(): \Generator
+    {
+        // InputOption::VALUE_NONE
+        foreach ([
+                     ['--fooBar', 'fooBar'],
+                     ['--0', '0'],
+                     ['--1', '1'],
+                     ['--foo\-%&Bar', 'foo\-%&Bar'],
+                     ['--ù+ì', 'ù+ì'],
+                 ] as list($expectedValue, $optionName)) {
+            $input = $this->createMock(InputInterface::class);
+
+            $option = $this->createMock(InputOption::class);
+            $option->expects($this->once())
+                ->method('getName')
+                ->willReturn($optionName);
+
+            $option->expects($this->once())
+                ->method('acceptValue')
+                ->willReturn(\false);
+
+            yield [
+                $expectedValue,
+                $input,
+                $option,
+            ];
+        }
+
+        // InputOption::VALUE_REQUIRED
+        foreach ([
+                     ['--fooBar=ciao', 'fooBar', 'ciao'],
+                     ['', 'fooBar', \null],
+                     ['', 'fooBar', ''],
+                     ['--fooBar=0', 'fooBar', '0'],
+                     ['--foo\-%&Bar=test', 'foo\-%&Bar', 'test'],
+                     ['--ù+ì=omg', 'ù+ì', 'omg'],
+                 ] as list($expectedValue, $optionName, $optionValue)) {
+            $input = $this->createMock(InputInterface::class);
+            $input->expects($this->once())
+                ->method('getOption')
+                ->willReturn($optionValue);
+
+            $option = $this->createMock(InputOption::class);
+            $option->expects($this->once())
+                ->method('getName')
+                ->willReturn($optionName);
+
+            $option->expects($this->once())
+                ->method('acceptValue')
+                ->willReturn(\true);
+
+            $option->expects($this->once())
+                ->method('isArray')
+                ->willReturn(\false);
+
+            $option->expects($this->any())
+                ->method('isValueOptional')
+                ->willReturn(\false);
+
+            yield [
+                $expectedValue,
+                $input,
+                $option,
+            ];
+        }
+
+        // InputOption::VALUE_OPTIONAL
+        foreach ([
+                     ['--fooBar=ciao', 'fooBar', 'ciao'],
+                     ['--fooBar', 'fooBar', \null],
+                     ['--fooBar', 'fooBar', ''],
+                     ['--fooBar=0', 'fooBar', '0'],
+                     ['--foo\-%&Bar=test', 'foo\-%&Bar', 'test'],
+                     ['--ù+ì=omg', 'ù+ì', 'omg'],
+                 ] as list($expectedValue, $optionName, $optionValue)) {
+            $input = $this->createMock(InputInterface::class);
+            $input->expects($this->once())
+                ->method('getOption')
+                ->willReturn($optionValue);
+
+            $option = $this->createMock(InputOption::class);
+            $option->expects($this->once())
+                ->method('getName')
+                ->willReturn($optionName);
+
+            $option->expects($this->once())
+                ->method('acceptValue')
+                ->willReturn(\true);
+
+            $option->expects($this->once())
+                ->method('isArray')
+                ->willReturn(\false);
+
+            $option->expects($this->any())
+                ->method('isValueOptional')
+                ->willReturn(\true);
+
+            yield [
+                $expectedValue,
+                $input,
+                $option,
+            ];
+        }
+
+        // InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY
+        foreach ([
+                     ['--fooBar=ciao --fooBar=Привет', 'fooBar', ['ciao', 'Привет']],
+                     ['--fooBar=ciao --fooBar=Привет', 'fooBar', ['ciao', \null, 'Привет']],
+                     ['', 'fooBar', [\null, '']],
+                     ['', 'fooBar', [\null]],
+                     ['', 'fooBar', ['']],
+                     ['--fooBar=0 --fooBar=1 --fooBar=2 --fooBar=...', 'fooBar', ['0', '1', '2', '...']],
+                 ] as list($expectedValue, $optionName, $optionValue)) {
+            $input = $this->createMock(InputInterface::class);
+            $input->expects($this->once())
+                ->method('getOption')
+                ->willReturn($optionValue);
+
+            $option = $this->createMock(InputOption::class);
+            $option->expects($this->once())
+                ->method('getName')
+                ->willReturn($optionName);
+
+            $option->expects($this->once())
+                ->method('acceptValue')
+                ->willReturn(\true);
+
+            $option->expects($this->once())
+                ->method('isArray')
+                ->willReturn(\true);
+
+            $option->expects($this->any())
+                ->method('isValueOptional')
+                ->willReturn(\false);
+
+            yield [
+                $expectedValue,
+                $input,
+                $option,
+            ];
+        }
+
+        // InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY
+        foreach ([
+                     ['--fooBar=ciao --fooBar=Привет', 'fooBar', ['ciao', 'Привет']],
+                     ['--fooBar=ciao --fooBar --fooBar=Привет', 'fooBar', ['ciao', \null, 'Привет']],
+                     ['--fooBar --fooBar', 'fooBar', [\null, '']],
+                     ['--fooBar', 'fooBar', [\null]],
+                     ['--fooBar', 'fooBar', ['']],
+                     ['--fooBar=0 --fooBar=1 --fooBar=2 --fooBar=...', 'fooBar', ['0', '1', '2', '...']],
+                 ] as list($expectedValue, $optionName, $optionValue)) {
+            $input = $this->createMock(InputInterface::class);
+            $input->expects($this->once())
+                ->method('getOption')
+                ->willReturn($optionValue);
+
+            $option = $this->createMock(InputOption::class);
+            $option->expects($this->once())
+                ->method('getName')
+                ->willReturn($optionName);
+
+            $option->expects($this->once())
+                ->method('acceptValue')
+                ->willReturn(\true);
+
+            $option->expects($this->once())
+                ->method('isArray')
+                ->willReturn(\true);
+
+            $option->expects($this->any())
+                ->method('isValueOptional')
+                ->willReturn(\true);
+
+            yield [
+                $expectedValue,
+                $input,
+                $option,
+            ];
+        }
+    }
+
+    /**
+     * @dataProvider toStringProvider
+     *
+     * @return void
+     */
+    public function testToString(
+        string $expectedValue,
+        InputInterface $input,
+        InputOption $option
+    ) {
+        $this->assertEquals(
+            $expectedValue,
+            Option::toString($input, $option)
+        );
+    }
+}