Write CLI errors to stderr (#2317)

Co-authored-by: Jens Hatlak <jens.hatlak@check24.de>
cakephp · Nov 18, 2024 · 411d94a · 411d94a
1 parent efd7cf8
commit 411d94a
Show file tree

Hide file tree

Showing 7 changed files with 121 additions and 45 deletions.
diff --git a/phpcs.xml b/phpcs.xml
@@ -4,6 +4,12 @@
 
     <rule ref="CakePHP"/>
 
+    <rule ref="SlevomatCodingStandard.Namespaces.UnusedUses">
+        <properties>
+            <property name="searchAnnotations" value="true"/>
+        </properties>
+    </rule>
+
     <arg value="nps"/>
 
     <file>src/</file>

diff --git a/src/Phinx/Console/Command/AbstractCommand.php b/src/Phinx/Console/Command/AbstractCommand.php
@@ -21,6 +21,7 @@
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\ConsoleOutputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use UnexpectedValueException;
 
@@ -439,7 +440,7 @@ protected function writeEnvironmentOutput(?string &$environment, OutputInterface
         }
 
         if (!$this->getConfig()->hasEnvironment($environment)) {
-            $output->writeln(sprintf('<error>The environment "%s" does not exist</error>', $environment));
+            self::getErrorOutput($output)->writeln(sprintf('<error>The environment "%s" does not exist</error>', $environment));
 
             return false;
         }
@@ -478,7 +479,7 @@ protected function writeInformationOutput(?string &$environment, OutputInterface
             }
             $output->writeln('<info>using database</info> ' . $name, $this->verbosityLevel);
         } else {
-            $output->writeln('<error>Could not determine database name! Please specify a database name in your config file.</error>');
+            self::getErrorOutput($output)->writeln('<error>Could not determine database name! Please specify a database name in your config file.</error>');
 
             return false;
         }
@@ -492,4 +493,15 @@ protected function writeInformationOutput(?string &$environment, OutputInterface
 
         return true;
     }
+
+    /**
+     * Returns the error output to use
+     *
+     * @param \Symfony\Component\Console\Output\OutputInterface $output Output
+     * @return \Symfony\Component\Console\Output\OutputInterface
+     */
+    protected static function getErrorOutput(OutputInterface $output): OutputInterface
+    {
+        return $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output;
+    }
 }
diff --git a/src/Phinx/Console/Command/ListAliases.php b/src/Phinx/Console/Command/ListAliases.php
@@ -70,10 +70,10 @@ function ($alias, $class) use ($maxAliasLength, $maxClassLength) {
             );
         } else {
             $output->writeln(
-                sprintf(
-                    '<error>No aliases defined in %s</error>',
-                    Util::relativePath($this->config->getConfigFilePath())
-                )
+                '<comment>warning</comment> no aliases defined in ' . Util::relativePath(
+                    $this->config->getConfigFilePath()
+                ),
+                $this->verbosityLevel
             );
         }
 

diff --git a/src/Phinx/Console/Command/Migrate.php b/src/Phinx/Console/Command/Migrate.php
@@ -9,7 +9,6 @@
 namespace Phinx\Console\Command;
 
 use DateTime;
-use Exception;
 use Symfony\Component\Console\Attribute\AsCommand;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
@@ -92,12 +91,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 $this->getManager()->migrate($environment, $version, $fake);
             }
             $end = microtime(true);
-        } catch (Exception $e) {
-            $output->writeln('<error>' . $e->__toString() . '</error>');
-
-            return self::CODE_ERROR;
         } catch (Throwable $e) {
-            $output->writeln('<error>' . $e->__toString() . '</error>');
+            self::getErrorOutput($output)->writeln('<error>' . $e->__toString() . '</error>');
 
             return self::CODE_ERROR;
         }

diff --git a/src/Phinx/Db/Adapter/SQLiteAdapter.php b/src/Phinx/Db/Adapter/SQLiteAdapter.php
@@ -204,7 +204,7 @@ public function connect(): void
      */
     public static function getSuffix(array $options): string
     {
-        if ($options['name'] === self::MEMORY) {
+        if (($options['name'] ?? '') === self::MEMORY) {
             return '';
         }
 

diff --git a/tests/Phinx/Console/Command/ListAliasesTest.php b/tests/Phinx/Console/Command/ListAliasesTest.php
@@ -38,18 +38,18 @@ public function testListingAliases($file, $hasAliases)
             ],
             ['decorated' => false]
         );
