Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate "Test" suffix for abstract test case classes #5132

Closed
sebastianbergmann opened this issue Jan 10, 2023 · 9 comments
Closed

Deprecate "Test" suffix for abstract test case classes #5132

sebastianbergmann opened this issue Jan 10, 2023 · 9 comments
Assignees
Labels
feature/test-runner CLI test runner type/deprecation Something will be/is deprecated
Milestone

Comments

@sebastianbergmann
Copy link
Owner

No description provided.

@blackdrago
Copy link

@sebastianbergmann I've been reading through various PRs on Github, the change log, and other PHPUnit documentation, looking for any background or details as to why this change was made and/or what naming convention (if any) is suggested for these abstract test classes, but I haven't found anything about it.

In our testing suite, we have a number of abstract "BaseTest" classes. Some of these establish shared testing setup elements (e.g., loading a specific data set that multiple tests use) while others contain core testing code that's used to test multiple classes (basic code reduction).

Was this change done simply to enforce a requirement (e.g., any class that uses the 'Test' suffix must be a class that PHPUnit can execute directly, and abstract test classes cannot be run directly, so they can't use the suffix), or is there more to it?

I know I'll be asked for 'the why' as soon as I create the ticket for updating the class names, so I figured I'd ask rather than assume.

@speedbomb
Copy link

@sebastianbergmann I have a question about this deprecation: As far as I can see, this should affect only abstract classes whose class name ends with "Test". There is no such class in our test suite, but we get this error:

PHPUnit 9.6.6 by Sebastian Bergmann and contributors.
Warning:       Abstract test case classes with "Test" suffix are deprecated (Tests\Unit\TestCases\BeModuleTestCase)

Is this behaviour intended?

@sebastianbergmann
Copy link
Owner Author

@speedbomb You may have run into an edge case that I will gladly investigate if you can provide a minimal, self-contained, reproducing test case that shows the problem. Please do so in a new issue. Thanks!

@speedbomb
Copy link

@speedbomb You may have run into an edge case [...]

It's conceivable that this edge case is the usage of the IDE PhpStorm: The abstract test case BeModuleTestCase has 3 inheritors. I can savely run the test method testCanCallView for each of them separately, but if i choose to run all 3 of them, the warning occurs.

Screenshot 2023-05-26 082234

@Jehong-Ahn
Copy link

Class Foo in Foo_Test.php causes this problem in my case.
I made the change to class Foo_Test.

athos-ribeiro added a commit to athos-ribeiro/composer that referenced this issue Jul 3, 2023
Abstract test case classes with Test suffix are deprecated in PHPUnit
10. We also change the ArchiverTest file name to match the new class
name (ArchiverTestCase).

* sebastianbergmann/phpunit#5132
@Mistralys
Copy link

Mistralys commented Jul 20, 2023

I am currently puzzling over this. On PHPunit 9.6.10 , I get the deprecation message even on classes that do not use any "Test" suffix at all, whether in the class name or the file name e.g.

BaseMailcodeChecks.php

Abstract test case classes with "Test" suffix are deprecated (BaseMailcodeChecks)

So I had a look at the PHPUnit code that triggers the message, and if I am not mistaken, it means that using abstract classes extending TestCase are deprecated in general?

TestSuite.php:459

if ($class->isAbstract() && $class->isSubclassOf(TestCase::class)) {
    $this->addWarning(
        sprintf(
            'Abstract test case classes with "Test" suffix are deprecated (%s)',
            $class->getName(),
        ),
    );
}

If that's the case, how should one handle methods common to test classes? With traits?

Seldaek pushed a commit to composer/composer that referenced this issue Jul 21, 2023
* Use static test data providers

Using non-static methods as a data providers was deprecated in phpunit
10.

* Rename abstract test class

Abstract test case classes with Test suffix are deprecated in PHPUnit
10. We also change the ArchiverTest file name to match the new class
name (ArchiverTestCase).

* sebastianbergmann/phpunit#5132
@midnite81
Copy link

Yeah, this is how it looks to me too, @Mistralys. If this is the case that no abstract classes can be used, this would have quite an impact on many codebases.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this issue Jul 29, 2023
The `BaseSniffTest` class, which is used as a basis for all test classes, should by rights be an `abstract` class as it doesn't contain any tests itself.

However, using the `Test` suffix for abstract test case classes is deprecated since PHPUnit 9.6.

Even though this wasn't causing us any problems so far (as the class wasn't `abstract`), let's tidy this up to future-proof PHPUnit cross-version compatibility.

So, in this commit:
* The `BaseSniffTest` class is renamed to `BaseSniffTestCase`, including updating the file name to match.
* All test classes which `extend` the class have been updated to now extend the `BaseSniffTestCase`.

Ref: sebastianbergmann/phpunit#5132
codemasher added a commit to codemasher/http-factory-tests that referenced this issue Oct 23, 2023
@borgogelli
Copy link

I am currently puzzling over this. On PHPunit 9.6.10 , I get the deprecation message even on classes that do not use any "Test" suffix at all, whether in the class name or the file name e.g.

BaseMailcodeChecks.php

Abstract test case classes with "Test" suffix are deprecated (BaseMailcodeChecks)

So I had a look at the PHPUnit code that triggers the message, and if I am not mistaken, it means that using abstract classes extending TestCase are deprecated in general?

TestSuite.php:459

if ($class->isAbstract() && $class->isSubclassOf(TestCase::class)) {
    $this->addWarning(
        sprintf(
            'Abstract test case classes with "Test" suffix are deprecated (%s)',
            $class->getName(),
        ),
    );
}

If that's the case, how should one handle methods common to test classes? With traits?

@Mistralys did you find any solutions ? I have the same problem with PHPUnit 10.4.2

@Mistralys
Copy link

@borgogelli I found out that the error message is only shown when I run a specific test. I am using PHPStorm, and when I choose a single test class or test method to run, I get the message. If I run all tests through phpunit.xml, PHPUnit does not complain. Maybe an issue with the scope of the test?

I have come to simply ignore the message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-runner CLI test runner type/deprecation Something will be/is deprecated
Projects
None yet
Development

No branches or pull requests

7 participants