Page MenuHomePhabricator
Files F18507992

D16417.id.diff
No OneTemporary
Actions

D16417.id.diff
View Options

diff --git a/resources/sql/autopatches/20160822.buildable.details.1.sql b/resources/sql/autopatches/20160822.buildable.details.1.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20160822.buildable.details.1.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildable
+ADD details LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};
diff --git a/resources/sql/autopatches/20160822.buildable.details.2.sql b/resources/sql/autopatches/20160822.buildable.details.2.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20160822.buildable.details.2.sql
@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_harbormaster.harbormaster_buildable
+SET details = '{}' WHERE details = '';
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
@@ -1172,6 +1172,7 @@
'HarbormasterCircleCIBuildableInterface' => 'applications/harbormaster/interface/HarbormasterCircleCIBuildableInterface.php',
'HarbormasterCircleCIHookController' => 'applications/harbormaster/controller/HarbormasterCircleCIHookController.php',
'HarbormasterConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php',
+ 'HarbormasterConstantsTestCase' => 'applications/harbormaster/__tests__/HarbormasterConstantsTestCase.php',
'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php',
'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php',
'HarbormasterCreatePlansCapability' => 'applications/harbormaster/capability/HarbormasterCreatePlansCapability.php',
@@ -1187,6 +1188,7 @@
'HarbormasterLeaseWorkingCopyBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php',
'HarbormasterLintMessagesController' => 'applications/harbormaster/controller/HarbormasterLintMessagesController.php',
'HarbormasterLintPropertyView' => 'applications/harbormaster/view/HarbormasterLintPropertyView.php',
+ 'HarbormasterLintStatus' => 'applications/harbormaster/constants/HarbormasterLintStatus.php',
'HarbormasterManagementArchiveLogsWorkflow' => 'applications/harbormaster/management/HarbormasterManagementArchiveLogsWorkflow.php',
'HarbormasterManagementBuildWorkflow' => 'applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php',
'HarbormasterManagementUpdateWorkflow' => 'applications/harbormaster/management/HarbormasterManagementUpdateWorkflow.php',
@@ -5761,6 +5763,7 @@
'HarbormasterCircleCIBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterCircleCIHookController' => 'HarbormasterController',
'HarbormasterConduitAPIMethod' => 'ConduitAPIMethod',
+ 'HarbormasterConstantsTestCase' => 'PhabricatorTestCase',
'HarbormasterController' => 'PhabricatorController',
'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod',
'HarbormasterCreatePlansCapability' => 'PhabricatorPolicyCapability',
@@ -5776,6 +5779,7 @@
'HarbormasterLeaseWorkingCopyBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterLintMessagesController' => 'HarbormasterController',
'HarbormasterLintPropertyView' => 'AphrontView',
+ 'HarbormasterLintStatus' => 'Phobject',
'HarbormasterManagementArchiveLogsWorkflow' => 'HarbormasterManagementWorkflow',
'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow',
'HarbormasterManagementUpdateWorkflow' => 'HarbormasterManagementWorkflow',
diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php
--- a/src/applications/differential/controller/DifferentialChangesetViewController.php
+++ b/src/applications/differential/controller/DifferentialChangesetViewController.php
@@ -403,37 +403,8 @@
}
private function loadCoverage(DifferentialChangeset $changeset) {
- $target_phids = $changeset->getDiff()->getBuildTargetPHIDs();
- if (!$target_phids) {
- return null;
- }
-
- $unit = id(new HarbormasterBuildUnitMessage())->loadAllWhere(
- 'buildTargetPHID IN (%Ls)',
- $target_phids);
-
- if (!$unit) {
- return null;
- }
-
- $coverage = array();
- foreach ($unit as $message) {
- $test_coverage = $message->getProperty('coverage');
- if ($test_coverage === null) {
- continue;
- }
- $coverage_data = idx($test_coverage, $changeset->getFileName());
- if (!strlen($coverage_data)) {
- continue;
- }
- $coverage[] = $coverage_data;
- }
-
- if (!$coverage) {
- return null;
- }
-
- return ArcanistUnitTestResult::mergeCoverage($coverage);
+ $coverage_map = $changeset->getDiff()->loadCoverageMap();
+ return idx($coverage_map, $changeset->getFileName());
}
}
diff --git a/src/applications/differential/controller/DifferentialController.php b/src/applications/differential/controller/DifferentialController.php
--- a/src/applications/differential/controller/DifferentialController.php
+++ b/src/applications/differential/controller/DifferentialController.php
@@ -108,8 +108,7 @@
}
}
-
- protected function loadHarbormasterData(array $diffs) {
+ protected function loadHarbormasterBuilds(array $diffs) {
$viewer = $this->getViewer();
$diffs = mpull($diffs, null, 'getPHID');
@@ -127,12 +126,18 @@
$diff->attachBuildable(idx($buildables, $phid));
}
+ }
+
+ protected function loadHarbormasterData(array $diffs) {
+ $diffs = mpull($diffs, null, 'getPHID');
+
$target_map = array();
foreach ($diffs as $phid => $diff) {
$target_map[$phid] = $diff->getBuildTargetPHIDs();
}
$all_target_phids = array_mergev($target_map);
+ // T10635: most of this information is already aggregated in BuildTarget.
if ($all_target_phids) {
$unit_messages = id(new HarbormasterBuildUnitMessage())->loadAllWhere(
'buildTargetPHID IN (%Ls)',
diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php
--- a/src/applications/differential/controller/DifferentialDiffViewController.php
+++ b/src/applications/differential/controller/DifferentialDiffViewController.php
@@ -113,7 +113,7 @@
$table_of_contents = $this->buildTableOfContents(
$changesets,
$changesets,
- $diff->loadCoverageMap($viewer));
+ $diff->loadCoverageMap());
$refs = array();
foreach ($changesets as $changeset) {
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -239,6 +239,8 @@
->setErrors($revision_warnings);
}
+ $this->loadHarbormasterBuilds($diffs);
+
$detail_diffs = array_select_keys(
$diffs,
array($diff_vs, $target->getID()));
@@ -322,7 +324,7 @@
$toc_view = $this->buildTableOfContents(
$changesets,
$visible_changesets,
- $target->loadCoverageMap($viewer));
+ $target->loadCoverageMap());
$tab_group = id(new PHUITabGroupView())
->addTab(
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
@@ -84,6 +84,7 @@
DifferentialDiff $diff,
array $messages) {
+ $status = $diff->getSummaryLintStatus();
$colors = array(
DifferentialLintStatus::LINT_NONE => 'grey',
DifferentialLintStatus::LINT_OKAY => 'green',
@@ -92,9 +93,10 @@
DifferentialLintStatus::LINT_SKIP => 'blue',
DifferentialLintStatus::LINT_AUTO_SKIP => 'blue',
);
- $icon_color = idx($colors, $diff->getLintStatus(), 'grey');
+ $icon_color = idx($colors, $status, 'grey');
- $message = DifferentialRevisionUpdateHistoryView::getDiffLintMessage($diff);
+ $message = DifferentialRevisionUpdateHistoryView::getDiffLintMessage(
+ $status);
$excuse = $diff->getProperty('arc:lint-excuse');
if (strlen($excuse)) {
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
@@ -32,7 +32,8 @@
}
public function getWarningsForDetailView() {
- $status = $this->getObject()->getActiveDiff()->getUnitStatus();
+ $active_diff = $this->getObject()->getActiveDiff();
+ $status = $active_diff->getSummaryUnitStatus();
$warnings = array();
if ($status < DifferentialUnitStatus::UNIT_WARN) {
@@ -50,6 +51,7 @@
}
public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+ $buildable = $diff->getBuildable();
$colors = array(
DifferentialUnitStatus::UNIT_NONE => 'grey',
@@ -59,19 +61,66 @@
DifferentialUnitStatus::UNIT_SKIP => 'blue',
DifferentialUnitStatus::UNIT_AUTO_SKIP => 'blue',
);
- $icon_color = idx($colors, $diff->getUnitStatus(), 'grey');
-
- $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff);
+ $view = id(new PHUIStatusListView());
+
+ $builds_unit_status = null;
+ if ($buildable) {
+ $builds_unit_status = $buildable->getDetail(
+ HarbormasterBuildable::DETAIL_UNIT_STATUS);
+ $builds_unit_status = HarbormasterUnitStatus::getDifferentialUnitStatus(
+ $builds_unit_status);
+ }
- $status = id(new PHUIStatusListView())
- ->addItem(
+ if ($builds_unit_status) {
+ $icon_color = idx($colors, $builds_unit_status, 'grey');
+ $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage(
+ $builds_unit_status);
+
+ $unit_count = $buildable->getDetail(
+ HarbormasterBuildable::DETAIL_UNIT_COUNTS);
+ $counts = array();
+ foreach ($unit_count as $status => $count) {
+ $counts[] = HarbormasterUnitStatus::getUnitStatusCountLabel(
+ $status,
+ $count);
+ }
+ $counts = implode(" \xC2\xB7 ", $counts);
+
+ $view->addItem(
id(new PHUIStatusItemView())
->setIcon(PHUIStatusItemView::ICON_STAR, $icon_color)
- ->setTarget($message));
-
- return $status;
- }
+ ->setTarget($message)
+ ->setNote($counts));
+ }
+ $arcanist_tests_status = $diff->getUnitStatus();
+ if ($arcanist_tests_status == DifferentialUnitStatus::UNIT_SKIP) {
+ $show_manual = true;
+ } else {
+ $show_manual = !$builds_unit_status;
+ }
+ if ($show_manual) {
+ $icon_color = idx($colors, $arcanist_tests_status, 'grey');
+ $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage(
+ $arcanist_tests_status);
+
+ $excuse = null;
+ if ($diff->hasDiffProperty('arc:unit-excuse')) {
+ $excuse = array(
+ phutil_tag('strong', array(), pht('Excuse:')),
+ ' ',
+ phutil_escape_html_newlines($diff->getProperty('arc:unit-excuse')),
+ );
+ }
+
+ $view->addItem(
+ id(new PHUIStatusItemView())
+ ->setIcon(PHUIStatusItemView::ICON_STAR, $icon_color)
+ ->setTarget($message)
+ ->setNote($excuse));
+ }
+ return $view;
+ }
}
diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php
--- a/src/applications/differential/storage/DifferentialDiff.php
+++ b/src/applications/differential/storage/DifferentialDiff.php
@@ -360,28 +360,27 @@
}
public function getBuildTargetPHIDs() {
+ $targets = $this->getBuildTargets();
+ return mpull($targets, 'getPHID');
+ }
+
+ public function getBuildTargets() {
$buildable = $this->getBuildable();
if (!$buildable) {
return array();
}
- $target_phids = array();
- foreach ($buildable->getBuilds() as $build) {
- foreach ($build->getBuildTargets() as $target) {
- $target_phids[] = $target->getPHID();
- }
- }
-
- return $target_phids;
+ $targets = mpull($buildable->getBuilds(), 'getBuildTargets');
+ return array_mergev($targets);
}
- public function loadCoverageMap(PhabricatorUser $viewer) {
+ public function loadCoverageMap() {
+ // T10635: most of this information is already aggregated in BuildTarget.
$target_phids = $this->getBuildTargetPHIDs();
if (!$target_phids) {
return array();
}
-
$unit = id(new HarbormasterBuildUnitMessage())->loadAllWhere(
'buildTargetPHID IN (%Ls)',
$target_phids);
@@ -412,11 +411,59 @@
return $this;
}
-
public function getUnitMessages() {
+ // T10635: practically no callsite of this method should want ALL messages.
return $this->assertAttached($this->unitMessages);
}
+ public function getSummaryLintStatus() {
+ // For now, I'll hack this with a clever summation of
+ // $builds_lint_status and $diff->getLintStatus.
+ // Next, actually add a "meta" message about lint total status.
+ // That will also give us a path twords having a message of "10,000 units
+ // passed" (For T10635).
+
+ // With modern arc (post Harbormaster), any lint event (message) that
+ // contributes to $this->lintStatus will also exist in the Buildable.
+ // So only these cases have information in the diff that isn't reflected
+ // in the Buildable: LINT_NONE, LINT_OKAY, LINT_SKIP, LINT_AUTOSKIP.
+ // Of these, NONE, OKAY and AUTOSKIP are not interesting if there are no
+ // lint messages in the Buildable.
+
+ $buildable = $this->getBuildable();
+ $buildable_lint = $buildable ?
+ $buildable->getDetail(HarbormasterBuildable::DETAIL_LINT_STATUS):
+ null;
+
+ $buildable_lint = HarbormasterLintStatus::getDifferentialLintStatus(
+ $buildable_lint);
+
+ $arc_status = $this->getLintStatus();
+
+ if ($arc_status == DifferentialLintStatus::LINT_SKIP || !$buildable_lint) {
+ return $arc_status;
+ }
+
+ return $buildable_lint;
+ }
+
+ public function getSummaryUnitStatus() {
+ $buildable = $this->getBuildable();
+ $buildable_unit = $buildable ?
+ $buildable->getDetail(HarbormasterBuildable::DETAIL_UNIT_STATUS) :
+ null;
+
+ $buildable_unit = HarbormasterUnitStatus::getDifferentialUnitStatus(
+ $buildable_unit);
+
+ $arc_status = $this->getUnitStatus();
+ if ($arc_status == DifferentialUnitStatus::UNIT_SKIP || !$buildable_unit) {
+ return $arc_status;
+ }
+
+ return $buildable_unit;
+ }
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
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
@@ -139,21 +139,24 @@
}
if ($diff) {
- $lint = self::renderDiffLintStar($row['obj']);
+ $lint_status = $diff->getSummaryLintStatus();
+ $unit_status = $diff->getSummaryUnitStatus();
+
+ $lint = self::renderDiffLintStar($lint_status);
$lint = phutil_tag(
'div',
array(
'class' => 'lintunit-star',
- 'title' => self::getDiffLintMessage($diff),
+ 'title' => self::getDiffLintMessage($lint_status),
),
$lint);
- $unit = self::renderDiffUnitStar($row['obj']);
+ $unit = self::renderDiffUnitStar($unit_status);
$unit = phutil_tag(
'div',
array(
'class' => 'lintunit-star',
- 'title' => self::getDiffUnitMessage($diff),
+ 'title' => self::getDiffUnitMessage($unit_status),
),
$unit);
@@ -312,7 +315,7 @@
const STAR_FAIL = 'fail';
const STAR_SKIP = 'skip';
- public static function renderDiffLintStar(DifferentialDiff $diff) {
+ private static function renderDiffLintStar($status) {
static $map = array(
DifferentialLintStatus::LINT_NONE => self::STAR_NONE,
DifferentialLintStatus::LINT_OKAY => self::STAR_OKAY,
@@ -322,12 +325,12 @@
DifferentialLintStatus::LINT_AUTO_SKIP => self::STAR_SKIP,
);
- $star = idx($map, $diff->getLintStatus(), self::STAR_FAIL);
+ $star = idx($map, $status, self::STAR_FAIL);
return self::renderDiffStar($star);
}
- public static function renderDiffUnitStar(DifferentialDiff $diff) {
+ private static function renderDiffUnitStar($unit_status) {
static $map = array(
DifferentialUnitStatus::UNIT_NONE => self::STAR_NONE,
DifferentialUnitStatus::UNIT_OKAY => self::STAR_OKAY,
@@ -337,13 +340,14 @@
DifferentialUnitStatus::UNIT_AUTO_SKIP => self::STAR_SKIP,
);
- $star = idx($map, $diff->getUnitStatus(), self::STAR_FAIL);
+
+ $star = idx($map, $unit_status, self::STAR_FAIL);
return self::renderDiffStar($star);
}
- public static function getDiffLintMessage(DifferentialDiff $diff) {
- switch ($diff->getLintStatus()) {
+ public static function getDiffLintMessage($status) {
+ switch ($status) {
case DifferentialLintStatus::LINT_NONE:
return pht('No Linters Available');
case DifferentialLintStatus::LINT_OKAY:
@@ -360,8 +364,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/__tests__/HarbormasterConstantsTestCase.php b/src/applications/harbormaster/__tests__/HarbormasterConstantsTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/applications/harbormaster/__tests__/HarbormasterConstantsTestCase.php
@@ -0,0 +1,95 @@
+<?php
+
+final class HarbormasterConstantsTestCase extends PhabricatorTestCase {
+
+ public function testSummarizeLintStatuses() {
+ $test_sets = array();
+
+ $test_sets['empty'] = array(
+ 'input' => array(),
+ 'expected' => null,
+ );
+
+ $test_sets['simple'] = array(
+ 'input' => array(
+ ArcanistLintSeverity::SEVERITY_ADVICE,
+ ArcanistLintSeverity::SEVERITY_WARNING,
+ ArcanistLintSeverity::SEVERITY_ERROR,
+ ),
+ 'expected' => ArcanistLintSeverity::SEVERITY_ERROR,
+ );
+
+ $test_sets['reversed simple'] = array(
+ 'input' => array(
+ ArcanistLintSeverity::SEVERITY_ERROR,
+ ArcanistLintSeverity::SEVERITY_WARNING,
+ ArcanistLintSeverity::SEVERITY_ADVICE,
+ ),
+ 'expected' => ArcanistLintSeverity::SEVERITY_ERROR,
+ );
+
+ $test_sets['all advice'] = array(
+ 'input' => array(
+ ArcanistLintSeverity::SEVERITY_ADVICE,
+ ArcanistLintSeverity::SEVERITY_ADVICE,
+ ArcanistLintSeverity::SEVERITY_ADVICE,
+ ),
+ 'expected' => ArcanistLintSeverity::SEVERITY_ADVICE,
+ );
+
+ foreach ($test_sets as $key => $set) {
+ $input = $set['input'];
+ $expected = $set['expected'];
+
+ $result = HarbormasterLintStatus::summarizeStatuses($input);
+
+ $this->assertEqual($expected, $result, 'test case: '.$key);
+ }
+ }
+
+ public function testSummarizeUnitStatuses() {
+ $test_sets = array();
+
+ $test_sets['empty'] = array(
+ 'input' => array(),
+ 'expected' => null,
+ );
+
+ $test_sets['simple'] = array(
+ 'input' => array(
+ ArcanistUnitTestResult::RESULT_PASS,
+ ArcanistUnitTestResult::RESULT_SKIP,
+ ArcanistUnitTestResult::RESULT_FAIL,
+ ),
+ 'expected' => ArcanistUnitTestResult::RESULT_FAIL,
+ );
+
+ $test_sets['reversed simple'] = array(
+ 'input' => array(
+ ArcanistUnitTestResult::RESULT_FAIL,
+ ArcanistUnitTestResult::RESULT_SKIP,
+ ArcanistUnitTestResult::RESULT_PASS,
+ ),
+ 'expected' => ArcanistUnitTestResult::RESULT_FAIL,
+ );
+
+ $test_sets['all pass'] = array(
+ 'input' => array(
+ ArcanistUnitTestResult::RESULT_PASS,
+ ArcanistUnitTestResult::RESULT_PASS,
+ ArcanistUnitTestResult::RESULT_PASS,
+ ),
+ 'expected' => ArcanistUnitTestResult::RESULT_PASS,
+ );
+
+ foreach ($test_sets as $key => $set) {
+ $input = $set['input'];
+ $expected = $set['expected'];
+
+ $result = HarbormasterUnitStatus::summarizeStatuses($input);
+
+ $this->assertEqual($expected, $result, 'test case: '.$key);
+ }
+ }
+
+}
diff --git a/src/applications/harbormaster/constants/HarbormasterLintStatus.php b/src/applications/harbormaster/constants/HarbormasterLintStatus.php
new file mode 100644
--- /dev/null
+++ b/src/applications/harbormaster/constants/HarbormasterLintStatus.php
@@ -0,0 +1,68 @@
+<?php
+
+final class HarbormasterLintStatus
+ extends Phobject {
+
+ public static function getLintStatusLabel($status) {
+ $map = self::getLintStatusDictionary($status);
+ $default = pht('Unknown Status ("%s")', $status);
+ return idx($map, 'label', $default);
+ }
+
+ public static function getLintStatusSort($status) {
+ $map = self::getLintStatusDictionary($status);
+ $default = 'N';
+ return idx($map, 'sort', $default);
+ }
+
+ public static function summarizeStatuses(array $statuses) {
+ if (!$statuses) {
+ return null;
+ }
+ $map = self::getLintStatusMap();
+ $map = ipull($map, 'sort');
+ $default = 'Z';
+ $summary = head($statuses);
+ $w_index = idx($map, $summary, $default);
+ foreach ($statuses as $status) {
+ $r = idx($map, $status, $default);
+ if ($r < $w_index) {
+ $summary = $status;
+ $w_index = $r;
+ }
+ }
+ return $summary;
+ }
+
+ public static function getDifferentialLintStatus($status) {
+ $map = self::getLintStatusDictionary($status);
+ return idx($map, 'differential_value', $status);
+ }
+
+ private static function getLintStatusDictionary($status) {
+ $map = self::getLintStatusMap();
+ $default = array();
+ return idx($map, $status, $default);
+ }
+
+ private static function getLintStatusMap() {
+ return array(
+ ArcanistLintSeverity::SEVERITY_ERROR => array(
+ 'differential_value' => DifferentialLintStatus::LINT_FAIL,
+ 'label' => pht('Error'),
+ 'sort' => 'A',
+ ),
+ ArcanistLintSeverity::SEVERITY_WARNING => array(
+ 'differential_value' => DifferentialLintStatus::LINT_WARN,
+ 'label' => pht('Warning'),
+ 'sort' => 'B',
+ ),
+ ArcanistLintSeverity::SEVERITY_ADVICE => array(
+ 'differential_value' => DifferentialLintStatus::LINT_OKAY,
+ 'label' => pht('Advice'),
+ 'sort' => 'Y',
+ ),
+ );
+ }
+
+}
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,30 @@
return idx($map, 'sort', $default);
}
+ public static function summarizeStatuses(array $statuses) {
+ if (!$statuses) {
+ return null;
+ }
+ $map = self::getUnitStatusMap();
+ $map = ipull($map, 'sort');
+ $default = 'Z';
+ $summary = head($statuses);
+ $w_index = idx($map, $summary, $default);
+ foreach ($statuses as $status) {
+ $r = idx($map, $status, $default);
+ if ($r < $w_index) {
+ $summary = $status;
+ $w_index = $r;
+ }
+ }
+ return $summary;
+ }
+
+ 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 +79,35 @@
private static function getUnitStatusMap() {
return array(
ArcanistUnitTestResult::RESULT_FAIL => array(
+ 'differantial_result' => DifferentialUnitStatus::UNIT_FAIL,
'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
@@ -12,6 +12,8 @@
private $artifactReleaseQueue = array();
private $forceBuildableUpdate;
+ private $consumedNewMessages = false;
+
public function setForceBuildableUpdate($force_buildable_update) {
$this->forceBuildableUpdate = $force_buildable_update;
return $this;
@@ -91,7 +93,10 @@
// If the build changed status, we might need to update the overall status
// on the buildable.
$new_status = $build->getBuildStatus();
- if ($new_status != $old_status || $this->shouldForceBuildableUpdate()) {
+ if ($new_status != $old_status ||
+ $this->consumedNewMessages ||
+ $this->shouldForceBuildableUpdate()) {
+
$this->updateBuildable($build->getBuildable());
}
@@ -375,7 +380,6 @@
$waiting_targets[$target->getPHID()] = $target;
}
}
-
if (!$waiting_targets) {
return;
}
@@ -386,6 +390,7 @@
->withConsumed(false)
->execute();
+ $updated_targets = array();
foreach ($messages as $message) {
$target = $waiting_targets[$message->getBuildTargetPHID()];
@@ -403,21 +408,22 @@
}
if ($new_status !== null) {
- $message->setIsConsumed(true);
- $message->save();
-
$target->setTargetStatus($new_status);
+
if ($target->isComplete()) {
$target->setDateCompleted(PhabricatorTime::getNow());
}
$target->save();
}
+
+ $this->consumedNewMessages = true;
+ $message->setIsConsumed(true);
+ $message->save();
}
}
-
/**
* Update the overall status of the buildable this build is attached to.
*
@@ -438,8 +444,12 @@
->setViewer($viewer)
->withIDs(array($buildable->getID()))
->needBuilds(true)
+ ->needTargets(true)
->executeOne();
+ // This doesn't effect the `should_publish` state.
+ $this->updateLintAndUnitStatus($buildable);
+
$all_pass = true;
$any_fail = false;
foreach ($buildable->getBuilds() as $build) {
@@ -535,6 +545,75 @@
array($template));
}
+ /**
+ * Update lint and unit aggregated information.
+ * Invoked in the context of the Buildable lock.
+ */
+ private function updateLintAndUnitStatus(HarbormasterBuildable $buildable) {
+ $targets = array_mergev(mpull($buildable->getBuilds(), 'getBuildTargets'));
+ $target_phids = mpull($targets, 'getPHID');
+
+ $buildable_lints = id(new HarbormasterBuildLintMessage())->loadAllWhere(
+ 'buildTargetPHID IN (%Ls)',
+ $target_phids);
+
+ $buildable_units = id(new HarbormasterBuildUnitMessage())->loadAllWhere(
+ 'buildTargetPHID IN (%Ls)',
+ $target_phids);
+
+ if ($buildable_lints) {
+ $severities = mpull($buildable_lints, 'getSeverity');
+ $lint_severity = HarbormasterLintStatus::summarizeStatuses($severities);
+ } else {
+ $lint_severity = null;
+ }
+
+ if ($buildable_units) {
+ $coverage_map = array();
+ foreach ($buildable_units as $unit) {
+ $coverage = $unit->getProperty('coverage', array());
+ foreach ($coverage as $path => $coverage_data) {
+ $coverage_map[$path][] = $coverage_data;
+ }
+ }
+
+ $groups = mgroup($buildable_units, 'getResult');
+ $unit_counts = array();
+ foreach ($groups as $status => $group) {
+ $unit_counts[$status] = count($group);
+ }
+
+ $statuses = array_keys($unit_counts);
+ $unit_result = HarbormasterUnitStatus::summarizeStatuses($statuses);
+
+ foreach ($coverage_map as $path => $items) {
+ $coverage_map[$path] = ArcanistUnitTestResult::mergeCoverage($items);
+ }
+ } else {
+ $unit_result = null;
+ $coverage_map = null;
+ $unit_counts = null;
+ }
+
+ $buildable->setDetail(
+ HarbormasterBuildable::DETAIL_LINT_STATUS,
+ $lint_severity);
+
+ $buildable->setDetail(
+ HarbormasterBuildable::DETAIL_UNIT_STATUS,
+ $unit_result);
+
+ $buildable->setDetail(
+ HarbormasterBuildable::DETAIL_UNIT_COUNTS,
+ $unit_counts);
+
+ $buildable->setDetail(
+ HarbormasterBuildable::DETAIL_COVERAGE_MAP,
+ $coverage_map);
+
+ $buildable->save();
+ }
+
private function releaseAllArtifacts(HarbormasterBuild $build) {
$targets = id(new HarbormasterBuildTargetQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
diff --git a/src/applications/harbormaster/storage/HarbormasterBuildable.php b/src/applications/harbormaster/storage/HarbormasterBuildable.php
--- a/src/applications/harbormaster/storage/HarbormasterBuildable.php
+++ b/src/applications/harbormaster/storage/HarbormasterBuildable.php
@@ -9,6 +9,7 @@
protected $buildablePHID;
protected $containerPHID;
protected $buildableStatus;
+ protected $details;
protected $isManualBuildable;
private $buildableObject = self::ATTACHABLE;
@@ -19,6 +20,11 @@
const STATUS_PASSED = 'passed';
const STATUS_FAILED = 'failed';
+ const DETAIL_LINT_STATUS = 'lint:status';
+ const DETAIL_UNIT_STATUS = 'unit:status';
+ const DETAIL_UNIT_COUNTS = 'unit:counts';
+ const DETAIL_COVERAGE_MAP = 'unit:coverage';
+
public static function getBuildableStatusName($status) {
$map = self::getBuildStatusMap();
return idx($map, $status, pht('Unknown ("%s")', $status));
@@ -197,6 +203,9 @@
protected function getConfiguration() {
return array(
self::CONFIG_AUX_PHID => true,
+ self::CONFIG_SERIALIZATION => array(
+ 'details' => self::SERIALIZATION_JSON,
+ ),
self::CONFIG_COLUMN_SCHEMA => array(
'containerPHID' => 'phid?',
'buildableStatus' => 'text32',
@@ -249,6 +258,14 @@
return $this->assertAttached($this->builds);
}
+ public function getDetail($key, $default = null) {
+ return idx($this->details, $key, $default);
+ }
+
+ public function setDetail($key, $value) {
+ $this->details[$key] = $value;
+ return $this;
+ }
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
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,6 +103,7 @@
if ($full_uri && (count($messages) > $limit)) {
$counts = array();
+ // T10635: most of this information is already aggregated in BuildTarget.
$groups = mgroup($messages, 'getResult');
foreach ($groups as $status => $group) {
$counts[] = HarbormasterUnitStatus::getUnitStatusCountLabel(
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,6 +40,7 @@
$id = $buildable->getID();
$full_uri = "/harbormaster/unit/{$id}/";
+ // T10635: most of this information is already aggregated in BuildTarget.
$messages = msort($messages, 'getSortKey');
$head_unit = head($messages);
if ($head_unit) {

File Metadata

Mime Type
text/plain
Expires
Sep 6 2025, 2:33 AM (15 w, 1 d ago)
Storage Engine
amazon-s3
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
phabricator/secure/vz/tv/yt6ixltry63vlcsl
Default Alt Text
D16417.id.diff (32 KB)

Event Timeline

Phabricator · Built by Phacility