From 5b00cd9d644a341a8f138547dfd3e213faf068ea Mon Sep 17 00:00:00 2001 From: Sven Reichel Date: Wed, 29 May 2024 00:46:38 +0200 Subject: [PATCH] Added unit tests to checks --- .php-cs-fixer.dist.php | 1 + .phpcs.php.xml.dist | 1 + .phpcs.xml.dist | 1 + composer.json | 1 + composer.lock | 11 +-- dev/tests/unit/Base/ClassLoadingTest.php | 2 +- dev/tests/unit/Base/XmlFileLoadingTest.php | 13 +-- .../Helper/EnvironmentConfigLoaderTest.php | 79 ++++++++++--------- .../EnvironmentConfigLoaderTestHelper.php | 19 +++++ .../unit/Mage/Core/Helper/SecurityTest.php | 16 ++-- .../unit/Mage/Core/Helper/StringTest.php | 2 +- .../Mage/Downloadable/Helper/FileTest.php | 2 +- .../unit/Mage/Uploader/Helper/FileTest.php | 8 +- dev/tests/unit/Varien/ObjectTest.php | 14 ++-- phpstan.dist.neon | 1 + 15 files changed, 103 insertions(+), 68 deletions(-) create mode 100644 dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTestHelper.php diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 653d7c3268e..39d5a5e0791 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -96,6 +96,7 @@ PhpCsFixer\Finder::create() ->in([ 'app/code/core/Mage/', + 'dev/tests/unit/', 'lib/Mage/', 'lib/Magento/', 'lib/Varien/', diff --git a/.phpcs.php.xml.dist b/.phpcs.php.xml.dist index 302d5e75ff9..beb3dc169ce 100644 --- a/.phpcs.php.xml.dist +++ b/.phpcs.php.xml.dist @@ -7,6 +7,7 @@ install.php app/Mage.php app/code/core/Mage/ + dev/tests/unit/ lib/Mage/ lib/Magento/ lib/Varien/ diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index a4a9c1ce8fe..c83ab1f2449 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -7,6 +7,7 @@ install.php app/Mage.php app/code/core/Mage/ + dev/tests/unit/ lib/Mage/ lib/Magento/ lib/Varien/ diff --git a/composer.json b/composer.json index e4bca138cae..daca1e558c0 100644 --- a/composer.json +++ b/composer.json @@ -21,6 +21,7 @@ "ext-pdo": "*", "ext-simplexml": "*", "ext-soap": "*", + "ext-xmlreader": "*", "ext-zlib": "*", "colinmollenhour/cache-backend-redis": "^1.14", "colinmollenhour/magento-redis-session": "^3.2.0", diff --git a/composer.lock b/composer.lock index 4534cf595de..9f5776839a8 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "5c217ee2c019c16647fa9060716afaf6", + "content-hash": "254552536d1db0594d6e197dbb2e17d3", "packages": [ { "name": "colinmollenhour/cache-backend-redis", @@ -405,12 +405,12 @@ "version": "v5.2.13", "source": { "type": "git", - "url": "https://github.com/justinrainbow/json-schema.git", + "url": "https://github.com/jsonrainbow/json-schema.git", "reference": "fbbe7e5d79f618997bc3332a6f49246036c45793" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/justinrainbow/json-schema/zipball/fbbe7e5d79f618997bc3332a6f49246036c45793", + "url": "https://api.github.com/repos/jsonrainbow/json-schema/zipball/fbbe7e5d79f618997bc3332a6f49246036c45793", "reference": "fbbe7e5d79f618997bc3332a6f49246036c45793", "shasum": "" }, @@ -465,8 +465,8 @@ "schema" ], "support": { - "issues": "https://github.com/justinrainbow/json-schema/issues", - "source": "https://github.com/justinrainbow/json-schema/tree/v5.2.13" + "issues": "https://github.com/jsonrainbow/json-schema/issues", + "source": "https://github.com/jsonrainbow/json-schema/tree/v5.2.13" }, "time": "2023-09-26T02:20:38+00:00" }, @@ -6325,6 +6325,7 @@ "ext-pdo": "*", "ext-simplexml": "*", "ext-soap": "*", + "ext-xmlreader": "*", "ext-zlib": "*" }, "platform-dev": [], diff --git a/dev/tests/unit/Base/ClassLoadingTest.php b/dev/tests/unit/Base/ClassLoadingTest.php index 0dbd0a3ec5a..3314e501699 100644 --- a/dev/tests/unit/Base/ClassLoadingTest.php +++ b/dev/tests/unit/Base/ClassLoadingTest.php @@ -19,7 +19,7 @@ public function testClassExists(bool $expectedResult, string $class): void } /** - * @return string[][] + * @return array> */ public function provideClassExistsData(): array { diff --git a/dev/tests/unit/Base/XmlFileLoadingTest.php b/dev/tests/unit/Base/XmlFileLoadingTest.php index 43b886fd436..37946e7351b 100644 --- a/dev/tests/unit/Base/XmlFileLoadingTest.php +++ b/dev/tests/unit/Base/XmlFileLoadingTest.php @@ -4,8 +4,10 @@ namespace OpenMage\Tests\Unit\Base; use PHPUnit\Framework\TestCase; +use SimpleXMLElement; +use XMLReader; -class XmlFileLoadingTest extends TestCase +class XmlFileLoadingTest extends TestCase { /** * @@ -15,10 +17,10 @@ class XmlFileLoadingTest extends TestCase */ public function testFileLoading(string $filepath): void { - //$simplexml = new \SimpleXMLElement(file_get_contents($filepath)); + /** @var SimpleXMLElement $simplexml */ $simplexml = simplexml_load_file( $filepath, - null, + SimpleXMLElement::class, LIBXML_PEDANTIC //not needed by OpenMage, but good to test more strictly ); self::assertNotEmpty($simplexml->asXML()); @@ -32,8 +34,9 @@ public function testFileLoading(string $filepath): void */ public function testXmlReaderIsValid(string $filepath): void { - $xml = \XMLReader::open($filepath); - $xml->setParserProperty(\XMLReader::VALIDATE, true); + /** @var XMLReader $xml */ + $xml = XMLReader::open($filepath); + $xml->setParserProperty(XMLReader::VALIDATE, true); self::assertTrue($xml->isValid()); } diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php index ed7911d29ba..e99e1d6a433 100644 --- a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -3,64 +3,57 @@ namespace OpenMage\Tests\Unit\Core\Helper; +use Mage; +use Mage_Core_Exception; use PHPUnit\Framework\TestCase; use Mage_Core_Helper_EnvironmentConfigLoader; -use Mage_Core_Model_Config; - -class TestEnvLoaderHelper extends Mage_Core_Helper_EnvironmentConfigLoader { - public function exposedBuildPath(string $section, string $group, string $field): string - { - return $this->buildPath($section, $group, $field); - } - - public function exposedBuildNodePath(string $scope, string $path): string - { - return $this->buildNodePath($scope, $path); - } -} +use Varien_Simplexml_Config; class EnvironmentConfigLoaderTest extends TestCase { + /** + * @throws Mage_Core_Exception + */ public function setup(): void { - \Mage::setRoot(''); + Mage::setRoot(); } - public function testBuildPath() + public function testBuildPath(): void { - $environmentConfigLoaderHelper = new TestEnvLoaderHelper(); + $environmentConfigLoaderHelper = new EnvironmentConfigLoaderTestHelper(); $path = $environmentConfigLoaderHelper->exposedBuildPath('GENERAL', 'STORE_INFORMATION', 'NAME'); - $this->assertEquals('general/store_information/name', $path); + self::assertEquals('general/store_information/name', $path); } - public function testBuildNodePath() + public function testBuildNodePath(): void { - $environmentConfigLoaderHelper = new TestEnvLoaderHelper(); + $environmentConfigLoaderHelper = new EnvironmentConfigLoaderTestHelper(); $nodePath = $environmentConfigLoaderHelper->exposedBuildNodePath('DEFAULT', 'general/store_information/name'); - $this->assertEquals('default/general/store_information/name', $nodePath); + self::assertEquals('default/general/store_information/name', $nodePath); } - public function test_xml_has_test_strings() + public function testXmlHasTestStrings(): void { $xmlStruct = $this->getTestXml(); - $xml = new \Varien_Simplexml_Config(); + $xml = new Varien_Simplexml_Config(); $xml->loadString($xmlStruct); - $this->assertEquals('test_default', (string)$xml->getNode('default/general/store_information/name')); - $this->assertEquals('test_website', (string)$xml->getNode('websites/base/general/store_information/name')); - $this->assertEquals('test_store', (string)$xml->getNode('stores/german/general/store_information/name')); + self::assertEquals('test_default', (string)$xml->getNode('default/general/store_information/name')); + self::assertEquals('test_website', (string)$xml->getNode('websites/base/general/store_information/name')); + self::assertEquals('test_store', (string)$xml->getNode('stores/german/general/store_information/name')); } /** - * @dataProvider env_overrides_correct_config_keys - * @test + * @dataProvider envOverridesCorrectConfigKeysDataProvider + * @param array $config */ - public function env_overrides_for_valid_config_keys(array $config) + public function testEnvOverridesForValidConfigKeys(array $config): void { $xmlStruct = $this->getTestXml(); - $xmlDefault = new \Varien_Simplexml_Config(); + $xmlDefault = new Varien_Simplexml_Config(); $xmlDefault->loadString($xmlStruct); - $xml = new \Varien_Simplexml_Config(); + $xml = new Varien_Simplexml_Config(); $xml->loadString($xmlStruct); // act @@ -75,10 +68,13 @@ public function env_overrides_for_valid_config_keys(array $config) $valueAfterOverride = $xml->getNode($configPath); // assert - $this->assertNotEquals((string)$defaultValue, (string)$valueAfterOverride, 'Default value was not overridden.'); + self::assertNotEquals((string)$defaultValue, (string)$valueAfterOverride, 'Default value was not overridden.'); } - public function env_overrides_correct_config_keys(): array + /** + * @return array>> + */ + public function envOverridesCorrectConfigKeysDataProvider(): array { $defaultPath = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME'; $defaultPathWithDash = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__FOO-BAR__NAME'; @@ -169,16 +165,16 @@ public function env_overrides_correct_config_keys(): array } /** - * @dataProvider env_does_not_override_on_wrong_config_keys - * @test + * @dataProvider envDoesNotOverrideOnWrongConfigKeysDataProvider + * @param array $config */ - public function env_does_not_override_for_invalid_config_keys(array $config) + public function testEnvDoesNotOverrideForInvalidConfigKeys(array $config): void { $xmlStruct = $this->getTestXml(); - $xmlDefault = new \Varien_Simplexml_Config(); + $xmlDefault = new Varien_Simplexml_Config(); $xmlDefault->loadString($xmlStruct); - $xml = new \Varien_Simplexml_Config(); + $xml = new Varien_Simplexml_Config(); $xml->loadString($xmlStruct); $defaultValue = 'test_default'; @@ -195,6 +191,7 @@ public function env_does_not_override_for_invalid_config_keys(array $config) ]); $loader->overrideEnvironment($xml); + $valueAfterCheck = ''; switch ($config['case']) { case 'DEFAULT': $valueAfterCheck = $xml->getNode('default/general/store_information/name'); @@ -208,14 +205,18 @@ public function env_does_not_override_for_invalid_config_keys(array $config) } // assert - $this->assertTrue(!str_contains('value_will_not_be_changed', (string)$valueAfterCheck), 'Default value was wrongfully overridden.'); + self::assertTrue(!str_contains('value_will_not_be_changed', (string)$valueAfterCheck), 'Default value was wrongfully overridden.'); } - public function env_does_not_override_on_wrong_config_keys(): array + /** + * @return array>> + */ + public function envDoesNotOverrideOnWrongConfigKeysDataProvider(): array { $defaultPath = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__ST'; $websitePath = 'OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__ST'; $storePath = 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__ST'; + return [ [ 'Case DEFAULT with ' . $defaultPath . ' will not override.' => [ diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTestHelper.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTestHelper.php new file mode 100644 index 00000000000..fdd368988fd --- /dev/null +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTestHelper.php @@ -0,0 +1,19 @@ +buildPath($section, $group, $field); + } + + public function exposedBuildNodePath(string $scope, string $path): string + { + return $this->buildNodePath($scope, $path); + } +} diff --git a/dev/tests/unit/Mage/Core/Helper/SecurityTest.php b/dev/tests/unit/Mage/Core/Helper/SecurityTest.php index c0269e822d2..6d0381fdbbd 100644 --- a/dev/tests/unit/Mage/Core/Helper/SecurityTest.php +++ b/dev/tests/unit/Mage/Core/Helper/SecurityTest.php @@ -25,6 +25,9 @@ public function setUp(): void $this->subject = Mage::helper('core/security'); } + /** + * @return array|Mage_Page_Block_Html_Topmenu_Renderer|Mage_Core_Block_Template|string>> + */ public function validateAgainstBlockMethodBlacklistDataProvider(): array { $topmenu = new Mage_Page_Block_Html_Topmenu_Renderer(); @@ -49,7 +52,7 @@ public function validateAgainstBlockMethodBlacklistDataProvider(): array * @doesNotPerformAssertions if data is correct, then NO exception is thrown, so we don't need an assertion * @param Mage_Core_Block_Abstract $block * @param string $method - * @param array $args + * @param string[] $args * @return void * @throws Mage_Core_Exception */ @@ -61,6 +64,9 @@ public function testValidateAgainstBlockMethodBlacklist( $this->subject->validateAgainstBlockMethodBlacklist($block, $method, $args); } + /** + * @return array|Mage_Page_Block_Html_Topmenu_Renderer|Mage_Core_Block_Template|string>> + */ public function forbiddenBlockMethodsDataProvider(): array { $topmenu = new Mage_Page_Block_Html_Topmenu_Renderer(); @@ -109,7 +115,7 @@ public function forbiddenBlockMethodsDataProvider(): array * @dataProvider forbiddenBlockMethodsDataProvider * @param Mage_Core_Block_Abstract $block * @param string $method - * @param array $args + * @param string[] $args * @return void * @throws Mage_Core_Exception */ @@ -118,7 +124,7 @@ public function testValidateAgainstBlockMethodBlacklistThrowsException( string $method, array $args ): void { - self::expectExceptionMessage(sprintf('Action with combination block %s and method %s is forbidden.', get_class($block), $method)); - $this->subject->validateAgainstBlockMethodBlacklist($block, $method, $args); + self::expectExceptionMessage(sprintf('Action with combination block %s and method %s is forbidden.', get_class($block), $method)); + $this->subject->validateAgainstBlockMethodBlacklist($block, $method, $args); } -} \ No newline at end of file +} diff --git a/dev/tests/unit/Mage/Core/Helper/StringTest.php b/dev/tests/unit/Mage/Core/Helper/StringTest.php index 3db0a43a310..e77c08b3851 100644 --- a/dev/tests/unit/Mage/Core/Helper/StringTest.php +++ b/dev/tests/unit/Mage/Core/Helper/StringTest.php @@ -9,7 +9,7 @@ class StringTest extends TestCase { - const TEST_STRING = '1234567890'; + public const TEST_STRING = '1234567890'; /** * @var Mage_Core_Helper_String diff --git a/dev/tests/unit/Mage/Downloadable/Helper/FileTest.php b/dev/tests/unit/Mage/Downloadable/Helper/FileTest.php index 12bb80a1a10..aad4e7347dd 100644 --- a/dev/tests/unit/Mage/Downloadable/Helper/FileTest.php +++ b/dev/tests/unit/Mage/Downloadable/Helper/FileTest.php @@ -34,7 +34,7 @@ public function testGetFilePath(string $expectedResult, string $path, ?string $f } /** - * @return string[][] + * @return array> */ public function provideGetFilePathData(): array { diff --git a/dev/tests/unit/Mage/Uploader/Helper/FileTest.php b/dev/tests/unit/Mage/Uploader/Helper/FileTest.php index fad36ced384..ec6defb230c 100644 --- a/dev/tests/unit/Mage/Uploader/Helper/FileTest.php +++ b/dev/tests/unit/Mage/Uploader/Helper/FileTest.php @@ -23,8 +23,8 @@ public function setUp(): void /** * @dataProvider provideGetMimeTypeFromExtensionListData - * @param array $expectedResult - * @param string|array $extensionsList + * @param array $expectedResult + * @param string|array $extensionsList * @return void */ public function testGetMimeTypeFromExtensionList(array $expectedResult, $extensionsList): void @@ -33,7 +33,7 @@ public function testGetMimeTypeFromExtensionList(array $expectedResult, $extensi } /** - * @return string[][] + * @return array|string>> */ public function provideGetMimeTypeFromExtensionListData(): array { @@ -103,7 +103,7 @@ public function testGetDataMaxSizeInBytes(int $expectedResult, string $maxSize): } /** - * @return string[][] + * @return array> */ public function provideGetDataMaxSizeInBytesData(): array { diff --git a/dev/tests/unit/Varien/ObjectTest.php b/dev/tests/unit/Varien/ObjectTest.php index bac3bb78d29..7ca4fec0ea8 100644 --- a/dev/tests/unit/Varien/ObjectTest.php +++ b/dev/tests/unit/Varien/ObjectTest.php @@ -3,7 +3,7 @@ namespace unit\Varien; -use StdClass; +use stdClass; use Varien_Exception; use Varien_Object; use PHPUnit\Framework\TestCase; @@ -23,9 +23,9 @@ public function setUp(): void /** * @dataProvider provideGetDataData * @param mixed $expectedResult - * @param string|array $setKey + * @param string $setKey * @param mixed $setValue - * @param string|array $key + * @param string $key * @param string|int|null $index * @return void */ @@ -36,7 +36,7 @@ public function testGetData($expectedResult, $setKey, $setValue, string $key, $i } /** - * @return string[][] + * @return array|int|string>|int|stdClass|string|Varien_Object|null>> */ public function provideGetDataData(): array { @@ -109,7 +109,7 @@ public function provideGetDataData(): array 'array_index_string_std_class' => [ null, 'array_index_string_std_class', - new StdClass(), + new stdClass(), 'array_index_string_std_class', 'not-exists', ], @@ -146,7 +146,7 @@ public function provideGetDataData(): array 'array_nested_std_class' => [ null, 'array_nested_std_class', - new StdClass(), + new stdClass(), 'array_nested_std_class/nested', ], 'array_nested_key_not_exists' => [ @@ -213,12 +213,12 @@ public function testGetSetUnsData(): void self::assertTrue($this->subject->isEmpty()); try { + /** @phpstan-ignore-next-line */ $this->subject->notData(); self::fail('Invalid __call'); } catch (Varien_Exception $exception) { self::assertStringStartsWith('Invalid method', $exception->getMessage()); } - } public function testOffset(): void diff --git a/phpstan.dist.neon b/phpstan.dist.neon index d82c837f678..c11e42caa7c 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -12,6 +12,7 @@ parameters: - install.php - app/Mage.php - app/code/core/Mage + - dev/tests/unit - lib/Mage - lib/Magento - lib/Varien