Merge pull request #1015 from goetas/allow-no-migration

Migrate returns 0 as exit codes for empty plans as in 2.x
doctrine · Jun 21, 2020 · 69eaf2c · 69eaf2c
2 parents 49490cf + bd985d3
commit 69eaf2c
Show file tree

Hide file tree

Showing 3 changed files with 131 additions and 47 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -13,9 +13,6 @@ please refer to the [Code BC breaks](#code-bc-breaks) section.
   use `migrations:list` instead.
 - The `--write-sql` option for `migrations:migrate` and `migrations:execute` does not imply dry-run anymore,  
 use the `--dry-run` parameter instead.  
-- The `migrations:migrate` command will return a non-zero exit code if there are no migrations to execute, 
-use the `--allow-no-migration` parameter to have a zero exit code.
-
 
 ## Migrations table
 

diff --git a/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php b/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php
@@ -140,10 +140,35 @@ protected function execute(InputInterface $input, OutputInterface $output) : int
             return 1;
         }
 
+        $migrationRepository = $this->getDependencyFactory()->getMigrationRepository();
+        if (count($migrationRepository->getMigrations()) === 0) {
+            $message = sprintf(
+                'The version "%s" couldn\'t be reached, there are no registered migrations.',
+                $versionAlias
+            );
+
+            if ($allowNoMigration) {
+                $this->io->warning($message);
+
+                return 0;
+            }
+
+            $this->io->error($message);
+
+            return 1;
+        }
+
         try {
             $version = $this->getDependencyFactory()->getVersionAliasResolver()->resolveVersionAlias($versionAlias);
-        } catch (UnknownMigrationVersion|NoMigrationsToExecute|NoMigrationsFoundWithCriteria $e) {
-            return $this->errorForAlias($versionAlias, $allowNoMigration);
+        } catch (UnknownMigrationVersion $e) {
+            $this->io->error(sprintf(
+                'Unknown version: %s',
+                OutputFormatter::escape($versionAlias)
+            ));
+
+            return 1;
+        } catch (NoMigrationsToExecute|NoMigrationsFoundWithCriteria $e) {
+            return $this->exitForAlias($versionAlias);
         }
 
         $planCalculator                = $this->getDependencyFactory()->getMigrationPlanCalculator();
