From 3b65e7260512c06cf01c3ec8fec1fcd9c6d4e792 Mon Sep 17 00:00:00 2001 From: Jens Hatlak Date: Wed, 16 Oct 2024 08:26:52 +0200 Subject: [PATCH 1/8] Use STDERR for errors (if available) --- src/Phinx/Console/Command/AbstractCommand.php | 16 ++++++++++++++-- src/Phinx/Console/Command/ListAliases.php | 18 ++++++++++-------- src/Phinx/Console/Command/Migrate.php | 7 +------ 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/Phinx/Console/Command/AbstractCommand.php b/src/Phinx/Console/Command/AbstractCommand.php index c53d95a9b..de5722654 100644 --- 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('The environment "%s" does not exist', $environment)); + self::getErrorOutput($output)->writeln(sprintf('The environment "%s" does not exist', $environment)); return false; } @@ -478,7 +479,7 @@ protected function writeInformationOutput(?string &$environment, OutputInterface } $output->writeln('using database ' . $name, $this->verbosityLevel); } else { - $output->writeln('Could not determine database name! Please specify a database name in your config file.'); + self::getErrorOutput($output)->writeln('Could not determine database name! Please specify a database name in your config file.'); 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 index ffe85e2bf..fc5889bb1 100644 --- a/src/Phinx/Console/Command/ListAliases.php +++ b/src/Phinx/Console/Command/ListAliases.php @@ -68,15 +68,17 @@ function ($alias, $class) use ($maxAliasLength, $maxClassLength) { ), $this->verbosityLevel ); - } else { - $output->writeln( - sprintf( - 'No aliases defined in %s', - Util::relativePath($this->config->getConfigFilePath()) - ) - ); + + return self::CODE_SUCCESS; } - return self::CODE_SUCCESS; + self::getErrorOutput($output)->writeln( + sprintf( + 'No aliases defined in %s', + Util::relativePath($this->config->getConfigFilePath()) + ) + ); + + return self::CODE_ERROR; } } diff --git a/src/Phinx/Console/Command/Migrate.php b/src/Phinx/Console/Command/Migrate.php index 6f9e7e143..699fa4a62 100644 --- 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('' . $e->__toString() . ''); - - return self::CODE_ERROR; } catch (Throwable $e) { - $output->writeln('' . $e->__toString() . ''); + self::getErrorOutput($output)->writeln('' . $e->__toString() . ''); return self::CODE_ERROR; } From 709482b7e59b5d9e82a0bde9906044ee997dfdc0 Mon Sep 17 00:00:00 2001 From: Jens Hatlak Date: Wed, 16 Oct 2024 08:30:55 +0200 Subject: [PATCH 2/8] Fix ListAliasesTest --- tests/Phinx/Console/Command/ListAliasesTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Phinx/Console/Command/ListAliasesTest.php b/tests/Phinx/Console/Command/ListAliasesTest.php index 0504da136..af1d9ca32 100644 --- a/tests/Phinx/Console/Command/ListAliasesTest.php +++ b/tests/Phinx/Console/Command/ListAliasesTest.php @@ -38,7 +38,8 @@ public function testListingAliases($file, $hasAliases) ], ['decorated' => false] ); - $this->assertEquals(AbstractCommand::CODE_SUCCESS, $exitCode); + $expectedExitCode = $hasAliases ? AbstractCommand::CODE_SUCCESS : AbstractCommand::CODE_ERROR; + $this->assertSame($expectedExitCode, $exitCode); $display = $commandTester->getDisplay(false); From 245cf62b0e475c32ae63fdf59ed0f7aaa0439e8c Mon Sep 17 00:00:00 2001 From: Jens Hatlak Date: Mon, 21 Oct 2024 08:02:56 +0200 Subject: [PATCH 3/8] Turn missing aliases into warning --- src/Phinx/Console/Command/ListAliases.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Phinx/Console/Command/ListAliases.php b/src/Phinx/Console/Command/ListAliases.php index fc5889bb1..d2e7fc374 100644 --- a/src/Phinx/Console/Command/ListAliases.php +++ b/src/Phinx/Console/Command/ListAliases.php @@ -72,13 +72,11 @@ function ($alias, $class) use ($maxAliasLength, $maxClassLength) { return self::CODE_SUCCESS; } - self::getErrorOutput($output)->writeln( - sprintf( - 'No aliases defined in %s', - Util::relativePath($this->config->getConfigFilePath()) - ) + $output->writeln( + 'warning no aliases defined in ' . Util::relativePath($this->config->getConfigFilePath()), + $this->verbosityLevel ); - return self::CODE_ERROR; + return self::CODE_SUCCESS; } } From 90ca13d42a9545fea2180c46a2783d0c7ffd70f3 Mon Sep 17 00:00:00 2001 From: Jens Hatlak Date: Mon, 21 Oct 2024 08:07:47 +0200 Subject: [PATCH 4/8] Fix ListAliasesTest --- tests/Phinx/Console/Command/ListAliasesTest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/Phinx/Console/Command/ListAliasesTest.php b/tests/Phinx/Console/Command/ListAliasesTest.php index af1d9ca32..27e0b346c 100644 --- a/tests/Phinx/Console/Command/ListAliasesTest.php +++ b/tests/Phinx/Console/Command/ListAliasesTest.php @@ -38,19 +38,18 @@ public function testListingAliases($file, $hasAliases) ], ['decorated' => false] ); - $expectedExitCode = $hasAliases ? AbstractCommand::CODE_SUCCESS : AbstractCommand::CODE_ERROR; - $this->assertSame($expectedExitCode, $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); } } } From 3e92fcaa79918016b6dd303a5b057403ce92f373 Mon Sep 17 00:00:00 2001 From: Jens Hatlak Date: Mon, 21 Oct 2024 09:10:38 +0200 Subject: [PATCH 5/8] Improve Migrate command coverage --- tests/Phinx/Console/Command/MigrateTest.php | 121 +++++++++++++++----- 1 file changed, 92 insertions(+), 29 deletions(-) diff --git a/tests/Phinx/Console/Command/MigrateTest.php b/tests/Phinx/Console/Command/MigrateTest.php index c948199d4..7ba4892b5 100644 --- 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); + } } From 5fa707ca55cf7dcb5acff69c6310894d4d5519a4 Mon Sep 17 00:00:00 2001 From: Jens Hatlak Date: Mon, 21 Oct 2024 09:11:32 +0200 Subject: [PATCH 6/8] Let PHPCS search PHPDoc annotations for uses --- phpcs.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/phpcs.xml b/phpcs.xml index 9ef5ab011..6040f3c23 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -4,6 +4,12 @@ + + + + + + src/ From 7f236adf378fcbe81665a1e0277c43bc79cc5259 Mon Sep 17 00:00:00 2001 From: Jens Hatlak Date: Mon, 21 Oct 2024 09:11:54 +0200 Subject: [PATCH 7/8] Fix undefined array key access --- src/Phinx/Db/Adapter/SQLiteAdapter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Phinx/Db/Adapter/SQLiteAdapter.php b/src/Phinx/Db/Adapter/SQLiteAdapter.php index 16cc10ee1..1dc1ddd3d 100644 --- 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 ''; } From c9de06370984fe3f3e4c9cfe40deb9d8595dab53 Mon Sep 17 00:00:00 2001 From: Jens Hatlak Date: Mon, 21 Oct 2024 09:17:07 +0200 Subject: [PATCH 8/8] Restore if/else/return flow --- src/Phinx/Console/Command/ListAliases.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Phinx/Console/Command/ListAliases.php b/src/Phinx/Console/Command/ListAliases.php index d2e7fc374..1f02f2fdc 100644 --- a/src/Phinx/Console/Command/ListAliases.php +++ b/src/Phinx/Console/Command/ListAliases.php @@ -68,15 +68,15 @@ function ($alias, $class) use ($maxAliasLength, $maxClassLength) { ), $this->verbosityLevel ); - - return self::CODE_SUCCESS; + } else { + $output->writeln( + 'warning no aliases defined in ' . Util::relativePath( + $this->config->getConfigFilePath() + ), + $this->verbosityLevel + ); } - $output->writeln( - 'warning no aliases defined in ' . Util::relativePath($this->config->getConfigFilePath()), - $this->verbosityLevel - ); - return self::CODE_SUCCESS; } }