Begin making Harbormaster unit test results a little easier to read

Summary: Ref T10457. These lack color and iconography and are difficult to parse. Make them easier to read.

Test Plan:
Before:

{F1135396}

After:

{F1135399}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10457

Differential Revision: https://secure.phabricator.com/D15362

src/__phutil_library_map__.php

@@ -1147,6 +1147,7 @@ phutil_register_library_map(array(
     'HarbormasterURIArtifact' => 'applications/harbormaster/artifact/HarbormasterURIArtifact.php',
     'HarbormasterUnitMessagesController' => 'applications/harbormaster/controller/HarbormasterUnitMessagesController.php',
     'HarbormasterUnitPropertyView' => 'applications/harbormaster/view/HarbormasterUnitPropertyView.php',
+    'HarbormasterUnitStatus' => 'applications/harbormaster/constants/HarbormasterUnitStatus.php',
     'HarbormasterUploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php',
     'HarbormasterWaitForPreviousBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php',
     'HarbormasterWorker' => 'applications/harbormaster/worker/HarbormasterWorker.php',
@@ -5314,6 +5315,7 @@ phutil_register_library_map(array(
     'HarbormasterURIArtifact' => 'HarbormasterArtifact',
     'HarbormasterUnitMessagesController' => 'HarbormasterController',
     'HarbormasterUnitPropertyView' => 'AphrontView',
+    'HarbormasterUnitStatus' => 'Phobject',
     'HarbormasterUploadArtifactBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterWaitForPreviousBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterWorker' => 'PhabricatorWorker',

src/applications/differential/customfield/DifferentialUnitField.php

@@ -54,7 +54,9 @@ final class DifferentialUnitField
       $this->getModernUnitMessageDictionary($message));
   }
 
-  protected function newHarbormasterMessageView(array $messages) {
+  protected function newHarbormasterMessageView(array $all_messages) {
+    $messages = $all_messages;
+
     foreach ($messages as $key => $message) {
       switch ($message->getResult()) {
         case ArcanistUnitTestResult::RESULT_PASS:
@@ -71,9 +73,18 @@ final class DifferentialUnitField
       return null;
     }
 
-    return id(new HarbormasterUnitPropertyView())
-      ->setLimit(10)
-      ->setUnitMessages($messages);
+    $table = id(new HarbormasterUnitPropertyView())
+      ->setLimit(5)
+      ->setUnitMessages($all_messages);
+
+    $diff = $this->getObject()->getActiveDiff();
+    $buildable = $diff->getBuildable();
+    if ($buildable) {
+      $full_results = '/harbormaster/unit/'.$buildable->getID().'/';
+      $table->setFullResultsURI($full_results);
+    }
+
+    return $table;
   }
 
   public function getWarningsForDetailView() {
@@ -112,53 +123,6 @@ final class DifferentialUnitField
 
     $note = array();
 
-    $groups = mgroup($messages, 'getResult');
-
-    $groups = array_select_keys(
-      $groups,
-      array(
-        ArcanistUnitTestResult::RESULT_FAIL,
-        ArcanistUnitTestResult::RESULT_BROKEN,
-        ArcanistUnitTestResult::RESULT_UNSOUND,
-        ArcanistUnitTestResult::RESULT_SKIP,
-        ArcanistUnitTestResult::RESULT_PASS,
-      )) + $groups;
-
-    foreach ($groups as $result => $group) {
-      $count = phutil_count($group);
-      switch ($result) {
-        case ArcanistUnitTestResult::RESULT_PASS:
-          $note[] = pht('%s Passed Test(s)', $count);
-          break;
-        case ArcanistUnitTestResult::RESULT_FAIL:
-          $note[] = pht('%s Failed Test(s)', $count);
-          break;
-        case ArcanistUnitTestResult::RESULT_SKIP:
-          $note[] = pht('%s Skipped Test(s)', $count);
-          break;
-        case ArcanistUnitTestResult::RESULT_BROKEN:
-          $note[] = pht('%s Broken Test(s)', $count);
-          break;
-        case ArcanistUnitTestResult::RESULT_UNSOUND:
-          $note[] = pht('%s Unsound Test(s)', $count);
-          break;
-        default:
-          $note[] = pht('%s Other Test(s)', $count);
-          break;
-      }
-    }
-
-    $buildable = $diff->getBuildable();
-    if ($buildable) {
-      $full_results = '/harbormaster/unit/'.$buildable->getID().'/';
-      $note[] = phutil_tag(
-        'a',
-        array(
-          'href' => $full_results,
-        ),
-        pht('View Full Results'));
-    }
-
     $excuse = $diff->getProperty('arc:unit-excuse');
     if (strlen($excuse)) {
       $excuse = array(

src/applications/harbormaster/constants/HarbormasterUnitStatus.php

@@ -0,0 +1,89 @@
+<?php
+
+final class HarbormasterUnitStatus
+  extends Phobject {
+
+  public static function getUnitStatusIcon($status) {
+    $map = self::getUnitStatusDictionary($status);
+    $default = 'fa-question-circle';
+    return idx($map, 'icon', $default);
+  }
+
+  public static function getUnitStatusColor($status) {
+    $map = self::getUnitStatusDictionary($status);
+    $default = 'violet';
+    return idx($map, 'color', $default);
+  }
+
+  public static function getUnitStatusLabel($status) {
+    $map = self::getUnitStatusDictionary($status);
+    $default = pht('Unknown Status ("%s")', $status);
+    return idx($map, 'label', $default);
+  }
+
+  public static function getUnitStatusSort($status) {
+    $map = self::getUnitStatusDictionary($status);
+    $default = 'N';
+    return idx($map, 'sort', $default);
+  }
+
+  private static function getUnitStatusDictionary($status) {
+    $map = self::getUnitStatusMap();
+    $default = array();
+    return idx($map, $status, $default);
+  }
+
+  public static function getUnitStatusCountLabel($status, $count) {
+    $count = new PhutilNumber($count);
+
+    switch ($status) {
+      case ArcanistUnitTestResult::RESULT_FAIL:
+        return pht('%s Failed Test(s)', $count);
+      case ArcanistUnitTestResult::RESULT_BROKEN:
+        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);
+    }
+
+    return pht('%s Other Test(s)', $count);
+  }
+
+  private static function getUnitStatusMap() {
+    return array(
+      ArcanistUnitTestResult::RESULT_FAIL => array(
+        'label' => pht('Failed'),
+        'icon' => 'fa-times',
+        'color' => 'red',
+        'sort' => 'A',
+      ),
+      ArcanistUnitTestResult::RESULT_BROKEN => array(
+        'label' => pht('Broken'),
+        'icon' => 'fa-bomb',
+        'color' => 'indigo',
+        'sort' => 'B',
+      ),
+      ArcanistUnitTestResult::RESULT_UNSOUND => array(
+        'label' => pht('Unsound'),
+        'icon' => 'fa-exclamation-triangle',
+        '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',
+      ),
+    );
+  }
+
+}

src/applications/harbormaster/controller/HarbormasterBuildableViewController.php

@@ -336,15 +336,32 @@ final class HarbormasterBuildableViewController
     }
 
     if ($unit_data) {
+      $unit_href = $this->getApplicationURI('unit/'.$buildable->getID().'/');
+
       $unit_table = id(new HarbormasterUnitPropertyView())
         ->setUser($viewer)
-        ->setLimit(25)
-        ->setUnitMessages($unit_data);
+        ->setLimit(5)
+        ->setUnitMessages($unit_data)
+        ->setFullResultsURI($unit_href);
 
-      $unit_href = $this->getApplicationURI('unit/'.$buildable->getID().'/');
+      $unit_data = msort($unit_data, 'getSortKey');
+      $head_unit = head($unit_data);
+      if ($head_unit) {
+        $status = $head_unit->getResult();
+
+        $tag_text = HarbormasterUnitStatus::getUnitStatusLabel($status);
+        $tag_color = HarbormasterUnitStatus::getUnitStatusColor($status);
+        $tag_icon = HarbormasterUnitStatus::getUnitStatusIcon($status);
+
+      } else {
+        $tag_text = pht('No Unit Tests');
+        $tag_color = 'grey';
+        $tag_icon = 'fa-ban';
+      }
 
       $unit_header = id(new PHUIHeaderView())
         ->setHeader(pht('Unit Tests'))
+        ->setStatus($tag_icon, $tag_color, $tag_text)
         ->addActionLink(
           id(new PHUIButtonView())
             ->setTag('a')

src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php

@@ -136,18 +136,11 @@ final class HarbormasterBuildUnitMessage
   }
 
   public function getSortKey() {
-    // TODO: Maybe use more numeric values after T6861.
-    $map = array(
-      ArcanistUnitTestResult::RESULT_FAIL => 'A',
-      ArcanistUnitTestResult::RESULT_BROKEN => 'B',
-      ArcanistUnitTestResult::RESULT_UNSOUND => 'C',
-      ArcanistUnitTestResult::RESULT_PASS => 'Z',
-    );
-
-    $result = idx($map, $this->getResult(), 'N');
+    $status = $this->getResult();
+    $sort = HarbormasterUnitStatus::getUnitStatusSort($status);
 
     $parts = array(
-      $result,
+      $sort,
       $this->getEngine(),
       $this->getNamespace(),
       $this->getName(),

src/applications/harbormaster/view/HarbormasterUnitPropertyView.php

@@ -5,6 +5,7 @@ final class HarbormasterUnitPropertyView extends AphrontView {
   private $pathURIMap = array();
   private $unitMessages = array();
   private $limit;
+  private $fullResultsURI;
 
   public function setPathURIMap(array $map) {
     $this->pathURIMap = $map;
@@ -22,18 +23,39 @@ final class HarbormasterUnitPropertyView extends AphrontView {
     return $this;
   }
 
+  public function setFullResultsURI($full_results_uri) {
+    $this->fullResultsURI = $full_results_uri;
+    return $this;
+  }
+
   public function render() {
     $messages = $this->unitMessages;
     $messages = msort($messages, 'getSortKey');
 
+    $limit = $this->limit;
+
     if ($this->limit) {
-      $messages = array_slice($messages, 0, $this->limit);
+      $display_messages = array_slice($messages, 0, $limit);
+    } else {
+      $display_messages = $messages;
     }
 
     $rows = array();
     $any_duration = false;
-    foreach ($messages as $message) {
-      $result = $this->renderResult($message->getResult());
+    foreach ($display_messages as $message) {
+      $status = $message->getResult();
+
+      $icon_icon = HarbormasterUnitStatus::getUnitStatusIcon($status);
+      $icon_color = HarbormasterUnitStatus::getUnitStatusColor($status);
+      $icon_label = HarbormasterUnitStatus::getUnitStatusLabel($status);
+
+      $result_icon = id(new PHUIIconView())
+        ->setIcon("{$icon_icon} {$icon_color}")
+        ->addSigil('has-tooltip')
+        ->setMetadata(
+          array(
+            'tip' => $icon_label,
+          ));
 
       $duration = $message->getDuration();
       if ($duration !== null) {
@@ -54,16 +76,44 @@ final class HarbormasterUnitPropertyView extends AphrontView {
       }
 
       $rows[] = array(
-        $result,
+        $result_icon,
         $duration,
         $name,
       );
     }
 
+    $full_uri = $this->fullResultsURI;
+    if ($full_uri && (count($messages) > $limit)) {
+      $counts = array();
+
+      $groups = mgroup($messages, 'getResult');
+      foreach ($groups as $status => $group) {
+        $counts[] = HarbormasterUnitStatus::getUnitStatusCountLabel(
+          $status,
+          count($group));
+      }
+
+      $link_text = pht(
+        'View Full Test Results (%s)',
+        implode(" \xC2\xB7 ", $counts));
+
+      $full_link = phutil_tag(
+        'a',
+        array(
+          'href' => $full_uri,
+        ),
+        $link_text);
+
+      $link_icon = id(new PHUIIconView())
+        ->setIcon('fa-ellipsis-h lightgreytext');
+
+      $rows[] = array($link_icon, null, $full_link);
+    }
+
     $table = id(new AphrontTableView($rows))
       ->setHeaders(
         array(
-          pht('Result'),
+          null,
           pht('Time'),
           pht('Test'),
         ))
@@ -82,19 +132,4 @@ final class HarbormasterUnitPropertyView extends AphrontView {
     return $table;
   }
 
-  private function renderResult($result) {
-    $names = array(
-      ArcanistUnitTestResult::RESULT_BROKEN     => pht('Broken'),
-      ArcanistUnitTestResult::RESULT_FAIL       => pht('Failed'),
-      ArcanistUnitTestResult::RESULT_UNSOUND    => pht('Unsound'),
-      ArcanistUnitTestResult::RESULT_SKIP       => pht('Skipped'),
-      ArcanistUnitTestResult::RESULT_PASS       => pht('Passed'),
-    );
-    $result = idx($names, $result, $result);
-
-    // TODO: Add some color.
-
-    return $result;
-  }
-
 }