Add phpunit v10 support (#373)

atk4 · Nov 26, 2023 · b0a1bdc · b0a1bdc
1 parent 936e077
commit b0a1bdc
Show file tree

Hide file tree

Showing 25 changed files with 200 additions and 90 deletions.
diff --git a/.github/workflows/build-changelog.yml b/.github/workflows/build-changelog.yml
@@ -6,10 +6,11 @@ on:
       - develop
 
 jobs:
-  update_release_draft:
+  update:
+    name: Update
     runs-on: ubuntu-latest
     steps:
-      - name: Run Release Drafter
+      - name: Run
         uses: release-drafter/release-drafter@v5
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml
@@ -46,15 +46,15 @@ jobs:
 
       - name: Install PHP dependencies
         run: |
-          if [ "${{ matrix.type }}" != "Phpunit" ] && [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpunit/phpunit johnkary/phpunit-speedtrap --dev; fi
+          if [ "${{ matrix.type }}" != "Phpunit" ] && [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpunit/phpunit atk4/ergebnis-phpunit-slow-test-detector --dev; fi
           if [ "${{ matrix.type }}" != "CodingStyle" ]; then composer remove --no-interaction --no-update friendsofphp/php-cs-fixer --dev; fi
           if [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpstan/\* --dev; fi
           composer update --ansi --prefer-dist --no-interaction --no-progress --optimize-autoloader
 
       - name: "Run tests (only for Phpunit)"
         if: startsWith(matrix.type, 'Phpunit')
         run: |
-          vendor/bin/phpunit --exclude-group none --no-coverage -v
+          vendor/bin/phpunit --exclude-group none --no-coverage --fail-on-warning --fail-on-risky $(if vendor/bin/phpunit --version | grep -q '^PHPUnit 9\.'; then echo -v; else echo --fail-on-notice --fail-on-deprecation --display-notices --display-deprecations --display-warnings --display-errors --display-incomplete --display-skipped; fi)
 
       - name: Check Coding Style (only for CodingStyle)
         if: matrix.type == 'CodingStyle'
@@ -108,21 +108,21 @@ jobs:
 
       - name: Install PHP dependencies
         run: |
-          if [ "${{ matrix.type }}" != "Phpunit" ] && [ "${{ matrix.type }}" != "Phpunit Lowest" ] && [ "${{ matrix.type }}" != "Phpunit Burn" ]; then composer remove --no-interaction --no-update phpunit/phpunit johnkary/phpunit-speedtrap --dev; fi
+          if [ "${{ matrix.type }}" != "Phpunit" ] && [ "${{ matrix.type }}" != "Phpunit Lowest" ] && [ "${{ matrix.type }}" != "Phpunit Burn" ]; then composer remove --no-interaction --no-update phpunit/phpunit atk4/ergebnis-phpunit-slow-test-detector --dev; fi
           if [ "${{ matrix.type }}" != "CodingStyle" ]; then composer remove --no-interaction --no-update friendsofphp/php-cs-fixer --dev; fi
           if [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpstan/\* --dev; fi
           if [ -n "$LOG_COVERAGE" ]; then composer require --no-interaction --no-install phpunit/phpcov; fi
           composer update --ansi --prefer-dist --no-interaction --no-progress --optimize-autoloader
           if [ "${{ matrix.type }}" = "Phpunit Lowest" ]; then composer update --ansi --prefer-dist --prefer-lowest --prefer-stable --no-interaction --no-progress --optimize-autoloader; fi
-          if [ "${{ matrix.type }}" = "Phpunit Burn" ]; then sed -i 's~ *public function runBare(): void~public function runBare(): void { gc_collect_cycles(); gc_collect_cycles(); $memDiffs = array_fill(0, '"$(if [ \"$GITHUB_EVENT_NAME\" == \"schedule\" ]; then echo 1024; else echo 64; fi)"', 0); for ($i = -1; $i < count($memDiffs); ++$i) { $this->_runBare(); gc_collect_cycles(); gc_collect_cycles(); $mem = memory_get_usage(); if ($i !== -1) { $memDiffs[$i] = $mem - $memPrev; } $memPrev = $mem; rsort($memDiffs); if (array_sum($memDiffs) >= 4096 * 1024 || $memDiffs[2] > 0) { $this->onNotSuccessfulTest(new AssertionFailedError("Memory leak detected! (" . implode(" + ", array_map(static fn ($v) => number_format($v / 1024, 3, ".", " "), array_filter($memDiffs))) . " KB, " . ($i + 2) . " iterations)")); } } } private function _runBare(): void~' vendor/phpunit/phpunit/src/Framework/TestCase.php && cat vendor/phpunit/phpunit/src/Framework/TestCase.php | grep '_runBare('; fi
+          if [ "${{ matrix.type }}" = "Phpunit Burn" ]; then sed -i 's~public function runBare(): void~public function runBare(): void { gc_collect_cycles(); gc_collect_cycles(); $memDiffs = array_fill(0, '"$(if [ \"$GITHUB_EVENT_NAME\" == \"schedule\" ]; then echo 1024; else echo 64; fi)"', 0); for ($i = -1; $i < count($memDiffs); ++$i) { $this->_runBare(); gc_collect_cycles(); gc_collect_cycles(); $mem = memory_get_usage(); if ($i !== -1) { $memDiffs[$i] = $mem - $memPrev; } $memPrev = $mem; rsort($memDiffs); if (array_sum($memDiffs) >= 4096 * 1024 || $memDiffs[2] > 0) { $this->onNotSuccessfulTest(new AssertionFailedError("Memory leak detected! (" . implode(" + ", array_map(static fn ($v) => number_format($v / 1024, 3, ".", " "), array_filter($memDiffs))) . " KB, " . ($i + 2) . " iterations)")); } } } private function _runBare(): void~' vendor/phpunit/phpunit/src/Framework/TestCase.php && cat vendor/phpunit/phpunit/src/Framework/TestCase.php | grep '_runBare('; fi
 
       - name: Init
         run: |
           if [ -n "$LOG_COVERAGE" ]; then mkdir coverage; fi
 
       - name: "Run tests"
         run: |
-          php -d opcache.enable_cli=1 vendor/bin/phpunit --exclude-group none $(if [ -n "$LOG_COVERAGE" ]; then echo --coverage-text; else echo --no-coverage; fi) -v
+          php -d opcache.enable_cli=1 vendor/bin/phpunit --exclude-group none $(if [ -n "$LOG_COVERAGE" ]; then echo --coverage-text; else echo --no-coverage; fi) --fail-on-warning --fail-on-risky $(if vendor/bin/phpunit --version | grep -q '^PHPUnit 9\.'; then echo -v; else echo --fail-on-notice --fail-on-deprecation --display-notices --display-deprecations --display-warnings --display-errors --display-incomplete --display-skipped; fi)
 
       - name: Upload coverage logs 1/2 (only for coverage)
         if: env.LOG_COVERAGE

diff --git a/composer.json b/composer.json
@@ -40,14 +40,14 @@
         "symfony/yaml": "^3.4 || ^4.4 || ^5.1 || ^6.0 || ^7.0"
     },
     "require-dev": {
+        "atk4/ergebnis-phpunit-slow-test-detector": "^2.4",
         "ergebnis/composer-normalize": "^2.13",
         "friendsofphp/php-cs-fixer": "^3.0",
-        "johnkary/phpunit-speedtrap": "^3.3",
         "phpstan/extension-installer": "^1.1",
         "phpstan/phpstan": "^1.0",
         "phpstan/phpstan-deprecation-rules": "^1.0",
         "phpstan/phpstan-strict-rules": "^1.3",
-        "phpunit/phpunit": "^9.5.5"
+        "phpunit/phpunit": "^9.5.5 || ^10.0"
     },
     "minimum-stability": "dev",
     "prefer-stable": true,

diff --git a/docs/container.md b/docs/container.md
@@ -231,7 +231,7 @@ $args = ['name' => 'child_name']; // obsolete, backward-compatible
 ```
 
 Method will return the object. Will throw exception if child with same
-name already exist.
+name already exists.
 :::
 
 :::{php:method} removeElement($shortName)

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -20,3 +20,29 @@ parameters:
             path: 'tests/DynamicMethodTraitTest.php'
             message: '~^Call to an undefined method Atk4\\Core\\Tests\\(DynamicMethodMock|DynamicMethodWithoutHookMock)::\w+\(\)\.$~'
             count: 10
+
+        # remove once PHPUnit 9.x support is removed
+        -
+            path: 'src/Phpunit/TestCase.php'
+            message: '~^Access to constant (STATUS_INCOMPLETE|STATUS_SKIPPED) on an unknown class PHPUnit\\Runner\\BaseTestRunner\.$~'
+            count: 2
+        -
+            path: 'src/Phpunit/TestCase.php'
+            message: '~^Call to an undefined method Atk4\\Core\\Phpunit\\TestCase::(getName|getStatus|getTestResultObject)\(\)\.$~'
+            count: 4
+        -
+            path: 'src/Phpunit/TestCase.php'
+            message: '~^Call to an undefined static method PHPUnit\\Util\\Test::(getLinesToBeCovered|getLinesToBeUsed)\(\)\.$~'
+            count: 2
+        -
+            path: 'tests/HookTraitTest.php'
+            message: '~^Call to an undefined method Atk4\\Core\\Tests\\HookTraitTest::getName\(\)\.$~'
+            count: 2
+        -
+            path: 'tests/Phpunit/TestCaseTest.php'
+            message: '~^Call to an undefined method Atk4\\Core\\Tests\\Phpunit\\TestCaseTest::getStatus\(\)\.$~'
+            count: 1
+        -
+            path: 'tests/Phpunit/TestCaseTest.php'
+            message: '~^Access to constant STATUS_INCOMPLETE on an unknown class PHPUnit\\Runner\\BaseTestRunner\.$~'
+            count: 1
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -4,14 +4,16 @@
             <directory>tests</directory>
         </testsuite>
     </testsuites>
-    <listeners>
-        <listener class="JohnKary\PHPUnit\Listener\SpeedTrapListener" />
-    </listeners>
-    <coverage>
+    <extensions>
+        <bootstrap class="Ergebnis\PHPUnit\SlowTestDetector\Extension" />
+    </extensions>
+    <source>
         <include>
             <directory>src</directory>
             <directory>tests</directory>
         </include>
+    </source>
+    <coverage>
         <report>
             <php outputFile="coverage/phpunit.cov" />
         </report>

diff --git a/src/CollectionTrait.php b/src/CollectionTrait.php
@@ -27,10 +27,10 @@ trait CollectionTrait
      *
      * @param string $collection property name
      */
-    public function _addIntoCollection(string $name, object $item, string $collection): object
+    protected function _addIntoCollection(string $name, object $item, string $collection): object
     {
         if (!isset($this->{$collection}) || !is_array($this->{$collection})) {
-            throw (new Exception('Collection does NOT exist'))
+            throw (new Exception('Collection does not exist'))
                 ->addMoreInfo('collection', $collection);
         }
 
@@ -41,11 +41,10 @@ public function _addIntoCollection(string $name, object $item, string $collectio
         }
 
         if ($this->_hasInCollection($name, $collection)) {
-            throw (new Exception('Element with the same name already exist in the collection'))
+            throw (new Exception('Element with the same name already exists in the collection'))
                 ->addMoreInfo('collection', $collection)
                 ->addMoreInfo('name', $name);
         }
-        $this->{$collection}[$name] = $item;
 
         // carry on reference to application if we have appScopeTraits set
         if ((TraitUtil::hasAppScopeTrait($this) && TraitUtil::hasAppScopeTrait($item))
@@ -69,6 +68,8 @@ public function _addIntoCollection(string $name, object $item, string $collectio
             }
         }
 
+        $this->{$collection}[$name] = $item;
+
         return $item;
     }
 
@@ -77,10 +78,10 @@ public function _addIntoCollection(string $name, object $item, string $collectio
      *
      * @param string $collection property name
      */
-    public function _removeFromCollection(string $name, string $collection): void
+    protected function _removeFromCollection(string $name, string $collection): void
     {
         if (!$this->_hasInCollection($name, $collection)) {
-            throw (new Exception('Element is NOT in the collection'))
+            throw (new Exception('Element is not in the collection'))
                 ->addMoreInfo('collection', $collection)
                 ->addMoreInfo('name', $name);
         }
@@ -94,7 +95,7 @@ public function _removeFromCollection(string $name, string $collection): void
      *
      * @param string $collectionName property name to be cloned
      */
-    public function _cloneCollection(string $collectionName): void
+    protected function _cloneCollection(string $collectionName): void
     {
         $this->{$collectionName} = array_map(function ($item) {
             $item = clone $item;
@@ -111,19 +112,19 @@ public function _cloneCollection(string $collectionName): void
      *
      * @param string $collection property name
      */
-    public function _hasInCollection(string $name, string $collection): bool
+    protected function _hasInCollection(string $name, string $collection): bool
     {
         return isset($this->{$collection}[$name]);
     }
 
     /**
      * @param string $collection property name
      */
-    public function _getFromCollection(string $name, string $collection): object
+    protected function _getFromCollection(string $name, string $collection): object
     {
         $res = $this->{$collection}[$name] ?? null;
         if ($res === null) {
-            throw (new Exception('Element is NOT in the collection'))
+            throw (new Exception('Element is not in the collection'))
                 ->addMoreInfo('collection', $collection)
                 ->addMoreInfo('name', $name);
         }

diff --git a/src/ContainerTrait.php b/src/ContainerTrait.php
@@ -24,7 +24,7 @@ trait ContainerTrait
     /**
      * Returns unique element name based on desired name.
      */
-    public function _uniqueElementName(string $desired): string
+    protected function _uniqueElementName(string $desired): string
     {
         if (!isset($this->_elementNameCounts[$desired])) {
             $this->_elementNameCounts[$desired] = 1;
@@ -130,7 +130,7 @@ public function removeElement($shortName)
         }
 
         if (!isset($this->elements[$shortName])) {
-            throw (new Exception('Could not remove child from parent. Instead of destroy() try using removeField / removeColumn / ..'))
+            throw (new Exception('Child element not found'))
                 ->addMoreInfo('parent', $this)
                 ->addMoreInfo('name', $shortName);
         }

diff --git a/src/DynamicMethodTrait.php b/src/DynamicMethodTrait.php
@@ -67,7 +67,7 @@ public function addMethod(string $name, \Closure $fx)
         }
 
         if ($this->hasMethod($name)) {
-            throw (new Exception('Registering method twice'))
+            throw (new Exception('Method is already defined'))
                 ->addMoreInfo('name', $name);
         }
 

diff --git a/src/Phpunit/TestCase.php b/src/Phpunit/TestCase.php
@@ -7,17 +7,43 @@
 use Atk4\Core\WarnDynamicPropertyTrait;
 use PHPUnit\Framework\TestCase as BaseTestCase;
 use PHPUnit\Framework\TestResult;
+use PHPUnit\Metadata\Api\CodeCoverage as CodeCoverageMetadata;
 use PHPUnit\Runner\BaseTestRunner;
+use PHPUnit\Runner\CodeCoverage;
 use PHPUnit\Util\Test as TestUtil;
-use SebastianBergmann\CodeCoverage\CodeCoverage;
+use SebastianBergmann\CodeCoverage\CodeCoverage as CodeCoverageRaw;
+
+if (\PHP_VERSION_ID >= 8_01_00) {
+    trait Phpunit9xTestCaseTrait
+    {
+        protected function onNotSuccessfulTest(\Throwable $e): never
+        {
+            $this->_onNotSuccessfulTest($e);
+        }
+    }
+} else {
+    trait Phpunit9xTestCaseTrait
+    {
+        protected function onNotSuccessfulTest(\Throwable $e): void
+        {
+            $this->_onNotSuccessfulTest($e);
+        }
+    }
+}
 
 /**
  * Generic TestCase for PHPUnit tests for ATK4 repos.
  */
 abstract class TestCase extends BaseTestCase
 {
+    use Phpunit9xTestCaseTrait;
     use WarnDynamicPropertyTrait;
 
+    final public static function isPhpunit9x(): bool
+    {
+        return (new \ReflectionClass(self::class))->hasMethod('getStatus');
+    }
+
     protected function setUp(): void
     {
         // rerun data providers to fix coverage when coverage for test files is enabled
@@ -89,15 +115,15 @@ protected function tearDown(): void
         gc_collect_cycles();
 
         // fix coverage for skipped/incomplete tests
-        // based on https://github.com/sebastianbergmann/phpunit/blob/9.5.21/src/Framework/TestResult.php#L830
-        // and https://github.com/sebastianbergmann/phpunit/blob/9.5.21/src/Framework/TestResult.php#L857
-        if (in_array($this->getStatus(), [BaseTestRunner::STATUS_SKIPPED, BaseTestRunner::STATUS_INCOMPLETE], true)) {
-            $coverage = $this->getTestResultObject()->getCodeCoverage();
+        // based on https://github.com/sebastianbergmann/phpunit/blob/9.5.21/src/Framework/TestResult.php#L830 https://github.com/sebastianbergmann/phpunit/blob/10.4.2/src/Framework/TestRunner.php#L154
+        // and https://github.com/sebastianbergmann/phpunit/blob/9.5.21/src/Framework/TestResult.php#L857 https://github.com/sebastianbergmann/phpunit/blob/10.4.2/src/Framework/TestRunner.php#L178
+        if (self::isPhpunit9x() ? in_array($this->getStatus(), [BaseTestRunner::STATUS_SKIPPED, BaseTestRunner::STATUS_INCOMPLETE], true) : $this->status()->isSkipped() || $this->status()->isIncomplete()) {
+            $coverage = self::isPhpunit9x() ? $this->getTestResultObject()->getCodeCoverage() : (CodeCoverage::instance()->isActive() ? CodeCoverage::instance() : null);
             if ($coverage !== null) {
-                $coverageId = \Closure::bind(static fn () => $coverage->currentId, null, CodeCoverage::class)();
+                $coverageId = self::isPhpunit9x() ? \Closure::bind(static fn () => $coverage->currentId, null, CodeCoverageRaw::class)() : (\Closure::bind(static fn () => $coverage->collecting, null, CodeCoverage::class)() ? $this : null);
                 if ($coverageId !== null) {
-                    $linesToBeCovered = TestUtil::getLinesToBeCovered(static::class, $this->getName(false));
-                    $linesToBeUsed = TestUtil::getLinesToBeUsed(static::class, $this->getName(false));
+                    $linesToBeCovered = self::isPhpunit9x() ? TestUtil::getLinesToBeCovered(static::class, $this->getName(false)) : (new CodeCoverageMetadata())->linesToBeCovered(static::class, $this->name());
+                    $linesToBeUsed = self::isPhpunit9x() ? TestUtil::getLinesToBeUsed(static::class, $this->getName(false)) : (new CodeCoverageMetadata())->linesToBeUsed(static::class, $this->name());
                     $coverage->stop(true, $linesToBeCovered, $linesToBeUsed);
                     $coverage->start($coverageId);
                 }
@@ -131,7 +157,10 @@ private function releaseObjectsFromExceptionTrace(\Throwable $e): void
         }
     }
 
-    protected function onNotSuccessfulTest(\Throwable $e): void
+    /**
+     * @return never
+     */
+    protected function _onNotSuccessfulTest(\Throwable $e): void
     {
         // release objects from uncaught exception as it is never released
         $this->releaseObjectsFromExceptionTrace($e);

diff --git a/src/TraitUtil.php b/src/TraitUtil.php
@@ -29,7 +29,7 @@ public static function hasTrait($class, string $traitName): bool
             // prevent mass use for other than internal use then we can decide
             // if we want to keep support this or replace with pure interfaces
             if (!str_starts_with($traitName, 'Atk4\Core\\')) {
-                throw new Exception(self::class . '::hasTrait is not intended for use with other than Atk4\Core\* traits');
+                throw new Exception(self::class . '::hasTrait() method is not intended for use with other than Atk4\Core\* traits');
             }
 
             $parentClass = get_parent_class($class);

diff --git a/src/TranslatableTrait.php b/src/TranslatableTrait.php
@@ -12,7 +12,7 @@
 trait TranslatableTrait
 {
     /**
-     * Translates the given message.
+     * Translate the given message.
      *
      * @param string               $message    The message to be translated
      * @param array<string, mixed> $parameters Array of parameters used to translate message

diff --git a/tests/CollectionMock.php b/tests/CollectionMock.php
@@ -15,7 +15,7 @@ class CollectionMock
     protected $fields = [];
 
     /**
-     * @param array<mixed>|object|null $seed
+     * @param array<mixed>|FieldMock|null $seed
      */
     public function addField(string $name, $seed = null): FieldMock
     {