From 8240e0f727069a2d11eb3f4d7913ca71516c5117 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 1 Mar 2016 05:58:28 -0800
Subject: [PATCH] Treat "skipped" unit tests as less interesting than "passed"

Summary:
Ref T10457. Skipped tests are almost always well-behaved (e.g., `testWindows()`, but the test is running on Linux) and not interesting, and we do not expect well-written, solid systems to necessarily have 0 skips.

Although skips //could// indicate that you have missing dependencies on a build server, and thus be a bit interesting, I think they almost always indicate that a particular test is not expected to run in the current environment.

If we wanted to tackle this problem in granular detail, we could eventually add a "Missing" status or similar which would serve as "a skip you //could// reasonably fix in this environment", but I don't think that's too interesting.

Test Plan:
Here's an example of a build result with skips: B10875

{F1136511}

I think this is clearer as "Passed", as this is the expected production state of the build.

Locally, looked at some builds.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10457

Differential Revision: https://secure.phabricator.com/D15369
---
 .../constants/HarbormasterUnitStatus.php        | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/applications/harbormaster/constants/HarbormasterUnitStatus.php b/src/applications/harbormaster/constants/HarbormasterUnitStatus.php
index ea79c7843d..de7c4dd42f 100644
--- a/src/applications/harbormaster/constants/HarbormasterUnitStatus.php
+++ b/src/applications/harbormaster/constants/HarbormasterUnitStatus.php
@@ -43,10 +43,10 @@ final class HarbormasterUnitStatus
         return pht('%s Broken Test(s)', $count);
       case ArcanistUnitTestResult::RESULT_UNSOUND:
         return pht('%s Unsound Test(s)', $count);
-      case ArcanistUnitTestResult::RESULT_SKIP:
-        return pht('%s Skipped Test(s)', $count);
       case ArcanistUnitTestResult::RESULT_PASS:
         return pht('%s Passed Test(s)', $count);
+      case ArcanistUnitTestResult::RESULT_SKIP:
+        return pht('%s Skipped Test(s)', $count);
     }
 
     return pht('%s Other Test(s)', $count);
@@ -72,16 +72,17 @@ final class HarbormasterUnitStatus
         'color' => 'yellow',
         'sort' => 'C',
       ),
-      ArcanistUnitTestResult::RESULT_SKIP => array(
-        'label' => pht('Skipped'),
-        'icon' => 'fa-fast-forward',
-        'color' => 'blue',
-      ),
       ArcanistUnitTestResult::RESULT_PASS => array(
         'label' => pht('Passed'),
         'icon' => 'fa-check',
         'color' => 'green',
-        'sort' => 'Z',
+        'sort' => 'D',
+      ),
+      ArcanistUnitTestResult::RESULT_SKIP => array(
+        'label' => pht('Skipped'),
+        'icon' => 'fa-fast-forward',
+        'color' => 'blue',
+        'sort' => 'E',
       ),
     );
   }