prevent uncovered classes from showing up in html coverage report

[yanghua-ola]

When @covers annotation was applied to tests, some uncovered classes still ended up in final coverage report, html in my case. Apart from cluttering the report and messing up the code coverage caculation, it also slowed down report generation significantly, especially for html.

It boils down to this two lines of code.

$this->initializeFilesThatAreSeenTheFirstTime
...
$this->applyCoversAnnotationFilter

applyCoversAnnotationFilter removes the uncovered files, however before that initializeFilesThatAreSeenTheFirstTime would have already added them to the output (unless that's intentional). In this commit I switched their orders. It is also slightly faster because now applyIgnoredLinesFilter has less work to do.

From what I read the change shouldn't have any other side effects. However the code is old, let me know if it breaks anything.

[sebastianbergmann]

@dvdoug Does this make sense to you? It's been a while since I looked at this in detail, and I do not have the time right now to dig into this. Thanks in advance!

[dvdoug]

I'll take a look in the next couple of days 👍

[dvdoug]

Hello @yanghva

Your diagnosis of the potential issue seems correct to me from code introspection, however without a minimal reproducing testcase it's hard to confirm the fix is the right one. Could you please add one? (even if it's not in the PR, just a separate comment on GitHub. Something I can QA with).

In this case, a reproducer for the bug is also important because php-code-coverage 7 and php-code-coverage 9 have very different code in this area so although I don't think v9 has the issue you refer to, I'd like to make sure...

Many thanks!

[yanghua-ola]

Hi @dvdoug here is a minimal example, coverage-minimal.zip. Unzip it and do the usual

compose install
./vendor/bin/phpunit

In this case, class A still showed up in the final report.

FYI I am using PCOV, below are the versions used for each components.

Generated by php-code-coverage 7.0.15 using PHP 7.2.34 with PCOV 1.0.9 and PHPUnit 8.5.20 at Wed Sep 22 2:39:15 UTC 2021.

[sebastianbergmann]

Out of curiosity and because I am thinking about removing includeUncoveredFiles, which is what addUncoveredFilesFromWhitelist is called in newer versions of PHPUnit: why do you want to exclude files (that are not ignored using metadata or the exclusion list) that are not covered from your code coverage report?

[yanghua-ola]

@sebastianbergmann For two reasons:

1. Unwanted extra files slows down coverage report generation significantly, especially for html report.
2. They also cluttered the report and skewed the coverage percentage, while I really just wanted to gauge the coverage of the files I intend to test.

[sebastianbergmann]

Thank you for your feedback.

[dvdoug]

@sebastianbergmann these changes look good to me for v7. v9 does suffer from the same bug, but the sequencing of events is different so will require a different fix. I'll work on that and send over an additional PR once I've got something.

[dvdoug]

Actually, I take that back - this branch is so old the tests haven't run on CI because they're configured to run on Travis not GitHub Actions. This PR breaks quite a lot of them. It may well be the tests are wrong of course, but this can't get merged as-is

[dvdoug]

Let's take CloverTest::testCloverForBankAccountTest, which runs https://github.com/sebastianbergmann/php-code-coverage/blob/7.0/tests/_files/BankAccountTest.php against https://github.com/sebastianbergmann/php-code-coverage/blob/7.0/tests/_files/BankAccount.php using some @covers annotations. The setBalance() method does not have any tests that cover it.

With this change, that method becomes excluded from the data entirely - all of the stats now claim the file has 3 methods instead of 4 etc. This is not correct.

Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
   <project timestamp="1632515646" name="BankAccount">
     <file name="C:\tmp\php-code-coverage\tests\_files\BankAccount.php">
       <class name="BankAccount" namespace="global">
-        <metrics complexity="5" methods="4" coveredmethods="3" conditionals="0" coveredconditionals="0" statements="10" coveredstatements="5" elements="14" coveredelements="8"/>
+        <metrics complexity="5" methods="3" coveredmethods="3" conditionals="0" coveredconditionals="0" statements="5" coveredstatements="5" elements="8" coveredelements="8"/>
       </class>
       <line num="6" type="method" name="getBalance" visibility="public" complexity="1" crap="1" count="2"/>
       <line num="8" type="stmt" count="2"/>
-      <line num="11" type="method" name="setBalance" visibility="protected" complexity="2" crap="6" count="0"/>
-      <line num="13" type="stmt" count="0"/>
-      <line num="14" type="stmt" count="0"/>
-      <line num="15" type="stmt" count="0"/>
-      <line num="16" type="stmt" count="0"/>
-      <line num="18" type="stmt" count="0"/>
       <line num="20" type="method" name="depositMoney" visibility="public" complexity="1" crap="1" count="2"/>
       <line num="22" type="stmt" count="2"/>
       <line num="24" type="stmt" count="1"/>
@@ @@
       <line num="27" type="method" name="withdrawMoney" visibility="public" complexity="1" crap="1" count="2"/>
       <line num="29" type="stmt" count="2"/>
       <line num="31" type="stmt" count="1"/>
-      <metrics loc="33" ncloc="33" classes="1" methods="4" coveredmethods="3" conditionals="0" coveredconditionals="0" statements="10" coveredstatements="5" elements="14" coveredelements="8"/>
+      <metrics loc="33" ncloc="33" classes="1" methods="3" coveredmethods="3" conditionals="0" coveredconditionals="0" statements="5" coveredstatements="5" elements="8" coveredelements="8"/>
     </file>
-    <metrics files="1" loc="33" ncloc="33" classes="1" methods="4" coveredmethods="3" conditionals="0" coveredconditionals="0" statements="10" coveredstatements="5" elements="14" coveredelements="8"/>
+    <metrics files="1" loc="33" ncloc="33" classes="1" methods="3" coveredmethods="3" conditionals="0" coveredconditionals="0" statements="5" coveredstatements="5" elements="8" coveredelements="8"/>
   </project>
 </coverage>

[sebastianbergmann]

Thank you for your contribution. I appreciate the time you invested in preparing this pull request. However, I have decided not to merge it.