Bring back more fine-grained code coverage targeting

[jnoordsij]

In PHPUnit 10.0, attributes were added to (optionally) replace code comment annotations (great feature btw!). However, these are currently limited only to apply to classes, where previously covers attributes were allowed to be on methods as well.

It would be fruitful for some usecases (as mentioned in various comments below) to allow for more fine-grained coverage measurement; in particular to allow targetting (class) methods by name using attributes.

Note: if this is an intentional change, and there is no plan to actively support this anymore, maybe this should be turned into two other issues:

• add a deprecation message for covers attributes on methods
• an issue on the Rector Rules repository to not automatically convert method-based covers annotations into attributes, as it will result into a broken tests suite (Attribute "PHPUnit\Framework\Attributes\CoversClass" cannot target method (allowed targets: class))

[sebastianbergmann]

This is intentional and explained in #4502.

[jnoordsij]

Thanks for pointing me there! I've raised https://github.com/rectorphp/rector-phpunit/issues/149 to highlight this issue there.

I think a separate deprecation for method-based annotations is probably not be needed, as this will be covered by deprecating all annotations in #4505. However it might be worth adding a somewhat more extended migration guide at that point, with some pointers on how to replace method-based annotations with class-based attributes.

[kylekatarnls]

This is intentional and explained in #4502.

I see it's intentional and noted "not recommended because too fine-grained"

But I'm searching with no luck where this design choice is explained. I've cases similar to #5558 (comment) where it's relevant to avoid a method to be unintentionally covered.

There I can't find any good reason why it would be bad to be too fine-grained in cover targeting.

Any reading you can recommend @sebastianbergmann to learn why we should not to that and then how we prevent accidental coverage of a method?

[mattvb91]

This is a crazy change to remove being able to cover methods directly... Defeats the whole purpose of having @covers in the first place...

Any recommendations on how to get targeted coverage results back?

[jrfnl]

I'm struggling with the same thing as everyone else in this thread.

Yes, of course, everyone should be using SOLID OO and classes should only have one responsibility and in an ideal world, using CoversClass would work brilliantly.

Except I do not live in such an ideal world. I live in a world of legacy code, where there are plenty of classes which do too much and where I want to know exactly what code (i.e. what method(s)) is covered by dedicated tests (vs incidentally covered) to know whether it is safe to refactor that code (preferably to more SOLID code).

Unfortunately, that will no longer be possible, which makes adding Covers* attributes at all, basically useless, as it no longer prevents unintentional code coverage from being recorded.

@sebastianbergmann Please, please, please reconsider this design choice, as CoversClass is next to useless for real world situations.

[sebastianbergmann]

Do you only want targetting methods back or do you also want to be able to use coverage target attributes on test methods in addition to test classes?

[jrfnl]

Do you only want targetting methods back or do you also want to be able to use coverage target attributes on test methods in addition to test classes?

@sebastianbergmann For me, the most important part is being able to target methods.

I don't have a problem with splitting up test classes which were targeting multiple different units of code to multiple test classes, each targeting just one unit of code.

Splitting test classes is not a breaking change, just busy-work while I have better things to do, but I can deal with that.
Having to refactor large parts of the code base under test just to be able to see what code is covered by tests, that's a whole other matter due to the breaking changes in the public API that would involve.