PHPUnit: added/updated tests

.gitattributes

@@ -23,6 +23,8 @@
 
 /README.md export-ignore
 
+/Makefile export-ignore
+
 # Enforce checkout with linux lf consistent over all platforms
 *.html text eol=lf
 *.css text eol=lf

.github/workflows/phpunit.yml

@@ -99,7 +99,7 @@ jobs:
             phpunit-result-cache-
 
       - name: Run phpUnit
-        run: php vendor/bin/phpunit --configuration .phpunit.dist.xml
+        run: php vendor/bin/phpunit --configuration .phpunit.dist.xml --testsuite Base,Error,Mage,Varien
 
       - name: Save result cache
         uses: actions/cache/save@v4

.php-cs-fixer.dist.php

@@ -15,7 +15,7 @@
         // PHP84: Adds or removes ? before single type declarations or |null at the end of union types when parameters have a default null value.
         'nullable_type_declaration_for_default_null_value' => true,
         // Calls to PHPUnit\Framework\TestCase static methods must all be of the same type, either $this->, self:: or static::
-        'php_unit_test_case_static_method_calls' => ['call_type' => 'this'],
+        'php_unit_test_case_static_method_calls' => ['call_type' => 'static'],
         // Convert double quotes to single quotes for simple strings.
         'single_quote' => true,
         // Arguments lists, array destructuring lists, arrays that are multi-line, match-lines and parameters lists must have a trailing comma.

.phpunit.dist.xml

@@ -31,6 +31,81 @@
         <testsuite name="Mage">
             <directory>tests/unit/Mage</directory>
         </testsuite>
+        <testsuite name="Mage_Admin">
+            <directory>tests/unit/Mage/Admin</directory>
+        </testsuite>
+        <testsuite name="Mage_Adminhtml">
+            <directory>tests/unit/Mage/Adminhtml</directory>
+        </testsuite>
+        <testsuite name="Mage_AdminNotification">
+            <directory>tests/unit/Mage/AdminNotification</directory>
+        </testsuite>
+        <testsuite name="Mage_Api">
+            <directory>tests/unit/Mage/Api</directory>
+        </testsuite>
+        <testsuite name="Mage_Catalog">
+            <directory>tests/unit/Mage/Catalog</directory>
+        </testsuite>
+        <testsuite name="Mage_Cms">
+            <directory>tests/unit/Mage/Cms</directory>
+        </testsuite>
+        <testsuite name="Mage_Contacts">
+            <directory>tests/unit/Mage/Contacts</directory>
+        </testsuite>
+        <testsuite name="Mage_Core">
+            <directory>tests/unit/Mage/Core</directory>
+        </testsuite>
+        <testsuite name="Mage_Customer">
+            <directory>tests/unit/Mage/Customer</directory>
+        </testsuite>
+        <testsuite name="Mage_Directory">
+            <directory>tests/unit/Mage/Directory</directory>
+        </testsuite>
+        <testsuite name="Mage_Downloadable">
+            <directory>tests/unit/Mage/Downloadable</directory>
+        </testsuite>
+        <testsuite name="Mage_Eav">
+            <directory>tests/unit/Mage/Eav</directory>
+        </testsuite>
+        <testsuite name="Mage_GiftMessage">
+            <directory>tests/unit/Mage/GiftMessage</directory>
+        </testsuite>
+        <testsuite name="Mage_Index">
+            <directory>tests/unit/Mage/Index</directory>
+        </testsuite>
+        <testsuite name="Mage_Log">
+            <directory>tests/unit/Mage/Log</directory>
+        </testsuite>
+        <testsuite name="Mage_Newsletter">
+            <directory>tests/unit/Mage/Newsletter</directory>
+        </testsuite>
+        <testsuite name="Mage_Page">
+            <directory>tests/unit/Mage/Page</directory>
+        </testsuite>
+        <testsuite name="Mage_Reports">
+            <directory>tests/unit/Mage/Reports</directory>
+        </testsuite>
+        <testsuite name="Mage_Review">
+            <directory>tests/unit/Mage/Review</directory>
+        </testsuite>
+        <testsuite name="Mage_Rule">
+            <directory>tests/unit/Mage/Rule</directory>
+        </testsuite>
+        <testsuite name="Mage_Sales">
+            <directory>tests/unit/Mage/Sales</directory>
+        </testsuite>
+        <testsuite name="Mage_Sitemap">
+            <directory>tests/unit/Mage/Sitemap</directory>
+        </testsuite>
+        <testsuite name="Mage_Tax">
+            <directory>tests/unit/Mage/Tax</directory>
+        </testsuite>
+        <testsuite name="Mage_Uploader">
+            <directory>tests/unit/Mage/Uploader</directory>
+        </testsuite>
+        <testsuite name="Mage_Wishlist">
+            <directory>tests/unit/Mage/Wishlist</directory>
+        </testsuite>
         <testsuite name="Varien">
             <directory>tests/unit/Varien</directory>
         </testsuite>

