Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
PHPUnit's code coverage tool is great, but unfortunately there is code that is:
a) either not unit testable [ procedural function call, external dependency, ... ]
b) a false positive like the ending line of an array according to Drupal's code standards.
As of now there are 2 functions using this annotation in Core.
Proposed resolution
Allow usage of @codeCoverageIgnore annotations on:
- classes
- class functions
- individual lines (for false positives)
Remaining tasks
- Discuss
- Change coding standards (if we want to do this)
User interface changes
- None
API changes
- None
Comments
Comment #1
tim.plunkettIn contrib I use the block-level @codeCoverageIgnore for one line methods that wrap functions like drupal_set_message.
There are a couple in HEAD on methods in MenuTreeParameters.
What I think is weird is something like
continue; // @codeCoverageIgnore
taken from #2395143-113: YAML parsing is very slow, cache it with FileCache.Mostly because that seems like a completely testable line of code...
Comment #2
neclimdulAm definitely for allowing them as well.
I'm not really sure we need to encourage them on trailing array lines though. I'm aware there was some weirdness around this at some point. Maybe it still exists but in the coverage I just ran of
\Drupal\Core\Form\ConfigFormBase
everything was fine. We should only be excluding actually non-testable code.Comment #3
Mile23tl;dr: Mark up the test as much as possible rather than the code under test. Not a fan of
@codeCoverageIgnore
.You *can* unit-test a procedural function, as long as it has dependencies you can mock.
If you want to test code that depends on global functions without covering those functions, then it's time to use
@uses
. https://phpunit.de/manual/current/en/appendixes.annotations.html#appendi... In fact, this helps with--strict-coverage
, and if we care about coverage at this level, we should re-investigate adding strict coverage to our test suite.We should use
@coversNothing
in test method docblocks that aren't unit-oriented, because it spells out the intent of the test. https://phpunit.de/manual/current/en/appendixes.annotations.html#appendi...Beyond that, I'm not sure whether there's a justification for code declaring itself exempt from coverage and complexity reports. That seems like it defeats the purpose.
For instance,
MenuTreeParameters::setMaxDepth()
(since its mentioned above). What do we gain by avoiding coverage there?setMaxDepth()
is a simple setter. Is it an annotation that's supposed to say, "Don't unit test this, it's only two lines?"False positives might be a valid reason for
@codeCoverageIgnore
, but really that's an upstream problem, and if its fixed then we're potentially stuck with the opposite problem.