D16417.diff
No OneTemporary
Actions

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

Mime Type: text/plain
Expires: Tue, Oct 14, 6:31 AM (1 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.diff (32 KB)

D16417.diffNo OneTemporaryActions

D16417.diffView Options

File Metadata

Event Timeline

D16417.diff
No OneTemporary
Actions

D16417.diff
View Options