Merge pull request #189 from shlinkio/develop

Release 8.4.1
shlinkio · Jun 8, 2023 · e8f7bbc · e8f7bbc
2 parents faf84bc + aeba96e
commit e8f7bbc
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## [8.4.1] - 2023-06-08
+### Added
+* *Nothing*
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* Fix silent error when trying to download rr binary
+
+
 ## [8.4.0] - 2023-05-23
 ### Added
 * [#183](https://github.com/shlinkio/shlink-installer/issues/183) Create new `init` command that can be used to set up and initialize the environment for Shlink.

diff --git a/config/config.php b/config/config.php
@@ -273,7 +273,7 @@
                 'printOutput' => true,
             ],
             InstallationCommand::ROAD_RUNNER_BINARY_DOWNLOAD->value => [
-                'command' => 'vendor/bin/rr get --no-interaction  --no-config --location bin/',
+                'command' => 'vendor/bin/rr get --no-interaction --no-config --location bin/',
                 'initMessage' => 'Downloading RoadRunner binary...',
                 'errorMessage' => 'Error downloading RoadRunner binary.',
                 'failOnError' => false,

diff --git a/src/Service/InstallationCommandsRunner.php b/src/Service/InstallationCommandsRunner.php
@@ -9,9 +9,11 @@
 use Symfony\Component\Console\Style\SymfonyStyle;
 use Symfony\Component\Process\PhpExecutableFinder;
 
+use function array_filter;
 use function explode;
 use function implode;
 use function sprintf;
+use function trim;
 
 class InstallationCommandsRunner implements InstallationCommandsRunnerInterface
 {
@@ -47,26 +49,38 @@ public function execPhpCommand(string $name, SymfonyStyle $io): bool
             return true;
         }
 
-        $command = [$this->phpBinary, ...explode(' ', $command)];
+        $command = [$this->phpBinary, ...$this->commandToArray($command)];
         $io->write(
             sprintf(' <options=bold>[Running "%s"]</> ', implode(' ', $command)),
             false,
             OutputInterface::VERBOSITY_VERBOSE,
         );
 
         $process = $this->processHelper->run($io, $command);
-        $isSuccessful = ! $failOnError || $process->isSuccessful();
+        $isSuccess = $process->isSuccessful();
+        $isWarning = ! $isSuccess && ! $failOnError;
+        $isVerbose = $io->isVerbose();
 
-        if ($isSuccessful) {
+        if ($isSuccess) {
             $io->writeln(' <info>Success!</info>');
-        } elseif (! $io->isVerbose()) {
+        } elseif ($isWarning) {
+            $io->write(' <comment>Warning!</comment>');
+            $io->writeln($isVerbose ? '' : ' Run with -vvv to see error.');
+        } elseif (! $isVerbose) {
             $io->error(sprintf('%s. Run this command with -vvv to see specific error info.', $errorMessage));
         }
 
         if ($printOutput) {
             $io->text($process->getOutput());
         }
 
-        return $isSuccessful;
+        return $isSuccess || $isWarning;
+    }
+
+    private function commandToArray(string $command): array
+    {
+        $splitBySpace = explode(' ', trim($command));
+        // array_filter ensures empty entries are removed, in case the command has duplicated spaces
+        return array_filter($splitBySpace);
     }
 }
diff --git a/test/Service/InstallationCommandsRunnerTest.php b/test/Service/InstallationCommandsRunnerTest.php
@@ -4,14 +4,14 @@
 
 namespace ShlinkioTest\Shlink\Installer\Service;
 
+use InvalidArgumentException;
 use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Installer\Service\InstallationCommandsRunner;
 use Symfony\Component\Console\Helper\ProcessHelper;
-use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
 use Symfony\Component\Process\PhpExecutableFinder;
 use Symfony\Component\Process\Process;
@@ -33,16 +33,18 @@ public function setUp(): void
         $phpFinder->method('find')->with(false)->willReturn('php');
 
         $this->processHelper = $this->createMock(ProcessHelper::class);
-        $this->commandsRunner = new InstallationCommandsRunner($this->processHelper, $phpFinder, $this->buildCommands(
-            ['foo', 'bar', 'null_command'],
-        ));
+        $this->commandsRunner = new InstallationCommandsRunner(
+            $this->processHelper,
+            $phpFinder,
+            $this->buildCommands(),
+        );
 
         $this->io = $this->createMock(SymfonyStyle::class);
-        $this->io->method('isVerbose')->willReturn(false);
     }
 
