Page MenuHomePhabricator

D16417.id39480.diff
No OneTemporary

D16417.id39480.diff

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
@@ -3,6 +3,8 @@
final class DifferentialLintField
extends DifferentialHarbormasterField {
+ // For lint, we're loading all the messages anyway, to display them inline.
+
public function getFieldKey() {
return 'differential:lint';
}
@@ -92,7 +94,7 @@
DifferentialLintStatus::LINT_SKIP => 'blue',
DifferentialLintStatus::LINT_AUTO_SKIP => 'blue',
);
- $icon_color = idx($colors, $diff->getLintStatus(), 'grey');
+ $icon_color = idx($colors, $diff->getLintStatus(), 'grey'); // TODO fetch from build/buildtarget
$message = DifferentialRevisionUpdateHistoryView::getDiffLintMessage($diff);
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
@@ -34,6 +34,7 @@
public function getWarningsForDetailView() {
$status = $this->getObject()->getActiveDiff()->getUnitStatus();
+
$warnings = array();
if ($status < DifferentialUnitStatus::UNIT_WARN) {
// Don't show any warnings.
@@ -51,6 +52,22 @@
public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+ $target_phids = $diff->getBuildTargetPHIDs();
+
+ if ($target_phids) {
+ $targets = id(new HarbormasterBuildTargetQuery())
+ ->setViewer($this->getViewer())
+ ->withPHIDs($target_phids)
+ ->execute();
+ }
+ $builds_unit_status = array();
+ foreach ($targets as $target) {
+ $builds_unit_status[] = $target->getDetail(HarbormasterBuildTarget::DETAIL_UNIT_STATUS);
+ }
+ $builds_unit_status = HarbormasterUnitStatus::getWorstStatus($builds_unit_status);
+ $builds_unit_status = HarbormasterUnitStatus::getDifferentialUnitStatus($builds_unit_status);
+ var_dump($builds_unit_status);
+
$colors = array(
DifferentialUnitStatus::UNIT_NONE => 'grey',
DifferentialUnitStatus::UNIT_OKAY => 'green',
@@ -59,17 +76,29 @@
DifferentialUnitStatus::UNIT_SKIP => 'blue',
DifferentialUnitStatus::UNIT_AUTO_SKIP => 'blue',
);
- $icon_color = idx($colors, $diff->getUnitStatus(), 'grey');
- $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff);
+ $view = id(new PHUIStatusListView());
- $status = id(new PHUIStatusListView())
- ->addItem(
+ if ($builds_unit_status) {
+ $icon_color = idx($colors, $builds_unit_status, 'grey');
+ $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($builds_unit_status);
+ $view->addItem(
id(new PHUIStatusItemView())
->setIcon(PHUIStatusItemView::ICON_STAR, $icon_color)
->setTarget($message));
+ }
- return $status;
+ $manual_tests_status = $diff->getUnitStatus(); // try to pull off the autoplan first.
+ // TODO this is actually only interesting if the local is "skip"; OTherwise, it should be included with the builds.
+ if ($manual_tests_status) {
+ $icon_color = idx($colors, $manual_tests_status, 'grey');
+ $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($manual_tests_status);
+ $view->addItem(
+ id(new PHUIStatusItemView())
+ ->setIcon(PHUIStatusItemView::ICON_STAR, $icon_color)
+ ->setTarget($message));
+ }
+ return $view;
}
diff --git a/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php b/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php
--- a/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php
+++ b/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php
@@ -153,7 +153,7 @@
'div',
array(
'class' => 'lintunit-star',
- 'title' => self::getDiffUnitMessage($diff),
+ 'title' => self::getDiffUnitMessage($diff->getUnitStatus()),
),
$unit);
@@ -360,8 +360,8 @@
return pht('Unknown');
}
- public static function getDiffUnitMessage(DifferentialDiff $diff) {
- switch ($diff->getUnitStatus()) {
+ public static function getDiffUnitMessage($status) {
+ switch ($status) {
case DifferentialUnitStatus::UNIT_NONE:
return pht('No Unit Test Coverage');
case DifferentialUnitStatus::UNIT_OKAY:
diff --git a/src/applications/harbormaster/constants/HarbormasterUnitStatus.php b/src/applications/harbormaster/constants/HarbormasterUnitStatus.php
--- a/src/applications/harbormaster/constants/HarbormasterUnitStatus.php
+++ b/src/applications/harbormaster/constants/HarbormasterUnitStatus.php
@@ -27,6 +27,26 @@
return idx($map, 'sort', $default);
}
+ public static function getWorstStatus(array $statuses) {
+ $map = self::getUnitStatusMap();
+ $default = 'Z';
+ $worst = head($statuses);
+ $w_index = idx($map, $worst, $default);
+ foreach ($statuses as $status) {
+ $r = idx($map, $status, $default);
+ if ($r < $w_index) {
+ $worst = $status;
+ $w_index = $r;
+ }
+ }
+ return $worst;
+ }
+
+ public static function getDifferentialUnitStatus($status) {
+ $map = self::getUnitStatusDictionary($status);
+ return idx($map, 'differantial_result', $status);
+ }
+
private static function getUnitStatusDictionary($status) {
$map = self::getUnitStatusMap();
$default = array();
@@ -55,30 +75,35 @@
private static function getUnitStatusMap() {
return array(
ArcanistUnitTestResult::RESULT_FAIL => array(
+ 'differantial_result' => DifferentialUnitStatus::UNIT_FAIL, // SOMEWHERER, we already making this translation.
'label' => pht('Failed'),
'icon' => 'fa-times',
'color' => 'red',
'sort' => 'A',
),
ArcanistUnitTestResult::RESULT_BROKEN => array(
+ 'differantial_result' => DifferentialUnitStatus::UNIT_WARN,
'label' => pht('Broken'),
'icon' => 'fa-bomb',
'color' => 'indigo',
'sort' => 'B',
),
ArcanistUnitTestResult::RESULT_UNSOUND => array(
+ 'differantial_result' => DifferentialUnitStatus::UNIT_WARN,
'label' => pht('Unsound'),
'icon' => 'fa-exclamation-triangle',
'color' => 'yellow',
'sort' => 'C',
),
ArcanistUnitTestResult::RESULT_PASS => array(
+ 'differantial_result' => DifferentialUnitStatus::UNIT_OKAY,
'label' => pht('Passed'),
'icon' => 'fa-check',
'color' => 'green',
'sort' => 'D',
),
ArcanistUnitTestResult::RESULT_SKIP => array(
+ 'differantial_result' => DifferentialUnitStatus::UNIT_SKIP,
'label' => pht('Skipped'),
'icon' => 'fa-fast-forward',
'color' => 'blue',
diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
--- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
+++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
@@ -375,7 +375,6 @@
$waiting_targets[$target->getPHID()] = $target;
}
}
-
if (!$waiting_targets) {
return;
}
@@ -386,6 +385,7 @@
->withConsumed(false)
->execute();
+ $updated_targets = array();
foreach ($messages as $message) {
$target = $waiting_targets[$message->getBuildTargetPHID()];
@@ -403,20 +403,106 @@
}
if ($new_status !== null) {
- $message->setIsConsumed(true);
- $message->save();
-
$target->setTargetStatus($new_status);
+
if ($target->isComplete()) {
$target->setDateCompleted(PhabricatorTime::getNow());
}
$target->save();
}
+
+ $updated_targets[] = $message->getBuildTargetPHID();
+ $message->setIsConsumed(true);
+ $message->save();
+ }
+ $updated_targets[] = 'PHID-HMBT-5bhailrqui5kssm67g27'; // -----------------
+ $updated_targets = array_select_keys($waiting_targets, $updated_targets);
+
+ if ($updated_targets) {
+ $this->updateLintAndUnitStatus($updated_targets);
}
}
+ private function updateLintAndUnitStatus(array $targets) { // TODO rename this function, and maybe use it for migration as well.
+ assert_instances_of($targets, 'HarbormasterBuildTarget');
+ $target_phids = mpull($targets, 'getPHID');
+
+ // TODO HarbormasterBuildLintMessageQuery ?
+ $lints = id(new HarbormasterBuildLintMessage())->loadAllWhere(
+ 'buildTargetPHID IN (%Ls)',
+ $target_phids);
+ $lints = mgroup($lints, 'getBuildTargetPHID');
+
+ $units = id(new HarbormasterBuildUnitMessage())->loadAllWhere(
+ 'buildTargetPHID IN (%Ls)',
+ $target_phids);
+ $units = mgroup($units, 'getBuildTargetPHID');
+
+ foreach ($targets as $target) {
+ $target_phid = $target->getPHID();
+ $target_lints = idx($lints, $target_phid);
+ if ($target_lints) {
+ $worst = head(msort($target_lints, 'getSortKey'));
+ $lint_severity = $worst->getSeverity();
+ } else {
+ $lint_severity = null;
+ }
+
+ // TODO also count lint by severity? We don't currently do that
+
+ $target_units = idx($units, $target_phid);
+ phlog($target_units);
+ if ($target_units) {
+ $worst = head(msort($target_units, 'getSortKey'));
+ $unit_result = $worst->getResult();
+
+ $coverage_map = array();
+ foreach ($target_units as $unit) {
+ $coverage = $unit->getProperty('coverage', array());
+ foreach ($coverage as $path => $coverage_data) {
+ $coverage_map[$path][] = $coverage_data;
+ }
+ }
+
+ $groups = mgroup($target_units, 'getResult');
+ $unit_counts = array();
+ foreach ($groups as $status => $group) {
+ $unit_counts[$status] = count($group);
+ }
+
+ foreach ($coverage_map as $path => $coverage_items) {
+ $coverage_map[$path] = ArcanistUnitTestResult::mergeCoverage(
+ $coverage_items);
+ }
+ } else {
+ $unit_result = null;
+ $coverage_map = null;
+ $unit_counts = null;
+ }
+
+ // evil magic: `ORDER BY FIELD(result, 'fail', 'broken', 'skip', 'fail')`, although god knows which versions of mysql allow this.
+
+ $target->setDetail(
+ HarbormasterBuildTarget::DETAIL_LINT_STATUS,
+ $lint_severity);
+
+ $target->setDetail(
+ HarbormasterBuildTarget::DETAIL_UNIT_STATUS,
+ $unit_result);
+
+ $target->setDetail(
+ HarbormasterBuildTarget::DETAIL_UNIT_COUNTS,
+ $unit_counts);
+
+ $target->setDetail(
+ HarbormasterBuildTarget::DETAIL_COVERAGE_MAP,
+ $coverage_map);
+
+ $target->save();
+ }
+ }
/**
* Update the overall status of the buildable this build is attached to.
diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php
--- a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php
+++ b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php
@@ -21,6 +21,11 @@
const STATUS_FAILED = 'target/failed';
const STATUS_ABORTED = 'target/aborted';
+ const DETAIL_LINT_STATUS = 'lint:status';
+ const DETAIL_UNIT_STATUS = 'unit:status';
+ const DETAIL_UNIT_COUNTS = 'unit:counts';
+ const DETAIL_COVERAGE_MAP = 'unit:coverage';
+
private $build = self::ATTACHABLE;
private $buildStep = self::ATTACHABLE;
private $implementation;
diff --git a/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php
--- a/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php
+++ b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php
@@ -103,7 +103,7 @@
if ($full_uri && (count($messages) > $limit)) {
$counts = array();
- $groups = mgroup($messages, 'getResult');
+ $groups = mgroup($messages, 'getResult'); // TODO use aggregated info
foreach ($groups as $status => $group) {
$counts[] = HarbormasterUnitStatus::getUnitStatusCountLabel(
$status,
diff --git a/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php b/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php
--- a/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php
+++ b/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php
@@ -40,7 +40,7 @@
$id = $buildable->getID();
$full_uri = "/harbormaster/unit/{$id}/";
- $messages = msort($messages, 'getSortKey');
+ $messages = msort($messages, 'getSortKey'); // TODO avoid msort again, use saved information
$head_unit = head($messages);
if ($head_unit) {
$status = $head_unit->getResult();

File Metadata

Mime Type
text/plain
Expires
Sun, Aug 3, 10:03 PM (4 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
8889711
Default Alt Text
D16417.id39480.diff (13 KB)

Event Timeline