From f626ccbcee1b67a13e14cae6584c21243b1ec79d Mon Sep 17 00:00:00 2001 From: Andrea Turso Date: Tue, 15 May 2012 21:25:18 +0100 Subject: [PATCH 1/3] Wrapped the listener invocation a try/catch block that catches any exception and throws a \RuntimeException. Added a test case for the EventDispatcher. Note: In order to test the doDispatch method I had to use a stub EventDispatcher with a getListeners that returned a preconfigured array. IMHO there should be a way to inject the listeners into the EventDispatcher. --- src/Composer/Script/EventDispatcher.php | 6 +- .../Test/Script/EventDispatcherTest.php | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/Composer/Test/Script/EventDispatcherTest.php diff --git a/src/Composer/Script/EventDispatcher.php b/src/Composer/Script/EventDispatcher.php index 83b4fb500..3b420ba70 100644 --- a/src/Composer/Script/EventDispatcher.php +++ b/src/Composer/Script/EventDispatcher.php @@ -91,7 +91,11 @@ class EventDispatcher throw new \UnexpectedValueException('Method '.$callable.' is not callable, can not call '.$event->getName().' script'); } - $className::$methodName($event); + try { + $className::$methodName($event); + } catch (\Exception $e) { + throw new \RuntimeException("'{$callable}' terminated with an exception."); + } } } diff --git a/tests/Composer/Test/Script/EventDispatcherTest.php b/tests/Composer/Test/Script/EventDispatcherTest.php new file mode 100644 index 000000000..b49e05fe1 --- /dev/null +++ b/tests/Composer/Test/Script/EventDispatcherTest.php @@ -0,0 +1,58 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Composer\Test\Script; + +use Exception; +use Composer\Test\TestCase; +use Composer\Script\Event; +use Composer\Script\EventDispatcher; + +/** + * + * @group + * @ticket #693 + * @author Andrea Turso + */ +class EventDispatcherTest extends TestCase +{ + /** + * @expectedException \RuntimeException + */ + public function testListenerExceptionsAreSuppressed() + { + $dispatcher = $this->getDispatcherStubForListenersTest(array( + "Composer\Test\Script\EventDispatcherTest::call" + )); + $dispatcher->dispatchCommandEvent("post-install-cmd"); + } + + private function getDispatcherStubForListenersTest($listeners) + { + $dispatcher = $this->getMockBuilder('Composer\Script\EventDispatcher') + ->setConstructorArgs(array( + $this->getMock('Composer\Composer'), + $this->getMock('Composer\IO\IOInterface'))) + ->setMethods(array('getListeners')) + ->getMock(); + + $dispatcher->expects($this->atLeastOnce()) + ->method('getListeners') + ->will($this->returnValue($listeners)); + + return $dispatcher; + } + + public static function call() + { + throw new Exception(); + } +} \ No newline at end of file From f8b2f203a1f62f1fcf6a529529f9b36c87c8128f Mon Sep 17 00:00:00 2001 From: Andrea Turso Date: Tue, 15 May 2012 21:47:17 +0100 Subject: [PATCH 2/3] Added exception class and message to the error string. --- src/Composer/Script/EventDispatcher.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Composer/Script/EventDispatcher.php b/src/Composer/Script/EventDispatcher.php index 3b420ba70..55519c803 100644 --- a/src/Composer/Script/EventDispatcher.php +++ b/src/Composer/Script/EventDispatcher.php @@ -94,7 +94,11 @@ class EventDispatcher try { $className::$methodName($event); } catch (\Exception $e) { - throw new \RuntimeException("'{$callable}' terminated with an exception."); + $message = "%s terminated with an exception.\n[%s] %s"; + throw new \RuntimeException(sprintf($message, + $callable, + get_class($e), + $e->getMessage())); } } } From 3a31b59473102af10bb3a8fd44de0b6ed5426e63 Mon Sep 17 00:00:00 2001 From: Andrea Turso Date: Wed, 16 May 2012 16:55:24 +0100 Subject: [PATCH 3/3] Minor changes to the EventDispatcherTest - Removed Exception class import - Added FQN at L60 - Added documentation - Added @group event-dispatcher --- tests/Composer/Test/Script/EventDispatcherTest.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/Composer/Test/Script/EventDispatcherTest.php b/tests/Composer/Test/Script/EventDispatcherTest.php index b49e05fe1..724eb182d 100644 --- a/tests/Composer/Test/Script/EventDispatcherTest.php +++ b/tests/Composer/Test/Script/EventDispatcherTest.php @@ -11,20 +11,24 @@ namespace Composer\Test\Script; -use Exception; use Composer\Test\TestCase; use Composer\Script\Event; use Composer\Script\EventDispatcher; /** + * Event Dispatcher Test Case * - * @group + * @group event-dispatcher * @ticket #693 * @author Andrea Turso */ class EventDispatcherTest extends TestCase { /** + * Test the doDispatch method properly catches any exception + * thrown from the listener invocation, i.e., $className::$methodName($event), + * and replaces it with a \RuntimeException. + * * @expectedException \RuntimeException */ public function testListenerExceptionsAreSuppressed() @@ -53,6 +57,6 @@ class EventDispatcherTest extends TestCase public static function call() { - throw new Exception(); + throw new \Exception(); } } \ No newline at end of file