Show additional details for failed builds in Harbormaster

Summary:
Ref T10457. When tests fail, it currently takes several clicks to figure out //why// they failed.

In this project, map rebuilds and `liberate` are fairly common failure conditions, but verifying that they were the root issue requires jumping into a build, then scrolling through a log.

Instead, display details if they're available.

Test Plan:
Before:

{F1135453}

After:

{F1135454}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9951, T10457

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

resources/celerity/map.php

@@ -7,7 +7,7 @@
  */
 return array(
   'names' => array(
-    'core.pkg.css' => 'ecdca229',
+    'core.pkg.css' => 'f7a91f6a',
     'core.pkg.js' => '7d8faf57',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '2de124c9',
@@ -25,7 +25,7 @@ return array(
     'rsrc/css/aphront/notification.css' => '7f684b62',
     'rsrc/css/aphront/panel-view.css' => '8427b78d',
     'rsrc/css/aphront/phabricator-nav-view.css' => 'ac79a758',
-    'rsrc/css/aphront/table-view.css' => '6d01d468',
+    'rsrc/css/aphront/table-view.css' => 'ec078a76',
     'rsrc/css/aphront/tokenizer.css' => '056da01b',
     'rsrc/css/aphront/tooltip.css' => '1a07aea8',
     'rsrc/css/aphront/typeahead-browse.css' => 'd8581d2c',
@@ -70,7 +70,7 @@ return array(
     'rsrc/css/application/feed/feed.css' => 'ecd4ec57',
     'rsrc/css/application/files/global-drag-and-drop.css' => '5c1b47c2',
     'rsrc/css/application/flag/flag.css' => '5337623f',
-    'rsrc/css/application/harbormaster/harbormaster.css' => 'b0758ca5',
+    'rsrc/css/application/harbormaster/harbormaster.css' => '834879db',
     'rsrc/css/application/herald/herald-test.css' => 'a52e323e',
     'rsrc/css/application/herald/herald.css' => '826075fa',
     'rsrc/css/application/maniphest/batch-editor.css' => 'b0f0b6d5',
@@ -523,7 +523,7 @@ return array(
     'aphront-list-filter-view-css' => '5d6f0526',
     'aphront-multi-column-view-css' => 'fd18389d',
     'aphront-panel-view-css' => '8427b78d',
-    'aphront-table-view-css' => '6d01d468',
+    'aphront-table-view-css' => 'ec078a76',
     'aphront-tokenizer-control-css' => '056da01b',
     'aphront-tooltip-css' => '1a07aea8',
     'aphront-typeahead-control-css' => 'd4f16145',
@@ -558,7 +558,7 @@ return array(
     'font-fontawesome' => 'c43323c5',
     'font-lato' => 'c7ccd872',
     'global-drag-and-drop-css' => '5c1b47c2',
-    'harbormaster-css' => 'b0758ca5',
+    'harbormaster-css' => '834879db',
     'herald-css' => '826075fa',
     'herald-rule-editor' => '746ca158',
     'herald-test-css' => 'a52e323e',

src/__phutil_library_map__.php

@@ -1145,7 +1145,8 @@ phutil_register_library_map(array(
     'HarbormasterThrowExceptionBuildStep' => 'applications/harbormaster/step/HarbormasterThrowExceptionBuildStep.php',
     'HarbormasterUIEventListener' => 'applications/harbormaster/event/HarbormasterUIEventListener.php',
     'HarbormasterURIArtifact' => 'applications/harbormaster/artifact/HarbormasterURIArtifact.php',
-    'HarbormasterUnitMessagesController' => 'applications/harbormaster/controller/HarbormasterUnitMessagesController.php',
+    'HarbormasterUnitMessageListController' => 'applications/harbormaster/controller/HarbormasterUnitMessageListController.php',
+    'HarbormasterUnitMessageViewController' => 'applications/harbormaster/controller/HarbormasterUnitMessageViewController.php',
     'HarbormasterUnitPropertyView' => 'applications/harbormaster/view/HarbormasterUnitPropertyView.php',
     'HarbormasterUnitStatus' => 'applications/harbormaster/constants/HarbormasterUnitStatus.php',
     'HarbormasterUploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php',
@@ -5313,7 +5314,8 @@ phutil_register_library_map(array(
     'HarbormasterThrowExceptionBuildStep' => 'HarbormasterBuildStepImplementation',
     'HarbormasterUIEventListener' => 'PhabricatorEventListener',
     'HarbormasterURIArtifact' => 'HarbormasterArtifact',
-    'HarbormasterUnitMessagesController' => 'HarbormasterController',
+    'HarbormasterUnitMessageListController' => 'HarbormasterController',
+    'HarbormasterUnitMessageViewController' => 'HarbormasterController',
     'HarbormasterUnitPropertyView' => 'AphrontView',
     'HarbormasterUnitStatus' => 'Phobject',
     'HarbormasterUploadArtifactBuildStepImplementation' => 'HarbormasterBuildStepImplementation',

src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php

@@ -85,7 +85,8 @@ final class PhabricatorHarbormasterApplication extends PhabricatorApplication {
           '(?P<id>\d+)/' => 'HarbormasterPlanViewController',
         ),
         'unit/' => array(
-          '(?P<id>\d+)/' => 'HarbormasterUnitMessagesController',
+          '(?P<id>\d+)/' => 'HarbormasterUnitMessageListController',
+          'view/(?P<id>\d+)/' => 'HarbormasterUnitMessageViewController',
         ),
         'lint/' => array(
           '(?P<id>\d+)/' => 'HarbormasterLintMessagesController',

src/applications/harbormaster/controller/HarbormasterUnitMessageListController.php

@@ -1,6 +1,6 @@
 <?php
 
-final class HarbormasterUnitMessagesController
+final class HarbormasterUnitMessageListController
   extends HarbormasterController {
 
   public function handleRequest(AphrontRequest $request) {

src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php

@@ -0,0 +1,122 @@
+<?php
+
+final class HarbormasterUnitMessageViewController
+  extends HarbormasterController {
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $this->getViewer();
+
+    $message_id = $request->getURIData('id');
+
+    $message = id(new HarbormasterBuildUnitMessage())->load($message_id);
+    if (!$message) {
+      return new Aphront404Response();
+    }
+
+    $build_target = id(new HarbormasterBuildTargetQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($message->getBuildTargetPHID()))
+      ->executeOne();
+    if (!$build_target) {
+      return new Aphront404Response();
+    }
+
+    $build = $build_target->getBuild();
+    $buildable = $build->getBuildable();
+    $buildable_id = $buildable->getID();
+
+    $id = $message->getID();
+    $display_name = $message->getUnitMessageDisplayName();
+
+    $status = $message->getResult();
+    $status_icon = HarbormasterUnitStatus::getUnitStatusIcon($status);
+    $status_color = HarbormasterUnitStatus::getUnitStatusColor($status);
+    $status_label = HarbormasterUnitStatus::getUnitStatusLabel($status);
+
+    $header = id(new PHUIHeaderView())
+      ->setHeader($display_name)
+      ->setStatus($status_icon, $status_color, $status_label);
+
+    $properties = $this->buildPropertyListView($message);
+    $actions = $this->buildActionView($message, $build);
+
+    $properties->setActionList($actions);
+
+    $unit = id(new PHUIObjectBoxView())
+      ->setHeader($header)
+      ->addPropertyList($properties);
+
+    $crumbs = $this->buildApplicationCrumbs();
+    $this->addBuildableCrumb($crumbs, $buildable);
+
+    $crumbs->addTextCrumb(
+      pht('Unit Tests'),
+      "/harbormaster/unit/{$buildable_id}/");
+
+    $crumbs->addTextCrumb(pht('Unit %d', $id));
+
+    $title = array(
+      $display_name,
+      $buildable->getMonogram(),
+    );
+
+    return $this->newPage()
+      ->setTitle($title)
+      ->setCrumbs($crumbs)
+      ->appendChild($unit);
+  }
+
+  private function buildPropertyListView(
+    HarbormasterBuildUnitMessage $message) {
+    $request = $this->getRequest();
+    $viewer = $request->getUser();
+
+    $view = id(new PHUIPropertyListView())
+      ->setUser($viewer);
+
+    $view->addProperty(
+      pht('Run At'),
+      phabricator_datetime($message->getDateCreated(), $viewer));
+
+    $details = $message->getUnitMessageDetails();
+    if (strlen($details)) {
+      // TODO: Use the log view here, once it gets cleaned up.
+      $details = phutil_tag(
+        'div',
+        array(
+          'class' => 'PhabricatorMonospaced',
+          'style' =>
+            'white-space: pre-wrap; '.
+            'color: #666666; '.
+            'overflow-x: auto;',
+        ),
+        $details);
+    } else {
+      $details = phutil_tag('em', array(), pht('No details provided.'));
+    }
+
+    $view->addSectionHeader(
+      pht('Details'),
+      PHUIPropertyListView::ICON_TESTPLAN);
+    $view->addTextContent($details);
+
+    return $view;
+  }
+
+  private function buildActionView(
+    HarbormasterBuildUnitMessage $message,
+    HarbormasterBuild $build) {
+    $viewer = $this->getViewer();
+
+    $view = id(new PhabricatorActionListView())
+      ->setUser($viewer);
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('View Build'))
+        ->setHref($build->getURI())
+        ->setIcon('fa-wrench'));
+
+    return $view;
+  }
+}

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

@@ -323,6 +323,11 @@ final class HarbormasterBuild extends HarbormasterDAO
     return ($this->getBuildStatus() == self::STATUS_PAUSED);
   }
 
+  public function getURI() {
+    $id = $this->getID();
+    return "/harbormaster/build/{$id}/";
+  }
+
 
 /* -(  Build Commands  )----------------------------------------------------- */
 

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

@@ -61,6 +61,11 @@ final class HarbormasterBuildUnitMessage
         'description' => pht(
           'Coverage information for this test.'),
       ),
+      'details' => array(
+        'type' => 'optional string',
+        'description' => pht(
+          'Additional human-readable information about the failure.'),
+      ),
     );
   }
 
@@ -94,6 +99,11 @@ final class HarbormasterBuildUnitMessage
       $obj->setProperty('coverage', $coverage);
     }
 
+    $details = idx($dict, 'details');
+    if ($details) {
+      $obj->setProperty('details', $details);
+    }
+
     return $obj;
   }
 
@@ -135,6 +145,30 @@ final class HarbormasterBuildUnitMessage
     return $this;
   }
 
+  public function getUnitMessageDetails() {
+    return $this->getProperty('details', '');
+  }
+
+  public function getUnitMessageDisplayName() {
+    $name = $this->getName();
+
+    $namespace = $this->getNamespace();
+    if (strlen($namespace)) {
+      $name = $namespace.'::'.$name;
+    }
+
+    $engine = $this->getEngine();
+    if (strlen($engine)) {
+      $name = $engine.' > '.$name;
+    }
+
+    if (!strlen($name)) {
+      return pht('Nameless Test (%d)', $this->getID());
+    }
+
+    return $name;
+  }
+
   public function getSortKey() {
     $status = $this->getResult();
     $sort = HarbormasterUnitStatus::getUnitStatusSort($status);

src/applications/harbormaster/view/HarbormasterUnitPropertyView.php

@@ -29,6 +29,8 @@ final class HarbormasterUnitPropertyView extends AphrontView {
   }
 
   public function render() {
+    require_celerity_resource('harbormaster-css');
+
     $messages = $this->unitMessages;
     $messages = msort($messages, 'getSortKey');
 
@@ -63,16 +65,22 @@ final class HarbormasterUnitPropertyView extends AphrontView {
         $duration = pht('%s ms', new PhutilNumber((int)(1000 * $duration)));
       }
 
-      $name = $message->getName();
+      $name = $message->getUnitMessageDisplayName();
+      $id = $message->getID();
 
-      $namespace = $message->getNamespace();
-      if (strlen($namespace)) {
-        $name = $namespace.'::'.$name;
-      }
+      $name = phutil_tag(
+        'a',
+        array(
+          'href' => "/harbormaster/unit/view/{$id}/",
+        ),
+        $name);
 
-      $engine = $message->getEngine();
-      if (strlen($engine)) {
-        $name = $engine.' > '.$name;
+      $details = $message->getUnitMessageDetails();
+      if (strlen($details)) {
+        $name = array(
+          $name,
+          $this->renderUnitTestDetails($details),
+        );
       }
 
       $rows[] = array(
@@ -119,9 +127,14 @@ final class HarbormasterUnitPropertyView extends AphrontView {
         ))
       ->setColumnClasses(
         array(
-          null,
-          null,
-          'pri wide',
+          'top',
+          'top',
+          'top wide',
+        ))
+      ->setColumnWidths(
+        array(
+          '32px',
+          '64px',
         ))
       ->setColumnVisibility(
         array(
@@ -132,4 +145,25 @@ final class HarbormasterUnitPropertyView extends AphrontView {
     return $table;
   }
 
+  private function renderUnitTestDetails($full_details) {
+    $details = id(new PhutilUTF8StringTruncator())
+      ->setMaximumBytes(2048)
+      ->truncateString($full_details);
+    $details = phutil_split_lines($details);
+
+    $limit = 3;
+    if (count($details) > $limit) {
+      $details = array_slice($details, 0, $limit);
+    }
+
+    $details = implode('', $details);
+
+    return phutil_tag(
+      'div',
+      array(
+        'class' => 'PhabricatorMonospaced harbormaster-unit-details',
+      ),
+      $details);
+  }
+
 }

src/view/control/AphrontTableView.php

@@ -15,6 +15,8 @@ final class AphrontTableView extends AphrontView {
   protected $columnVisibility = array();
   private $deviceVisibility = array();
 
+  private $columnWidths = array();
+
   protected $sortURI;
   protected $sortParam;
   protected $sortSelected;
@@ -46,6 +48,11 @@ final class AphrontTableView extends AphrontView {
     return $this;
   }
 
+  public function setColumnWidths(array $widths) {
+    $this->columnWidths = $widths;
+    return $this;
+  }
+
   public function setNoDataString($no_data_string) {
     $this->noDataString = $no_data_string;
     return $this;
@@ -131,6 +138,8 @@ final class AphrontTableView extends AphrontView {
     $visibility = array_values($this->columnVisibility);
     $device_visibility = array_values($this->deviceVisibility);
 
+    $column_widths = $this->columnWidths;
+
     $headers = $this->headers;
     $short_headers = $this->shortHeaders;
     $sort_values = $this->sortValues;
@@ -236,7 +245,18 @@ final class AphrontTableView extends AphrontView {
           $header = hsprintf('%s %s', $header_nodevice, $header_device);
         }
 
-        $tr[] = phutil_tag('th', array('class' => $class), $header);
+        $style = null;
+        if (isset($column_widths[$col_num])) {
+          $style = 'width: '.$column_widths[$col_num].';';
+        }
+
+        $tr[] = phutil_tag(
+          'th',
+          array(
+            'class' => $class,
+            'style' => $style,
+          ),
+          $header);
       }
       $table[] = phutil_tag('tr', array(), $tr);
     }
@@ -283,7 +303,13 @@ final class AphrontTableView extends AphrontView {
           if (!empty($this->cellClasses[$row_num][$col_num])) {
             $class = trim($class.' '.$this->cellClasses[$row_num][$col_num]);
           }
-          $tr[] = phutil_tag('td', array('class' => $class), $value);
+
+          $tr[] = phutil_tag(
+            'td',
+            array(
+              'class' => $class,
+            ),
+            $value);
           ++$col_num;
         }
 
@@ -315,10 +341,15 @@ final class AphrontTableView extends AphrontView {
     if ($this->className !== null) {
       $classes[] = $this->className;
     }
+
     if ($this->deviceReadyTable) {
       $classes[] = 'aphront-table-view-device-ready';
     }
 
+    if ($this->columnWidths) {
+      $classes[] = 'aphront-table-view-fixed';
+    }
+
     $html = phutil_tag(
       'table',
       array(

webroot/rsrc/css/aphront/table-view.css

@@ -15,6 +15,10 @@
   border-bottom: 1px solid {$blueborder};
 }
 
+.aphront-table-view-fixed {
+  table-layout: fixed;
+}
+
 .aphront-table-view td.aphront-table-notice {
   padding: 12px 16px;
   font-size: {$normalfontsize};

webroot/rsrc/css/application/harbormaster/harbormaster.css

@@ -20,3 +20,11 @@
   padding: 12px;
   color: {$darkgreytext};
 }
+
+.harbormaster-unit-details {
+  margin: 8px 0 4px;
+  overflow: hidden;
+  white-space: pre;
+  text-overflow: ellipsis;
+  color: {$lightgreytext};
+}