[PHPUnit60] Skip tests in classes with asserting lifecycle methods in AddDoesNotPerformAssertionToNonAssertingTestRector

bench/does_not_perform_assertion_lifecycle_bench.php

@@ -6,7 +6,14 @@
  * Benchmark for AddDoesNotPerformAssertionToNonAssertingTestRector on test classes with deep parent chains.
  *
  * Each test class has parent lifecycle hooks (setUp(), tearDown()) and parent::test*() calls. Resolving these
- * via AstResolver parses and decorates parent files on first use, so this bench measures one cold pass, as in a real run.
+ * via AstResolver parses and decorates parent files on first use, so every scenario measures one cold pass, as in a real run.
+ *
+ * Scenarios, all with the same shape (see the constants below):
+ * - inherited:       parents have non-asserting hooks, the test classes have none. Every test method changes.
+ * - own set up:      like "inherited", but each test class has its own setUp() calling parent::setUp(). The file of
+ *                    the test class is already parsed by Rector, it must not be parsed a second time to look at its hooks.
+ * - asserting hooks: the top parent of each chain has an asserting hook (setUp(), assertPostConditions() or a
+ *                    #[Before] method), so the lifecycle analyzer walks the whole chain and no test method changes.
  *
  * Run on both branches to compare. CPU (user+sys) is the reliable metric; wall is noisy.
  */
@@ -28,15 +35,30 @@
 const FILLER_METHODS = 15;  // extra methods per parent class, to make parsing realistic
 const MAX_CPU_SECONDS = 1.0; // fails in CI above this
 
-$directory = sys_get_temp_dir() . '/rector_phpunit_bench_' . getmypid();
-@mkdir($directory);
+const SCENARIOS = [
+    'inherited' => [
+        'own_set_up' => false,
+        'asserting_hooks' => false,
+    ],
+    'own set up' => [
+        'own_set_up' => true,
+        'asserting_hooks' => false,
+    ],
+    'asserting hooks' => [
+        'own_set_up' => false,
+        'asserting_hooks' => true,
+    ],
+];
 
-$writeClass = static function (string $className, string $body) use ($directory): string {
-    $filePath = $directory . '/' . $className . '.php';
-    file_put_contents($filePath, "<?php\n\nnamespace Bench;\n\n" . $body);
-    return $filePath;
+$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;
 };
 
+$directory = sys_get_temp_dir() . '/rector_phpunit_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\\')) {
@@ -49,103 +71,138 @@
     }
 });
 
-$testFilePaths = [];
+$isFailed = false;
+$scenarioIndex = 0;
 
