Rector: updated UnitTests (OpenMage#4391)

sreichel · web-flow · commit 0b965e92ae2e · 2024-11-28T18:34:42.000+01:00
* rector unit tests * rector phpunit * rector privatization * Revert "rector privatization" This reverts commit 0ac365a.
diff --git a/.rector.php b/.rector.php
@@ -39,4 +39,23 @@
         DeadCode\ClassMethod\RemoveUselessReturnTagRector::class,
         DeadCode\Property\RemoveUselessVarTagRector::class,
         TypeDeclaration\ClassMethod\ReturnNeverTypeRector::class,
-    ]);
+    ])
+    ->withPreparedSets(
+        false,
+        false,
+        false,
+        false,
+        false,
+        false,
+        false,
+        false,
+        false,
+        false,
+        false,
+        true,
+        false,
+        false,
+        false,
+        false,
+        true,
+    );
diff --git a/tests/unit/Base/XmlFileLoadingTest.php b/tests/unit/Base/XmlFileLoadingTest.php
@@ -17,6 +17,7 @@
 
 namespace OpenMage\Tests\Unit\Base;
 
+use Generator;
 use PHPUnit\Framework\TestCase;
 use SimpleXMLElement;
 use XMLReader;
@@ -50,17 +51,12 @@ public function testXmlReaderIsValid(string $filepath): void
         $this->assertTrue($xml->isValid());
     }
 
-    /**
-     * @return string[][]
-     */
-    public function provideXmlFiles(): array
+    public function provideXmlFiles(): Generator
     {
         $root = realpath(__DIR__ . '/../../../') . '/';
 
-        return [
-            'file from vendor directory' => [
-                $root . 'vendor/shardj/zf1-future/library/Zend/Locale/Data/es_419.xml'
-            ],
+        yield 'file from vendor directory' => [
+            $root . 'vendor/shardj/zf1-future/library/Zend/Locale/Data/es_419.xml'
         ];
     }
 }
diff --git a/tests/unit/Mage/Admin/Model/BlockTest.php b/tests/unit/Mage/Admin/Model/BlockTest.php
@@ -48,8 +48,8 @@ public function testValidate(array $expectedResult, array $methods): void
             ])
             ->getMock();
 
-        $mock->expects($this->any())->method('getBlockName')->willReturn($methods['getBlockName']);
-        $mock->expects($this->any())->method('getIsAllowed')->willReturn($methods['getIsAllowed']);
+        $mock->method('getBlockName')->willReturn($methods['getBlockName']);
+        $mock->method('getIsAllowed')->willReturn($methods['getIsAllowed']);
         $this->assertEquals($expectedResult, $mock->validate());
     }
 
diff --git a/tests/unit/Mage/Admin/Model/UserTest.php b/tests/unit/Mage/Admin/Model/UserTest.php
@@ -56,13 +56,13 @@ public function testAuthenticate($expectedResult, array $methods): void
             ])
             ->getMock();
 
-        $mock->expects($this->any())->method('loadByUsername')->willReturnSelf();
-        $mock->expects($this->any())->method('getId')->willReturn($methods['getId']);
+        $mock->method('loadByUsername')->willReturnSelf();
+        $mock->method('getId')->willReturn($methods['getId']);
 //        $mock->expects($this->any())->method('getUsername')->willReturn($methods['getUsername']);
-        $mock->expects($this->any())->method('getPassword')->willReturn($methods['getPassword']);
-        $mock->expects($this->any())->method('validatePasswordHash')->willReturn($methods['validatePasswordHash']);
-        $mock->expects($this->any())->method('getIsActive')->willReturn($methods['getIsActive']);
-        $mock->expects($this->any())->method('hasAssigned2Role')->willReturn($methods['hasAssigned2Role']);
+        $mock->method('getPassword')->willReturn($methods['getPassword']);
+        $mock->method('validatePasswordHash')->willReturn($methods['validatePasswordHash']);
+        $mock->method('getIsActive')->willReturn($methods['getIsActive']);
+        $mock->method('hasAssigned2Role')->willReturn($methods['hasAssigned2Role']);
 
         try {
             $this->assertSame($expectedResult, $mock->authenticate($methods['getUsername'], $methods['getPassword']));
@@ -136,10 +136,10 @@ public function testValidate($expectedResult, array $methods): void
             ])
             ->getMock();
 
