From 086f5399bfbb65513b1b0e2c5371522202a9beb7 Mon Sep 17 00:00:00 2001
From: Eric Stern <eric@ericstern.com>
Date: Wed, 24 Feb 2016 12:48:37 -0800
Subject: [PATCH] Filter out some Clover coverage to prevent false positives

Summary: Clover reports generated from PHPUnit sometimes give false positives of lines that were not covered by a test that should have actually been not coverable, apparently due to inaccurate static analysis of files that weren't actually executed. This filters coverage reports of files that show no coverage which avoids these false positives. Fixes T10420.

Test Plan:
Looked at coverage reports of files before and after the change

Before: {F1124115}

After: {F1124113}

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, aurelijus

Maniphest Tasks: T10420

Differential Revision: https://secure.phabricator.com/D15343
---
 .../ArcanistPhpunitTestResultParser.php       | 39 +++++++++----------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/src/unit/parser/ArcanistPhpunitTestResultParser.php b/src/unit/parser/ArcanistPhpunitTestResultParser.php
index 46e9c131..5ccff970 100644
--- a/src/unit/parser/ArcanistPhpunitTestResultParser.php
+++ b/src/unit/parser/ArcanistPhpunitTestResultParser.php
@@ -129,34 +129,33 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
       $line_count = count(file($class_path));
 
       $coverage = '';
+      $any_line_covered = false;
       $start_line = 1;
       $lines = $file->getElementsByTagName('line');
-      for ($ii = 0; $ii < $lines->length; $ii++) {
-        $line = $lines->item($ii);
-        for (; $start_line < $line->getAttribute('num'); $start_line++) {
-          $coverage .= 'N';
-        }
 
+      $coverage = str_repeat('N', $line_count);
+      foreach ($lines as $line) {
         if ($line->getAttribute('type') != 'stmt') {
-          $coverage .= 'N';
-        } else {
-          if ((int)$line->getAttribute('count') == 0) {
-            $coverage .= 'U';
-          } else if ((int)$line->getAttribute('count') > 0) {
-            $coverage .= 'C';
-          }
+          continue;
         }
-
-        $start_line++;
+        if ((int)$line->getAttribute('count') > 0) {
+          $is_covered = 'C';
+          $any_line_covered = true;
+        } else {
+          $is_covered = 'U';
+        }
+        $line_no = (int)$line->getAttribute('num');
+        $coverage[$line_no - 1] = $is_covered;
       }
 
-      for (; $start_line <= $line_count; $start_line++) {
-        $coverage .= 'N';
+      // Sometimes the Clover coverage gives false positives on uncovered lines
+      // when the file wasn't actually part of the test. This filters out files
+      // with no coverage which helps give more accurate overall results.
+      if ($any_line_covered) {
+        $len = strlen($this->projectRoot.DIRECTORY_SEPARATOR);
+        $class_path = substr($class_path, $len);
+        $reports[$class_path] = $coverage;
       }
-
-      $len = strlen($this->projectRoot.DIRECTORY_SEPARATOR);
-      $class_path = substr($class_path, $len);
-      $reports[$class_path] = $coverage;
     }
 
     return $reports;