From 5485fb8aa9e7f2652c2a15accd7da855d4179c02 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 10 Aug 2015 15:24:15 -0700 Subject: [PATCH] Read modern coverage information for tables of contents Summary: Ref T8096. This modernizes the last thing which was reading the old datasource. Also fix a bug where it didn't work. Test Plan: {F698405} Reviewers: chad Reviewed By: chad Maniphest Tasks: T8096 Differential Revision: https://secure.phabricator.com/D13852 --- .../DifferentialChangesetViewController.php | 10 +++++--- .../DifferentialDiffViewController.php | 2 +- .../DifferentialRevisionViewController.php | 2 +- .../differential/storage/DifferentialDiff.php | 25 +++++++++++++++++++ .../DifferentialDiffTableOfContentsView.php | 25 +++---------------- 5 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index 5efb8561af..a73875c8f8 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -251,15 +251,19 @@ final class DifferentialChangesetViewController extends DifferentialController { ->setMask($mask); if ($request->isAjax()) { + // NOTE: We must render the changeset before we render coverage + // information, since it builds some caches. + $rendered_changeset = $parser->renderChangeset(); + $mcov = $parser->renderModifiedCoverage(); - $coverage = array( + $coverage_data = array( 'differential-mcoverage-'.md5($changeset->getFilename()) => $mcov, ); return id(new PhabricatorChangesetResponse()) - ->setRenderedChangeset($parser->renderChangeset()) - ->setCoverage($coverage) + ->setRenderedChangeset($rendered_changeset) + ->setCoverage($coverage_data) ->setUndoTemplates($parser->getRenderer()->renderUndoTemplates()); } diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php index d095465604..9756634a81 100644 --- a/src/applications/differential/controller/DifferentialDiffViewController.php +++ b/src/applications/differential/controller/DifferentialDiffViewController.php @@ -108,7 +108,7 @@ final class DifferentialDiffViewController extends DifferentialController { $table_of_contents = id(new DifferentialDiffTableOfContentsView()) ->setChangesets($changesets) ->setVisibleChangesets($changesets) - ->setUnitTestData(idx($props, 'arc:unit', array())); + ->setCoverageMap($diff->loadCoverageMap($viewer)); $refs = array(); foreach ($changesets as $changeset) { diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 6c695b8ce4..085e888fe5 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -353,7 +353,7 @@ final class DifferentialRevisionViewController extends DifferentialController { $toc_view->setChangesets($changesets); $toc_view->setVisibleChangesets($visible_changesets); $toc_view->setRenderingReferences($rendering_references); - $toc_view->setUnitTestData(idx($props, 'arc:unit', array())); + $toc_view->setCoverageMap($target->loadCoverageMap($user)); if ($repository) { $toc_view->setRepository($repository); } diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index e30a401306..e48ed5630d 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -348,6 +348,31 @@ final class DifferentialDiff return $target_phids; } + public function loadCoverageMap(PhabricatorUser $viewer) { + $target_phids = $this->getBuildTargetPHIDs(); + if (!$target_phids) { + return array(); + } + + $unit = id(new HarbormasterBuildUnitMessage())->loadAllWhere( + 'buildTargetPHID IN (%Ls)', + $target_phids); + + $map = array(); + foreach ($unit as $message) { + $coverage = $message->getProperty('coverage', array()); + foreach ($coverage as $path => $coverage_data) { + $map[$path][] = $coverage_data; + } + } + + foreach ($map as $path => $coverage_items) { + $map[$path] = ArcanistUnitTestResult::mergeCoverage($coverage_items); + } + + return $map; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/differential/view/DifferentialDiffTableOfContentsView.php b/src/applications/differential/view/DifferentialDiffTableOfContentsView.php index f20c7ce19b..26161096fe 100644 --- a/src/applications/differential/view/DifferentialDiffTableOfContentsView.php +++ b/src/applications/differential/view/DifferentialDiffTableOfContentsView.php @@ -10,7 +10,7 @@ final class DifferentialDiffTableOfContentsView extends AphrontView { private $renderURI = '/differential/changeset/'; private $revisionID; private $whitespace; - private $unitTestData; + private $coverageMap; public function setChangesets($changesets) { $this->changesets = $changesets; @@ -37,8 +37,8 @@ final class DifferentialDiffTableOfContentsView extends AphrontView { return $this; } - public function setUnitTestData($unit_test_data) { - $this->unitTestData = $unit_test_data; + public function setCoverageMap(array $coverage_map) { + $this->coverageMap = $coverage_map; return $this; } @@ -60,23 +60,6 @@ final class DifferentialDiffTableOfContentsView extends AphrontView { $rows = array(); - $coverage = array(); - if ($this->unitTestData) { - $coverage_by_file = array(); - foreach ($this->unitTestData as $result) { - $test_coverage = idx($result, 'coverage'); - if (!$test_coverage) { - continue; - } - foreach ($test_coverage as $file => $results) { - $coverage_by_file[$file][] = $results; - } - } - foreach ($coverage_by_file as $file => $coverages) { - $coverage[$file] = ArcanistUnitTestResult::mergeCoverage($coverages); - } - } - $changesets = $this->changesets; $paths = array(); foreach ($changesets as $id => $changeset) { @@ -144,7 +127,7 @@ final class DifferentialDiffTableOfContentsView extends AphrontView { 'M'); $fname = $changeset->getFilename(); - $cov = $this->renderCoverage($coverage, $fname); + $cov = $this->renderCoverage($this->coverageMap, $fname); if ($cov === null) { $mcov = $cov = phutil_tag('em', array(), '-'); } else {