-        $mock->expects($this->any())->method('hasNewPassword')->willReturn($methods['hasNewPassword']);
-        $mock->expects($this->any())->method('getNewPassword')->willReturn($methods['getNewPassword']);
-        $mock->expects($this->any())->method('hasPassword')->willReturn($methods['hasPassword']);
-        $mock->expects($this->any())->method('getPassword')->willReturn($methods['getPassword']);
+        $mock->method('hasNewPassword')->willReturn($methods['hasNewPassword']);
+        $mock->method('getNewPassword')->willReturn($methods['getNewPassword']);
+        $mock->method('hasPassword')->willReturn($methods['hasPassword']);
+        $mock->method('getPassword')->willReturn($methods['getPassword']);
         $this->assertSame($expectedResult, $mock->validate());
     }
 
@@ -231,8 +231,8 @@ public function testIsResetPasswordLinkTokenExpired(bool $expectedResult, array
             ])
             ->getMock();
 
-        $mock->expects($this->any())->method('getRpToken')->willReturn($methods['getRpToken']);
-        $mock->expects($this->any())->method('getRpTokenCreatedAt')->willReturn($methods['getRpTokenCreatedAt']);
+        $mock->method('getRpToken')->willReturn($methods['getRpToken']);
+        $mock->method('getRpTokenCreatedAt')->willReturn($methods['getRpTokenCreatedAt']);
         $this->assertSame($expectedResult, $mock->isResetPasswordLinkTokenExpired());
     }
 
diff --git a/tests/unit/Mage/Admin/Model/VariableTest.php b/tests/unit/Mage/Admin/Model/VariableTest.php
@@ -45,8 +45,8 @@ public function testValidate($expectedResult, string $variableName, string $isAl
             ->setMethods(['getVariableName', 'getIsAllowed'])
             ->getMock();
 
-        $mock->expects($this->any())->method('getVariableName')->willReturn($variableName);
-        $mock->expects($this->any())->method('getIsAllowed')->willReturn($isAllowed);
+        $mock->method('getVariableName')->willReturn($variableName);
+        $mock->method('getIsAllowed')->willReturn($isAllowed);
         $this->assertSame($expectedResult, $mock->validate());
     }
 
diff --git a/tests/unit/Mage/Adminhtml/Block/Customer/Edit/Tab/AddressesTest.php b/tests/unit/Mage/Adminhtml/Block/Customer/Edit/Tab/AddressesTest.php
@@ -43,12 +43,12 @@ public function testInitForm(): void
             ->setMethods(['getRegistryCurrentCustomer', 'isReadonly'])
             ->getMock();
 
-        $mock->expects($this->any())
+        $mock
             ->method('getRegistryCurrentCustomer')
             // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
             ->willReturn(new Mage_Customer_Model_Customer());
 
-        $mock->expects($this->any())
+        $mock
             ->method('isReadonly')
             ->willReturn(true);
 
diff --git a/tests/unit/Mage/Adminhtml/Block/Customer/Edit/Tab/NewsletterTest.php b/tests/unit/Mage/Adminhtml/Block/Customer/Edit/Tab/NewsletterTest.php
@@ -44,7 +44,7 @@ public function testInitForm(): void
             ->setMethods(['getRegistryCurrentCustomer'])
             ->getMock();
 
-        $mock->expects($this->any())
+        $mock
             ->method('getRegistryCurrentCustomer')
             // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
             ->willReturn(new Mage_Customer_Model_Customer());
diff --git a/tests/unit/Mage/Adminhtml/Block/System/Convert/Gui/Edit/Tab/ViewTest.php b/tests/unit/Mage/Adminhtml/Block/System/Convert/Gui/Edit/Tab/ViewTest.php
@@ -43,7 +43,7 @@ public function testInitForm(): void
             ->setMethods(['getRegistryCurrentConvertProfile'])
             ->getMock();
 
-        $mock->expects($this->any())
+        $mock
             ->method('getRegistryCurrentConvertProfile')
             // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
             ->willReturn(new Mage_Dataflow_Model_Profile());
diff --git a/tests/unit/Mage/Adminhtml/Block/System/Convert/Profile/Edit/Tab/EditTest.php b/tests/unit/Mage/Adminhtml/Block/System/Convert/Profile/Edit/Tab/EditTest.php
@@ -43,7 +43,7 @@ public function testInitForm(): void
             ->setMethods(['getRegistryCurrentConvertProfile'])
             ->getMock();
 
