coverage-reporter
coverage-reporter copied to clipboard
In clover.xml, count all method definitions once
The clover.xml specification does not answer what a count means for a line with type="method".
In the case of PHP's CodeCoverage library, it is set to the maximum count for any statement within the method.
Therefore, even if a file is parsed and run (so the method is defined), if the method is never called then it will have a count of 0. This differs from how other PHP tools display "method" lines, which instead treat them as run.
So, treat all methods as if they have a count of 1.
:ballot_box_with_check: Checklist
- [x] Add specs
Pull Request Test Coverage Report for Build 8443662829
Details
- 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage increased (+0.01%) to 93.731%
| Totals | |
|---|---|
| Change from base Build 6721884478: | 0.01% |
| Covered Lines: | 942 |
| Relevant Lines: | 1005 |
💛 - Coveralls
Hi @Thompson1985. Thanks for reviewing the changes. We had left this open because we're not sure about the proper representation of coverage for method declarations in clover reports.
It's been a while since I spent time with this, but I left last time feeling that the expectation is that method declarations should not be shown as covered (but instead as non-relevant) in clover reports.
Unfortunately, I also seem to remember a lack of canonical documentation on the matter.
Are you a user of clover reports? And can you provide your perspective?
TIA.
Closing this PR without accepting its changes
Thank you @mike-burns and @Thompson1985 for your contributions and for starting this discussion!
After reviewing this proposal to treat all method definitions as covered (count=1) in Clover reports, we’ve decided not to merge it. Here is our reasoning:
-
Preserving Coverage Accuracy
We believe coverage metrics should reflect actual method invocations. Marking uninvoked methods as “covered” could inflate coverage percentages and mislead teams into thinking a method is at least partially tested when it’s never called. -
Keeping Configuration at the Coverage Library Level
Our product aims to display coverage results exactly as they come from the underlying coverage tools. If some users prefer method definitions to have a non-zero count, that configuration is best handled in the coverage reporting library itself. This approach keeps our platform simpler for everyone and avoids additional format-specific settings in our UI or.coveralls.yml. -
Avoiding Extra Complexity for Most Users
Introducing a new configuration toggle or option here could cause confusion, especially since most teams just want the coverage details they see in our interface to match those in their CI/CD coverage reports.
Ultimately, we welcome either methodology (“method definition = 0 if uncalled” or “method definition = 1 by default”)—but we do not want to enforce, override, or expand upon the coverage library’s logic in this project. We’re therefore closing this PR.
Thank you again for your time and effort. And apologies for the long delay in resolving this.
For end users: If you need a different approach to how methods are counted, please see the documentation for your chosen coverage library, as many of them offer configuration options that might suit your needs. We appreciate your contribution and look forward to future improvements you might bring to the project.
—
Team Coveralls