Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F19071510
D16417.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
32 KB
Referenced Files
None
Subscribers
None
D16417.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
Details
Attached
Mime Type
text/plain
Expires
Dec 1 2025, 7:49 PM (6 w, 4 d ago)
Storage Engine
amazon-s3
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
phabricator/secure/vz/tv/yt6ixltry63vlcsl
Default Alt Text
D16417.diff (32 KB)
Attached To
Mode
D16417: Aggregate lint, unit information in HarbormasterBuildable
Attached
Detach File
Event Timeline
Log In to Comment