Wrong default value for optional parameter of PHPUnit\Util\Test::parseTestMethodAnnotations() causes ReflectionException

[vemod]

Q	A
PHPUnit version	9.6.3
PHP version	7.4
Installation Method	PHAR

Summary

A lot of ReflectionException messages in the test output when using testdox.
prettifyTestClass($className = 'MyTest') should not throw ReflectionException

Current behavior

When xdebug is active bunch of noise due to not properly handled ReflectionExceptions.
This issue exists since ages: see #4379

How to reproduce

cat > MyTest.php <<FILE
<?php

class MyTest extends \PHPUnit\Framework\TestCase {

  public function testFoo(): void {
    \$this->assertTrue(true);
  }
}
FILE

php /tmp/phpunit.phar --testdox  MyTest.php

PHP ReflectionException:  Method suite does not exist in phar:///tmp/phpunit.phar/phpunit/Runner/BaseTestRunner.php on line 100
PHP Stack trace:
PHP   1. {main}() /tmp/phpunit.phar:0
PHP   2. PHPUnit\TextUI\Command::main($exit = *uninitialized*) /tmp/phpunit.phar:2305
PHP   3. PHPUnit\TextUI\Command->run($argv = [0 => '/tmp/phpunit.phar', 1 => '--testdox', 2 => 'MyTest.php'], $exit = TRUE) phar:///tmp/phpunit.phar/phpunit/TextUI/Command.php:94
PHP   4. PHPUnit\Runner\BaseTestRunner->getTest($suiteClassFile = '/tmp/MyTest.php', $suffixes = [0 => 'Test.php', 1 => '.phpt']) phar:///tmp/phpunit.phar/phpunit/TextUI/Command.php:109
PHP   5. ReflectionClass->getMethod($name = 'suite') phar:///tmp/phpunit.phar/phpunit/Runner/BaseTestRunner.php:100
PHPUnit 9.6.3 by Sebastian Bergmann and contributors.

PHP ReflectionException:  Method MyTest::() does not exist in phar:///tmp/phpunit.phar/phpunit/Util/Annotation/Registry.php on line 70
PHP Stack trace:
PHP   1. {main}() /tmp/phpunit.phar:0
PHP   2. PHPUnit\TextUI\Command::main($exit = *uninitialized*) /tmp/phpunit.phar:2305
PHP   3. PHPUnit\TextUI\Command->run($argv = [0 => '/tmp/phpunit.phar', 1 => '--testdox', 2 => 'MyTest.php'], $exit = TRUE) phar:///tmp/phpunit.phar/phpunit/TextUI/Command.php:94
...
PHP  13. PHPUnit\Util\TestDox\NamePrettifier->prettifyTestClass($className = 'MyTest') phar:///tmp/phpunit.phar/phpunit/Util/TestDox/CliTestDoxPrinter.php:83
PHP  14. PHPUnit\Util\Test::parseTestMethodAnnotations($className = 'MyTest', $methodName = *uninitialized*) phar:///tmp/phpunit.phar/phpunit/Util/TestDox/NamePrettifier.php:76
PHP  15. PHPUnit\Util\Annotation\Registry->forMethod($classInHierarchy = 'MyTest', $method = '') phar:///tmp/phpunit.phar/phpunit/Util/Test.php:279
PHP  16. ReflectionMethod->__construct($class_or_method = 'MyTest', $name = '') phar:///tmp/phpunit.phar/phpunit/Util/Annotation/Registry.php:70


My
✔

Time: 00:00.003, Memory: 10.00 MB

OK (1 test, 1 assertion)

Expected behavior

No exceptions in the output if nothing is wrong with the code.

Testdox processors should either catch exceptions not expose them, or let the developer know what is wrong with the code. Throwing exception when some generated method doesn't exist doesn't help anyone.

PHP  13. PHPUnit\Util\TestDox\NamePrettifier->prettifyTestClass($className = 'MyTest') 

should not throw ReflectionException

[sebastianbergmann]

I cannot reproduce this and do not understand what you are trying to report.

[sebastianbergmann]

I do not know if this is related, but the the PHPUnit documentation suggests to configure xdebug.show_exception_trace=0.

[vemod]

yes xdebug.show_exception_trace=0 makes the exception disappear as many other exceptions which wanted to see/catch.

in the end the issue is with defining default value "" but checking against null. If default value would have been null or the check afterwards would check the default value instead of just arbitrary null, then we would not have this false positive exception.

public static function parseTestMethodAnnotations(string $className, ?string $methodName = '') : array
    {
        $registry = Registry::getInstance();
        if ($methodName !== null) {

it should be

public static function parseTestMethodAnnotations(string $className, ?string $methodName = null) : array