[PHPUnit60] Skip test methods in abstract classes in AddDoesNotPerformAssertionToNonAssertingTestRector

.github/workflows/code_analysis.yaml

@@ -40,6 +40,10 @@ jobs:
                         name: 'Bench'
                         run: php bench/does_not_perform_assertion_lifecycle_bench.php
 
+                    -
+                        name: 'Bench abstract classes'
+                        run: php bench/does_not_perform_assertion_abstract_class_bench.php
+
         name: ${{ matrix.actions.name }}
         runs-on: ubuntu-latest
         timeout-minutes: 10

bench/does_not_perform_assertion_abstract_class_bench.php

@@ -0,0 +1,106 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * Benchmark for AddDoesNotPerformAssertionToNonAssertingTestRector on abstract and final test classes.
+ *
+ * Half of the test classes are abstract, so the abstract class check runs on every test method.
+ * Run on both branches to compare. CPU (user+sys) is the reliable metric; wall is noisy.
+ */
+
+use PhpParser\Node\Stmt\ClassMethod;
+use Rector\DependencyInjection\LazyContainerFactory;
+use Rector\PhpParser\Node\BetterNodeFinder;
+use Rector\PHPUnit\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector;
+use Rector\Testing\TestingParser\TestingParser;
+
+require __DIR__ . '/../vendor/autoload.php';
+
+const TEST_CLASSES = 300;    // every other one is abstract
+const TEST_METHODS = 6;      // non-asserting test methods per test class
+const MAX_CPU_SECONDS = 1.0; // fails in CI above this
+
+$directory = sys_get_temp_dir() . '/rector_phpunit_abstract_bench_' . getmypid();
+@mkdir($directory);
+
+// make generated classes visible to PHPStan reflection
+spl_autoload_register(static function (string $class) use ($directory): void {
+    if (! str_starts_with($class, 'Bench\\')) {
+        return;
+    }
+
+    $filePath = $directory . '/' . substr($class, 6) . '.php';
+    if (is_file($filePath)) {
+        require_once $filePath;
+    }
+});
+
+$testFilePaths = [];
+
+for ($test = 0; $test < TEST_CLASSES; ++$test) {
+    $isAbstract = $test % 2 === 0;
+    $className = sprintf('%sTest%d', $isAbstract ? 'Abstract' : 'Final', $test);
+
+    $methods = '';
+    for ($testMethod = 0; $testMethod < TEST_METHODS; ++$testMethod) {
+        $methods .= "    public function testSomething{$testMethod}(): void\n    {\n        \$value = {$testMethod};\n    }\n\n";
+    }
+
+    $filePath = $directory . '/' . $className . '.php';
+    file_put_contents(
+        $filePath,
+        "<?php\n\nnamespace Bench;\n\n" . ($isAbstract ? 'abstract' : 'final')
+        . " class {$className} extends \PHPUnit\Framework\TestCase\n{\n{$methods}}\n"
+    );
+
+    $testFilePaths[] = $filePath;
+}
+
+$rectorConfig = (new LazyContainerFactory())->create();
+$rectorConfig->boot();
+
+/** @var TestingParser $testingParser */
+$testingParser = $rectorConfig->make(TestingParser::class);
+
+/** @var BetterNodeFinder $betterNodeFinder */
+$betterNodeFinder = $rectorConfig->make(BetterNodeFinder::class);
+
+$rector = $rectorConfig->make(AddDoesNotPerformAssertionToNonAssertingTestRector::class);
+
+$classMethods = [];
+foreach ($testFilePaths as $testFilePath) {
+    $stmts = $testingParser->parseFileToDecoratedNodes($testFilePath);
+    $classMethods = [...$classMethods, ...$betterNodeFinder->findInstanceOf($stmts, ClassMethod::class)];
+}
+
+$cpuSeconds = static function (): float {
+    $usage = getrusage();
+    return $usage['ru_utime.tv_sec'] + $usage['ru_utime.tv_usec'] / 1_000_000
+        + $usage['ru_stime.tv_sec'] + $usage['ru_stime.tv_usec'] / 1_000_000;
+};
+
+$wallStart = microtime(true);
+$cpuStart = $cpuSeconds();
+
+$changedCount = 0;
+foreach ($classMethods as $classMethod) {
+    if ($rector->refactor($classMethod) instanceof ClassMethod) {
+        ++$changedCount;
+    }
+}
+
+$cpu = $cpuSeconds() - $cpuStart;
+$wall = microtime(true) - $wallStart;
+
+array_map('unlink', glob($directory . '/*.php'));
+@rmdir($directory);
+
+printf('test classes: %d, test methods: %d, changed: %d%s', count($testFilePaths), count($classMethods), $changedCount, PHP_EOL);
+printf('CPU:  %.4f s  (%.3f ms/method)%s', $cpu, $cpu / count($classMethods) * 1000, PHP_EOL);
+printf('wall: %.4f s%s', $wall, PHP_EOL);
+
+if ($cpu > MAX_CPU_SECONDS) {
+    printf('CPU %.4f s is over the %.1f s limit%s', $cpu, MAX_CPU_SECONDS, PHP_EOL);
+    exit(1);
+}

rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_abstract_class_inherited_test_with_asserting_child.php.inc

@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+abstract class AbstractBase extends TestCase
+{
+    public function testInherited()
+    {
+        $value = 1;
+    }
+}
+
+final class AbstractChild extends AbstractBase
+{
+    protected function setUp(): void
+    {
+        $this->assertTrue(true);
+    }
+}

rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_abstract_class_test_method.php.inc

@@ -0,0 +1,14 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+abstract class SkipAbstractClassTestMethod extends TestCase
+{
+    // a child class can add asserting lifecycle methods
+    public function testSomething()
+    {
+        $value = 5;
+    }
+}

rules/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector.php

@@ -150,6 +150,11 @@ private function shouldSkipClassMethod(ClassMethod $classMethod): bool
             return true;
         }
 
+        // a child class can add asserting lifecycle methods, that count toward the inherited tests
+        if ($this->isInAbstractClass($classMethod)) {
+            return true;
+        }
+
         // the parent test case asserts in its own integration methods
         if ($this->isInTwigIntegrationTestCase($classMethod)) {
             return true;
@@ -220,6 +225,16 @@ private function isInTrait(ClassMethod $classMethod): bool
         return $classReflection->isTrait();
     }
 
+    private function isInAbstractClass(ClassMethod $classMethod): bool
+    {
+        $classReflection = $this->reflectionResolver->resolveClassReflection($classMethod);
+        if (! $classReflection instanceof ClassReflection) {
+            return false;
+        }
+
+        return $classReflection->isAbstract();
+    }
+
     private function isInTwigIntegrationTestCase(ClassMethod $classMethod): bool
     {
         $classReflection = $this->reflectionResolver->resolveClassReflection($classMethod);