Phpunit: Remove support for "ClassName<*>" as values for @covers and @uses annotations

[sebastianbergmann]

PHPUnit 10 should not support the following annotations anymore:

• @covers ClassName::<public>
• @covers ClassName::<protected>
• @covers ClassName::<private>
• @covers ClassName::<!public>
• @covers ClassName::<!protected>
• @covers ClassName::<!private>
• @covers ClassName<extended>
• @uses ClassName::<public>
• @uses ClassName::<protected>
• @uses ClassName::<private>
• @uses ClassName::<!public>
• @uses ClassName::<!protected>
• @uses ClassName::<!private>
• @uses ClassName<extended>

[andrewnicols]

I'm just wondering whether there will be any replacement for these and what the rationale is.

Generally I/we prefer to only @cover the public part of an API that we are actively testing, and to use @uses ClassName::<!public> to cover the private/protected methods. The rationale for this being that we are testing the public interface, but how the class actually achieves the result is its business. Technically I guess these could be considered to be functional tests, but the alternative is to add copious @covers tags for things that I'm not really testing but are covered, or to use Reflection to actively test business logic.

[Ocramius]

The rationale for this being that we are testing the public interface, but how the class actually achieves the result is its business.

The test is still supposed to figure out whether the internals of your system are sound and well covered or not. Having @covers Foo::<public> (for example) could lead to developers only writing public methods, or not having any coverage report at all on private API.

@uses can be limited to just the target class names: it's an external collaborator anyway, and public/private does not matter there either.

[Firehed]

Hi, somewhat late to the game here, but I agree with @andrewnicols's philosophy on coverage, and this is already blocking me from upgrading to 9.x due to (presumably) some change in the docblock parser that was probably related to plans for this. I'll assume for discussion purposes that breakage is a bug, but it would be just as relevant in the future if this happens for PHPUnit 10.

My tests all start off with the following:

/**
 * @coversDefaultClass Company\Project\My
 * @covers ::<protected>
 * @covers ::<private>
 */
class MyTest extends \PHPUnit\Framework\TestCase
{

Tests can and do figure out whether the internals of my code are sound, and the coverage reports confirm as much. Test cases interact with the public APIs, and exercise logic that's contained in the public and private APIs. The coverage reports for public APIs must be explicit, and anything internal is an implementation detail.

In my opinion, it's actively harmful for tests to be forced to know which private/protected methods any public API uses for the purpose of generating coverage reports, but as of the docs for 9.3, the only _recommended_ way to annotate coverage is class-wide; even covering specific class methods is explicitly "not recommended".

So while my gut reaction is that this isn't entirely a good direction to go, I'm open to alternatives. Maybe the best practices in the documentation could be made clearer? My current approach is based on a test generator skeleton that probably dates back to PHPUnit 4 or so. It would be really nice to not introduce code churn on thousands of test files in an upgrade, but if there's a better way I'll deal with it.

To be clear: I have no issue with dropping <public> support, or equivalents. There's a lot of value in being explicit about which part(s) of the public API a unit test is covering. But being able to either implicitly or explicitly cover any part of the private API (whether via <!public> or <private> or something else) seems like a fundamental goal of proper encapsulation.

[andrewnicols]

@sebastianbergmann it would be good to have some discussion on what the expected alternative is here.