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

.github/workflows/code_analysis.yaml

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

bench/does_not_perform_assertion_lifecycle_bench.php

@@ -12,10 +12,12 @@
  */
 
 use PhpParser\Node\Stmt\ClassMethod;
+use Rector\Application\Provider\CurrentFileProvider;
 use Rector\DependencyInjection\LazyContainerFactory;
 use Rector\PhpParser\Node\BetterNodeFinder;
 use Rector\PHPUnit\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector;
 use Rector\Testing\TestingParser\TestingParser;
+use Rector\ValueObject\Application\File;
 
 require __DIR__ . '/../vendor/autoload.php';
 
@@ -98,11 +100,17 @@
 
 $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
-$classMethods = [];
+// 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);
-    $classMethods = [...$classMethods, ...$betterNodeFinder->findInstanceOf($stmts, ClassMethod::class)];
+    $file = new File($testFilePath, (string) file_get_contents($testFilePath));
+    $file->hydrateStmtsAndTokens($stmts, $stmts, []);
+    $classMethodsByFile[] = [$file, $betterNodeFinder->findInstanceOf($stmts, ClassMethod::class)];
 }
 
 $cpuSeconds = static function (): float {
@@ -115,9 +123,15 @@
 $cpuStart = $cpuSeconds();
 
 $changedCount = 0;
-foreach ($classMethods as $classMethod) {
-    if ($rector->refactor($classMethod) instanceof ClassMethod) {
-        ++$changedCount;
+$classMethodCount = 0;
+foreach ($classMethodsByFile as [$file, $classMethods]) {
+    $currentFileProvider->setFile($file);
+
+    foreach ($classMethods as $classMethod) {
+        ++$classMethodCount;
+        if ($rector->refactor($classMethod) instanceof ClassMethod) {
+            ++$changedCount;
+        }
     }
 }
 
@@ -127,8 +141,8 @@
 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('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) {

bench/does_not_perform_assertion_own_lifecycle_bench.php

@@ -0,0 +1,122 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * Benchmark for AddDoesNotPerformAssertionToNonAssertingTestRector on test classes with own lifecycle methods.
+ *
+ * Every test class has its own setUp() and tearDown(), every other one asserts in setUp(). The class is taken from
+ * the current file, so the rule looks into its lifecycle methods for 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\Application\Provider\CurrentFileProvider;
+use Rector\DependencyInjection\LazyContainerFactory;
+use Rector\PhpParser\Node\BetterNodeFinder;
+use Rector\PHPUnit\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector;
+use Rector\Testing\TestingParser\TestingParser;
+use Rector\ValueObject\Application\File;
+
+require __DIR__ . '/../vendor/autoload.php';
+
+const TEST_CLASSES = 300;    // every other one asserts in setUp()
+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_own_lifecycle_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) {
+    $isAsserting = $test % 2 === 0;
+    $className = sprintf('%sTest%d', $isAsserting ? 'Asserting' : 'NonAsserting', $test);
+
+    $setUpBody = $isAsserting ? '$this->assertTrue(true);' : '$this->ready = true;';
+    $methods = "    protected function setUp(): void\n    {\n        parent::setUp();\n        {$setUpBody}\n    }\n\n";
+    $methods .= "    protected function tearDown(): void\n    {\n        \$this->ready = null;\n        parent::tearDown();\n    }\n\n";
+    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\nfinal class {$className} extends \\PHPUnit\\Framework\\TestCase\n{\n    protected \$ready;\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);
+
+/** @var CurrentFileProvider $currentFileProvider */
+$currentFileProvider = $rectorConfig->make(CurrentFileProvider::class);
+
+// 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)];
+}
+
+$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;
+$classMethodCount = 0;
+foreach ($classMethodsByFile as [$file, $classMethods]) {
+    $currentFileProvider->setFile($file);
+
+    foreach ($classMethods as $classMethod) {
+        ++$classMethodCount;
+        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), $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);
+}

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

@@ -0,0 +1,42 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+class NonAssertingSetUp extends TestCase
+{
+    protected function setUp(): void
+    {
+        $this->value = 1;
+    }
+
+    public function testSomething()
+    {
+        $value = 5;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+class NonAssertingSetUp extends TestCase
+{
+    protected function setUp(): void
+    {
+        $this->value = 1;
+    }
+
+    #[\PHPUnit\Framework\Attributes\DoesNotPerformAssertions]
+    public function testSomething()
+    {
+        $value = 5;
+    }
+}
+
+?>

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

@@ -0,0 +1,56 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+abstract class ShadowBase extends TestCase
+{
+    protected function setUp(): void
+    {
+        $this->assertTrue(true);
+    }
+}
+
+final class ShadowChild extends ShadowBase
+{
+    protected function setUp(): void
+    {
+        $value = 1;
+    }
+
+    public function testSomething()
+    {
+        $value = 1;
+    }
+}
+?>
+-----
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+abstract class ShadowBase extends TestCase
+{
+    protected function setUp(): void
+    {
+        $this->assertTrue(true);
+    }
+}
+
+final class ShadowChild extends ShadowBase
+{
+    protected function setUp(): void
+    {
+        $value = 1;
+    }
+
+    #[\PHPUnit\Framework\Attributes\DoesNotPerformAssertions]
+    public function testSomething()
+    {
+        $value = 1;
+    }
+}
+?>

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

@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+class SkipAssertingAfterAnnotation extends TestCase
+{
+    /**
+     * @after
+     */
+    protected function cleanup(): void
+    {
+        $this->assertTrue(true);
+    }
+
+    public function testSomething()
+    {
+        $value = 5;
+    }
+}

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

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

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

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

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

@@ -0,0 +1,26 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class AnonThenHook extends TestCase
+{
+    public function testFirst()
+    {
+        $object = new class {
+            public function run() {}
+        };
+        $this->assertNotNull($object);
+    }
+
+    public function testSecond()
+    {
+        $value = 1;
+    }
+
+    protected function tearDown(): void
+    {
+        $this->assertTrue(true);
+    }
+}