-        $this->assertEquals(AbstractCommand::CODE_SUCCESS, $exitCode);
+        $this->assertSame(AbstractCommand::CODE_SUCCESS, $exitCode);
 
         $display = $commandTester->getDisplay(false);
 
         if ($hasAliases) {
-            $this->assertStringNotContainsString('No aliases defined in ', $display);
+            $this->assertStringNotContainsString('no aliases defined in ', $display);
             $this->assertStringContainsString('Alias            Class                                             ', $display);
             $this->assertStringContainsString('================ ==================================================', $display);
             $this->assertStringContainsString('MakePermission   Vendor\Package\Migration\Creation\MakePermission  ', $display);
             $this->assertStringContainsString('RemovePermission Vendor\Package\Migration\Creation\RemovePermission', $display);
         } else {
-            $this->assertStringContainsString('No aliases defined in ', $display);
+            $this->assertStringContainsString('no aliases defined in ', $display);
         }
     }
 }
diff --git a/tests/Phinx/Console/Command/MigrateTest.php b/tests/Phinx/Console/Command/MigrateTest.php
@@ -3,31 +3,29 @@
 
 namespace Test\Phinx\Console\Command;
 
+use DateTime;
 use Phinx\Config\Config;
+use Phinx\Config\ConfigInterface;
 use Phinx\Console\Command\AbstractCommand;
 use Phinx\Console\Command\Migrate;
 use Phinx\Console\PhinxApplication;
+use Phinx\Migration\Manager;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use RuntimeException;
 use Symfony\Component\Console\Input\ArrayInput;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Output\StreamOutput;
 use Symfony\Component\Console\Tester\CommandTester;
 
 class MigrateTest extends TestCase
 {
-    /**
-     * @var ConfigInterface|array
-     */
-    protected $config = [];
+    private ConfigInterface $config;
 
-    /**
-     * @var InputInterface $input
-     */
-    protected $input;
+    private InputInterface $input;
 
-    /**
-     * @var OutputInterface $output
-     */
-    protected $output;
+    private OutputInterface $output;
 
     protected function setUp(): void
     {
@@ -50,7 +48,7 @@ protected function setUp(): void
         ]);
 
         $this->input = new ArrayInput([]);
-        $this->output = new StreamOutput(fopen('php://memory', 'a', false));
+        $this->output = new StreamOutput(fopen('php://memory', 'ab'));
     }
 
     public function testExecute()
@@ -62,8 +60,8 @@ public function testExecute()
         $command = $application->find('migrate');
 
         // mock the manager class
-        /** @var Manager|\PHPUnit\Framework\MockObject\MockObject $managerStub */
-        $managerStub = $this->getMockBuilder('\Phinx\Migration\Manager')
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
             ->setConstructorArgs([$this->config, $this->input, $this->output])
             ->getMock();
         $managerStub->expects($this->once())
@@ -103,8 +101,8 @@ public function testExecuteWithDsn()
         ]);
 
         // mock the manager class
-        /** @var Manager|\PHPUnit\Framework\MockObject\MockObject $managerStub */
-        $managerStub = $this->getMockBuilder('\Phinx\Migration\Manager')
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
             ->setConstructorArgs([$config, $this->input, $this->output])
             ->getMock();
         $managerStub->expects($this->once())
@@ -131,8 +129,8 @@ public function testExecuteWithEnvironmentOption()
         $command = $application->find('migrate');
 
         // mock the manager class
-        /** @var Manager|\PHPUnit\Framework\MockObject\MockObject $managerStub */
-        $managerStub = $this->getMockBuilder('\Phinx\Migration\Manager')
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
             ->setConstructorArgs([$this->config, $this->input, $this->output])
             ->getMock();
         $managerStub->expects($this->once())
@@ -157,8 +155,8 @@ public function testExecuteWithInvalidEnvironmentOption()
         $command = $application->find('migrate');
 
         // mock the manager class
-        /** @var Manager|\PHPUnit\Framework\MockObject\MockObject $managerStub */
-        $managerStub = $this->getMockBuilder('\Phinx\Migration\Manager')
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
             ->setConstructorArgs([$this->config, $this->input, $this->output])
             ->getMock();
         $managerStub->expects($this->never())
