From 4ce92f7488f3af4c37d8bcaf2188b890427581f4 Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 12:45:37 +0100 Subject: [PATCH 01/11] Move the TestLoggingHandler class to the classes/NanoBaseTest.class.php file --- classes/NanoBaseTest.class.php | 15 +++++++++++++++ tests/NanoLoggerTest.php | 13 +------------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/classes/NanoBaseTest.class.php b/classes/NanoBaseTest.class.php index 731508a1..7169a750 100644 --- a/classes/NanoBaseTest.class.php +++ b/classes/NanoBaseTest.class.php @@ -4,6 +4,8 @@ use PHPUnit\Framework\TestCase; use PHPUnit\Framework\MockObject\MockObject; +use Monolog\LogRecord; +use Monolog\Handler\SyslogHandler; /** * Base class for PHPUnit-based unit tests @@ -69,3 +71,16 @@ protected function setNanoAppMock(NanoObject $obj, MockObject $mock) $reflection_property->setValue($obj, $mock); } } + + +/** + * No-op logging handler. Keeps track of LogRecords sent to it. + */ +class TestLoggingHandler extends SyslogHandler +{ + public ?LogRecord $lastRecord = null; + protected function write(LogRecord $record): void + { + $this->lastRecord = $record; + } +} diff --git a/tests/NanoLoggerTest.php b/tests/NanoLoggerTest.php index b0571183..2a8d0aac 100644 --- a/tests/NanoLoggerTest.php +++ b/tests/NanoLoggerTest.php @@ -4,24 +4,13 @@ use Monolog\LogRecord; use Nano\Logger\NanoLogger; -/** - * No-op logging handler. Keeps track of LogRecords sent to it. - */ -class TestLoggingHandler extends Monolog\Handler\SyslogHandler -{ - public ?LogRecord $lastRecord = null; - protected function write(LogRecord $record): void - { - $this->lastRecord = $record; - } -} class NanoLoggerTest extends \Nano\NanoBaseTest { public function testGetLogger(): void { // register a global logging handler for easier testing - $handler = new TestLoggingHandler(ident: 'foo'); + $handler = new Nano\TestLoggingHandler(ident: 'foo'); NanoLogger::pushHandler($handler); $logger = NanoLogger::getLogger(name: __CLASS__, extraFields: ['foo'=>'bar']); From 4435d4292b07a983b57604804d3c95b50485c9db Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 12:46:43 +0100 Subject: [PATCH 02/11] DatabaseMysql: logger->pushProcessor accepts \Monolog\LogRecord now --- classes/database/DatabaseMysql.class.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/database/DatabaseMysql.class.php b/classes/database/DatabaseMysql.class.php index 2403bbc9..e26c4e9f 100644 --- a/classes/database/DatabaseMysql.class.php +++ b/classes/database/DatabaseMysql.class.php @@ -80,8 +80,8 @@ protected function doConnect() if ($res) { // add more information to Monolog logs - $this->logger->pushProcessor(function ($record) use ($hostInfo) { - $record['extra']['database'] = [ + $this->logger->pushProcessor(function (\Monolog\LogRecord $record) use ($hostInfo) { + $record->extra['database'] = [ 'host' => $hostInfo, 'name' => $this->name, ]; From 37d512d84d8a106dee98f6f61f49bf1e3cdc8247 Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 12:54:46 +0100 Subject: [PATCH 03/11] NanoScriptTest: test init() method exceptions handling --- classes/NanoScript.class.php | 30 +++++++++++------------------- tests/NanoScriptTest.php | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 19 deletions(-) create mode 100644 tests/NanoScriptTest.php diff --git a/classes/NanoScript.class.php b/classes/NanoScript.class.php index b73b9071..8878b85b 100644 --- a/classes/NanoScript.class.php +++ b/classes/NanoScript.class.php @@ -11,8 +11,8 @@ abstract class NanoScript extends NanoObject { const LOGFILE = 'debug'; - private $isDebug = false; - private $arguments = []; + private bool $isDebug = false; + private array $arguments = []; /** * @param NanoApp $app @@ -24,17 +24,14 @@ public function __construct(NanoApp $app) $this->arguments = $argv; $this->isDebug = (bool) getenv('DEBUG'); - parent::__construct(); if ($this->isInDebugMode()) { - $this->debug->log(); - $this->debug->log('Running in debug mode'); - $this->debug->log(); + $this->logger->debug('Running in debug mode'); } - $this->logger->pushProcessor(function ($record) { - $record['extra']['script_class'] = get_class($this); + $this->logger->pushProcessor(function (\Monolog\LogRecord $record) { + $record->extra['script_class'] = get_class($this); return $record; }); @@ -42,15 +39,15 @@ public function __construct(NanoApp $app) try { $this->init(); - } catch (Exception $e) { - $this->logger->error(__METHOD__ . ' failed', [ + } catch (Throwable $e) { + $this->logger->error(get_class($this) . '::init() failed', [ 'exception' => $e, ]); } } /** - * Setup the script + * Set up the script */ protected function init() { @@ -63,10 +60,8 @@ abstract public function run(); /** * Called when the script execution is completed - * - * @param NanoApp $app */ - public function onTearDown(NanoApp $app) + public function onTearDown(NanoApp $app): void { $this->logger->info('Script completed', [ 'time' => $app->getResponse()->getResponseTime() * 1000, // [ms] @@ -80,18 +75,15 @@ public function onTearDown(NanoApp $app) * * @return bool is script run in debug mode? */ - protected function isInDebugMode() + protected function isInDebugMode(): bool { return $this->isDebug; } /** * Was given option passed in command line as "--option"? - * - * @param string $opt - * @return bool */ - protected function hasOption($opt) + protected function hasOption(string $opt): bool { $opt = sprintf('--%s', $opt); return in_array($opt, $this->arguments); diff --git a/tests/NanoScriptTest.php b/tests/NanoScriptTest.php new file mode 100644 index 00000000..8d1d31da --- /dev/null +++ b/tests/NanoScriptTest.php @@ -0,0 +1,36 @@ +foo); + } + + public function run() + { + } +} +class NanoScriptTest extends \Nano\NanoBaseTest +{ + public function testInitExceptions() + { + // register a global logging handler for easier testing + $handler = new Nano\TestLoggingHandler(ident: 'foo'); + NanoLogger::pushHandler($handler); + + $instance = Nano::script(__DIR__ . '/..', TestScript::class); + $this->assertInstanceOf(TestScript::class, $instance); + + // our init method failed, there should be a log message about it + $this->assertInstanceOf(\Monolog\LogRecord::class, $handler->lastRecord); + + $this->assertEquals('TestScript::init() failed', $handler->lastRecord->message); + $this->assertInstanceOf(TypeError::class, $handler->lastRecord->context['exception']); + } +} From 3dc2be5908a636f19abae0c11a9b1c8e89afefee Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 13:00:48 +0100 Subject: [PATCH 04/11] Do have the log/ directory for tests --- .gitignore | 1 + log/.gitkeep | 0 2 files changed, 1 insertion(+) create mode 100644 log/.gitkeep diff --git a/.gitignore b/.gitignore index 7c932886..873b8934 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ composer.phar .phpunit.result.cache .phpunit.cache/ +log/* vendor/ .idea/ .php-cs-fixer.cache diff --git a/log/.gitkeep b/log/.gitkeep new file mode 100644 index 00000000..e69de29b From 8b60c8629d1c0ec970579df1611a8b77db5e1d72 Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 13:01:06 +0100 Subject: [PATCH 05/11] phpunit.xml: show warnings details --- phpunit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit.xml b/phpunit.xml index 95c01459..02a5755b 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,6 +1,6 @@ - + From 85f27cdac6f425bf539cec40dcda76e2a1ae514c Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 13:01:30 +0100 Subject: [PATCH 06/11] NanoScript will now re-throw exceptions from init() method --- classes/NanoScript.class.php | 3 +++ tests/NanoScriptTest.php | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/classes/NanoScript.class.php b/classes/NanoScript.class.php index 8878b85b..5367c883 100644 --- a/classes/NanoScript.class.php +++ b/classes/NanoScript.class.php @@ -16,6 +16,7 @@ abstract class NanoScript extends NanoObject /** * @param NanoApp $app + * @throws Throwable */ public function __construct(NanoApp $app) { @@ -43,6 +44,8 @@ public function __construct(NanoApp $app) $this->logger->error(get_class($this) . '::init() failed', [ 'exception' => $e, ]); + + throw $e; } } diff --git a/tests/NanoScriptTest.php b/tests/NanoScriptTest.php index 8d1d31da..72172428 100644 --- a/tests/NanoScriptTest.php +++ b/tests/NanoScriptTest.php @@ -24,11 +24,14 @@ public function testInitExceptions() $handler = new Nano\TestLoggingHandler(ident: 'foo'); NanoLogger::pushHandler($handler); - $instance = Nano::script(__DIR__ . '/..', TestScript::class); - $this->assertInstanceOf(TestScript::class, $instance); + try { + Nano::script(__DIR__ . '/..', TestScript::class); + } catch (Throwable) { + } // our init method failed, there should be a log message about it $this->assertInstanceOf(\Monolog\LogRecord::class, $handler->lastRecord); + $this->assertEquals(\Monolog\Level::Error, $handler->lastRecord->level); $this->assertEquals('TestScript::init() failed', $handler->lastRecord->message); $this->assertInstanceOf(TypeError::class, $handler->lastRecord->context['exception']); From 25207c30254a5b80584ec48101cb017e7adf43ba Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 13:11:35 +0100 Subject: [PATCH 07/11] More tests in NanoScriptTest --- tests/NanoScriptTest.php | 47 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/tests/NanoScriptTest.php b/tests/NanoScriptTest.php index 72172428..36cfa9e6 100644 --- a/tests/NanoScriptTest.php +++ b/tests/NanoScriptTest.php @@ -2,7 +2,7 @@ use Nano\Logger\NanoLogger; -class TestScript extends NanoScript +class FailingTestScript extends NanoScript { private ?array $foo = null; @@ -16,16 +16,39 @@ public function run() { } } + +class WorkingTestScript extends FailingTestScript +{ + public static ?Throwable $throw = null; + + protected function init(): int + { + return 0; + } + + /** + * @return int + * @throws Throwable + */ + public function run(): int + { + if (self::$throw) { + throw self::$throw; + } + return 42; + } +} + class NanoScriptTest extends \Nano\NanoBaseTest { - public function testInitExceptions() + public function testFailingTestScriptInitException() { // register a global logging handler for easier testing $handler = new Nano\TestLoggingHandler(ident: 'foo'); NanoLogger::pushHandler($handler); try { - Nano::script(__DIR__ . '/..', TestScript::class); + Nano::script(__DIR__ . '/..', FailingTestScript::class); } catch (Throwable) { } @@ -33,7 +56,23 @@ public function testInitExceptions() $this->assertInstanceOf(\Monolog\LogRecord::class, $handler->lastRecord); $this->assertEquals(\Monolog\Level::Error, $handler->lastRecord->level); - $this->assertEquals('TestScript::init() failed', $handler->lastRecord->message); + $this->assertEquals('FailingTestScript::init() failed', $handler->lastRecord->message); $this->assertInstanceOf(TypeError::class, $handler->lastRecord->context['exception']); } + + /** + * @throws Throwable + */ + public function testWorkingTestScript() + { + $instance = Nano::script(__DIR__ . '/..', WorkingTestScript::class); + $this->assertInstanceOf(WorkingTestScript::class, $instance); + $this->assertEquals(42, $instance->run()); + + // make the run() method throw an exception + $ex = new Exception('foo'); + WorkingTestScript::$throw = $ex; + $this->expectExceptionObject($ex); + $instance->run(); + } } From 6fe6ef85cadbf4f1b8f11a3a424e7d84ab15b12f Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 13:24:35 +0100 Subject: [PATCH 08/11] NanoScript: add runAndCatch() helper --- classes/NanoScript.class.php | 18 ++++++++++++++++++ tests/NanoScriptTest.php | 22 ++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/classes/NanoScript.class.php b/classes/NanoScript.class.php index 5367c883..6342cbb8 100644 --- a/classes/NanoScript.class.php +++ b/classes/NanoScript.class.php @@ -61,6 +61,24 @@ protected function init() */ abstract public function run(); + /** + * Runs the script (by calling the run method), but wraps the call in the exceptions handling code. + * + * @throws Throwable + */ + public function runAndCatch(): mixed + { + try { + return $this->run(); + } catch(Throwable $e) { + $message = get_class($this) . '::run() failed'; + $this->logger->error($message, ['exception' => $e]); + + echo "{$message}:\n{$e->getMessage()}\n\n{$e->getTraceAsString()}\n"; + throw $e; + } + } + /** * Called when the script execution is completed */ diff --git a/tests/NanoScriptTest.php b/tests/NanoScriptTest.php index 36cfa9e6..67f01015 100644 --- a/tests/NanoScriptTest.php +++ b/tests/NanoScriptTest.php @@ -75,4 +75,26 @@ public function testWorkingTestScript() $this->expectExceptionObject($ex); $instance->run(); } + + public function testWorkingTestScriptRunAndCatchException() + { + // make the run() method throw an exception + $ex = new Exception('foo'); + WorkingTestScript::$throw = $ex; + + // register a global logging handler for easier testing + $handler = new Nano\TestLoggingHandler(ident: 'foo'); + NanoLogger::pushHandler($handler); + + try { + Nano::script(__DIR__ . '/..', WorkingTestScript::class)->runAndCatch(); + } catch(Throwable $e) { + $this->assertEquals($ex->getMessage(), $e->getMessage()); + } + + // assert the logging + $this->assertEquals('WorkingTestScript::run() failed', $handler->lastRecord->message); + $this->assertEquals(Monolog\Level::Error, $handler->lastRecord->level); + $this->assertEquals($ex->getMessage(), $handler->lastRecord->context['exception']['message']); + } } From b6130927b8186ddf9a96263d8a8ec093a248ff43 Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 13:33:32 +0100 Subject: [PATCH 09/11] runAndCatch() should pass the value returned by the run() method --- classes/NanoScript.class.php | 2 +- tests/NanoScriptTest.php | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/classes/NanoScript.class.php b/classes/NanoScript.class.php index 6342cbb8..58dea924 100644 --- a/classes/NanoScript.class.php +++ b/classes/NanoScript.class.php @@ -66,7 +66,7 @@ abstract public function run(); * * @throws Throwable */ - public function runAndCatch(): mixed + final public function runAndCatch(): mixed { try { return $this->run(); diff --git a/tests/NanoScriptTest.php b/tests/NanoScriptTest.php index 67f01015..14d67dea 100644 --- a/tests/NanoScriptTest.php +++ b/tests/NanoScriptTest.php @@ -41,6 +41,11 @@ public function run(): int class NanoScriptTest extends \Nano\NanoBaseTest { + public function setUp(): void + { + WorkingTestScript::$throw = null; + } + public function testFailingTestScriptInitException() { // register a global logging handler for easier testing @@ -97,4 +102,13 @@ public function testWorkingTestScriptRunAndCatchException() $this->assertEquals(Monolog\Level::Error, $handler->lastRecord->level); $this->assertEquals($ex->getMessage(), $handler->lastRecord->context['exception']['message']); } + + /** + * @throws Throwable + */ + public function testWorkingTestScriptRunAndCatchReturnsTheValue() + { + $ret = Nano::script(__DIR__ . '/..', WorkingTestScript::class)->runAndCatch(); + $this->assertEquals(42, $ret); + } } From 6313816fc35fcb5e3fd69f8dab4a6bfcf61b09e5 Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 13:36:24 +0100 Subject: [PATCH 10/11] NanoScriptTest: assert that the Logger's extra 'script_class' field is set properly --- tests/NanoScriptTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/NanoScriptTest.php b/tests/NanoScriptTest.php index 14d67dea..6eb5ecb5 100644 --- a/tests/NanoScriptTest.php +++ b/tests/NanoScriptTest.php @@ -35,6 +35,8 @@ public function run(): int if (self::$throw) { throw self::$throw; } + + $this->logger->info('Hi!'); return 42; } } @@ -108,7 +110,16 @@ public function testWorkingTestScriptRunAndCatchException() */ public function testWorkingTestScriptRunAndCatchReturnsTheValue() { + // register a global logging handler for easier testing + $handler = new Nano\TestLoggingHandler(ident: 'foo'); + NanoLogger::pushHandler($handler); + $ret = Nano::script(__DIR__ . '/..', WorkingTestScript::class)->runAndCatch(); $this->assertEquals(42, $ret); + + // assert the logging + $this->assertEquals('Hi!', $handler->lastRecord->message); + $this->assertEquals(Monolog\Level::Info, $handler->lastRecord->level); + $this->assertEquals(WorkingTestScript::class, $handler->lastRecord->extra['script_class']); } } From 5a3ed6a7554690d1b7439ddd64e31114bcac8406 Mon Sep 17 00:00:00 2001 From: macbre Date: Thu, 10 Aug 2023 13:42:24 +0100 Subject: [PATCH 11/11] Make the linter happy 1) tests/NanoLoggerTest.php (single_line_after_imports) --- tests/NanoLoggerTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/NanoLoggerTest.php b/tests/NanoLoggerTest.php index 2a8d0aac..ec137619 100644 --- a/tests/NanoLoggerTest.php +++ b/tests/NanoLoggerTest.php @@ -4,7 +4,6 @@ use Monolog\LogRecord; use Nano\Logger\NanoLogger; - class NanoLoggerTest extends \Nano\NanoBaseTest { public function testGetLogger(): void