-        $mock->expects($this->any())
+        $mock
             ->method('getRegistryCurrentConvertProfile')
             // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
             ->willReturn(new Mage_Dataflow_Model_Profile());
diff --git a/tests/unit/Mage/Catalog/Helper/Product/UrlTest.php b/tests/unit/Mage/Catalog/Helper/Product/UrlTest.php
@@ -39,7 +39,7 @@ public function setUp(): void
     public function testGetConvertTable(): void
     {
         $this->assertIsArray($this->subject->getConvertTable());
-        $this->assertSame(317, count($this->subject->getConvertTable()));
+        $this->assertCount(317, $this->subject->getConvertTable());
     }
 
     /**
diff --git a/tests/unit/Mage/Catalog/Helper/ProductTest.php b/tests/unit/Mage/Catalog/Helper/ProductTest.php
@@ -48,7 +48,7 @@ public function testCanUseCanonicalTag(): void
      */
     public function testGetAttributeInputTypes(int $expectedResult, ?string $inputType = null): void
     {
-        $this->assertSame($expectedResult, count($this->subject->getAttributeInputTypes($inputType)));
+        $this->assertCount($expectedResult, $this->subject->getAttributeInputTypes($inputType));
     }
 
     public function provideGetAttributeInputTypes(): Generator
diff --git a/tests/unit/Mage/Cms/Block/BlockTest.php b/tests/unit/Mage/Cms/Block/BlockTest.php
@@ -34,7 +34,7 @@ public function testGetCacheKeyInfo(string $blockId): void
             ->setMethods(['getBlockId'])
             ->getMock();
 
-        $mock->expects($this->any())->method('getBlockId')->willReturn($blockId);
+        $mock->method('getBlockId')->willReturn($blockId);
         $this->assertIsArray($mock->getCacheKeyInfo());
     }
 
diff --git a/tests/unit/Mage/Cms/Block/PageTest.php b/tests/unit/Mage/Cms/Block/PageTest.php
@@ -35,7 +35,7 @@ public function testGetPage(string $pageId): void
             ->setMethods(['getPageId'])
             ->getMock();
 
-        $mock->expects($this->any())->method('getPageId')->willReturn($pageId);
+        $mock->method('getPageId')->willReturn($pageId);
         $this->assertInstanceOf(Mage_Cms_Model_Page::class, $mock->getPage());
     }
 
diff --git a/tests/unit/Mage/Cms/Block/Widget/BlockTest.php b/tests/unit/Mage/Cms/Block/Widget/BlockTest.php
@@ -44,7 +44,7 @@ public function testGetCacheKeyInfo(string $blockId): void
             ->setMethods(['getBlockId'])
             ->getMock();
 
-        $mock->expects($this->any())->method('getBlockId')->willReturn($blockId);
+        $mock->method('getBlockId')->willReturn($blockId);
         $this->assertIsArray($mock->getCacheKeyInfo());
     }
 
diff --git a/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php
@@ -106,7 +106,7 @@ public function testEnvOverridesForValidConfigKeys(array $config): void
         $valueAfterOverride = $xml->getNode($configPath);
 
         // assert
-        $this->assertNotEquals((string)$defaultValue, (string)$valueAfterOverride, 'Default value was not overridden.');
+        $this->assertNotSame((string)$defaultValue, (string)$valueAfterOverride, 'Default value was not overridden.');
     }
 
     public function envOverridesCorrectConfigKeysDataProvider(): Generator
@@ -222,7 +222,7 @@ public function testEnvDoesNotOverrideForInvalidConfigKeys(array $config): void
         }
 
         // assert
-        $this->assertTrue(!str_contains('value_will_not_be_changed', (string)$valueAfterCheck), 'Default value was wrongfully overridden.');
+        $this->assertFalse(str_contains('value_will_not_be_changed', (string)$valueAfterCheck), 'Default value was wrongfully overridden.');
     }
 
     public function envDoesNotOverrideOnWrongConfigKeysDataProvider(): Generator
diff --git a/tests/unit/Mage/Core/Helper/SecurityTest.php b/tests/unit/Mage/Core/Helper/SecurityTest.php
@@ -17,6 +17,7 @@
 
 namespace OpenMage\Tests\Unit\Mage\Core\Helper;
 
+use Generator;
 use Mage;
 use Mage_Core_Block_Abstract;
 use Mage_Core_Block_Template;
@@ -37,29 +38,22 @@ public function setUp(): void
     }
 
     /**
-     * @return array<int|string, array<int, array<empty, empty>|Mage_Page_Block_Html_Topmenu_Renderer|Mage_Core_Block_Template|string>>
-     *
      * @group Mage_Core
      * @group Mage_Core_Helper
      */
-    public function validateAgainstBlockMethodBlacklistDataProvider(): array
+    public function validateAgainstBlockMethodBlacklistDataProvider(): Generator
     {
-        // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
         $topmenu = new Mage_Page_Block_Html_Topmenu_Renderer();
-        // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
         $template = new Mage_Core_Block_Template();
-
-        return [
-            [
-                $topmenu,
-                'setData',
-                []
-            ],
-            [
-                $template,
-                'setData',
-                []
-            ],
+        yield [
+            $topmenu,
+            'setData',
+            []
+        ];
+        yield [
+            $template,
+            'setData',
+            []
         ];
     }
 
@@ -80,52 +74,46 @@ public function testValidateAgainstBlockMethodBlacklist(
         $this->subject->validateAgainstBlockMethodBlacklist($block, $method, $args);
     }
 
-    /**
-     * @return array<int|string, array<int, array<empty, empty>|Mage_Page_Block_Html_Topmenu_Renderer|Mage_Core_Block_Template|string>>
-     */
-    public function forbiddenBlockMethodsDataProvider(): array
+    public function forbiddenBlockMethodsDataProvider(): Generator
     {
         // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
         $topmenu = new Mage_Page_Block_Html_Topmenu_Renderer();
         // phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
         $template = new Mage_Core_Block_Template();
-
-        return [
-            [
-                $template,
-                'fetchView',
-                []
-            ],
-            [
-                $topmenu,
-                'fetchView',
-                []
-            ],
-            [
-                $topmenu,
-                'render',
-                []
-            ],
-            [
-                $template,
-                'Mage_Core_Block_Template::fetchView',
-                []
-            ],
-            [
-                $topmenu,
-                'Mage_Page_Block_Html_Topmenu_Renderer::fetchView',
-                []
-            ],
-            'parent class name is passed as second arg' => [
-                $topmenu,
-                'Mage_Core_Block_Template::fetchView',
-                []
-            ],
-            'parent class name is passed as second arg2' => [
-                $topmenu,
-                'Mage_Core_Block_Template::render',
-                []
-            ],
+        yield [
+            $template,
+            'fetchView',
+            []
+        ];
+        yield [
+            $topmenu,
+            'fetchView',
+            []
+        ];
+        yield [
+            $topmenu,
+            'render',
+            []
+        ];
+        yield [
+            $template,
+            'Mage_Core_Block_Template::fetchView',
+            []
+        ];
+        yield [
+            $topmenu,
+            'Mage_Page_Block_Html_Topmenu_Renderer::fetchView',
+            []
+        ];
+        yield 'parent class name is passed as second arg' => [
+            $topmenu,
+            'Mage_Core_Block_Template::fetchView',
+            []
+        ];
+        yield 'parent class name is passed as second arg2' => [
+            $topmenu,
+            'Mage_Core_Block_Template::render',
+            []
         ];
     }
 
diff --git a/tests/unit/Mage/Log/Model/CustomerTest.php b/tests/unit/Mage/Log/Model/CustomerTest.php
@@ -43,7 +43,7 @@ public function testGetLoginAtTimestamp(): void
 
         $this->assertNull($mock->getLoginAtTimestamp());
 
-        $mock->expects($this->any())->method('getLoginAt')->willReturn(true);
+        $mock->method('getLoginAt')->willReturn(true);
         $this->assertIsInt($mock->getLoginAtTimestamp());
     }
 }
diff --git a/tests/unit/Mage/Sitemap/Model/SitemapTest.php b/tests/unit/Mage/Sitemap/Model/SitemapTest.php
@@ -41,7 +41,7 @@ public function testGetPreparedFilename(): void
             ->setMethods(['getSitemapFilename'])
             ->getMock();
 
-        $mock->expects($this->any())->method('getSitemapFilename')->willReturn('text.xml');
+        $mock->method('getSitemapFilename')->willReturn('text.xml');
         $this->assertIsString($mock->getPreparedFilename());
     }
 