-for ($chain = 0; $chain < CHAINS; ++$chain) {
-    $parentClassName = '\PHPUnit\Framework\TestCase';
+foreach (SCENARIOS as $scenarioName => $scenario) {
+    // own class names per scenario, so no scenario profits from the parsed files of another
+    $prefix = 'S' . $scenarioIndex++;
 
-    for ($level = 0; $level < DEPTH; ++$level) {
-        $className = sprintf('AbstractChain%dLevel%d', $chain, $level);
+    $writeClass = static function (string $className, string $body) use ($directory): string {
+        $filePath = $directory . '/' . $className . '.php';
+        file_put_contents($filePath, "<?php\n\nnamespace Bench;\n\n" . $body);
+        return $filePath;
+    };
 
-        $methods = "    protected function setUp(): void\n    {\n        parent::setUp();\n        \$this->prepare{$level}();\n    }\n\n";
-        $methods .= "    protected function tearDown(): void\n    {\n        \$this->cleanup = null;\n        parent::tearDown();\n    }\n\n";
-        $methods .= "    public function testInherited(): void\n    {\n        \$value = {$level};\n    }\n\n";
-        $methods .= "    private function prepare{$level}(): void\n    {\n        \$this->ready = true;\n    }\n\n";
+    $testFilePaths = [];
 
-        for ($filler = 0; $filler < FILLER_METHODS; ++$filler) {
-            $methods .= "    protected function helper{$filler}(array \$items): array\n    {\n        return array_map(fn (\$item) => \$item * {$filler}, \$items);\n    }\n\n";
-        }
+    for ($chain = 0; $chain < CHAINS; ++$chain) {
+        $parentClassName = '\PHPUnit\Framework\TestCase';
 
-        $writeClass(
-            $className,
-            "abstract class {$className} extends {$parentClassName}\n{\n    protected \$ready;\n    protected \$cleanup;\n\n{$methods}}\n"
-        );
+        for ($level = 0; $level < DEPTH; ++$level) {
+            $className = sprintf('%sAbstractChain%dLevel%d', $prefix, $chain, $level);
 
-        $parentClassName = $className;
-    }
+            $methods = "    protected function setUp(): void\n    {\n        parent::setUp();\n        \$this->prepare{$level}();\n    }\n\n";
+            $methods .= "    protected function tearDown(): void\n    {\n        \$this->cleanup = null;\n        parent::tearDown();\n    }\n\n";
+            $methods .= "    public function testInherited(): void\n    {\n        \$value = {$level};\n    }\n\n";
+            $methods .= "    private function prepare{$level}(): void\n    {\n        \$this->ready = true;\n    }\n\n";
+
+            if ($scenario['asserting_hooks'] && $level === 0) {
+                // the asserting hook is in the top parent, furthest away from the test classes
+                $methods .= match ($chain % 3) {
+                    0 => "    protected function assertPostConditions(): void\n    {\n        \$this->assertTrue(\$this->ready);\n    }\n\n",
+                    1 => "    #[\PHPUnit\Framework\Attributes\Before]\n    protected function checkReady(): void\n    {\n        \$this->assertNotNull(\$this->cleanup);\n    }\n\n",
+                    default => "    protected function assertPreConditions(): void\n    {\n        \$this->assertFalse(\$this->ready);\n    }\n\n",
+                };
+            }
 
-    for ($test = 0; $test < TESTS_PER_CHAIN; ++$test) {
-        $className = sprintf('Chain%dTest%d', $chain, $test);
+            for ($filler = 0; $filler < FILLER_METHODS; ++$filler) {
+                $methods .= "    protected function helper{$filler}(array \$items): array\n    {\n        return array_map(fn (\$item) => \$item * {$filler}, \$items);\n    }\n\n";
+            }
 
-        $methods = "    public function testInherited(): void\n    {\n        parent::testInherited();\n    }\n\n";
-        for ($testMethod = 0; $testMethod < TEST_METHODS; ++$testMethod) {
-            $methods .= "    public function testSomething{$testMethod}(): void\n    {\n        \$value = \$this->helper{$testMethod}([1, 2, 3]);\n    }\n\n";
+            $writeClass(
+                $className,
+                "abstract class {$className} extends {$parentClassName}\n{\n    protected \$ready;\n    protected \$cleanup;\n\n{$methods}}\n"
+            );
+
+            $parentClassName = $className;
         }
 
-        $testFilePaths[] = $writeClass(
-            $className,
-            "final class {$className} extends {$parentClassName}\n{\n{$methods}}\n"
-        );
+        for ($test = 0; $test < TESTS_PER_CHAIN; ++$test) {
+            $className = sprintf('%sChain%dTest%d', $prefix, $chain, $test);
+
+            $methods = '';
+            if ($scenario['own_set_up']) {
+                $methods .= "    protected function setUp(): void\n    {\n        parent::setUp();\n        \$this->ready = true;\n    }\n\n";
+            }
+
+            $methods .= "    public function testInherited(): void\n    {\n        parent::testInherited();\n    }\n\n";
+            for ($testMethod = 0; $testMethod < TEST_METHODS; ++$testMethod) {
+                $methods .= "    public function testSomething{$testMethod}(): void\n    {\n        \$value = \$this->helper{$testMethod}([1, 2, 3]);\n    }\n\n";
+            }
+
+            $testFilePaths[] = $writeClass(
+                $className,
+                "final class {$className} extends {$parentClassName}\n{\n{$methods}}\n"
+            );
+        }
     }
-}
 
-$rectorConfig = (new LazyContainerFactory())->create();
-$rectorConfig->boot();
+    $rectorConfig = (new LazyContainerFactory())->create();
+    $rectorConfig->boot();
 
-/** @var TestingParser $testingParser */
-$testingParser = $rectorConfig->make(TestingParser::class);
+    /** @var TestingParser $testingParser */
+    $testingParser = $rectorConfig->make(TestingParser::class);
 
-/** @var BetterNodeFinder $betterNodeFinder */
-$betterNodeFinder = $rectorConfig->make(BetterNodeFinder::class);
+    /** @var BetterNodeFinder $betterNodeFinder */
+    $betterNodeFinder = $rectorConfig->make(BetterNodeFinder::class);
 
-$rector = $rectorConfig->make(AddDoesNotPerformAssertionToNonAssertingTestRector::class);
+    $rector = $rectorConfig->make(AddDoesNotPerformAssertionToNonAssertingTestRector::class);
 
-/** @var CurrentFileProvider $currentFileProvider */
-$currentFileProvider = $rectorConfig->make(CurrentFileProvider::class);
+    // parse test files up front, parent files are left for the rule to resolve
+    // the rule finds the class of a method in the stmts of the current file, as it does in a real run
+    /** @var CurrentFileProvider $currentFileProvider */
+    $currentFileProvider = $rectorConfig->make(CurrentFileProvider::class);
 
-// parse test files up front, parent files are left for the rule to resolve
-// the rule finds the class of a method in the stmts of the current file, as in a real run
-$classMethodsByFile = [];
-foreach ($testFilePaths as $testFilePath) {
-    $stmts = $testingParser->parseFileToDecoratedNodes($testFilePath);
-    $file = new File($testFilePath, (string) file_get_contents($testFilePath));
-    $file->hydrateStmtsAndTokens($stmts, $stmts, []);
-    $classMethodsByFile[] = [$file, $betterNodeFinder->findInstanceOf($stmts, ClassMethod::class)];
-}
+    $classMethodsByFile = [];
+    $methodCount = 0;
+    foreach ($testFilePaths as $testFilePath) {
+        $stmts = $testingParser->parseFileToDecoratedNodes($testFilePath);
 
-$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;
-};
+        $file = new File($testFilePath, (string) file_get_contents($testFilePath));
+        $file->hydrateStmtsAndTokens($stmts, $stmts, []);
 
-$wallStart = microtime(true);
-$cpuStart = $cpuSeconds();
+        $classMethods = $betterNodeFinder->findInstanceOf($stmts, ClassMethod::class);
+        $classMethodsByFile[] = [$file, $classMethods];
+        $methodCount += count($classMethods);
+    }
+
+    $wallStart = microtime(true);
+    $cpuStart = $cpuSeconds();
 
-$changedCount = 0;
-$classMethodCount = 0;
-foreach ($classMethodsByFile as [$file, $classMethods]) {
-    $currentFileProvider->setFile($file);
+    $changedCount = 0;
+    foreach ($classMethodsByFile as [$file, $classMethods]) {
+        $currentFileProvider->setFile($file);
 
-    foreach ($classMethods as $classMethod) {
-        ++$classMethodCount;
-        if ($rector->refactor($classMethod) instanceof ClassMethod) {
-            ++$changedCount;
+        foreach ($classMethods as $classMethod) {
+            if ($rector->refactor($classMethod) instanceof ClassMethod) {
+                ++$changedCount;
+            }
         }
     }
-}
 
-$cpu = $cpuSeconds() - $cpuStart;
-$wall = microtime(true) - $wallStart;
+    $cpu = $cpuSeconds() - $cpuStart;
+    $wall = microtime(true) - $wallStart;
+
+    printf(
+        '%s: test classes: %d, test methods: %d, changed: %d%s',
+        $scenarioName,
+        count($testFilePaths),
+        $methodCount,
+        $changedCount,
+        PHP_EOL
+    );
+    printf('CPU:  %.4f s  (%.3f ms/method)%s', $cpu, $cpu / $methodCount * 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);
+        $isFailed = true;
+    }
+
+}
 
 array_map('unlink', glob($directory . '/*.php'));
 @rmdir($directory);
 
-printf('test classes: %d, test methods: %d, changed: %d%s', count($testFilePaths), $classMethodCount, $changedCount, PHP_EOL);
-printf('CPU:  %.4f s  (%.3f ms/method)%s', $cpu, $cpu / $classMethodCount * 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);
-}
+exit($isFailed ? 1 : 0);

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

@@ -0,0 +1,24 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+trait AssertingBeforeTrait
+{
+    #[\PHPUnit\Framework\Attributes\Before]
+    protected function check(): void
+    {
+        $this->assertTrue(true);
+    }
+}
+
+final class TraitBefore extends TestCase
+{
+    use AssertingBeforeTrait;
+
+    public function testSomething()
+    {
+        $value = 1;
+    }
+}

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

@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+abstract class Deep0 extends TestCase { protected function setUp(): void { $this->assertTrue(true); } }
+abstract class Deep1 extends Deep0 { protected function setUp(): void { parent::setUp(); } }
+abstract class Deep2 extends Deep1 { protected function setUp(): void { parent::setUp(); } }
+abstract class Deep3 extends Deep2 { protected function setUp(): void { parent::setUp(); } }
+abstract class Deep4 extends Deep3 { protected function setUp(): void { parent::setUp(); } }
+abstract class Deep5 extends Deep4 { protected function setUp(): void { parent::setUp(); } }
+abstract class Deep6 extends Deep5 { protected function setUp(): void { parent::setUp(); } }
+
+final class DeepTest extends Deep6
+{
+    public function testSomething()
+    {
+        $value = 1;
+    }
+}

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

@@ -0,0 +1,29 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+abstract class DeepC0 extends TestCase { protected function setUp(): void { $this->assertTrue(true); } }
+abstract class DeepC1 extends DeepC0 { protected function setUp(): void { parent::setUp(); } }
+abstract class DeepC2 extends DeepC1 { protected function setUp(): void { parent::setUp(); } }
+abstract class DeepC3 extends DeepC2 { protected function setUp(): void { parent::setUp(); } }
+abstract class DeepC4 extends DeepC3 { protected function setUp(): void { parent::setUp(); } }
+abstract class DeepC5 extends DeepC4 { protected function setUp(): void { parent::setUp(); } }
+abstract class DeepC6 extends DeepC5 { protected function setUp(): void { parent::setUp(); } }
+
+final class DeepCTest extends DeepC6
+{
+    public function testSomething()
+    {
+        $value = 1;
+    }
+}
+
+final class DeepCShallowTest extends DeepC2
+{
+    public function testOther()
+    {
+        $other = 2;
+    }
+}

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

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

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

@@ -0,0 +1,16 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+use Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Source\TraitWithAssertingSetUp;
+
+final class SkipAssertingSetUpInTraitFromOtherFile extends TestCase
+{
+    use TraitWithAssertingSetUp;
+
+    public function testSomething()
+    {
+        $value = 5;
+    }
+}

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

@@ -0,0 +1,13 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Source\AbstractClassUsingAssertingTrait;
+
+final class SkipAssertingSetUpInTraitOfParent extends AbstractClassUsingAssertingTrait
+{
+    public function testSomething()
+    {
+        $value = 5;
+    }
+}