From cf52d88f8b5cd82412759696df2c97dc73cf07cd Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Fri, 7 Mar 2014 10:16:27 -0800
Subject: [PATCH] Fix unit test coverage for `NoseTestEngine`.

Summary:
I noticed that code coverage wasn't showing in Differential for
some repositories that we are using with Phabricator.

`arc unit` would should unit test coverage, but the paths were messy
(for example, `.//foo/bar.py` instead of `foo/bar.py`). As a result,
the code coverage info wasn't recognised as being for the correct
module.

I'm not sure why this logic is the way that it is... perhaps this is to
do with an older version of `nose` (I am using v1.3.0).

Test Plan:
I created a diff for an internal repository that we have, and
observed that code coverage was displayed in Differential.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran, seporaitis, avive, dctrwatson, zeeg

Differential Revision: https://secure.phabricator.com/D8433
---
 src/unit/engine/NoseTestEngine.php | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/src/unit/engine/NoseTestEngine.php b/src/unit/engine/NoseTestEngine.php
index 807e4161..dc7087f4 100644
--- a/src/unit/engine/NoseTestEngine.php
+++ b/src/unit/engine/NoseTestEngine.php
@@ -122,35 +122,15 @@ final class NoseTestEngine extends ArcanistBaseUnitTestEngine {
     $classes = $coverage_dom->getElementsByTagName("class");
 
     foreach ($classes as $class) {
-      // filename is actually python module path with ".py" at the end,
-      // e.g.: tornado.web.py
-      $relative_path = explode(".", $class->getAttribute("filename"));
-      array_pop($relative_path);
-      $relative_path = $source_path .'/'. implode("/", $relative_path);
-      $relative_path = Filesystem::readablePath($relative_path);
+      $path = $class->getAttribute("filename");
+      $root = $this->getWorkingCopy()->getProjectRoot();
 
-      // first we check if the path is a directory (a Python package), if it is
-      // set relative and absolute paths to have __init__.py at the end.
-      $absolute_path = Filesystem::resolvePath($relative_path);
-      if (is_dir($absolute_path)) {
-        $relative_path .= "/__init__.py";
-        $absolute_path .= "/__init__.py";
-      }
-
-      // then we check if the path with ".py" at the end is file (a Python
-      // submodule), if it is - set relative and absolute paths to have
-      // ".py" at the end.
-      if (is_file($absolute_path.".py")) {
-        $relative_path .= ".py";
-        $absolute_path .= ".py";
-      }
-
-      if (!file_exists($absolute_path)) {
+      if (!Filesystem::isDescendant($path, $root)) {
         continue;
       }
 
       // get total line count in file
-      $line_count = count(file($absolute_path));
+      $line_count = count(phutil_split_lines(Filesystem::readFile($path)));
 
       $coverage = "";
       $start_line = 1;
@@ -178,7 +158,7 @@ final class NoseTestEngine extends ArcanistBaseUnitTestEngine {
         }
       }
 
-      $reports[$relative_path] = $coverage;
+      $reports[$path] = $coverage;
     }
 
     return $reports;