From 2e83a0ed1e6e6e9923e32ea48911964a3309784e Mon Sep 17 00:00:00 2001 From: Nico Hoffmann Date: Wed, 27 Dec 2023 17:31:42 +0100 Subject: [PATCH 1/4] Refactor deprecation warning tests --- tests/Cms/Helpers/HelperFunctionsTest.php | 20 ++--- tests/Cms/Helpers/HelpersTest.php | 104 ++++++++++------------ tests/Cms/HelpersTestCase.php | 48 ++++++++++ vendor/composer/ClassLoader.php | 96 ++++++++++---------- 4 files changed, 151 insertions(+), 117 deletions(-) create mode 100644 tests/Cms/HelpersTestCase.php diff --git a/tests/Cms/Helpers/HelperFunctionsTest.php b/tests/Cms/Helpers/HelperFunctionsTest.php index 9e7c109d62..7c4821f999 100644 --- a/tests/Cms/Helpers/HelperFunctionsTest.php +++ b/tests/Cms/Helpers/HelperFunctionsTest.php @@ -8,10 +8,8 @@ use Kirby\Image\QrCode; use Kirby\Toolkit\Collection; use Kirby\Toolkit\Obj; -use PHPUnit\Framework\Assert; -use PHPUnit\Framework\Error\Deprecated; -class HelperFunctionsTest extends TestCase +class HelperFunctionsTest extends HelpersTestCase { protected $fixtures; protected $kirby; @@ -189,17 +187,11 @@ public function testCssWithArray() public function testDeprecated() { - // the deprecation warnings are always triggered in testing mode, - // so we cannot test it with disabled debug mode - - try { - deprecated('The xyz method is deprecated.'); - } catch (Deprecated $e) { - $this->assertSame('The xyz method is deprecated.', $e->getMessage()); - return; - } - - Assert::fail('Expected deprecation warning was not generated'); + $this->assertError( + E_USER_DEPRECATED, + 'The xyz method is deprecated.', + fn () => deprecated('The xyz method is deprecated.') + ); } public function testDumpOnCli() diff --git a/tests/Cms/Helpers/HelpersTest.php b/tests/Cms/Helpers/HelpersTest.php index e5ac2445a7..9da08b238d 100644 --- a/tests/Cms/Helpers/HelpersTest.php +++ b/tests/Cms/Helpers/HelpersTest.php @@ -4,17 +4,13 @@ use Kirby\Exception\InvalidArgumentException; use Kirby\Toolkit\Obj; -use PHPUnit\Framework\Assert; -use PHPUnit\Framework\Error\Deprecated; -use PHPUnit\Framework\Error\Warning; /** * @coversDefaultClass Kirby\Cms\Helpers */ -class HelpersTest extends TestCase +class HelpersTest extends HelpersTestCase { protected $deprecations = []; - protected $hasErrorHandler = false; public function setUp(): void { @@ -28,11 +24,6 @@ public function tearDown(): void parent::tearDown(); Helpers::$deprecations = $this->deprecations; - - if ($this->hasErrorHandler === true) { - restore_error_handler(); - $this->hasErrorHandler = false; - } } /** @@ -40,17 +31,11 @@ public function tearDown(): void */ public function testDeprecated() { - // the deprecation warnings are always triggered in testing mode, - // so we cannot test it with disabled debug mode - - try { - Helpers::deprecated('The xyz method is deprecated.'); - } catch (Deprecated $e) { - $this->assertSame('The xyz method is deprecated.', $e->getMessage()); - return; - } - - Assert::fail('Expected deprecation warning was not generated'); + $this->assertError( + E_USER_DEPRECATED, + 'The xyz method is deprecated.', + fn () => Helpers::deprecated('The xyz method is deprecated.') + ); } /** @@ -58,14 +43,11 @@ public function testDeprecated() */ public function testDeprecatedKeyUndefined() { - try { - Helpers::deprecated('The xyz method is deprecated.', 'my-key'); - } catch (Deprecated $e) { - $this->assertSame('The xyz method is deprecated.', $e->getMessage()); - return; - } - - Assert::fail('Expected deprecation warning was not generated'); + $this->assertError( + E_USER_DEPRECATED, + 'The xyz method is deprecated.', + fn () => Helpers::deprecated('The xyz method is deprecated.', 'my-key') + ); } /** @@ -73,15 +55,14 @@ public function testDeprecatedKeyUndefined() */ public function testDeprecatedActivated() { - try { - Helpers::$deprecations = ['my-key' => true]; - Helpers::deprecated('The xyz method is deprecated.', 'my-key'); - } catch (Deprecated $e) { - $this->assertSame('The xyz method is deprecated.', $e->getMessage()); - return; - } - - Assert::fail('Expected deprecation warning was not generated'); + $this->assertError( + E_USER_DEPRECATED, + 'The xyz method is deprecated.', + function () { + Helpers::$deprecations = ['my-key' => true]; + Helpers::deprecated('The xyz method is deprecated.', 'my-key'); + } + ); } /** @@ -89,8 +70,16 @@ public function testDeprecatedActivated() */ public function testDeprecatedKeyDeactivated() { - Helpers::$deprecations = ['my-key' => false]; - $this->assertFalse(Helpers::deprecated('The xyz method is deprecated.', 'my-key')); + $result = $this->assertError( + E_USER_DEPRECATED, + 'The xyz method is deprecated.', + function () { + Helpers::$deprecations = ['my-key' => false]; + return Helpers::deprecated('The xyz method is deprecated.', 'my-key'); + }, + false + ); + $this->assertFalse($result); } /** @@ -211,24 +200,23 @@ public function testHandleErrorsWarningCaughtCallbackValue() */ public function testHandleErrorsWarningNotCaught() { - try { - Helpers::handleErrors( - fn () => trigger_error('Some warning', E_USER_WARNING), - function (int $errno, string $errstr) { - $this->assertSame(E_USER_WARNING, $errno); - $this->assertSame('Some warning', $errstr); - - // continue the handler chain - return false; - }, - 'handled' - ); - } catch (Warning $e) { - $this->assertSame('Some warning', $e->getMessage()); - return; - } - - Assert::fail('Expected warning was not generated'); + $this->assertError( + E_USER_WARNING, + 'Some warning', + function () { + Helpers::handleErrors( + fn () => trigger_error('Some warning', E_USER_WARNING), + function (int $errno, string $errstr) { + $this->assertSame(E_USER_WARNING, $errno); + $this->assertSame('Some warning', $errstr); + + // continue the handler chain + return false; + }, + 'handled' + ); + } + ); } /** diff --git a/tests/Cms/HelpersTestCase.php b/tests/Cms/HelpersTestCase.php new file mode 100644 index 0000000000..0ebd8ce556 --- /dev/null +++ b/tests/Cms/HelpersTestCase.php @@ -0,0 +1,48 @@ +hasErrorHandler === true) { + restore_error_handler(); + $this->hasErrorHandler = false; + } + } + + public function assertError( + int $expectedErrorType, + string $exptectedErrorMessage, + Closure $callback, + bool $expectedFailure = true + ) { + $this->hasErrorHandler = true; + + $called = false; + + set_error_handler( + function (int $errno, string $errstr) use ($expectedErrorType, $exptectedErrorMessage, &$called) { + $called = true; + $this->assertSame($expectedErrorType, $errno); + $this->assertSame($exptectedErrorMessage, $errstr); + } + ); + + $result = $callback(); + + if ($expectedFailure === false) { + $this->assertFalse($called); + return $result; + } + + $this->assertTrue($called); + } +} diff --git a/vendor/composer/ClassLoader.php b/vendor/composer/ClassLoader.php index 7824d8f7ea..a72151c77c 100644 --- a/vendor/composer/ClassLoader.php +++ b/vendor/composer/ClassLoader.php @@ -45,34 +45,35 @@ class ClassLoader /** @var \Closure(string):void */ private static $includeFile; - /** @var string|null */ + /** @var ?string */ private $vendorDir; // PSR-4 /** - * @var array> + * @var array[] + * @psalm-var array> */ private $prefixLengthsPsr4 = array(); /** - * @var array> + * @var array[] + * @psalm-var array> */ private $prefixDirsPsr4 = array(); /** - * @var list + * @var array[] + * @psalm-var array */ private $fallbackDirsPsr4 = array(); // PSR-0 /** - * List of PSR-0 prefixes - * - * Structured as array('F (first letter)' => array('Foo\Bar (full prefix)' => array('path', 'path2'))) - * - * @var array>> + * @var array[] + * @psalm-var array> */ private $prefixesPsr0 = array(); /** - * @var list + * @var array[] + * @psalm-var array */ private $fallbackDirsPsr0 = array(); @@ -80,7 +81,8 @@ class ClassLoader private $useIncludePath = false; /** - * @var array + * @var string[] + * @psalm-var array */ private $classMap = array(); @@ -88,20 +90,21 @@ class ClassLoader private $classMapAuthoritative = false; /** - * @var array + * @var bool[] + * @psalm-var array */ private $missingClasses = array(); - /** @var string|null */ + /** @var ?string */ private $apcuPrefix; /** - * @var array + * @var self[] */ private static $registeredLoaders = array(); /** - * @param string|null $vendorDir + * @param ?string $vendorDir */ public function __construct($vendorDir = null) { @@ -110,7 +113,7 @@ public function __construct($vendorDir = null) } /** - * @return array> + * @return string[] */ public function getPrefixes() { @@ -122,7 +125,8 @@ public function getPrefixes() } /** - * @return array> + * @return array[] + * @psalm-return array> */ public function getPrefixesPsr4() { @@ -130,7 +134,8 @@ public function getPrefixesPsr4() } /** - * @return list + * @return array[] + * @psalm-return array */ public function getFallbackDirs() { @@ -138,7 +143,8 @@ public function getFallbackDirs() } /** - * @return list + * @return array[] + * @psalm-return array */ public function getFallbackDirsPsr4() { @@ -146,7 +152,8 @@ public function getFallbackDirsPsr4() } /** - * @return array Array of classname => path + * @return string[] Array of classname => path + * @psalm-return array */ public function getClassMap() { @@ -154,7 +161,8 @@ public function getClassMap() } /** - * @param array $classMap Class to filename map + * @param string[] $classMap Class to filename map + * @psalm-param array $classMap * * @return void */ @@ -171,25 +179,24 @@ public function addClassMap(array $classMap) * Registers a set of PSR-0 directories for a given prefix, either * appending or prepending to the ones previously set for this prefix. * - * @param string $prefix The prefix - * @param list|string $paths The PSR-0 root directories - * @param bool $prepend Whether to prepend the directories + * @param string $prefix The prefix + * @param string[]|string $paths The PSR-0 root directories + * @param bool $prepend Whether to prepend the directories * * @return void */ public function add($prefix, $paths, $prepend = false) { - $paths = (array) $paths; if (!$prefix) { if ($prepend) { $this->fallbackDirsPsr0 = array_merge( - $paths, + (array) $paths, $this->fallbackDirsPsr0 ); } else { $this->fallbackDirsPsr0 = array_merge( $this->fallbackDirsPsr0, - $paths + (array) $paths ); } @@ -198,19 +205,19 @@ public function add($prefix, $paths, $prepend = false) $first = $prefix[0]; if (!isset($this->prefixesPsr0[$first][$prefix])) { - $this->prefixesPsr0[$first][$prefix] = $paths; + $this->prefixesPsr0[$first][$prefix] = (array) $paths; return; } if ($prepend) { $this->prefixesPsr0[$first][$prefix] = array_merge( - $paths, + (array) $paths, $this->prefixesPsr0[$first][$prefix] ); } else { $this->prefixesPsr0[$first][$prefix] = array_merge( $this->prefixesPsr0[$first][$prefix], - $paths + (array) $paths ); } } @@ -219,9 +226,9 @@ public function add($prefix, $paths, $prepend = false) * Registers a set of PSR-4 directories for a given namespace, either * appending or prepending to the ones previously set for this namespace. * - * @param string $prefix The prefix/namespace, with trailing '\\' - * @param list|string $paths The PSR-4 base directories - * @param bool $prepend Whether to prepend the directories + * @param string $prefix The prefix/namespace, with trailing '\\' + * @param string[]|string $paths The PSR-4 base directories + * @param bool $prepend Whether to prepend the directories * * @throws \InvalidArgumentException * @@ -229,18 +236,17 @@ public function add($prefix, $paths, $prepend = false) */ public function addPsr4($prefix, $paths, $prepend = false) { - $paths = (array) $paths; if (!$prefix) { // Register directories for the root namespace. if ($prepend) { $this->fallbackDirsPsr4 = array_merge( - $paths, + (array) $paths, $this->fallbackDirsPsr4 ); } else { $this->fallbackDirsPsr4 = array_merge( $this->fallbackDirsPsr4, - $paths + (array) $paths ); } } elseif (!isset($this->prefixDirsPsr4[$prefix])) { @@ -250,18 +256,18 @@ public function addPsr4($prefix, $paths, $prepend = false) throw new \InvalidArgumentException("A non-empty PSR-4 prefix must end with a namespace separator."); } $this->prefixLengthsPsr4[$prefix[0]][$prefix] = $length; - $this->prefixDirsPsr4[$prefix] = $paths; + $this->prefixDirsPsr4[$prefix] = (array) $paths; } elseif ($prepend) { // Prepend directories for an already registered namespace. $this->prefixDirsPsr4[$prefix] = array_merge( - $paths, + (array) $paths, $this->prefixDirsPsr4[$prefix] ); } else { // Append directories for an already registered namespace. $this->prefixDirsPsr4[$prefix] = array_merge( $this->prefixDirsPsr4[$prefix], - $paths + (array) $paths ); } } @@ -270,8 +276,8 @@ public function addPsr4($prefix, $paths, $prepend = false) * Registers a set of PSR-0 directories for a given prefix, * replacing any others previously set for this prefix. * - * @param string $prefix The prefix - * @param list|string $paths The PSR-0 base directories + * @param string $prefix The prefix + * @param string[]|string $paths The PSR-0 base directories * * @return void */ @@ -288,8 +294,8 @@ public function set($prefix, $paths) * Registers a set of PSR-4 directories for a given namespace, * replacing any others previously set for this namespace. * - * @param string $prefix The prefix/namespace, with trailing '\\' - * @param list|string $paths The PSR-4 base directories + * @param string $prefix The prefix/namespace, with trailing '\\' + * @param string[]|string $paths The PSR-4 base directories * * @throws \InvalidArgumentException * @@ -475,9 +481,9 @@ public function findFile($class) } /** - * Returns the currently registered loaders keyed by their corresponding vendor directories. + * Returns the currently registered loaders indexed by their corresponding vendor directories. * - * @return array + * @return self[] */ public static function getRegisteredLoaders() { From 985c647902fa1a547a74ca6875648146f5dba021 Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Sat, 30 Dec 2023 18:23:04 +0100 Subject: [PATCH 2/4] Handle multiple error handlers per test We need to pop exactly as many error handlers off the stack as we added, even if e.g. the `assertError()` method was called multiple times --- tests/Cms/Helpers/HelpersTest.php | 6 +++--- tests/Cms/HelpersTestCase.php | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/Cms/Helpers/HelpersTest.php b/tests/Cms/Helpers/HelpersTest.php index 9da08b238d..d72f0f380d 100644 --- a/tests/Cms/Helpers/HelpersTest.php +++ b/tests/Cms/Helpers/HelpersTest.php @@ -130,7 +130,7 @@ public function testHandleErrorsNoWarning() */ public function testHandleErrorsWarningCaught1() { - $this->hasErrorHandler = true; + $this->activeErrorHandlers++; $called = false; set_error_handler(function (int $errno, string $errstr) use (&$called) { @@ -156,7 +156,7 @@ function (int $errno, string $errstr) { */ public function testHandleErrorsWarningCaught2() { - $this->hasErrorHandler = true; + $this->activeErrorHandlers++; $called = false; set_error_handler(function (int $errno, string $errstr) use (&$called) { @@ -186,7 +186,7 @@ function (int $errno, string $errstr) { */ public function testHandleErrorsWarningCaughtCallbackValue() { - $this->hasErrorHandler = true; + $this->activeErrorHandlers++; $this->assertSame('handled', Helpers::handleErrors( fn () => trigger_error('Some warning', E_USER_WARNING), diff --git a/tests/Cms/HelpersTestCase.php b/tests/Cms/HelpersTestCase.php index 0ebd8ce556..8bbc048f66 100644 --- a/tests/Cms/HelpersTestCase.php +++ b/tests/Cms/HelpersTestCase.php @@ -6,15 +6,15 @@ class HelpersTestCase extends TestCase { - protected $hasErrorHandler = false; + protected int $activeErrorHandlers = 0; public function tearDown(): void { parent::tearDown(); - if ($this->hasErrorHandler === true) { + while ($this->activeErrorHandlers > 0) { restore_error_handler(); - $this->hasErrorHandler = false; + $this->activeErrorHandlers--; } } @@ -24,7 +24,7 @@ public function assertError( Closure $callback, bool $expectedFailure = true ) { - $this->hasErrorHandler = true; + $this->activeErrorHandlers++; $called = false; From 0cbf7c8a7598c9dc7f3d94734de834a160ddd637 Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Sat, 30 Dec 2023 18:23:23 +0100 Subject: [PATCH 3/4] Fix typo --- tests/Cms/HelpersTestCase.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Cms/HelpersTestCase.php b/tests/Cms/HelpersTestCase.php index 8bbc048f66..bf4b848824 100644 --- a/tests/Cms/HelpersTestCase.php +++ b/tests/Cms/HelpersTestCase.php @@ -20,7 +20,7 @@ public function tearDown(): void public function assertError( int $expectedErrorType, - string $exptectedErrorMessage, + string $expectedErrorMessage, Closure $callback, bool $expectedFailure = true ) { @@ -29,10 +29,10 @@ public function assertError( $called = false; set_error_handler( - function (int $errno, string $errstr) use ($expectedErrorType, $exptectedErrorMessage, &$called) { + function (int $errno, string $errstr) use ($expectedErrorType, $expectedErrorMessage, &$called) { $called = true; $this->assertSame($expectedErrorType, $errno); - $this->assertSame($exptectedErrorMessage, $errstr); + $this->assertSame($expectedErrorMessage, $errstr); } ); From 2e4f383a0467791275c416d18a94a33c9338da6d Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Sat, 30 Dec 2023 20:34:46 +0100 Subject: [PATCH 4/4] Add TODO comment --- tests/Cms/Helpers/HelpersTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Cms/Helpers/HelpersTest.php b/tests/Cms/Helpers/HelpersTest.php index d72f0f380d..0139300778 100644 --- a/tests/Cms/Helpers/HelpersTest.php +++ b/tests/Cms/Helpers/HelpersTest.php @@ -186,6 +186,8 @@ function (int $errno, string $errstr) { */ public function testHandleErrorsWarningCaughtCallbackValue() { + // TODO: This line should not be necessary + // https://github.com/getkirby/kirby/pull/6093#issuecomment-1872569768 $this->activeErrorHandlers++; $this->assertSame('handled', Helpers::handleErrors(