@@ -184,8 +182,8 @@ public function testDatabaseNameSpecified()
         $command = $application->find('migrate');
 
         // mock the manager class
-        /** @var Manager|\PHPUnit\Framework\MockObject\MockObject $managerStub */
-        $managerStub = $this->getMockBuilder('\Phinx\Migration\Manager')
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
             ->setConstructorArgs([$this->config, $this->input, $this->output])
             ->getMock();
         $managerStub->expects($this->once())
@@ -210,8 +208,8 @@ public function testFakeMigrate()
         $command = $application->find('migrate');
 
         // mock the manager class
-        /** @var Manager|\PHPUnit\Framework\MockObject\MockObject $managerStub */
-        $managerStub = $this->getMockBuilder('\Phinx\Migration\Manager')
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
             ->setConstructorArgs([$this->config, $this->input, $this->output])
             ->getMock();
         $managerStub->expects($this->once())
@@ -238,8 +236,8 @@ public function testMigrateExecutionOrder()
         $command = $application->find('migrate');
 
         // mock the manager class
-        /** @var Manager|\PHPUnit\Framework\MockObject\MockObject $managerStub */
-        $managerStub = $this->getMockBuilder('\Phinx\Migration\Manager')
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
             ->setConstructorArgs([$this->config, $this->input, $this->output])
             ->getMock();
         $managerStub->expects($this->once())
@@ -280,8 +278,8 @@ public function testMigrateMemorySqlite()
         $command = $application->find('migrate');
 
         // mock the manager class
-        /** @var Manager|\PHPUnit\Framework\MockObject\MockObject $managerStub */
-        $managerStub = $this->getMockBuilder('\Phinx\Migration\Manager')
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
             ->setConstructorArgs([$config, $this->input, $this->output])
             ->getMock();
         $managerStub->expects($this->once())
@@ -301,4 +299,69 @@ public function testMigrateMemorySqlite()
         ]) . PHP_EOL, $commandTester->getDisplay());
         $this->assertSame(AbstractCommand::CODE_SUCCESS, $exitCode);
     }
+
+    public function testExecuteWithDate(): void
+    {
+        $application = new PhinxApplication();
+        $application->add(new Migrate());
+
+        /** @var Migrate $command */
+        $command = $application->find('migrate');
+
+        // mock the manager class
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
+            ->setConstructorArgs([$this->config, $this->input, $this->output])
+            ->getMock();
+        $managerStub->expects($this->never())
+            ->method('migrate');
+        $managerStub->expects($this->once())
+            ->method('migrateToDateTime')
+            ->with('development', new DateTime('yesterday'), false);
+
+        $command->setConfig($this->config);
+        $command->setManager($managerStub);
+
+        $commandTester = new CommandTester($command);
+        $exitCode = $commandTester->execute(
+            ['command' => $command->getName(), '--environment' => 'development', '--date' => 'yesterday'],
+            ['decorated' => false]
+        );
+
+        $this->assertStringContainsString('using environment development', $commandTester->getDisplay());
+        $this->assertSame(AbstractCommand::CODE_SUCCESS, $exitCode);
+    }
+
+    public function testExecuteWithError(): void
+    {
+        $exception = new RuntimeException('oops');
+
+        $application = new PhinxApplication();
+        $application->add(new Migrate());
+
+        /** @var Migrate $command */
+        $command = $application->find('migrate');
+
+        // mock the manager class
+        /** @var Manager&MockObject $managerStub */
+        $managerStub = $this->getMockBuilder(Manager::class)
+            ->setConstructorArgs([$this->config, $this->input, $this->output])
+            ->getMock();
+        $managerStub->expects($this->once())
+            ->method('migrate')
+            ->willThrowException($exception);
+
+        $command->setConfig($this->config);
+        $command->setManager($managerStub);
+
+        $commandTester = new CommandTester($command);
+        $exitCode = $commandTester->execute(
+            ['command' => $command->getName(), '--environment' => 'development'],
+            ['decorated' => false, 'capture_stderr_separately' => true]
+        );
+
+        $this->assertStringContainsString('using environment development', $commandTester->getDisplay());
+        $this->assertStringContainsString('RuntimeException: oops', $commandTester->getErrorOutput());
+        $this->assertSame(AbstractCommand::CODE_ERROR, $exitCode);
+    }
 }