Skip to content

[PHPUnit60] Follow parent::test() calls in AddDoesNotPerformAssertionToNonAssertingTestRector - #826

Merged
TomasVotruba merged 1 commit into
rectorphp:mainfrom
bbrala:does-not-perform-assertions-parent-call
Nov 14, 2026
Merged

TomasVotruba merged 1 commit into
rectorphp:mainfrom
bbrala:does-not-perform-assertions-parent-call

Conversation

@bbrala

@bbrala bbrala commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

Part of rectorphp/rector#9965, split from #822 as requested. The expectUserDeprecationMessage() part is already in #825.

AddDoesNotPerformAssertionToNonAssertingTestRector adds #[DoesNotPerformAssertions] to tests that only call the parent test:

abstract class BaseTest extends TestCase
{
    public function testInherited(): void
    {
        $this->assertTrue(true);
    }
}

final class SomeTest extends BaseTest
{
    public function testInherited(): void
    {
        parent::testInherited();
    }
}

With failOnRisky that test then fails with "This test is not expected to perform assertions but performed 1 assertion".

Change

  • parent::test() is resolved to the direct parent class of the current class. The type resolver gives a union of all ancestors for parent, so the call was never followed.
  • PHPUnit\Framework\TestCase is never resolved as a parent: it has no asserting test methods and is slow to parse.
  • The containsAssertCall() cache key now includes the name of the class owning the method. Identical bodies (parent::test(), but also $this->helper()) can mean different things per class. This also fixes a false positive that already happens on main: two sibling classes with the same $this->helper(); body but different helpers shared a cache entry (sibling_classes_with_same_helper_body fixture).

Bench

bench/does_not_perform_assertion_lifecycle_bench.php (#824), CPU, 3 runs, local ddev PHP 8.5:

CPU
main 0.12-0.16 s
this PR 0.23-0.28 s

The extra time is real work: on main the parent:: calls in the bench were never followed. The class name in the cache key costs part of it too; it is needed for correctness.

Tests

New fixtures: parent_test_call_without_assert, skip_parent_test_call_with_assert, sibling_classes_with_same_helper_body. Full suite, ECS, Rector, class-leak and finalize-classes pass. PHPStan reports the same 6 offsetAccess.invalidOffset errors as main, in files this PR doesn't touch.

Lifecycle hooks (setUp(), assertPostConditions(), #[Before] ...) follow in a separate PR stacked on this one.

…ToNonAssertingTestRector (rectorphp/rector#9965)

"parent::test()" was not resolved, as "parent" is a union of all ancestors in the type resolver. Resolve it to the direct parent class of the current class instead.

The containsAssertCall() cache key now includes the name of the class owning the method. Identical bodies ("parent::test()" or "$this->helper()") can mean different things per class, so sibling classes sharing a cache entry could get a wrong result. This also fixes a false positive that already happens without the parent:: change, see the sibling_classes_with_same_helper_body fixture.

PHPUnit\Framework\TestCase itself is never resolved as a parent, it has no asserting test methods and is slow to parse.
@TomasVotruba

Copy link
Copy Markdown
Member

Lets ship it, thank you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants