From 6f1de11c5990c1b995413f5c94ddd32dc11c419e Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Thu, 23 Nov 2023 15:54:10 +0100 Subject: [PATCH] =?UTF-8?q?N=C2=B06976=20Fix=20DeprecatedCallsLog=20error?= =?UTF-8?q?=20handler=20never=20set=20(#576)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caused by N°6274 (disabling error handler when running phpunit) There is now a test testing the handler is really fixed when not in the phpunit context Also a TRACE log is made on setting the handler --- approot.inc.php | 6 +- core/log.class.inc.php | 6 +- .../src/BaseTestCase/ItopTestCase.php | 12 +++- .../DeprecatedCallsLogErrorHandlerTest.php | 70 +++++++++++++++++++ 4 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 tests/php-unit-tests/unitary-tests/core/Log/DeprecatedCallsLogErrorHandlerTest.php diff --git a/approot.inc.php b/approot.inc.php index 36a984ab1..0d6246062 100644 --- a/approot.inc.php +++ b/approot.inc.php @@ -26,8 +26,10 @@ define('ITOP_DESIGN_LATEST_VERSION', '3.0'); define('ITOP_CORE_VERSION', '3.0.3'); /** - * @since 3.0.4 N°6274 Allow to test if PHPUnit is currently running. Starting with PHPUnit 9.5 we'll be able to replace it with $GLOBALS['phpunit_version'] + * @var string + * @since 3.0.4 3.1.0 3.2.0 N°6274 Allow to test if PHPUnit is currently running. Starting with PHPUnit 9.5 we'll be able to replace it with $GLOBALS['phpunit_version'] + * @since 3.0.4 3.1.1 3.2.0 N°6976 Fix constant name (DeprecatedCallsLog error handler was never set) */ -define('ITOP_PHPUNIT_RUNNING_CONSTANT_NAME', 'ITOP_PHPUNIT_RUNNING'); +const ITOP_PHPUNIT_RUNNING_CONSTANT_NAME = 'ITOP_PHPUNIT_RUNNING'; require_once APPROOT.'bootstrap.inc.php'; diff --git a/core/log.class.inc.php b/core/log.class.inc.php index a0f5c7526..20344db7a 100644 --- a/core/log.class.inc.php +++ b/core/log.class.inc.php @@ -1080,9 +1080,13 @@ class DeprecatedCallsLog extends LogAPI parent::Enable($sTargetFile); if ( - (false === defined('ITOP_PHPUNIT_RUNNING_CONSTANT_NAME')) + ( + (false === defined(ITOP_PHPUNIT_RUNNING_CONSTANT_NAME)) + || (defined(ITOP_PHPUNIT_RUNNING_CONSTANT_NAME) && (constant(ITOP_PHPUNIT_RUNNING_CONSTANT_NAME) !== true)) + ) && static::IsLogLevelEnabledSafe(self::LEVEL_WARNING, self::ENUM_CHANNEL_PHP_LIBMETHOD) ) { + IssueLog::Trace('Setting '.static::class.' error handler to catch DEPRECATED', static::ENUM_CHANNEL_PHP_LIBMETHOD); set_error_handler([static::class, 'DeprecatedNoticesErrorHandler'], E_DEPRECATED | E_USER_DEPRECATED); } } diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index 2d5afafd6..d70578ef2 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -7,6 +7,7 @@ namespace Combodo\iTop\Test\UnitTest; use CMDBSource; +use DeprecatedCallsLog; use MySQLTransactionNotClosedException; use PHPUnit\Framework\TestCase; use SetupUtils; @@ -24,6 +25,12 @@ define('DEBUG_UNIT_TEST', true); abstract class ItopTestCase extends TestCase { public const TEST_LOG_DIR = 'test'; + + /** + * @var bool + * @since 3.0.4 3.1.1 3.2.0 N°6976 Allow to enable/disable {@see DeprecatedCallsLog} error handler + */ + public const DISABLE_DEPRECATEDCALLSLOG_ERRORHANDLER = true; public static $DEBUG_UNIT_TEST = false; protected static $aBackupStaticProperties = []; @@ -46,9 +53,10 @@ abstract class ItopTestCase extends TestCase require_once static::GetAppRoot() . 'approot.inc.php'; - if (false === defined('ITOP_PHPUNIT_RUNNING_CONSTANT_NAME')) { + if ((static::DISABLE_DEPRECATEDCALLSLOG_ERRORHANDLER) + && (false === defined(ITOP_PHPUNIT_RUNNING_CONSTANT_NAME))) { // setUp might be called multiple times, so protecting the define() call ! - define('ITOP_PHPUNIT_RUNNING_CONSTANT_NAME', true); + define(ITOP_PHPUNIT_RUNNING_CONSTANT_NAME, true); } } diff --git a/tests/php-unit-tests/unitary-tests/core/Log/DeprecatedCallsLogErrorHandlerTest.php b/tests/php-unit-tests/unitary-tests/core/Log/DeprecatedCallsLogErrorHandlerTest.php new file mode 100644 index 000000000..7e3a8aaa2 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/core/Log/DeprecatedCallsLogErrorHandlerTest.php @@ -0,0 +1,70 @@ +fail('Constant to disable error handler is set, so we cannot test :('); + } + + $sNoticeMessage = __METHOD__.uniqid(' @trigger_error unique message - ', true); + + // to check that error handler is really set + $oMockIssueLogFile = $this->createMock(FileLog::class); + $oMockIssueLogFile->expects($this->exactly(1)) + ->method(LogAPI::LEVEL_TRACE) + ->with($this->stringContains(DeprecatedCallsLog::class), DeprecatedCallsLog::ENUM_CHANNEL_PHP_LIBMETHOD, []); + + // to check the error handler is logging correctly + $oMockDeprecatedLogFile = $this->createMock(FileLog::class); + $oMockDeprecatedLogFile->expects($this->exactly(1)) + ->method(LogAPI::LEVEL_WARNING) + ->with($this->stringContains($sNoticeMessage), DeprecatedCallsLog::ENUM_CHANNEL_PHP_LIBMETHOD, []); + + $oMockConfig = $this->createMock(Config::class); + $oMockConfig + ->method("Get") + ->willReturnCallback(function ($sConfigParameterName) { + if ($sConfigParameterName==='log_level_min'){ + return [ + DeprecatedCallsLog::ENUM_CHANNEL_PHP_LIBMETHOD => LogAPI::LEVEL_TRACE + ]; + } + /** @noinspection NullPointerExceptionInspection */ + return utils::GetConfig()->Get($sConfigParameterName); + }); + + $this->RequireOnceItopFile('core/log.class.inc.php'); + IssueLog::Enable(APPROOT.'log/error.log'); // to get log when setting error handler + IssueLog::MockStaticObjects($oMockIssueLogFile, $oMockConfig); + DeprecatedCallsLog::Enable(); // will set error handler + DeprecatedCallsLog::MockStaticObjects($oMockDeprecatedLogFile, $oMockConfig); + + @trigger_error($sNoticeMessage, E_USER_DEPRECATED); + } +}