Prevent vendor deprecations to break the test suite

glpi-project · Nov 26, 2024 · 1977788 · 1977788
1 parent eb1429a
commit 1977788
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 53 deletions.
diff --git a/phpunit/functional/TicketTest.php b/phpunit/functional/TicketTest.php
@@ -7579,7 +7579,10 @@ public function testDynamicProperties(): void
     {
         $ticket = new \Ticket();
 
+        $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
         $ticket->plugin_xxx_data = 'test';
+        \error_reporting($reporting_level); // restore previous level
+
         $this->hasPhpLogRecordThatContains(
             'Creation of dynamic property Ticket::$plugin_xxx_data is deprecated',
             LogLevel::INFO

diff --git a/phpunit/functional/ToolboxTest.php b/phpunit/functional/ToolboxTest.php
@@ -1080,7 +1080,10 @@ public function testIsValidWebUrl(string $url, bool $result)
 
     public function testDeprecated()
     {
+        $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
         \Toolbox::deprecated('Calling this function is deprecated');
+        \error_reporting($reporting_level); // restore previous level
+
         $this->hasPhpLogRecordThatContains(
             'Calling this function is deprecated',
             LogLevel::INFO
@@ -1090,7 +1093,10 @@ public function testDeprecated()
     public function testDeprecatedPast()
     {
         // Test planned deprecation in the past
+        $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
         \Toolbox::deprecated('Calling this function is deprecated', true, '10.0');
+        \error_reporting($reporting_level); // restore previous level
+
         $this->hasPhpLogRecordThatContains(
             'Calling this function is deprecated',
             LogLevel::INFO
@@ -1100,7 +1106,10 @@ public function testDeprecatedPast()
     public function testDeprecatedCurrent()
     {
         // Test planned deprecation in current version
+        $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
         \Toolbox::deprecated('Calling this function is deprecated', true, GLPI_VERSION);
+        \error_reporting($reporting_level); // restore previous level
+
         $this->hasPhpLogRecordThatContains(
             'Calling this function is deprecated',
             LogLevel::INFO
@@ -1110,7 +1119,10 @@ public function testDeprecatedCurrent()
     public function testFutureDeprecated()
     {
         // Test planned deprecation in the future does NOT throw an error
+        $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
         \Toolbox::deprecated('Calling this function is deprecated', true, '99.0');
+        \error_reporting($reporting_level); // restore previous level
+
         $this->assertTrue(true); //non empty test
     }
 

diff --git a/src/Glpi/Application/ErrorHandler.php b/src/Glpi/Application/ErrorHandler.php
@@ -63,7 +63,7 @@ class ErrorHandler
         E_USER_ERROR        => LogLevel::ERROR,
         E_USER_WARNING      => LogLevel::WARNING,
         E_USER_NOTICE       => LogLevel::NOTICE,
-        E_STRICT            => LogLevel::NOTICE,
+        2048                => LogLevel::NOTICE, // 2048 = deprecated E_STRICT
         E_RECOVERABLE_ERROR => LogLevel::ERROR,
         E_DEPRECATED        => LogLevel::INFO,
         E_USER_DEPRECATED   => LogLevel::INFO,
@@ -227,9 +227,34 @@ public function register(): void
             set_exception_handler([$this, 'handleException']);
         }
 
-        // Force reporting of all errors, to ensure that all log levels that are supposed to be
-        // pushed in logs according to `GLPI_LOG_LVL`/`GLPI_ENVIRONMENT_TYPE` are actually reported.
-        error_reporting(E_ALL);
+        // Adjust reporting level to the environment, to ensure that all the errors supposed to be logged are
+        // actually reported, and to prevent reporting other errors.
+        $reporting_level = E_ALL;
+        foreach (self::ERROR_LEVEL_MAP as $value => $log_level) {
+            if (
+                $this->env !== GLPI::ENV_DEVELOPMENT
+                && in_array($log_level, [LogLevel::DEBUG, LogLevel::INFO])
+            ) {
+                // Do not report debug and info messages unless in development env.
+                // Suppressing the INFO level will prevent deprecations to be pushed in other environments logs.
+                //
+                // Suppressing the deprecations in the testing environment is mandatory to prevent deprecations
+                // triggered in vendor code to make our test suite fail.
+                // We may review this part once we will have migrate all our test suite on PHPUnit.
+                // For now, we rely on PHPStan to detect usages of deprecated code.
+                $reporting_level = $reporting_level & ~$value;
+            }
+
+            if (
+                !in_array($this->env, [GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING], true)
+                && $log_level === LogLevel::NOTICE
+            ) {
+                // Do not report notice messages unless in development/testing env.
+                // Notices are errors with no functional impact, so we do not want people to report them as issues.
+                $reporting_level = $reporting_level & ~$value;
+            }
+        }
+        error_reporting($reporting_level);
 
         // Disable native error displaying as it will be handled by `self::outputDebugMessage()`.
         ini_set('display_errors', 'Off');
@@ -435,15 +460,6 @@ private function outputDebugMessage(string $error_type, string $message, string
             return;
         }
 
-        if (
-            in_array($this->env, [GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION])
-            && in_array($log_level, [LogLevel::DEBUG, LogLevel::INFO])
-        ) {
-            // On staging/production environment, do not output debug/info messages.
-            // These messages are not supposed to be intended for end users, as they have no functional impact.
-            return;
-        }
-
         $is_dev_env         = $this->env === GLPI::ENV_DEVELOPMENT;
         $is_debug_mode      = isset($_SESSION['glpi_use_mode']) && $_SESSION['glpi_use_mode'] == \Session::DEBUG_MODE;
         $is_console_context = $this->output_handler instanceof OutputInterface;
@@ -513,7 +529,7 @@ private function codeToString(int $error_code): string
             E_USER_ERROR        => 'User Error',
             E_USER_WARNING      => 'User Warning',
             E_USER_NOTICE       => 'User Notice',
-            E_STRICT            => 'Runtime Notice',
+            2048                => 'Runtime Notice', // 2048 = deprecated E_STRICT
             E_RECOVERABLE_ERROR => 'Catchable Fatal Error',
             E_DEPRECATED        => 'Deprecated function',
             E_USER_DEPRECATED   => 'User deprecated function',

diff --git a/tests/functional/DBmysqlIterator.php b/tests/functional/DBmysqlIterator.php
@@ -1635,7 +1635,9 @@ public function testConvertOldRequestArgsToCriteria(array $params, array $expect
         $result = null;
         $this->when(
             function () use ($iterator, $params, &$result) {
+                $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
                 $result = $this->callPrivateMethod($iterator, 'convertOldRequestArgsToCriteria', $params, 'test');
+                \error_reporting($reporting_level); // restore previous level
             }
         )
          ->error
@@ -1665,7 +1667,9 @@ public function testExecuteWithOldSignature(array $params, array $expected, stri
         $iterator = new \DBmysqlIterator($db);
         $this->when(
             function () use ($iterator, $params) {
+                $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
                 $iterator = $iterator->execute(...$params);
+                \error_reporting($reporting_level); // restore previous level
             }
         )
          ->error
@@ -1701,7 +1705,9 @@ public function testBuildQueryWithOldSignature(array $params, array $expected, s
         $iterator = new \DBmysqlIterator($db);
         $this->when(
             function () use ($iterator, $params) {
+                $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
                 $iterator = $iterator->buildQuery(...$params);
+                \error_reporting($reporting_level); // restore previous level
             }
         )
          ->error

diff --git a/tests/functional/Glpi/Application/ErrorHandler.php b/tests/functional/Glpi/Application/ErrorHandler.php
@@ -52,6 +52,9 @@ protected function errorProvider(): iterable
 
         foreach ([Session::NORMAL_MODE, Session::DEBUG_MODE] as $session_mode) {
             foreach ([GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING, GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION] as $env) {
+                $suppress_info   = $env !== GLPI::ENV_DEVELOPMENT;
+                $suppress_notice = !in_array($env, [GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING], true);
+
                 yield [
                     'session_mode'         => $session_mode,
                     'env'                  => $env,
@@ -82,8 +85,8 @@ protected function errorProvider(): iterable
                     'error_call'           => function () {
                         trigger_error('some notice', E_USER_NOTICE);
                     },
-                    'expected_log_level'   => Level::Notice,
-                    'expected_msg_pattern' => $log_prefix
+                    'expected_log_level'   => $suppress_notice ? null : Level::Notice,
+                    'expected_msg_pattern' => $suppress_notice ? null : $log_prefix
                         . preg_quote('PHP User Notice (' . E_USER_NOTICE . '): some notice', '/')
                         . $log_suffix,
                 ];
@@ -94,8 +97,8 @@ protected function errorProvider(): iterable
                     'error_call'           => function () {
                         trigger_error('this method is deprecated', E_USER_DEPRECATED);
                     },
-                    'expected_log_level'   => Level::Info,
-                    'expected_msg_pattern' => $log_prefix
+                    'expected_log_level'   => $suppress_info ? null : Level::Info,
+                    'expected_msg_pattern' => $suppress_info ? null : $log_prefix
                         . preg_quote('PHP User deprecated function (' . E_USER_DEPRECATED . '): this method is deprecated', '/')
                         . $log_suffix,
                 ];
@@ -107,8 +110,8 @@ protected function errorProvider(): iterable
                         $param = new \ReflectionParameter([\Config::class, 'getTypeName'], 0);
                         $param->isCallable();
                     },
-                    'expected_log_level'   => Level::Info,
-                    'expected_msg_pattern' => $log_prefix
+                    'expected_log_level'   => $suppress_info ? null : Level::Info,
+                    'expected_msg_pattern' => $suppress_info ? null : $log_prefix
                         . preg_quote('PHP Deprecated function (' . E_DEPRECATED . '): Method ReflectionParameter::isCallable() is deprecated', '/')
                         . $log_suffix,
                 ];
@@ -128,13 +131,13 @@ public function testRegisteredErrorHandler(
         int $session_mode,
         string $env,
         callable $error_call,
-        Level $expected_log_level,
-        string $expected_msg_pattern
+        ?Level $expected_log_level,
+        ?string $expected_msg_pattern
     ) {
         $handler = new TestHandler();
         $logger = $this->newMockInstance('Monolog\\Logger', null, null, ['test-logger', [$handler]]);
 
-        $previous_use_mode         = $_SESSION['glpi_use_mode'];
+        $previous_use_mode = $_SESSION['glpi_use_mode'];
 
         $error_handler = $this->callPrivateConstructor(
             \Glpi\Application\ErrorHandler::class,
@@ -154,15 +157,13 @@ public function testRegisteredErrorHandler(
         $_SESSION['glpi_use_mode'] = $session_mode;
         $error_call();
         $_SESSION['glpi_use_mode'] = $previous_use_mode;
-        $this->integer(count($handler->getRecords()))->isEqualTo(1);
-        $this->boolean($handler->hasRecordThatMatches($expected_msg_pattern, $expected_log_level));
-        if (
-            $env === GLPI::ENV_DEVELOPMENT
-            || (
-                $session_mode === Session::DEBUG_MODE
-                && ($expected_log_level->isHigherThan(Level::Info)  || !in_array($env, [GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION]))
-            )
-        ) {
+        if ($expected_log_level !== null) {
+            $this->integer(count($handler->getRecords()))->isEqualTo(1);
+            $this->boolean($handler->hasRecordThatMatches($expected_msg_pattern, $expected_log_level));
+        } else {
+            $this->integer(count($handler->getRecords()))->isEqualTo(0);
+        }
+        if ($expected_msg_pattern !== null && ($env === GLPI::ENV_DEVELOPMENT || $session_mode === Session::DEBUG_MODE)) {
             $this->output->matches($expected_msg_pattern);
         } else {
             $this->output->isEmpty();
@@ -177,6 +178,9 @@ protected function handleErrorProvider(): iterable
 
         foreach ([Session::NORMAL_MODE, Session::DEBUG_MODE] as $session_mode) {
             foreach ([GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING, GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION] as $env) {
+                $suppress_info   = $env !== GLPI::ENV_DEVELOPMENT;
+                $suppress_notice = !in_array($env, [GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING], true);
+
                 yield [
                     'session_mode'         => $session_mode,
                     'env'                  => $env,
@@ -190,8 +194,8 @@ protected function handleErrorProvider(): iterable
                     'session_mode'         => $session_mode,
                     'env'                  => $env,
                     'error_code'           => E_NOTICE,
-                    'expected_log_level'   => Level::Notice,
-                    'expected_msg_pattern' => $log_prefix . 'PHP Notice' . $log_suffix,
+                    'expected_log_level'   => $suppress_notice ? null : Level::Notice,
+                    'expected_msg_pattern' => $suppress_notice ? null : $log_prefix . 'PHP Notice' . $log_suffix,
                     'is_fatal_error'       => false,
                 ];
 
@@ -226,26 +230,26 @@ protected function handleErrorProvider(): iterable
                     'session_mode'         => $session_mode,
                     'env'                  => $env,
                     'error_code'           => E_USER_NOTICE,
-                    'expected_log_level'   => Level::Notice,
-                    'expected_msg_pattern' => $log_prefix . 'PHP User Notice' . $log_suffix,
+                    'expected_log_level'   => $suppress_notice ? null : Level::Notice,
+                    'expected_msg_pattern' => $suppress_notice ? null : $log_prefix . 'PHP User Notice' . $log_suffix,
                     'is_fatal_error'       => false,
                 ];
 
                 yield [
                     'session_mode'         => $session_mode,
                     'env'                  => $env,
                     'error_code'           => E_DEPRECATED,
-                    'expected_log_level'   => Level::Info,
-                    'expected_msg_pattern' => $log_prefix . 'PHP Deprecated function' . $log_suffix,
+                    'expected_log_level'   => $suppress_info ? null : Level::Info,
+                    'expected_msg_pattern' => $suppress_info ? null : $log_prefix . 'PHP Deprecated function' . $log_suffix,
                     'is_fatal_error'       => false,
                 ];
 
                 yield [
                     'session_mode'         => $session_mode,
                     'env'                  => $env,
                     'error_code'           => E_USER_DEPRECATED,
-                    'expected_log_level'   => Level::Info,
-                    'expected_msg_pattern' => $log_prefix . 'PHP User deprecated function' . $log_suffix,
+                    'expected_log_level'   => $suppress_info ? null : Level::Info,
+                    'expected_msg_pattern' => $suppress_info ? null : $log_prefix . 'PHP User deprecated function' . $log_suffix,
                     'is_fatal_error'       => false,
                 ];
             }
@@ -261,20 +265,21 @@ public function testHandleError(
         int $session_mode,
         string $env,
         int $error_code,
-        Level $expected_log_level,
-        string $expected_msg_pattern,
+        ?Level $expected_log_level,
+        ?string $expected_msg_pattern,
         bool $is_fatal_error
     ) {
         $handler = new TestHandler();
         $logger = $this->newMockInstance('Monolog\\Logger', null, null, ['test-logger', [$handler]]);
 
-        $previous_use_mode         = $_SESSION['glpi_use_mode'];
+        $previous_use_mode = $_SESSION['glpi_use_mode'];
 
         $error_handler = $this->callPrivateConstructor(
             \Glpi\Application\ErrorHandler::class,
             [$logger, $env]
         );
         $error_handler->setForwardToInternalHandler(false);
+        $error_handler->register();
 
         // Assert that nothing is logged when using '@' operator
         $_SESSION['glpi_use_mode'] = $session_mode;
@@ -305,16 +310,13 @@ public function testHandleError(
             $_SESSION['glpi_use_mode'] = $previous_use_mode;
         }
 
-        $this->integer(count($handler->getRecords()))->isEqualTo(1);
-        $this->boolean($handler->hasRecordThatMatches($expected_msg_pattern, $expected_log_level));
-
-        if (
-            $env === GLPI::ENV_DEVELOPMENT
-            || (
-                $session_mode === Session::DEBUG_MODE
-                && ($expected_log_level->isHigherThan(Level::Info)  || !in_array($env, [GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION]))
-            )
-        ) {
+        if ($expected_log_level !== null) {
+            $this->integer(count($handler->getRecords()))->isEqualTo(1);
+            $this->boolean($handler->hasRecordThatMatches($expected_msg_pattern, $expected_log_level));
+        } else {
+            $this->integer(count($handler->getRecords()))->isEqualTo(0);
+        }
+        if ($expected_msg_pattern !== null && ($env === GLPI::ENV_DEVELOPMENT || $session_mode === Session::DEBUG_MODE)) {
             $this->output->matches($expected_msg_pattern);
         } else {
             $this->output->isEmpty();

diff --git a/tests/functional/Glpi/Http/LegacyAssetsListener.php b/tests/functional/Glpi/Http/LegacyAssetsListener.php
@@ -594,7 +594,9 @@ public function testServeLegacyAssetsFromDeprecatedMarketplacePath(): void
         $response = null;
         $this->when(
             function () use ($request, &$response) {
+                $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
                 $response = $this->callPrivateMethod($this->testedInstance, 'serveLegacyAssets', $request);
+                \error_reporting($reporting_level); // restore previous level
             }
         )->error
             ->withMessage('Accessing the plugins resources from the `/marketplace/` path is deprecated. Use the `/plugins/` path instead.')

diff --git a/tests/functional/Glpi/Http/LegacyRouterListener.php b/tests/functional/Glpi/Http/LegacyRouterListener.php
@@ -594,7 +594,9 @@ public function testRunLegacyRouterFromDeprecatedMarketplacePath(): void
 
         $this->when(
             function () use ($event) {
+                $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations
                 $this->testedInstance->onKernelRequest($event);
+                \error_reporting($reporting_level); // restore previous level
             }
         )->error
             ->withMessage('Accessing the plugins resources from the `/marketplace/` path is deprecated. Use the `/plugins/` path instead.')