1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-18 19:40:55 +01:00

Show broken units in revision history

Summary:
This is hacky, and I'm not sure I'm happy with it; Until T9365 is done, this will show up
broken tests with an appropriate star in the Revision History.

Test Plan: Created 1M messages in a couple of old diffs in a revision. The query took ~80us (On SSD drive).

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D16483
This commit is contained in:
Aviv Eyal 2016-09-02 10:16:14 -07:00
parent d0013d0898
commit 31c5f39506
3 changed files with 62 additions and 9 deletions

View file

@ -292,9 +292,12 @@ final class DifferentialRevisionViewController extends DifferentialController {
'/differential/comment/inline/edit/'.$revision->getID().'/'); '/differential/comment/inline/edit/'.$revision->getID().'/');
} }
$broken_diffs = $this->loadHistoryDiffStatus($diffs);
$history = id(new DifferentialRevisionUpdateHistoryView()) $history = id(new DifferentialRevisionUpdateHistoryView())
->setUser($viewer) ->setUser($viewer)
->setDiffs($diffs) ->setDiffs($diffs)
->setDiffUnitStatuses($broken_diffs)
->setSelectedVersusDiffID($diff_vs) ->setSelectedVersusDiffID($diff_vs)
->setSelectedDiffID($target->getID()) ->setSelectedDiffID($target->getID())
->setSelectedWhitespace($whitespace) ->setSelectedWhitespace($whitespace)
@ -776,6 +779,45 @@ final class DifferentialRevisionViewController extends DifferentialController {
return $actions_dict; return $actions_dict;
} }
private function loadHistoryDiffStatus(array $diffs) {
assert_instances_of($diffs, 'DifferentialDiff');
$diff_phids = mpull($diffs, 'getPHID');
$bad_unit_status = array(
ArcanistUnitTestResult::RESULT_FAIL,
ArcanistUnitTestResult::RESULT_BROKEN,
);
$message = new HarbormasterBuildUnitMessage();
$target = new HarbormasterBuildTarget();
$build = new HarbormasterBuild();
$buildable = new HarbormasterBuildable();
$broken_diffs = queryfx_all(
$message->establishConnection('r'),
'SELECT distinct a.buildablePHID
FROM %T m
JOIN %T t ON m.buildTargetPHID = t.phid
JOIN %T b ON t.buildPHID = b.phid
JOIN %T a ON b.buildablePHID = a.phid
WHERE a.buildablePHID IN (%Ls)
AND m.result in (%Ls)',
$message->getTableName(),
$target->getTableName(),
$build->getTableName(),
$buildable->getTableName(),
$diff_phids,
$bad_unit_status);
$unit_status = array();
foreach ($broken_diffs as $broken) {
$phid = $broken['buildablePHID'];
$unit_status[$phid] = DifferentialUnitStatus::UNIT_FAIL;
}
return $unit_status;
}
private function loadChangesetsAndVsMap( private function loadChangesetsAndVsMap(
DifferentialDiff $target, DifferentialDiff $target,
DifferentialDiff $diff_vs = null, DifferentialDiff $diff_vs = null,

View file

@ -61,7 +61,8 @@ final class DifferentialUnitField
); );
$icon_color = idx($colors, $diff->getUnitStatus(), 'grey'); $icon_color = idx($colors, $diff->getUnitStatus(), 'grey');
$message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff); $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage(
$diff->getUnitStatus());
$status = id(new PHUIStatusListView()) $status = id(new PHUIStatusListView())
->addItem( ->addItem(

View file

@ -7,6 +7,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
private $selectedDiffID; private $selectedDiffID;
private $selectedWhitespace; private $selectedWhitespace;
private $commitsForLinks = array(); private $commitsForLinks = array();
private $unitStatus = array();
public function setDiffs(array $diffs) { public function setDiffs(array $diffs) {
assert_instances_of($diffs, 'DifferentialDiff'); assert_instances_of($diffs, 'DifferentialDiff');
@ -35,6 +36,11 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
return $this; return $this;
} }
public function setDiffUnitStatuses(array $unit_status) {
$this->unitStatus = $unit_status;
return $this;
}
public function render() { public function render() {
$this->requireResource('differential-core-view-css'); $this->requireResource('differential-core-view-css');
$this->requireResource('differential-revision-history-css'); $this->requireResource('differential-revision-history-css');
@ -139,6 +145,11 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
} }
if ($diff) { if ($diff) {
$unit_status = idx(
$this->unitStatus,
$diff->getPHID(),
$diff->getUnitStatus());
$lint = self::renderDiffLintStar($row['obj']); $lint = self::renderDiffLintStar($row['obj']);
$lint = phutil_tag( $lint = phutil_tag(
'div', 'div',
@ -148,12 +159,12 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
), ),
$lint); $lint);
$unit = self::renderDiffUnitStar($row['obj']); $unit = self::renderDiffUnitStar($unit_status);
$unit = phutil_tag( $unit = phutil_tag(
'div', 'div',
array( array(
'class' => 'lintunit-star', 'class' => 'lintunit-star',
'title' => self::getDiffUnitMessage($diff), 'title' => self::getDiffUnitMessage($unit_status),
), ),
$unit); $unit);
@ -312,7 +323,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
const STAR_FAIL = 'fail'; const STAR_FAIL = 'fail';
const STAR_SKIP = 'skip'; const STAR_SKIP = 'skip';
public static function renderDiffLintStar(DifferentialDiff $diff) { private static function renderDiffLintStar(DifferentialDiff $diff) {
static $map = array( static $map = array(
DifferentialLintStatus::LINT_NONE => self::STAR_NONE, DifferentialLintStatus::LINT_NONE => self::STAR_NONE,
DifferentialLintStatus::LINT_OKAY => self::STAR_OKAY, DifferentialLintStatus::LINT_OKAY => self::STAR_OKAY,
@ -327,7 +338,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
return self::renderDiffStar($star); return self::renderDiffStar($star);
} }
public static function renderDiffUnitStar(DifferentialDiff $diff) { private static function renderDiffUnitStar($unit_status) {
static $map = array( static $map = array(
DifferentialUnitStatus::UNIT_NONE => self::STAR_NONE, DifferentialUnitStatus::UNIT_NONE => self::STAR_NONE,
DifferentialUnitStatus::UNIT_OKAY => self::STAR_OKAY, DifferentialUnitStatus::UNIT_OKAY => self::STAR_OKAY,
@ -336,8 +347,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
DifferentialUnitStatus::UNIT_SKIP => self::STAR_SKIP, DifferentialUnitStatus::UNIT_SKIP => self::STAR_SKIP,
DifferentialUnitStatus::UNIT_AUTO_SKIP => self::STAR_SKIP, DifferentialUnitStatus::UNIT_AUTO_SKIP => self::STAR_SKIP,
); );
$star = idx($map, $unit_status, self::STAR_FAIL);
$star = idx($map, $diff->getUnitStatus(), self::STAR_FAIL);
return self::renderDiffStar($star); return self::renderDiffStar($star);
} }
@ -360,8 +370,8 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
return pht('Unknown'); return pht('Unknown');
} }
public static function getDiffUnitMessage(DifferentialDiff $diff) { public static function getDiffUnitMessage($unit_status) {
switch ($diff->getUnitStatus()) { switch ($unit_status) {
case DifferentialUnitStatus::UNIT_NONE: case DifferentialUnitStatus::UNIT_NONE:
return pht('No Unit Test Coverage'); return pht('No Unit Test Coverage');
case DifferentialUnitStatus::UNIT_OKAY: case DifferentialUnitStatus::UNIT_OKAY: