Improve coverage handling of value objects

[gbirke]

With the advent of readonly classes in PHP 8.2, we're now able to have a lot of very simple value objects like this:

readonly class BookResult {
   public function construct(
      string $title,
      string $author,
      int $numberOfPages,
   ) {}
}

These value objects have no code that could be tested for behavior or functionality, but they still show up as uncovered in the coverage report, with the constructor being uncovered. I was wondering how we could improve this and would like to suggest some ideas:

• Give an example of best practices around value objects in the PHPUnit documentation
  • Should I use a CoversClass attribute in another test that uses this as a parameter / result? Which test, all tests that use the class, etc?
  • Should I just use the @codeCoverageIgnore annotation, with an explanation?
• Add a new PHPUnit attribute ValueObject that influences the coverage reports, but might have a different behavior than @codeCoverageIgnore (e.g. regarding the disableCodeCoverageIgnore flag)
• Do some light static analysis on classes (either on the PHPUnit side or the code coverage engine side) that marks classes as covered if they are readonly and have only a constructor method.
• Make it possible to have a naming convention in the coverage configuration section to ignore certain classes by name, eg. all classes ending in DTO, Result, Data, ValueObject, etc.

The last two improvements would have the benefit of saving a lines of code, as we don't have to annotate every value object. The ValueObject attribute would make the intent very clear, without being too "magic".

What do you think?

[sebastianbergmann]

Adding a complete example for reference:

BookResult.php

<?php declare(strict_types=1);
final readonly class BookResult
{
   public function __construct(
      string $title,
      string $author,
      int $numberOfPages,
   ) {}
}

Code Coverage for BookResult.php when no BookResult object is created during tests

Code Coverage for BookResult.php when a BookResult object is created during tests

I used PHP 8.3.4, Xdebug 3.3.1, PHPUnit 11.0.8, and php-code-coverage 11.0.3.

[sebastianbergmann]

Constructor property promotion is used to avoid boilerplate code. But even if you do not need to write that code and do not see it when you look at the class, it is still there somehow. If no object of the class in question is created while the tests are running then the constructor of that class is correctly reported by the code coverage source, Xdebug in this case, to be executable and not executed.

I understand that the current behavior is annoying. If I would use constructor property promotion then it would annoy me for sure. But as I do not, I rely on others such as yourself to be annoyed and escalate that annoyance to me :-)

Jokes aside, I though that we were already "cleaning up" constructor property promotion and force line 8 in the above example to be not executable. Do I remember this wrong, @Slamdunk?

[Slamdunk]

You remember wrong :smile:

Many users rely on this very functionality to track down unused Value Objects: I would suggest to keep the current behaviour in php-code-coverage.

[sebastianbergmann]

I would suggest to keep the current behaviour in php-code-coverage.

I agree with you 😄, but unfortunately that does not help @gbirke.

[sebastianbergmann]

• Should I just use the @codeCoverageIgnore annotation, with an explanation?

@gbirke I think that would be the best way to deal with this.

[gbirke]

You remember wrong 😄

Many users rely on this very functionality to track down unused Value Objects: I would suggest to keep the current behaviour in php-code-coverage.

What would be a typical setup for this use case? The way I'd set it up would be to add the coverage annotations/attributes to the test cases that use the value object (as described in my initial question). When the class and its test get deleted but not the value object, you get an uncovered class error in CI.

I can see additional the benefit of annotating your tests rather than your production code (with tool-specific annotations).

@sebastianbergmann An extreme example, just for my understanding: If I'd use CoversClass for my value object on every test class and then suddenly don't use the value object any more (neither in prod nor in test code) would it show up as covered or uncovered? Or would it show up at all?

@Slamdunk Do you have heuristics for where to put the annotation/attribute? One test class, all test classes, all test methods? Can you point me to repos where people do this, so I can see it in action (also maybe look at the CI setup)?

[Slamdunk]

If I'd use CoversClass for my value object on every test class and then suddenly don't use the value object any more (neither in prod nor in test code) would it show up as covered or uncovered? Or would it show up at all?

I would show up as uncovered

Do you have heuristics for where to put the annotation/attribute? One test class, all test classes, all test methods?

This choice is up to the developer. I prefer to put the #[CoversClass] only on tests that make sensible usage of that class, which in good projects should be just a few.

[sebastianbergmann]

@gbirke Can this be closed?