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 @@ -127,31 +127,34 @@ $changeset = $choice; } - $coverage = null; - if ($right && $right->getDiffID()) { - $unit = id(new DifferentialDiffProperty())->loadOneWhere( - 'diffID = %d AND name = %s', - $right->getDiffID(), - 'arc:unit'); - - if ($unit) { - $coverage = array(); - foreach ($unit->getData() as $result) { - $result_coverage = idx($result, 'coverage'); - if (!$result_coverage) { - continue; - } - $file_coverage = idx($result_coverage, $right->getFileName()); - if (!$file_coverage) { - continue; - } - $coverage[] = $file_coverage; - } + if ($left_new || $right_new) { + $diff_map = array(); + if ($left) { + $diff_map[] = $left->getDiff(); + } + if ($right) { + $diff_map[] = $right->getDiff(); + } + $diff_map = mpull($diff_map, null, 'getPHID'); - $coverage = ArcanistUnitTestResult::mergeCoverage($coverage); + $buildables = id(new HarbormasterBuildableQuery()) + ->setViewer($viewer) + ->withBuildablePHIDs(array_keys($diff_map)) + ->withManualBuildables(false) + ->needBuilds(true) + ->needTargets(true) + ->execute(); + $buildables = mpull($buildables, null, 'getBuildablePHID'); + foreach ($diff_map as $diff_phid => $changeset_diff) { + $changeset_diff->attachBuildable(idx($buildables, $diff_phid)); } } + $coverage = null; + if ($right_new) { + $coverage = $this->loadCoverage($right); + } + $spec = $request->getStr('range'); list($range_s, $range_e, $mask) = DifferentialChangesetParser::parseRangeSpecification($spec); @@ -203,29 +206,6 @@ $inlines = array(); } - if ($left_new || $right_new) { - $diff_map = array(); - if ($left) { - $diff_map[] = $left->getDiff(); - } - if ($right) { - $diff_map[] = $right->getDiff(); - } - $diff_map = mpull($diff_map, null, 'getPHID'); - - $buildables = id(new HarbormasterBuildableQuery()) - ->setViewer($viewer) - ->withBuildablePHIDs(array_keys($diff_map)) - ->withManualBuildables(false) - ->needBuilds(true) - ->needTargets(true) - ->execute(); - $buildables = mpull($buildables, null, 'getBuildablePHID'); - foreach ($diff_map as $diff_phid => $changeset_diff) { - $changeset_diff->attachBuildable(idx($buildables, $diff_phid)); - } - } - if ($left_new) { $inlines = array_merge( $inlines, @@ -381,18 +361,7 @@ private function buildLintInlineComments($changeset) { $diff = $changeset->getDiff(); - $buildable = $diff->getBuildable(); - if (!$buildable) { - return array(); - } - - $target_phids = array(); - foreach ($buildable->getBuilds() as $build) { - foreach ($build->getBuildTargets() as $target) { - $target_phids[] = $target->getPHID(); - } - } - + $target_phids = $diff->getBuildTargetPHIDs(); if (!$target_phids) { return array(); } @@ -425,4 +394,27 @@ return $inlines; } + private function loadCoverage(DifferentialChangeset $changeset) { + $target_phids = $changeset->getDiff()->getBuildTargetPHIDs(); + if (!$target_phids) { + return array(); + } + + $unit = id(new HarbormasterBuildUnitMessage())->loadAllWhere( + 'buildTargetPHID IN (%Ls)', + $target_phids); + + $coverage = array(); + foreach ($unit as $message) { + $test_coverage = $message->getProperty('coverage', array()); + $coverage_data = idx($test_coverage, $changeset->getFileName()); + if (!strlen($coverage_data)) { + continue; + } + $coverage[] = $coverage_data; + } + + return ArcanistUnitTestResult::mergeCoverage($coverage); + } + } diff --git a/src/applications/differential/customfield/DifferentialHarbormasterField.php b/src/applications/differential/customfield/DifferentialHarbormasterField.php --- a/src/applications/differential/customfield/DifferentialHarbormasterField.php +++ b/src/applications/differential/customfield/DifferentialHarbormasterField.php @@ -29,20 +29,11 @@ $diff->attachProperty($key, idx($properties, $key)); } - $messages = array(); - - $buildable = $diff->getBuildable(); - if ($buildable) { - $target_phids = array(); - foreach ($buildable->getBuilds() as $build) { - foreach ($build->getBuildTargets() as $target) { - $target_phids[] = $target->getPHID(); - } - } - - if ($target_phids) { - $messages = $this->loadHarbormasterTargetMessages($target_phids); - } + $target_phids = $diff->getBuildTargetPHIDs(); + if ($target_phids) { + $messages = $this->loadHarbormasterTargetMessages($target_phids); + } else { + $messages = array(); } if (!$messages) { 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 @@ -331,6 +331,22 @@ return $this->assertAttached($this->buildable); } + public function getBuildTargetPHIDs() { + $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; + } /* -( PhabricatorPolicyInterface )----------------------------------------- */