@@ -55,7 +55,7 @@ public function testGenerateXml(): void
             ->setMethods(['getSitemapFilename'])
             ->getMock();
 
-        $mock->expects($this->any())->method('getSitemapFilename')->willReturn('text.xml');
+        $mock->method('getSitemapFilename')->willReturn('text.xml');
         $this->assertInstanceOf(Mage_Sitemap_Model_Sitemap::class, $mock->generateXml());
     }
 }
diff --git a/tests/unit/Mage/Tax/Helper/DataTest.php b/tests/unit/Mage/Tax/Helper/DataTest.php
diff --git a/tests/unit/Varien/Db/Adapter/Pdo/MysqlTest.php b/tests/unit/Varien/Db/Adapter/Pdo/MysqlTest.php
diff --git a/tests/unit/Varien/ObjectTest.php b/tests/unit/Varien/ObjectTest.php

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@`
`17`	`17`
`18`	`18`	`namespace OpenMage\Tests\Unit\Base;`
`19`	`19`
	`20`	`+use Generator;`
`20`	`21`	`use PHPUnit\Framework\TestCase;`
`21`	`22`	`use SimpleXMLElement;`
`22`	`23`	`use XMLReader;`
`@@ -50,17 +51,12 @@ public function testXmlReaderIsValid(string $filepath): void`
`50`	`51`	`$this->assertTrue($xml->isValid());`
`51`	`52`	`}`
`52`	`53`
`53`		`- /**`
`54`		`- * @return string[][]`
`55`		`- */`
`56`		`- public function provideXmlFiles(): array`
	`54`	`+ public function provideXmlFiles(): Generator`
`57`	`55`	`{`
`58`	`56`	`$root = realpath(__DIR__ . '/../../../') . '/';`
`59`	`57`
`60`		`- return [`
`61`		`- 'file from vendor directory' => [`
`62`		`- $root . 'vendor/shardj/zf1-future/library/Zend/Locale/Data/es_419.xml'`
`63`		`- ],`
	`58`	`+ yield 'file from vendor directory' => [`
	`59`	`+ $root . 'vendor/shardj/zf1-future/library/Zend/Locale/Data/es_419.xml'`
`64`	`60`	`];`
`65`	`61`	`}`
`66`	`62`	`}`
Original file line number	Diff line number	Diff line change
`@@ -48,8 +48,8 @@ public function testValidate(array $expectedResult, array $methods): void`
`48`	`48`	`])`
`49`	`49`	`->getMock();`
`50`	`50`
`51`		`- $mock->expects($this->any())->method('getBlockName')->willReturn($methods['getBlockName']);`
`52`		`- $mock->expects($this->any())->method('getIsAllowed')->willReturn($methods['getIsAllowed']);`
	`51`	`+ $mock->method('getBlockName')->willReturn($methods['getBlockName']);`
	`52`	`+ $mock->method('getIsAllowed')->willReturn($methods['getIsAllowed']);`
`53`	`53`	`$this->assertEquals($expectedResult, $mock->validate());`
`54`	`54`	`}`
`55`	`55`
Original file line number	Diff line number	Diff line change
`@@ -45,8 +45,8 @@ public function testValidate($expectedResult, string $variableName, string $isAl`
`45`	`45`	`->setMethods(['getVariableName', 'getIsAllowed'])`
`46`	`46`	`->getMock();`
`47`	`47`
`48`		`- $mock->expects($this->any())->method('getVariableName')->willReturn($variableName);`
`49`		`- $mock->expects($this->any())->method('getIsAllowed')->willReturn($isAllowed);`
	`48`	`+ $mock->method('getVariableName')->willReturn($variableName);`
	`49`	`+ $mock->method('getIsAllowed')->willReturn($isAllowed);`
`50`	`50`	`$this->assertSame($expectedResult, $mock->validate());`
`51`	`51`	`}`
`52`	`52`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public function setUp(): void`
`39`	`39`	`public function testGetConvertTable(): void`
`40`	`40`	`{`
`41`	`41`	`$this->assertIsArray($this->subject->getConvertTable());`
`42`		`- $this->assertSame(317, count($this->subject->getConvertTable()));`
	`42`	`+ $this->assertCount(317, $this->subject->getConvertTable());`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`/**`