Browse Source

Fix missing check.

We need to check the collection generated by both the app and plugin
hooks, as we don't yet have return types.
Mark Story 8 years ago
parent
commit
c1958d5100

+ 23 - 8
src/Console/CommandRunner.php

@@ -138,17 +138,12 @@ class CommandRunner implements EventDispatcherInterface
             'help' => HelpCommand::class,
         ]);
         $commands = $this->app->console($commands);
+        $this->checkCollection($commands, 'console');
+
         if ($this->app instanceof PluginApplicationInterface) {
             $commands = $this->app->pluginConsole($commands);
         }
-
-        if (!($commands instanceof CommandCollection)) {
-            $type = getTypeName($commands);
-            throw new RuntimeException(
-                "The application's `console` method did not return a CommandCollection." .
-                " Got '{$type}' instead."
-            );
-        }
+        $this->checkCollection($commands, 'pluginConsole');
         $this->dispatchEvent('Console.buildCommands', ['commands' => $commands]);
 
         if (empty($argv)) {
@@ -197,6 +192,26 @@ class CommandRunner implements EventDispatcherInterface
     }
 
     /**
+     * Check the created CommandCollection
+     *
+     * @param mixed $commands The CommandCollection to check, could be anything though.
+     * @param string $method The method that was used.
+     * @return void
+     * @throws \RuntimeException
+     * @deprecated 3.6.0 This method should be replaced with return types in 4.x
+     */
+    protected function checkCollection($commands, $method)
+    {
+        if (!($commands instanceof CommandCollection)) {
+            $type = getTypeName($commands);
+            throw new RuntimeException(
+                "The application's `{$method}` method did not return a CommandCollection." .
+                " Got '{$type}' instead."
+            );
+        }
+    }
+
+    /**
      * Get the application's event manager or the global one.
      *
      * @return \Cake\Event\EventManagerInterface

+ 2 - 1
tests/TestCase/Console/CommandCollectionTest.php

@@ -245,7 +245,8 @@ class CommandCollectionTest extends TestCase
      */
     public function testDiscoverPluginUnknown()
     {
-        $this->assertSame([], $collection = new CommandCollection());
+        $collection = new CommandCollection();
+        $this->assertSame([], $collection->discoverPlugin('Nope'));
     }
 
     /**

+ 18 - 0
tests/TestCase/Console/CommandRunnerTest.php

@@ -133,6 +133,24 @@ class CommandRunnerTest extends TestCase
     }
 
     /**
+     * Test that the console hook not returning a command collection
+     * raises an error.
+     *
+     * @return void
+     */
+    public function testRunPluginConsoleHookFailure()
+    {
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('The application\'s `pluginConsole` method did not return a CommandCollection.');
+        $app = $this->getMockBuilder(BaseApplication::class)
+            ->setMethods(['pluginConsole', 'middleware', 'bootstrap'])
+            ->setConstructorArgs([$this->config])
+            ->getMock();
+        $runner = new CommandRunner($app);
+        $runner->run(['cake', '-h']);
+    }
+
+    /**
      * Test that running with empty argv fails
      *
      * @return void