.rector.php

@@ -17,6 +17,7 @@
 use Rector\Php82\Rector as Php82;
 use Rector\Php83\Rector as Php83;
 use Rector\Php84\Rector as Php84;
+use Rector\PHPUnit\CodeQuality\Rector\Class_\PreferPHPUnitThisCallRector;
 use Rector\TypeDeclaration\Rector as TypeDeclaration;
 
 try {
@@ -49,6 +50,8 @@
             Php80\Class_\ClassPropertyAssignToConstructorPromotionRector::class, # todo: wait for php80
             Php80\Class_\StringableForToStringRector::class, # todo: wait for php80
             TypeDeclaration\ClassMethod\ReturnNeverTypeRector::class,
+            # use static methods
+            PreferPHPUnitThisCallRector::class,
             __DIR__ . '/shell/translations.php',
             __DIR__ . '/shell/update-copyright.php',
             __DIR__ . '/tests/unit/Mage/Reports/Model/Resource/Report/CollectionTest.php',

Makefile

@@ -0,0 +1,13 @@
+mkdocs-serve:
+	@if command -v mkdocs >/dev/null ; then \
+		mkdocs serve; \
+	else \
+		echo "mkdocs is not installed, see https://docs.openmage.org/developers/mkdocs/" && exit 2; \
+	fi; \
+
+phpunit-serve:
+	@if test -f build/coverage/index.html ; then \
+		xdg-open build/coverage/index.html; \
+	else \
+		echo "phpunit coverage not found, run \"composer run phpunit:coverage-local\"" && exit 2; \
+	fi; \

app/code/core/Mage/Adminhtml/Block/Cache/Grid.php

@@ -107,15 +107,14 @@ public function decorateStatus($value, $row, $column, $isExport)
     {
         $class = '';
         if (isset($this->_invalidatedTypes[$row->getId()])) {
-            $cell = '<span class="grid-severity-minor"><span>' . $this->__('Invalidated') . '</span></span>';
+            $class = self::CSS_SEVERITY_MINOR;
+            $value = $this->__('Invalidated');
+        } elseif ($row->getStatus()) {
+            $class = self::CSS_SEVERITY_NOTICE;
         } else {
-            if ($row->getStatus()) {
-                $cell = '<span class="grid-severity-notice"><span>' . $value . '</span></span>';
-            } else {
-                $cell = '<span class="grid-severity-critical"><span>' . $value . '</span></span>';
-            }
+            $class = self::CSS_SEVERITY_CRITICAL;
         }
-        return $cell;
+        return sprintf(self::PATTERN_SEVERITY, $class, $value);
     }
 
     /**

app/code/core/Mage/Adminhtml/Block/Permissions/OrphanedResource/Grid.php

@@ -15,6 +15,8 @@
  * @license    https://opensource.org/licenses/osl-3.0.php  Open Software License (OSL 3.0)
  */
 
+use Mage_Adminhtml_Block_Widget_Grid_Massaction_Abstract as MassAction;
+
 /**
  * Adminhtml permissions orphanedResource grid
  *
@@ -75,7 +77,7 @@ protected function _prepareMassaction()
         $this->setMassactionIdField('resource_id');
         $this->getMassactionBlock()->setFormFieldName('resource_id');
 
-        $this->getMassactionBlock()->addItem('delete', [
+        $this->getMassactionBlock()->addItem(MassAction::DELETE, [
             'label'    => Mage::helper('adminhtml')->__('Delete'),
             'url'      => $this->getUrl('*/*/massDelete'),
             'confirm'  => Mage::helper('adminhtml')->__('Are you sure you want to do this?'),

app/code/core/Mage/Adminhtml/Block/Widget/Grid.php

@@ -25,6 +25,12 @@
  */
 class Mage_Adminhtml_Block_Widget_Grid extends Mage_Adminhtml_Block_Widget
 {
+    public const PATTERN_SEVERITY           = '<span class="%s"><span>%s</span></span>';
+    public const CSS_SEVERITY_CRITICAL      = 'grid-severity-critical';
+    public const CSS_SEVERITY_MAJOR         = 'grid-severity-major';
+    public const CSS_SEVERITY_MINOR         = 'grid-severity-minor';
+    public const CSS_SEVERITY_NOTICE        = 'grid-severity-notice';
+
     /**
      * Columns array
      *

app/code/core/Mage/Cms/Block/Widget/Page/Link.php

@@ -68,6 +68,7 @@ public function getHref()
      * as parameter or retrieve page title from DB using passed identifier or page id.
      *
      * @return string
+     * @throws Mage_Core_Model_Store_Exception
      */
     public function getTitle()
     {
@@ -77,10 +78,9 @@ public function getTitle()
                 // compare to null used here bc user can specify blank title
                 $this->_title = $this->getData('title');
             } elseif ($this->getData('page_id')) {
-                $this->_title = Mage::getResourceSingleton('cms/page')->getCmsPageTitleById($this->getData('page_id'));
+                $this->_title = $this->getCmsPageTitleById($this->getData('page_id'));
             } elseif ($this->getData('href')) {
-                $this->_title = Mage::getResourceSingleton('cms/page')->setStore(Mage::app()->getStore())
-                    ->getCmsPageTitleByIdentifier($this->getData('href'));
+                $this->_title = $this->getCmsPageTitleByIdentifier($this->getData('href'));
             }
         }
 
@@ -93,6 +93,7 @@ public function getTitle()
      * if title will be blank string, page identifier will be used.
      *
      * @return string
+     * @throws Mage_Core_Model_Store_Exception
      */
     public function getAnchorText()
     {
@@ -101,14 +102,27 @@ public function getAnchorText()
         } elseif ($this->getTitle()) {
             $this->_anchorText = $this->getTitle();
         } elseif ($this->getData('href')) {
-            $this->_anchorText = Mage::getResourceSingleton('cms/page')->setStore(Mage::app()->getStore())
-                ->getCmsPageTitleByIdentifier($this->getData('href'));
+            $this->_anchorText = $this->getCmsPageTitleByIdentifier($this->getData('href'));
         } elseif ($this->getData('page_id')) {
-            $this->_anchorText = Mage::getResourceSingleton('cms/page')->getCmsPageTitleById($this->getData('page_id'));
+            $this->_anchorText = $this->getCmsPageTitleById($this->getData('page_id'));
         } else {
             $this->_anchorText = $this->getData('href');
         }
 
         return $this->_anchorText;
     }
