Test suite loader produces warning and does not run any tests if the test class name is partially similar to the parent test case name

[mbessolov]

Q	A
PHPUnit version	9.1.4
PHP version	7.3
Installation Method	Both Composer and PHAR

Summary

When the current test class (e.g. FooTest) extends the parent class with a partially matching name (ending with FooTest, for example ParentFooTest, AbstractFooTest or similar), the testsuite loader does not run any tests and produces a warning:

<?php
class FooTest extends ParentFooTest
{
    public function testSomething()
    {
        static::assertTrue(true);
    }
}

$ vendor/bin/phpunit tests/FooTest.php
PHPUnit 9.1.4 by Sebastian Bergmann and contributors.

W                                                                   1 / 1 (100%)

Time: 00:00.026, Memory: 6.00 MB

There was 1 warning:

1) Warning
No tests found in class "App\Tests\ParentFooTest".

WARNINGS!
Tests: 1, Assertions: 0, Warnings: 1.

Current behavior

phpunit/src/Runner/StandardTestSuiteLoader.php

Lines 44 to 48 in 1dc40a5

	$offset = 0 - \strlen($suiteClassName);
	foreach ($loadedClasses as $loadedClass) {
	if (\substr($loadedClass, $offset) === $suiteClassName) {
	$suiteClassName = $loadedClass;

- the purpose of this is not clear to me, so I cannot comment on whether it does what it is supposed to do or not, but it produces the bug I described.

How to reproduce

Here is the reproducer - https://github.com/mbessolov/phpunit-loader-bug

Expected behavior

It should not matter if there is any partial similary between the names of the current test class and any of its parents.

[sebastianbergmann]

@flow-control Can you have a look? Thanks!

[realFlowControl]

Hey @mbessolov 🖖

I could reproduce your problem, thanks for the well working minimal example 👍

This bug only exists in PHP 7.3 with PHPUnit ^9. PHP 7.4 and 7.2 (the later with older PHPUnit) work as expected.

The problem is in the behavior of get_declared_classes() which returns the declared classes in another order. In PHP 7.3 we first get the ParentFooTest and then the FooTest, with PHP 7.4 there is no guaranteed order anymore and out of luck we get it in the "right" order.

For documentation purposes:

• this works in PHPUnit < 9 because of additional guard clauses in the StandardTestSuiteLoader
• this bug exists in PHPUnit >= 9.0.0 when used with PHP 7.3

I'll provide a fix.

Now to the code snippet you found: The purpose of it is to find the class when it is namespaced. From CLI you call with the filename test/FooTest.php from that we create the class name FooTest and search for it after including the given file. But it may be that the class is in a namespace, so we can not find a class FooTest, but only a class App\Tests\FooTest. Finding the "right" class is the sole purpose of this snippet.
And also this is the part with the 🐛

Hope I could help.

/Flo

[realFlowControl]

@sebastianbergmann should we also provide a fix for this in 9.0 or is a fix in 9.1 sufficient?

[sebastianbergmann]

There is no 9.0 branch anymore :)

[TysonAndre]

What about legacy unit test suite classes that use _ as a separator instead of actual namespaces, such as MyNamespace_FooTest?

• Would you recommend just stopping using them entirely given that namespaces have been around since php 5.3?
• The notes for 9.1.5 could note that some class names are now rejected

                if (
                    \substr($loadedClass, $offset) === $suiteClassName &&
                    \basename(\str_replace('\\', '/', $loadedClass)) == $suiteClassName
                ) {

A broader check that could be used instead of the above check:

                if (
                    \substr($loadedClass, $offset) === $suiteClassName &&
                    (\basename(\str_replace('\\', '/', $loadedClass)) == $suiteClassName || \basename(\str_replace('_', '/', $loadedClass)) === $suiteClassName)
                ) {

[realFlowControl]

Hey there 🖖

I personally would recommend stopping using pseudo namespaces in class names. Not only that namespaces have been around since quite a while, thanks to tools like Rector it has become straight forwarded to refactor from pseudo namespaces to real namespaces.

The StandardTestSuiteLoader was marked deprecated in order to clean up with the legacy stuff and problems inherited in it. Although your proposed change looks small and easy, since v9.1 it is deprecated and gives you a warning when the test case class name does not match the filename, which would be the case with pseudo namespaces. So folks see that there is a problem upcoming and have time to adopt to it.

All in all, I would rather suggest to refactor from pseudo namespaces to real ones 😃

Hope I could help

/Flo