-    private function buildCommands(array $names): array
+    private function buildCommands(): array
     {
+        $names = ['foo', 'bar', 'null_command', 'multiple  spaces   '];
         return array_combine($names, map($names, fn (string $name) => [
             'command' => $name === 'null_command' ? null : sprintf('%s something', $name),
             'initMessage' => sprintf('%s_init', $name),
@@ -58,25 +60,23 @@ public function doesNothingWhenRequestedCommandDoesNotExist(): void
         self::assertFalse($this->commandsRunner->execPhpCommand('invalid', $this->io));
     }
 
-    #[Test, DataProvider('provideCommandNames')]
-    public function returnsSuccessWhenProcessIsProperlyRunOrDoesNotFailOnError(string $name): void
+    #[Test]
+    public function returnsSuccessWhenProcessIsProperlyRunOrDoesNotFailOnError(): void
     {
+        $name = 'foo';
         $command = ['php', $name, 'something'];
 
-        $process = $this->createProcessMock($name === 'foo');
+        $process = $this->createProcessMock(true);
         $this->processHelper->expects($this->once())->method('run')->with($this->io, $command)->willReturn($process);
 
-        $callCount = 0;
-        $this->io->expects($this->exactly(2))->method('write')->willReturnCallback(
-            function (string $messages, bool $newline, int $type) use (&$callCount, $name, $command): void {
-                if ($callCount === 0) {
-                    Assert::assertEquals(sprintf('%s_init', $name), $messages);
-                } elseif ($callCount === 1) {
-                    Assert::assertStringContainsString(sprintf('Running "%s"', implode(' ', $command)), $messages);
-                    Assert::assertFalse($newline);
-                    Assert::assertEquals(OutputInterface::VERBOSITY_VERBOSE, $type);
-                }
-                $callCount++;
+        $writeCallMatcher = $this->exactly(2);
+        $this->io->expects($writeCallMatcher)->method('write')->willReturnCallback(
+            function (string $message) use ($writeCallMatcher, $name, $command): void {
+                match ($writeCallMatcher->numberOfInvocations()) {
+                    1 => Assert::assertEquals(sprintf('%s_init', $name), $message),
+                    2 => Assert::assertStringContainsString(sprintf('Running "%s"', implode(' ', $command)), $message),
+                    default => throw new InvalidArgumentException('Not valid case'),
+                };
             },
         );
         $this->io->expects($this->once())->method('writeln')->with(' <info>Success!</info>', $this->anything());
@@ -85,9 +85,37 @@ function (string $messages, bool $newline, int $type) use (&$callCount, $name, $
         self::assertTrue($this->commandsRunner->execPhpCommand($name, $this->io));
     }
 
-    public static function provideCommandNames(): array
+    #[Test, DataProvider('provideExtraLines')]
+    public function returnsWarningWhenProcessFailsButErrorIsAllowed(bool $isVerbose, string $extraLine): void
+    {
+        $name = 'bar';
+        $command = ['php', $name, 'something'];
+
+        $process = $this->createProcessMock(false);
+        $this->processHelper->expects($this->once())->method('run')->with($this->io, $command)->willReturn($process);
+        $this->io->method('isVerbose')->willReturn($isVerbose);
+
+        $writeCallMatcher = $this->exactly(3);
+        $this->io->expects($writeCallMatcher)->method('write')->willReturnCallback(
+            function (string $message) use ($writeCallMatcher, $name, $command): void {
+                match ($writeCallMatcher->numberOfInvocations()) {
+                    1 => Assert::assertEquals(sprintf('%s_init', $name), $message),
+                    2 => Assert::assertStringContainsString(sprintf('Running "%s"', implode(' ', $command)), $message),
+                    3 => Assert::assertEquals(' <comment>Warning!</comment>', $message),
+                    default => throw new InvalidArgumentException('Not valid case'),
+                };
+            },
+        );
+        $this->io->expects($this->once())->method('writeln')->with($extraLine);
+        $this->io->expects($this->never())->method('error');
+
+        self::assertTrue($this->commandsRunner->execPhpCommand($name, $this->io));
+    }
+
+    public static function provideExtraLines(): iterable
     {
-        return [['foo'], ['bar']];
+        yield 'verbose output' => [true, ''];
+        yield 'not verbose output' => [false, ' Run with -vvv to see error.'];
     }
 
     #[Test]
@@ -99,17 +127,14 @@ public function returnsErrorWhenProcessIsNotProperlyRun(): void
         $process = $this->createProcessMock(false);
         $this->processHelper->expects($this->once())->method('run')->with($this->io, $command)->willReturn($process);
 
-        $callCount = 0;
-        $this->io->expects($this->exactly(2))->method('write')->willReturnCallback(
-            function (string $messages, bool $newline, int $type) use (&$callCount, $name, $command): void {
-                if ($callCount === 0) {
-                    Assert::assertEquals(sprintf('%s_init', $name), $messages);
-                } elseif ($callCount === 1) {
-                    Assert::assertStringContainsString(sprintf('Running "%s"', implode(' ', $command)), $messages);
-                    Assert::assertFalse($newline);
-                    Assert::assertEquals(OutputInterface::VERBOSITY_VERBOSE, $type);
-                }
-                $callCount++;
+        $writeCallMatcher = $this->exactly(2);
+        $this->io->expects($writeCallMatcher)->method('write')->willReturnCallback(
+            function (string $message) use ($writeCallMatcher, $name, $command): void {
+                match ($writeCallMatcher->numberOfInvocations()) {
+                    1 => Assert::assertEquals(sprintf('%s_init', $name), $message),
+                    2 => Assert::assertStringContainsString(sprintf('Running "%s"', implode(' ', $command)), $message),
+                    default => throw new InvalidArgumentException('Not valid case'),
+                };
             },
         );
         $this->io->expects($this->once())->method('error')->with($this->stringContains(sprintf('%s_error', $name)));