+
+    protected function getCmsPageTitleById(int|string $pageId): string
+    {
+        return Mage::getResourceSingleton('cms/page')->getCmsPageTitleById($pageId);
+    }
+
+    /**
+     * @throws Mage_Core_Model_Store_Exception
+     */
+    protected function getCmsPageTitleByIdentifier(int|string $identifier): string
+    {
+        return Mage::getResourceSingleton('cms/page')->setStore(Mage::app()->getStore())
+            ->getCmsPageTitleByIdentifier($identifier);
+    }
 }

app/code/core/Mage/Core/Helper/Array.php

@@ -38,12 +38,10 @@ public function mergeRecursiveWithoutOverwriteNumKeys(array $baseArray, array $m
                 } else {
                     $baseArray[$key] = $value;
                 }
+            } elseif ($key) {
+                $baseArray[$key] = $value;
             } else {
-                if ($key) {
-                    $baseArray[$key] = $value;
-                } else {
-                    $baseArray[] = $value;
-                }
+                $baseArray[] = $value;
             }
         }
 

app/code/core/Mage/Index/Block/Adminhtml/Process/Grid.php

@@ -52,6 +52,7 @@ public function __construct()
      * Prepare grid collection
      *
      * @return $this
+     * @throws Exception
      */
     protected function _prepareCollection()
     {
@@ -66,6 +67,7 @@ protected function _prepareCollection()
      * Add name and description to collection elements
      *
      * @return $this
+     * @throws Mage_Core_Exception
      */
     protected function _afterLoadCollection()
     {
@@ -89,10 +91,10 @@ protected function _afterLoadCollection()
      * Prepare grid columns
      *
      * @return $this
+     * @throws Exception
      */
     protected function _prepareColumns()
     {
-        $baseUrl = $this->getUrl();
         $this->addColumn('indexer_code', [
             'header'    => Mage::helper('index')->__('Index'),
             'width'     => '180',
@@ -183,16 +185,16 @@ public function decorateStatus($value, $row, $column, $isExport)
         $class = '';
         switch ($row->getStatus()) {
             case Mage_Index_Model_Process::STATUS_PENDING:
-                $class = 'grid-severity-notice';
+                $class = self::CSS_SEVERITY_NOTICE;
                 break;
             case Mage_Index_Model_Process::STATUS_RUNNING:
-                $class = 'grid-severity-major';
+                $class = self::CSS_SEVERITY_MAJOR;
                 break;
             case Mage_Index_Model_Process::STATUS_REQUIRE_REINDEX:
-                $class = 'grid-severity-critical';
+                $class = self::CSS_SEVERITY_CRITICAL;
                 break;
         }
-        return '<span class="' . $class . '"><span>' . $value . '</span></span>';
+        return sprintf(self::PATTERN_SEVERITY, $class, $value);
     }
 
     /**
@@ -210,13 +212,13 @@ public function decorateUpdateRequired($value, $row, $column, $isExport)
         $class = '';
         switch ($row->getUpdateRequired()) {
             case 0:
-                $class = 'grid-severity-notice';
+                $class = self::CSS_SEVERITY_NOTICE;
                 break;
             case 1:
-                $class = 'grid-severity-critical';
+                $class = self::CSS_SEVERITY_CRITICAL;
                 break;
         }
-        return '<span class="' . $class . '"><span>' . $value . '</span></span>';
+        return sprintf(self::PATTERN_SEVERITY, $class, $value);
     }
 
     /**

app/code/core/Mage/Index/Model/Indexer.php

@@ -84,7 +84,7 @@ public function getProcessesCollection()
      * Get index process by specific id
      *
      * @param int $processId
-     * @return Mage_Index_Model_Process | false
+     * @return Mage_Index_Model_Process|false
      */
     public function getProcessById($processId)
     {

app/code/core/Mage/Index/Model/Process.php

@@ -23,6 +23,10 @@
  * @method string getIndexCode()
  * @method string getIndexerCode()
  * @method $this setIndexerCode(string $value)
+ * @method string getName()
+ * @method $this setName(string $value)
+ * @method string getDescription()
+ * @method $this setDescription(string $value)
  * @method string getStatus()
  * @method $this setStatus(string $value)
  * @method string getStartedAt()
@@ -33,6 +37,8 @@
  * @method $this setMode(string $value)
  * @method bool getForcePartialReindex()
  * @method $this setForcePartialReindex(bool $value)
+ * @method int getUpdateRequired()
+ * @method $this setUpdateRequired(int $value)
  */
 class Mage_Index_Model_Process extends Mage_Core_Model_Abstract
 {

app/code/core/Mage/Reports/Model/Resource/Report/Collection.php

@@ -39,7 +39,7 @@ class Mage_Reports_Model_Resource_Report_Collection
     /**
      * Report period
      *
-     * @var int
+     * @var string
      */
     protected $_period;
 
@@ -76,7 +76,7 @@ protected function _construct() {}
     /**
      * Set period
      *
-     * @param int $period
+     * @param string $period
      * @return $this
      */
     public function setPeriod($period)