@@ -157,7 +182,7 @@ protected function execute(InputInterface $input, OutputInterface $output) : int
         $plan = $planCalculator->getPlanUntilVersion($version);
 
         if (count($plan) === 0) {
-            return $this->errorForAlias($versionAlias, $allowNoMigration);
+            return $this->exitForAlias($versionAlias);
         }
 
         $this->getDependencyFactory()->getLogger()->notice(
@@ -213,33 +238,36 @@ private function checkExecutedUnavailableMigrations(
         return true;
     }
 
-    private function errorForAlias(string $versionAlias, bool $allowNoMigration) : int
+    private function exitForAlias(string $versionAlias) : int
     {
-        if (in_array($versionAlias, ['first', 'next', 'latest'], true) || strpos($versionAlias, 'current') === 0) {
-            $version = $this->getDependencyFactory()->getVersionAliasResolver()->resolveVersionAlias('current');
+        $version = $this->getDependencyFactory()->getVersionAliasResolver()->resolveVersionAlias('current');
 
+        // Allow meaningful message when latest version already reached.
+        if (in_array($versionAlias, ['current', 'latest', 'first'], true)) {
             $message = sprintf(
-                'The version "%s" couldn\'t be reached, you are at version "%s"',
+                'Already at the %s version ("%s")',
                 $versionAlias,
                 (string) $version
             );
 
-            if ($allowNoMigration) {
-                $this->io->warning($message);
-
-                return 0;
-            }
+            $this->io->success($message);
+        } elseif (in_array($versionAlias, ['next', 'prev'], true) || strpos($versionAlias, 'current') === 0) {
+            $message = sprintf(
+                'The version "%s" couldn\'t be reached, you are at version "%s"',
+                $versionAlias,
+                (string) $version
+            );
 
             $this->io->error($message);
+        } else {
+            $message = sprintf(
+                'You are already at version "%s"',
+                (string) $version
+            );
 
-            return 1;
+            $this->io->success($message);
         }
 
-        $this->io->error(sprintf(
-            'Unknown version: %s',
-            OutputFormatter::escape($versionAlias)
-        ));
-
-        return 1;
+        return 0;
     }
 }
diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php
@@ -36,6 +36,7 @@
 use Symfony\Component\Console\Helper\QuestionHelper;
 use Symfony\Component\Console\Tester\CommandTester;
 use function getcwd;
+use function in_array;
 use function sprintf;
 use function strpos;
 use function trim;
@@ -74,63 +75,121 @@ class MigrateCommandTest extends MigrationTestCase
 
     public function testTargetUnknownVersion() : void
     {
-        $result = new ExecutionResult(new Version('A'));
-        $this->storage->complete($result);
-
         $this->migrateCommandTester->execute(
-            ['version' => 'A'],
+            ['version' => 'B'],
             ['interactive' => false]
         );
 
-        self::assertStringContainsString('[ERROR] Unknown version: A', $this->migrateCommandTester->getDisplay(true));
+        self::assertStringContainsString('[ERROR] Unknown version: B', $this->migrateCommandTester->getDisplay(true));
         self::assertSame(1, $this->migrateCommandTester->getStatusCode());
     }
 
+    /**
+     * @return array<array<int, bool|int>>
+     */
+    public function getMigrateWithMigrationsOrWithout() : array
+    {
+        return [
+            // migrations available, allow-no-migrations, expected exit code
+            [false, false, 1],
+            [true, false, 0],
+            [false, true, 0],
+            [true, true, 0],
+        ];
+    }
+
+    /**
+     * @dataProvider getMigrateWithMigrationsOrWithout
+     */
+    public function testMigrateWhenNoMigrationsAvailable(bool $hasMigrations, bool $allowNoMigration, int $expectedExitCode) : void
+    {
+        $finder                    = $this->createMock(Finder::class);
+        $factory                   = $this->createMock(MigrationFactory::class);
+        $this->migrationRepository = new FilesystemMigrationsRepository([], [], $finder, $factory);
+        $this->dependencyFactory->setService(MigrationsRepository::class, $this->migrationRepository);
+
+        if ($hasMigrations) {
+            $migration = $this->createMock(AbstractMigration::class);
+            Helper::registerMigrationInstance($this->migrationRepository, new Version('A'), $migration);
+        }
+
+        $this->migrateCommandTester->execute(
+            ['--allow-no-migration' => $allowNoMigration],
+            ['interactive' => false]
+        );
+
+        if (! $hasMigrations) {
+            $display = trim($this->migrateCommandTester->getDisplay(true));
+            self::assertStringContainsString(
+                sprintf(
+                    '[%s] The version "latest" couldn\'t be reached, there are no registered migrations.',
+                    $allowNoMigration ? 'WARNING' : 'ERROR'
+                ),
+                $display
+            );
+        }
+
+        self::assertSame($expectedExitCode, $this->migrateCommandTester->getStatusCode());
+    }
+
     /**
      * @return array<array<bool|string|null>>
      */
     public function getTargetAliases() : array
     {
         return [
-            ['latest', true, 'A'],
-            ['latest', false, 'A'],
-            ['first', true, null],
-            ['first', false, null],
-            ['next', true, 'A'],
-            ['next', false, 'A'],
-            ['current+1', false, 'A'],
-            ['current+1', true, 'A'],
+            ['A', 'OK', 'A'], // already at A
+            ['latest', 'OK', 'A'], // already at latest
+            ['first', 'OK', null], // already at first
+            ['next', 'ERROR', 'A'], // already at latest, no next available
+            ['prev', 'ERROR', null], // no prev, already at first
+            ['current', 'OK', 'A'], // already at latest, always
+            ['current+1', 'ERROR', 'A'], // no current+1
         ];
     }
 
     /**
      * @dataProvider getTargetAliases
      */
-    public function testExecuteAtVersion(string $targetAlias, bool $allowNoMigration, ?string $executedMigration) : void
+    public function testExecuteAtVersion(string $targetAlias, string $level, ?string $executedMigration) : void
     {
         if ($executedMigration !== null) {
             $result = new ExecutionResult(new Version($executedMigration));
             $this->storage->complete($result);
         }
 
         $this->migrateCommandTester->execute(
-            [
-                'version' => $targetAlias,
-                '--allow-no-migration' => $allowNoMigration,
-            ],
+            ['version' => $targetAlias],
             ['interactive' => false]
         );
 
-        self::assertStringContainsString(
-            trim($this->migrateCommandTester->getDisplay(true)),
-            sprintf(
+        $display = trim($this->migrateCommandTester->getDisplay(true));
+        $aliases = ['current', 'latest', 'first'];
+
+        if (in_array($targetAlias, $aliases, true)) {
+            $message = sprintf(
+                '[%s] Already at the %s version ("%s")',
+                $level,
+                $targetAlias,
+                ($executedMigration ?? '0')
+            );
+        } elseif ($targetAlias === 'A') {
+            $message = sprintf(
+                '[%s] You are already at version "%s"',
+                $level,
+                $targetAlias
+            );
+        } else {
+            $message = sprintf(
                 '[%s] The version "%s" couldn\'t be reached, you are at version "%s"',
-                ($allowNoMigration ? 'WARNING' : 'ERROR'),
+                $level,
                 $targetAlias,
                 ($executedMigration ?? '0')
-            )
-        );
-        self::assertSame($allowNoMigration ? 0 : 1, $this->migrateCommandTester->getStatusCode());
+            );
+        }
+
+        self::assertStringContainsString($message, $display);
+        self::assertSame(0, $this->migrateCommandTester->getStatusCode());
     }
 
     public function testExecuteUnknownVersion() : void