Page MenuHomePhabricator
Files F15400273

D13378.diff
No OneTemporary
Actions

D13378.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -919,6 +919,7 @@
'HarbormasterTargetWorker' => 'applications/harbormaster/worker/HarbormasterTargetWorker.php',
'HarbormasterThrowExceptionBuildStep' => 'applications/harbormaster/step/HarbormasterThrowExceptionBuildStep.php',
'HarbormasterUIEventListener' => 'applications/harbormaster/event/HarbormasterUIEventListener.php',
+ 'HarbormasterUnitPropertyView' => 'applications/harbormaster/view/HarbormasterUnitPropertyView.php',
'HarbormasterUploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php',
'HarbormasterWaitForPreviousBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php',
'HarbormasterWorker' => 'applications/harbormaster/worker/HarbormasterWorker.php',
@@ -4380,6 +4381,7 @@
'HarbormasterTargetWorker' => 'HarbormasterWorker',
'HarbormasterThrowExceptionBuildStep' => 'HarbormasterBuildStepImplementation',
'HarbormasterUIEventListener' => 'PhabricatorEventListener',
+ 'HarbormasterUnitPropertyView' => 'AphrontView',
'HarbormasterUploadArtifactBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterWaitForPreviousBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterWorker' => 'PhabricatorWorker',
diff --git a/src/applications/differential/customfield/DifferentialLintField.php b/src/applications/differential/customfield/DifferentialLintField.php
--- a/src/applications/differential/customfield/DifferentialLintField.php
+++ b/src/applications/differential/customfield/DifferentialLintField.php
@@ -56,7 +56,6 @@
// TODO: Look for Harbormaster messages here.
-
if (!$lint) {
// No Harbormaster messages, so look for legacy messages and make them
// look like modern messages.
@@ -71,10 +70,14 @@
$target = new HarbormasterBuildTarget();
foreach ($legacy_lint as $message) {
- $modern = HarbormasterBuildLintMessage::newFromDictionary(
- $target,
- $this->getModernLintMessageDictionary($message));
- $lint[] = $modern;
+ try {
+ $modern = HarbormasterBuildLintMessage::newFromDictionary(
+ $target,
+ $this->getModernLintMessageDictionary($message));
+ $lint[] = $modern;
+ } catch (Exception $ex) {
+ // Ignore any poorly formatted messages.
+ }
}
}
}
@@ -160,6 +163,13 @@
}
private function getModernLintMessageDictionary(array $map) {
+ // Strip out `null` values to satisfy stricter typechecks.
+ foreach ($map as $key => $value) {
+ if ($value === null) {
+ unset($map[$key]);
+ }
+ }
+
// TODO: We might need to remap some stuff here?
return $map;
}
diff --git a/src/applications/differential/customfield/DifferentialUnitField.php b/src/applications/differential/customfield/DifferentialUnitField.php
--- a/src/applications/differential/customfield/DifferentialUnitField.php
+++ b/src/applications/differential/customfield/DifferentialUnitField.php
@@ -48,182 +48,55 @@
$diff->attachProperty($key, idx($properties, $key));
}
- $ustar = DifferentialRevisionUpdateHistoryView::renderDiffUnitStar($diff);
- $umsg = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff);
-
- $rows = array();
-
- $rows[] = array(
- 'style' => 'star',
- 'name' => $ustar,
- 'value' => $umsg,
- 'show' => true,
- );
-
- $excuse = $diff->getProperty('arc:unit-excuse');
- if ($excuse) {
- $rows[] = array(
- 'style' => 'excuse',
- 'name' => pht('Excuse'),
- 'value' => phutil_escape_html_newlines($excuse),
- 'show' => true,
- );
- }
-
- $show_limit = 10;
- $hidden = array();
-
- $udata = $diff->getProperty('arc:unit');
- if ($udata) {
- $sort_map = array(
- ArcanistUnitTestResult::RESULT_BROKEN => 0,
- ArcanistUnitTestResult::RESULT_FAIL => 1,
- ArcanistUnitTestResult::RESULT_UNSOUND => 2,
- ArcanistUnitTestResult::RESULT_SKIP => 3,
- ArcanistUnitTestResult::RESULT_POSTPONED => 4,
- ArcanistUnitTestResult::RESULT_PASS => 5,
- );
-
- foreach ($udata as $key => $test) {
- $udata[$key]['sort'] = idx($sort_map, idx($test, 'result'));
- }
- $udata = isort($udata, 'sort');
- $engine = new PhabricatorMarkupEngine();
- $engine->setViewer($this->getViewer());
- $markup_objects = array();
- foreach ($udata as $key => $test) {
- $userdata = idx($test, 'userdata');
- if ($userdata) {
- if ($userdata !== false) {
- $userdata = str_replace("\000", '', $userdata);
+ $status = $this->renderUnitStatus($diff);
+
+ $unit = array();
+
+ // TODO: Look for Harbormaster results here.
+
+ if (!$unit) {
+ $legacy_unit = $diff->getProperty('arc:unit');
+ if ($legacy_unit) {
+ // Show the top 100 legacy unit messages.
+ $legacy_unit = array_slice($legacy_unit, 0, 100);
+
+ $target = new HarbormasterBuildTarget();
+ foreach ($legacy_unit as $message) {
+ try {
+ $modern = HarbormasterBuildUnitMessage::newFromDictionary(
+ $target,
+ $this->getModernUnitMessageDictionary($message));
+ $unit[] = $modern;
+ } catch (Exception $ex) {
+ // Just ignore it if legacy messages aren't formatted like
+ // we expect.
}
- $markup_object = id(new PhabricatorMarkupOneOff())
- ->setContent($userdata)
- ->setPreserveLinebreaks(true);
- $engine->addObject($markup_object, 'default');
- $markup_objects[$key] = $markup_object;
- }
- }
- $engine->process();
- foreach ($udata as $key => $test) {
- $result = idx($test, 'result');
-
- $default_hide = false;
- switch ($result) {
- case ArcanistUnitTestResult::RESULT_POSTPONED:
- case ArcanistUnitTestResult::RESULT_PASS:
- $default_hide = true;
- break;
- }
-
- if ($show_limit && !$default_hide) {
- --$show_limit;
- $show = true;
- } else {
- $show = false;
- if (empty($hidden[$result])) {
- $hidden[$result] = 0;
- }
- $hidden[$result]++;
- }
-
- $value = idx($test, 'name');
-
- $namespace = idx($test, 'namespace');
- if ($namespace) {
- $value = $namespace.'::'.$value;
- }
-
- if (!empty($test['link'])) {
- $value = phutil_tag(
- 'a',
- array(
- 'href' => $test['link'],
- 'target' => '_blank',
- ),
- $value);
- }
- $rows[] = array(
- 'style' => $this->getResultStyle($result),
- 'name' => ucwords($result),
- 'value' => $value,
- 'show' => $show,
- );
-
- if (isset($markup_objects[$key])) {
- $rows[] = array(
- 'style' => 'details',
- 'value' => $engine->getOutput($markup_objects[$key], 'default'),
- 'show' => false,
- );
- if (empty($hidden['details'])) {
- $hidden['details'] = 0;
- }
- $hidden['details']++;
}
}
}
- $show_string = $this->renderShowString($hidden);
-
- $view = new DifferentialResultsTableView();
- $view->setRows($rows);
- $view->setShowMoreString($show_string);
+ if ($unit) {
+ $path_map = mpull($diff->loadChangesets(), 'getID', 'getFilename');
+ foreach ($path_map as $path => $id) {
+ $href = '#C'.$id.'NL';
- return $view->render();
- }
+ // TODO: When the diff is not the right-hand-size diff, we should
+ // ideally adjust this URI to be absolute.
- private function getResultStyle($result) {
- $map = array(
- ArcanistUnitTestResult::RESULT_PASS => 'green',
- ArcanistUnitTestResult::RESULT_FAIL => 'red',
- ArcanistUnitTestResult::RESULT_SKIP => 'blue',
- ArcanistUnitTestResult::RESULT_BROKEN => 'red',
- ArcanistUnitTestResult::RESULT_UNSOUND => 'yellow',
- ArcanistUnitTestResult::RESULT_POSTPONED => 'blue',
- );
- return idx($map, $result);
- }
+ $path_map[$path] = $href;
+ }
- private function renderShowString(array $hidden) {
- if (!$hidden) {
- return null;
+ $view = id(new HarbormasterUnitPropertyView())
+ ->setPathURIMap($path_map)
+ ->setUnitMessages($unit);
+ } else {
+ $view = null;
}
- // Reorder hidden things by severity.
- $hidden = array_select_keys(
- $hidden,
- array(
- ArcanistUnitTestResult::RESULT_BROKEN,
- ArcanistUnitTestResult::RESULT_FAIL,
- ArcanistUnitTestResult::RESULT_UNSOUND,
- ArcanistUnitTestResult::RESULT_SKIP,
- ArcanistUnitTestResult::RESULT_POSTPONED,
- ArcanistUnitTestResult::RESULT_PASS,
- 'details',
- )) + $hidden;
-
- $noun = array(
- ArcanistUnitTestResult::RESULT_BROKEN => pht('Broken'),
- ArcanistUnitTestResult::RESULT_FAIL => pht('Failed'),
- ArcanistUnitTestResult::RESULT_UNSOUND => pht('Unsound'),
- ArcanistUnitTestResult::RESULT_SKIP => pht('Skipped'),
- ArcanistUnitTestResult::RESULT_POSTPONED => pht('Postponed'),
- ArcanistUnitTestResult::RESULT_PASS => pht('Passed'),
+ return array(
+ $status,
+ $view,
);
-
- $show = array();
- foreach ($hidden as $key => $value) {
- if ($key == 'details') {
- $show[] = pht('%d Detail(s)', $value);
- } else {
- $show[] = $value.' '.idx($noun, $key);
- }
- }
-
- return pht(
- 'Show Full Unit Results (%s)',
- implode(', ', $show));
}
public function getWarningsForDetailView() {
@@ -248,4 +121,51 @@
}
+ private function renderUnitStatus(DifferentialDiff $diff) {
+ $colors = array(
+ DifferentialUnitStatus::UNIT_NONE => 'grey',
+ DifferentialUnitStatus::UNIT_OKAY => 'green',
+ DifferentialUnitStatus::UNIT_WARN => 'yellow',
+ DifferentialUnitStatus::UNIT_FAIL => 'red',
+ DifferentialUnitStatus::UNIT_SKIP => 'blue',
+ DifferentialUnitStatus::UNIT_AUTO_SKIP => 'blue',
+ DifferentialUnitStatus::UNIT_POSTPONED => 'blue',
+ );
+ $icon_color = idx($colors, $diff->getUnitStatus(), 'grey');
+
+ $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff);
+
+ $excuse = $diff->getProperty('arc:unit-excuse');
+ if (strlen($excuse)) {
+ $excuse = array(
+ phutil_tag('strong', array(), pht('Excuse:')),
+ ' ',
+ phutil_escape_html_newlines($excuse),
+ );
+ }
+
+ $status = id(new PHUIStatusListView())
+ ->addItem(
+ id(new PHUIStatusItemView())
+ ->setIcon(PHUIStatusItemView::ICON_STAR, $icon_color)
+ ->setTarget($message)
+ ->setNote($excuse));
+
+ return $status;
+ }
+
+ private function getModernUnitMessageDictionary(array $map) {
+ // Strip out `null` values to satisfy stricter typechecks.
+ foreach ($map as $key => $value) {
+ if ($value === null) {
+ unset($map[$key]);
+ }
+ }
+
+ // TODO: Remap more stuff here?
+
+ return $map;
+ }
+
+
}
diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php
--- a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php
+++ b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php
@@ -15,7 +15,7 @@
public static function initializeNewUnitMessage(
HarbormasterBuildTarget $build_target) {
- return id(new HarbormasterBuildLintMessage())
+ return id(new HarbormasterBuildUnitMessage())
->setBuildTargetPHID($build_target->getPHID());
}
diff --git a/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php
new file mode 100644
--- /dev/null
+++ b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php
@@ -0,0 +1,90 @@
+<?php
+
+final class HarbormasterUnitPropertyView extends AphrontView {
+
+ private $pathURIMap;
+ private $unitMessages = array();
+
+ public function setPathURIMap(array $map) {
+ $this->pathURIMap = $map;
+ return $this;
+ }
+
+ public function setUnitMessages(array $messages) {
+ assert_instances_of($messages, 'HarbormasterBuildUnitMessage');
+ $this->unitMessages = $messages;
+ return $this;
+ }
+
+ public function render() {
+
+ $rows = array();
+ $any_duration = false;
+ foreach ($this->unitMessages as $message) {
+ $result = $this->renderResult($message->getResult());
+
+ $duration = $message->getDuration();
+ if ($duration !== null) {
+ $any_duration = true;
+ $duration = pht('%s ms', new PhutilNumber((int)(1000 * $duration)));
+ }
+
+ $name = $message->getName();
+
+ $namespace = $message->getNamespace();
+ if (strlen($namespace)) {
+ $name = $namespace.'::'.$name;
+ }
+
+ $engine = $message->getEngine();
+ if (strlen($engine)) {
+ $name = $engine.' > '.$name;
+ }
+
+ $rows[] = array(
+ $result,
+ $duration,
+ $name,
+ );
+ }
+
+
+ $table = id(new AphrontTableView($rows))
+ ->setHeaders(
+ array(
+ pht('Result'),
+ pht('Time'),
+ pht('Test'),
+ ))
+ ->setColumnClasses(
+ array(
+ null,
+ null,
+ 'pri wide',
+ ))
+ ->setColumnVisibility(
+ array(
+ true,
+ $any_duration,
+ ));
+
+ 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_POSTPONED => pht('Postponed'),
+ ArcanistUnitTestResult::RESULT_PASS => pht('Passed'),
+ );
+ $result = idx($names, $result, $result);
+
+ // TODO: Add some color.
+
+ return $result;
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 18, 10:44 AM (5 d, 2 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7390889
Default Alt Text
D13378.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility