Skip to content

Commit

Permalink
Remove exit() from ConsoleErrorHandler
Browse files Browse the repository at this point in the history
This fixes a number of failing tests, and simplifies exception handling
in the console.
  • Loading branch information
markstory committed Sep 28, 2012
1 parent 80cab2d commit 88637b5
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 52 deletions.
2 changes: 1 addition & 1 deletion App/Console/cake.php
Expand Up @@ -19,4 +19,4 @@
*/
include dirname(__DIR__) . '/Config/bootstrap.php';

return Cake\Console\ShellDispatcher::run($argv);
exit(Cake\Console\ShellDispatcher::run($argv));
10 changes: 3 additions & 7 deletions lib/Cake/Console/ConsoleErrorHandler.php
@@ -1,9 +1,5 @@
<?php
/**
* ErrorHandler for Console Shells
*
* PHP 5
*
* CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
* Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org)
*
Expand All @@ -16,6 +12,7 @@
* @license MIT License (http://www.opensource.org/licenses/mit-license.php)
*/
namespace Cake\Console;

use Cake\Core\Configure;
use Cake\Error\ErrorHandler;

Expand Down Expand Up @@ -50,16 +47,15 @@ public static function getStderr() {
* Handle a exception in the console environment. Prints a message to stderr.
*
* @param Exception $exception The exception to handle
* @return void
* @return integer Exit code from exception caught.
*/
public static function handleException(\Exception $exception) {
$stderr = static::getStderr();
$stderr->write(__d('cake_console', "<error>Error:</error> %s\n%s",
$exception->getMessage(),
$exception->getTraceAsString()
));
// TODO this makes this method impossible to test.
exit($exception->getCode() ? $exception->getCode() : 1);
return $exception->getCode() ?: 1;
}

/**
Expand Down
31 changes: 24 additions & 7 deletions lib/Cake/Console/ShellDispatcher.php
Expand Up @@ -64,11 +64,11 @@ public function __construct($args = array(), $bootstrap = true) {
* Run the dispatcher
*
* @param array $argv The argv from PHP
* @return void
* @return integer The exit code of the shell process.
*/
public static function run($argv) {
$dispatcher = new ShellDispatcher($argv);
$dispatcher->_stop($dispatcher->dispatch() === false ? 1 : 0);
return $dispatcher->dispatch();
}

/**
Expand All @@ -82,7 +82,9 @@ protected function _initConstants() {
ini_set('implicit_flush', true);
ini_set('max_execution_time', 0);
}
define('CAKEPHP_SHELL', true);
if (!defined('CAKEPHP_SHELL')) {
define('CAKEPHP_SHELL', true);
}
}

/**
Expand Down Expand Up @@ -135,23 +137,38 @@ public function setErrorHandlers() {
$errorHandler = new ConsoleErrorHandler();
if (empty($error['consoleHandler'])) {
$error['consoleHandler'] = array($errorHandler, 'handleError');
Configure::write('error', $error);
Configure::write('Error', $error);
}
if (empty($exception['consoleHandler'])) {
$exception['consoleHandler'] = array($errorHandler, 'handleException');
Configure::write('exception', $exception);
Configure::write('Exception', $exception);
}
set_exception_handler($exception['consoleHandler']);
set_error_handler($error['consoleHandler'], Configure::read('Error.level'));
}

/**
* Dispatches a CLI request
*
* @return integer The cli command exit code. 0 is success.
*/
public function dispatch() {
try {
$exit = 0;
$this->_dispatch();
} catch (\Exception $e) {
$handler = Configure::read('Exception.consoleHandler');
$exit = $handler($e);
}
return $exit;
}

/**
* Dispatch a request.
*
* @return boolean
* @throws MissingShellMethodException
*/
public function dispatch() {
protected function _dispatch() {
$shell = $this->shiftArgs();

if (!$shell) {
Expand Down
2 changes: 1 addition & 1 deletion lib/Cake/Console/cake.php
Expand Up @@ -31,4 +31,4 @@
require $root . '/App/Config/bootstrap.php';
}
unset($root, $loaded, $appIndex, $dir);
return Cake\Console\ShellDispatcher::run($argv);
exit(Cake\Console\ShellDispatcher::run($argv));
34 changes: 9 additions & 25 deletions lib/Cake/Test/TestCase/Console/ConsoleErrorHandlerTest.php
@@ -1,9 +1,5 @@
<?php
/**
* ConsoleErrorHandler Test case
*
* PHP versions 5
*
* CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
* Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org)
*
Expand All @@ -12,11 +8,11 @@
*
* @copyright Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org)
* @link http://cakephp.org CakePHP(tm) Project
* @package Cake.Test.Case.Console
* @since CakePHP(tm) v 2.0
* @license MIT License (http://www.opensource.org/licenses/mit-license.php)
*/
namespace Cake\Test\TestCase\Console;

use Cake\Console\ConsoleErrorHandler;
use Cake\Error;
use Cake\TestSuite\TestCase;
Expand Down Expand Up @@ -72,11 +68,8 @@ public function testCakeErrors() {
ConsoleErrorHandler::$stderr->expects($this->once())->method('write')
->with($this->stringContains('Missing action'));

$this->Error->expects($this->once())
->method('_stop')
->with(404);

$this->Error->handleException($exception);
$result = $this->Error->handleException($exception);
$this->assertEquals(404, $result);
}

/**
Expand All @@ -90,11 +83,8 @@ public function testNonCakeExceptions() {
ConsoleErrorHandler::$stderr->expects($this->once())->method('write')
->with($this->stringContains('Too many parameters.'));

$this->Error->expects($this->once())
->method('_stop')
->with(1);

$this->Error->handleException($exception);
$result = $this->Error->handleException($exception);
$this->assertEquals(1, $result);
}

/**
Expand All @@ -108,11 +98,8 @@ public function testError404Exception() {
ConsoleErrorHandler::$stderr->expects($this->once())->method('write')
->with($this->stringContains('dont use me in cli.'));

$this->Error->expects($this->once())
->method('_stop')
->with(404);

$this->Error->handleException($exception);
$result = $this->Error->handleException($exception);
$this->assertEquals(404, $result);
}

/**
Expand All @@ -126,11 +113,8 @@ public function testError500Exception() {
ConsoleErrorHandler::$stderr->expects($this->once())->method('write')
->with($this->stringContains('dont use me in cli.'));

$this->Error->expects($this->once())
->method('_stop')
->with(500);

$this->Error->handleException($exception);
$result = $this->Error->handleException($exception);
$this->assertEquals(500, $result);
}

}
18 changes: 7 additions & 11 deletions lib/Cake/Test/TestCase/Console/ShellDispatcherTest.php
@@ -1,9 +1,5 @@
<?php
/**
* ShellDispatcherTest file
*
* PHP 5
*
* CakePHP(tm) Tests <http://book.cakephp.org/2.0/en/development/testing.html>
* Copyright 2005-2012, Cake Software Foundation, Inc.
*
Expand All @@ -12,11 +8,11 @@
*
* @copyright Copyright 2005-2012, Cake Software Foundation, Inc.
* @link http://book.cakephp.org/2.0/en/development/testing.html CakePHP(tm) Tests
* @package Cake.Test.Case.Console
* @since CakePHP(tm) v 1.2.0.5432
* @license MIT License (http://www.opensource.org/licenses/mit-license.php)
*/
namespace Cake\Test\TestCase\Console;

use Cake\Console\ShellDispatcher;
use Cake\Core\App;
use Cake\Core\Configure;
Expand Down Expand Up @@ -454,7 +450,7 @@ public function testDispatchShellWithMain() {

$Dispatcher->args = array('mock_with_main');
$result = $Dispatcher->dispatch();
$this->assertTrue($result);
$this->assertEquals(0, $result);
$this->assertEquals(array(), $Dispatcher->args);
}

Expand All @@ -480,7 +476,7 @@ public function testDispatchShellWithoutMain() {

$Dispatcher->args = array('mock_without_main', 'initdb');
$result = $Dispatcher->dispatch();
$this->assertTrue($result);
$this->assertEquals(0, $result);
}

/**
Expand All @@ -502,7 +498,7 @@ public function testDispatchNotAShellWithMain() {

$Dispatcher->args = array('mock_with_main_not_a');
$result = $Dispatcher->dispatch();
$this->assertTrue($result);
$this->assertEquals(0, $result);
$this->assertEquals(array(), $Dispatcher->args);

$Shell = new \MockWithMainNotAShell($Dispatcher);
Expand All @@ -513,7 +509,7 @@ public function testDispatchNotAShellWithMain() {

$Dispatcher->args = array('mock_with_main_not_a', 'initdb');
$result = $Dispatcher->dispatch();
$this->assertTrue($result);
$this->assertEquals(0, $result);
}

/**
Expand All @@ -535,7 +531,7 @@ public function testDispatchNotAShellWithoutMain() {

$Dispatcher->args = array('mock_without_main_not_a');
$result = $Dispatcher->dispatch();
$this->assertTrue($result);
$this->assertEquals(0, $result);
$this->assertEquals(array(), $Dispatcher->args);

$Shell = new \MockWithoutMainNotAShell($Dispatcher);
Expand All @@ -546,7 +542,7 @@ public function testDispatchNotAShellWithoutMain() {

$Dispatcher->args = array('mock_without_main_not_a', 'initdb');
$result = $Dispatcher->dispatch();
$this->assertTrue($result);
$this->assertEquals(0, $result);
}

/**
Expand Down

0 comments on commit 88637b5

Please sign in to comment.