1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-23 07:12:41 +01:00

Show coverage percentages in table of contents

Summary:
Rough cut -- this needs style / color / tooltips, etc. Show raw coverage and "modified coverage" (coverage on lines you touched) in the table of contents.

https://secure.phabricator.com/file/data/id3apce5p5gevkee6tg2/PHID-FILE-kxcxlbsej454t4xiht2o/Screen_Shot_2012-03-12_at_3.30.30_PM.png

Test Plan: See screenshot above.

Reviewers: tuomaspelkonen, btrahan, zeeg

Reviewed By: tuomaspelkonen

CC: aran, epriestley

Maniphest Tasks: T965

Differential Revision: https://secure.phabricator.com/D1864
This commit is contained in:
epriestley 2012-03-12 17:06:55 -07:00
parent 09c1bd34f1
commit 85bdcbdd43
8 changed files with 217 additions and 56 deletions

View file

@ -250,7 +250,7 @@ celerity_register_resource_map(array(
), ),
'differential-table-of-contents-css' => 'differential-table-of-contents-css' =>
array( array(
'uri' => '/res/e4c089fe/rsrc/css/application/differential/table-of-contents.css', 'uri' => '/res/a633259f/rsrc/css/application/differential/table-of-contents.css',
'type' => 'css', 'type' => 'css',
'requires' => 'requires' =>
array( array(
@ -576,7 +576,7 @@ celerity_register_resource_map(array(
), ),
'javelin-behavior-differential-populate' => 'javelin-behavior-differential-populate' =>
array( array(
'uri' => '/res/6efe5cd2/rsrc/js/application/differential/behavior-populate.js', 'uri' => '/res/3c430bff/rsrc/js/application/differential/behavior-populate.js',
'type' => 'js', 'type' => 'js',
'requires' => 'requires' =>
array( array(
@ -1917,27 +1917,6 @@ celerity_register_resource_map(array(
), array( ), array(
'packages' => 'packages' =>
array( array(
'09c86840' =>
array(
'name' => 'differential.pkg.css',
'symbols' =>
array(
0 => 'differential-core-view-css',
1 => 'differential-changeset-view-css',
2 => 'differential-revision-detail-css',
3 => 'differential-revision-history-css',
4 => 'differential-table-of-contents-css',
5 => 'differential-revision-comment-css',
6 => 'differential-revision-add-comment-css',
7 => 'differential-revision-comment-list-css',
8 => 'phabricator-object-selector-css',
9 => 'aphront-headsup-action-list-view-css',
10 => 'phabricator-content-source-view-css',
11 => 'differential-local-commits-view-css',
),
'uri' => '/res/pkg/09c86840/differential.pkg.css',
'type' => 'css',
),
'2af849fb' => '2af849fb' =>
array( array(
'name' => 'typeahead.pkg.js', 'name' => 'typeahead.pkg.js',
@ -2024,7 +2003,28 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/86fc0b0c/maniphest.pkg.js', 'uri' => '/res/pkg/86fc0b0c/maniphest.pkg.js',
'type' => 'js', 'type' => 'js',
), ),
'e8b28c4a' => '9d02b654' =>
array(
'name' => 'differential.pkg.css',
'symbols' =>
array(
0 => 'differential-core-view-css',
1 => 'differential-changeset-view-css',
2 => 'differential-revision-detail-css',
3 => 'differential-revision-history-css',
4 => 'differential-table-of-contents-css',
5 => 'differential-revision-comment-css',
6 => 'differential-revision-add-comment-css',
7 => 'differential-revision-comment-list-css',
8 => 'phabricator-object-selector-css',
9 => 'aphront-headsup-action-list-view-css',
10 => 'phabricator-content-source-view-css',
11 => 'differential-local-commits-view-css',
),
'uri' => '/res/pkg/9d02b654/differential.pkg.css',
'type' => 'css',
),
'ffca9dae' =>
array( array(
'name' => 'differential.pkg.js', 'name' => 'differential.pkg.js',
'symbols' => 'symbols' =>
@ -2047,7 +2047,7 @@ celerity_register_resource_map(array(
15 => 'javelin-behavior-differential-dropdown-menus', 15 => 'javelin-behavior-differential-dropdown-menus',
16 => 'javelin-behavior-buoyant', 16 => 'javelin-behavior-buoyant',
), ),
'uri' => '/res/pkg/e8b28c4a/differential.pkg.js', 'uri' => '/res/pkg/ffca9dae/differential.pkg.js',
'type' => 'js', 'type' => 'js',
), ),
31583232 => 31583232 =>
@ -2092,7 +2092,7 @@ celerity_register_resource_map(array(
'aphront-crumbs-view-css' => '78e8854e', 'aphront-crumbs-view-css' => '78e8854e',
'aphront-dialog-view-css' => '78e8854e', 'aphront-dialog-view-css' => '78e8854e',
'aphront-form-view-css' => '78e8854e', 'aphront-form-view-css' => '78e8854e',
'aphront-headsup-action-list-view-css' => '09c86840', 'aphront-headsup-action-list-view-css' => '9d02b654',
'aphront-list-filter-view-css' => '78e8854e', 'aphront-list-filter-view-css' => '78e8854e',
'aphront-pager-view-css' => '78e8854e', 'aphront-pager-view-css' => '78e8854e',
'aphront-panel-view-css' => '78e8854e', 'aphront-panel-view-css' => '78e8854e',
@ -2100,40 +2100,40 @@ celerity_register_resource_map(array(
'aphront-table-view-css' => '78e8854e', 'aphront-table-view-css' => '78e8854e',
'aphront-tokenizer-control-css' => '78e8854e', 'aphront-tokenizer-control-css' => '78e8854e',
'aphront-typeahead-control-css' => '78e8854e', 'aphront-typeahead-control-css' => '78e8854e',
'differential-changeset-view-css' => '09c86840', 'differential-changeset-view-css' => '9d02b654',
'differential-core-view-css' => '09c86840', 'differential-core-view-css' => '9d02b654',
'differential-inline-comment-editor' => 'e8b28c4a', 'differential-inline-comment-editor' => 'ffca9dae',
'differential-local-commits-view-css' => '09c86840', 'differential-local-commits-view-css' => '9d02b654',
'differential-revision-add-comment-css' => '09c86840', 'differential-revision-add-comment-css' => '9d02b654',
'differential-revision-comment-css' => '09c86840', 'differential-revision-comment-css' => '9d02b654',
'differential-revision-comment-list-css' => '09c86840', 'differential-revision-comment-list-css' => '9d02b654',
'differential-revision-detail-css' => '09c86840', 'differential-revision-detail-css' => '9d02b654',
'differential-revision-history-css' => '09c86840', 'differential-revision-history-css' => '9d02b654',
'differential-table-of-contents-css' => '09c86840', 'differential-table-of-contents-css' => '9d02b654',
'diffusion-commit-view-css' => '61f9d480', 'diffusion-commit-view-css' => '61f9d480',
'javelin-behavior' => '4fbae2af', 'javelin-behavior' => '4fbae2af',
'javelin-behavior-aphront-basic-tokenizer' => '2af849fb', 'javelin-behavior-aphront-basic-tokenizer' => '2af849fb',
'javelin-behavior-aphront-drag-and-drop' => 'e8b28c4a', 'javelin-behavior-aphront-drag-and-drop' => 'ffca9dae',
'javelin-behavior-aphront-drag-and-drop-textarea' => 'e8b28c4a', 'javelin-behavior-aphront-drag-and-drop-textarea' => 'ffca9dae',
'javelin-behavior-aphront-form-disable-on-submit' => '95944588', 'javelin-behavior-aphront-form-disable-on-submit' => '95944588',
'javelin-behavior-buoyant' => 'e8b28c4a', 'javelin-behavior-buoyant' => 'ffca9dae',
'javelin-behavior-differential-accept-with-errors' => 'e8b28c4a', 'javelin-behavior-differential-accept-with-errors' => 'ffca9dae',
'javelin-behavior-differential-add-reviewers-and-ccs' => 'e8b28c4a', 'javelin-behavior-differential-add-reviewers-and-ccs' => 'ffca9dae',
'javelin-behavior-differential-comment-jump' => 'e8b28c4a', 'javelin-behavior-differential-comment-jump' => 'ffca9dae',
'javelin-behavior-differential-diff-radios' => 'e8b28c4a', 'javelin-behavior-differential-diff-radios' => 'ffca9dae',
'javelin-behavior-differential-dropdown-menus' => 'e8b28c4a', 'javelin-behavior-differential-dropdown-menus' => 'ffca9dae',
'javelin-behavior-differential-edit-inline-comments' => 'e8b28c4a', 'javelin-behavior-differential-edit-inline-comments' => 'ffca9dae',
'javelin-behavior-differential-feedback-preview' => 'e8b28c4a', 'javelin-behavior-differential-feedback-preview' => 'ffca9dae',
'javelin-behavior-differential-keyboard-navigation' => 'e8b28c4a', 'javelin-behavior-differential-keyboard-navigation' => 'ffca9dae',
'javelin-behavior-differential-populate' => 'e8b28c4a', 'javelin-behavior-differential-populate' => 'ffca9dae',
'javelin-behavior-differential-show-more' => 'e8b28c4a', 'javelin-behavior-differential-show-more' => 'ffca9dae',
'javelin-behavior-maniphest-batch-selector' => '86fc0b0c', 'javelin-behavior-maniphest-batch-selector' => '86fc0b0c',
'javelin-behavior-maniphest-transaction-controls' => '86fc0b0c', 'javelin-behavior-maniphest-transaction-controls' => '86fc0b0c',
'javelin-behavior-maniphest-transaction-expand' => '86fc0b0c', 'javelin-behavior-maniphest-transaction-expand' => '86fc0b0c',
'javelin-behavior-maniphest-transaction-preview' => '86fc0b0c', 'javelin-behavior-maniphest-transaction-preview' => '86fc0b0c',
'javelin-behavior-phabricator-autofocus' => '95944588', 'javelin-behavior-phabricator-autofocus' => '95944588',
'javelin-behavior-phabricator-keyboard-shortcuts' => '95944588', 'javelin-behavior-phabricator-keyboard-shortcuts' => '95944588',
'javelin-behavior-phabricator-object-selector' => 'e8b28c4a', 'javelin-behavior-phabricator-object-selector' => 'ffca9dae',
'javelin-behavior-phabricator-watch-anchor' => '95944588', 'javelin-behavior-phabricator-watch-anchor' => '95944588',
'javelin-behavior-refresh-csrf' => '95944588', 'javelin-behavior-refresh-csrf' => '95944588',
'javelin-behavior-workflow' => '95944588', 'javelin-behavior-workflow' => '95944588',
@ -2158,20 +2158,20 @@ celerity_register_resource_map(array(
'maniphest-task-summary-css' => '31583232', 'maniphest-task-summary-css' => '31583232',
'maniphest-transaction-detail-css' => '31583232', 'maniphest-transaction-detail-css' => '31583232',
'phabricator-app-buttons-css' => '78e8854e', 'phabricator-app-buttons-css' => '78e8854e',
'phabricator-content-source-view-css' => '09c86840', 'phabricator-content-source-view-css' => '9d02b654',
'phabricator-core-buttons-css' => '78e8854e', 'phabricator-core-buttons-css' => '78e8854e',
'phabricator-core-css' => '78e8854e', 'phabricator-core-css' => '78e8854e',
'phabricator-directory-css' => '78e8854e', 'phabricator-directory-css' => '78e8854e',
'phabricator-drag-and-drop-file-upload' => 'e8b28c4a', 'phabricator-drag-and-drop-file-upload' => 'ffca9dae',
'phabricator-dropdown-menu' => '95944588', 'phabricator-dropdown-menu' => '95944588',
'phabricator-jump-nav' => '78e8854e', 'phabricator-jump-nav' => '78e8854e',
'phabricator-keyboard-shortcut' => '95944588', 'phabricator-keyboard-shortcut' => '95944588',
'phabricator-keyboard-shortcut-manager' => '95944588', 'phabricator-keyboard-shortcut-manager' => '95944588',
'phabricator-menu-item' => '95944588', 'phabricator-menu-item' => '95944588',
'phabricator-object-selector-css' => '09c86840', 'phabricator-object-selector-css' => '9d02b654',
'phabricator-paste-file-upload' => '95944588', 'phabricator-paste-file-upload' => '95944588',
'phabricator-remarkup-css' => '78e8854e', 'phabricator-remarkup-css' => '78e8854e',
'phabricator-shaped-request' => 'e8b28c4a', 'phabricator-shaped-request' => 'ffca9dae',
'phabricator-standard-page-view' => '78e8854e', 'phabricator-standard-page-view' => '78e8854e',
'phabricator-transaction-view-css' => '78e8854e', 'phabricator-transaction-view-css' => '78e8854e',
'syntax-highlighting-css' => '78e8854e', 'syntax-highlighting-css' => '78e8854e',

View file

@ -208,9 +208,17 @@ final class DifferentialChangesetViewController extends DifferentialController {
$output = $parser->render($range_s, $range_e, $mask); $output = $parser->render($range_s, $range_e, $mask);
$mcov = $parser->renderModifiedCoverage();
if ($request->isAjax()) { if ($request->isAjax()) {
$content = array(
'coverage' => array(
'differential-mcoverage-'.md5($changeset->getFilename()) => $mcov,
),
'changeset' => $output,
);
return id(new AphrontAjaxResponse()) return id(new AphrontAjaxResponse())
->setContent($output); ->setContent($content);
} }
Javelin::initBehavior('differential-show-more', array( Javelin::initBehavior('differential-show-more', array(

View file

@ -68,6 +68,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
$target, $target,
array( array(
'local:commits', 'local:commits',
'arc:unit',
)); ));
list($changesets, $vs_map, $rendering_references) = list($changesets, $vs_map, $rendering_references) =
@ -262,6 +263,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
$toc_view = new DifferentialDiffTableOfContentsView(); $toc_view = new DifferentialDiffTableOfContentsView();
$toc_view->setChangesets($changesets); $toc_view->setChangesets($changesets);
$toc_view->setUnitTestData(idx($props, 'arc:unit', array()));
if ($repository) { if ($repository) {
$toc_view->setRepository($repository); $toc_view->setRepository($repository);
} }

View file

@ -1714,4 +1714,49 @@ class DifferentialChangesetParser {
return array($range_s, $range_e, $mask); return array($range_s, $range_e, $mask);
} }
/**
* Render "modified coverage" information; test coverage on modified lines.
* This synthesizes diff information with unit test information into a useful
* indicator of how well tested a change is.
*/
public function renderModifiedCoverage() {
$na = '<em>-</em>';
if (!$this->coverage) {
return $na;
}
$covered = 0;
$not_covered = 0;
foreach ($this->new as $k => $new) {
if (!$new['line']) {
continue;
}
if (!$new['type']) {
continue;
}
if (empty($this->coverage[$new['line'] - 1])) {
continue;
}
switch ($this->coverage[$new['line']]) {
case 'C':
$covered++;
break;
case 'U':
$not_covered++;
break;
}
}
if (!$covered && !$not_covered) {
return $na;
}
return sprintf('%d%%', 100 * ($covered / ($covered + $not_covered)));
}
} }

View file

@ -26,6 +26,7 @@ final class DifferentialDiffTableOfContentsView extends AphrontView {
private $renderURI = '/differential/changeset/'; private $renderURI = '/differential/changeset/';
private $revisionID; private $revisionID;
private $whitespace; private $whitespace;
private $unitTestData;
public function setChangesets($changesets) { public function setChangesets($changesets) {
$this->changesets = $changesets; $this->changesets = $changesets;
@ -42,6 +43,11 @@ final class DifferentialDiffTableOfContentsView extends AphrontView {
return $this; return $this;
} }
public function setUnitTestData($unit_test_data) {
$this->unitTestData = $unit_test_data;
return $this;
}
public function setUser(PhabricatorUser $user) { public function setUser(PhabricatorUser $user) {
$this->user = $user; $this->user = $user;
return $this; return $this;
@ -74,6 +80,23 @@ final class DifferentialDiffTableOfContentsView extends AphrontView {
$rows = array(); $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; $changesets = $this->changesets;
$paths = array(); $paths = array();
foreach ($changesets as $changeset) { foreach ($changesets as $changeset) {
@ -129,6 +152,20 @@ final class DifferentialDiffTableOfContentsView extends AphrontView {
? null ? null
: '<span title="Properties Changed">M</span>'; : '<span title="Properties Changed">M</span>';
$fname = $changeset->getFilename();
$cov = $this->renderCoverage($coverage, $fname);
if ($cov === null) {
$mcov = $cov = '<em>-</em>';
} else {
$mcov = phutil_render_tag(
'div',
array(
'id' => 'differential-mcoverage-'.md5($fname),
'class' => 'differential-mcoverage-loading',
),
'Loading...');
}
$rows[] = $rows[] =
'<tr>'. '<tr>'.
'<td class="differential-toc-char" title='.$chartitle.'>'.$char. '<td class="differential-toc-char" title='.$chartitle.'>'.$char.
@ -136,6 +173,8 @@ final class DifferentialDiffTableOfContentsView extends AphrontView {
'<td class="differential-toc-prop">'.$pchar.'</td>'. '<td class="differential-toc-prop">'.$pchar.'</td>'.
'<td class="differential-toc-ftype">'.$desc.'</td>'. '<td class="differential-toc-ftype">'.$desc.'</td>'.
'<td class="differential-toc-file">'.$link.$lines.'</td>'. '<td class="differential-toc-file">'.$link.$lines.'</td>'.
'<td class="differential-toc-cov">'.$cov.'</td>'.
'<td class="differential-toc-mcov">'.$mcov.'</td>'.
'</tr>'; '</tr>';
if ($meta) { if ($meta) {
$rows[] = $rows[] =
@ -172,11 +211,36 @@ final class DifferentialDiffTableOfContentsView extends AphrontView {
$editor_link. $editor_link.
'<h1>Table of Contents</h1>'. '<h1>Table of Contents</h1>'.
'<table>'. '<table>'.
'<tr>'.
'<th></th>'.
'<th></th>'.
'<th></th>'.
'<th>Path</th>'.
'<th class="differential-toc-cov">Coverage (All)</th>'.
'<th class="differential-toc-mcov">Coverage (Touched)</th>'.
'</tr>'.
implode("\n", $rows). implode("\n", $rows).
'</table>'. '</table>'.
'</div>'; '</div>';
} }
private function renderCoverage(array $coverage, $file) {
$info = idx($coverage, $file);
if (!$info) {
return null;
}
$not_covered = substr_count($info, 'U');
$covered = substr_count($info, 'C');
if (!$not_covered && !$covered) {
return null;
}
return sprintf('%d%%', 100 * ($covered / ($covered + $not_covered)));
}
private function renderChangesetLink(DifferentialChangeset $changeset) { private function renderChangesetLink(DifferentialChangeset $changeset) {
$display_file = $changeset->getDisplayFilename(); $display_file = $changeset->getDisplayFilename();

View file

@ -6,6 +6,8 @@
phutil_require_module('arcanist', 'unit/result');
phutil_require_module('phabricator', 'applications/differential/constants/changetype'); phutil_require_module('phabricator', 'applications/differential/constants/changetype');
phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'infrastructure/celerity/api');
phutil_require_module('phabricator', 'view/base'); phutil_require_module('phabricator', 'view/base');

View file

@ -49,3 +49,34 @@
.diff-star-skip { .diff-star-skip {
color: #ff00aa; color: #ff00aa;
} }
.differential-toc table {
width: 100%;
}
.differential-toc table td.differential-toc-cov,
.differential-toc table td.differential-toc-mcov {
width: 120px;
text-align: right;
padding-right: 6px;
}
.differential-toc table th {
color: #666666;
font-size: 11px;
padding: 0 4px 4px;
white-space: nowrap;
}
.differential-toc table th.differential-toc-cov,
.differential-toc table th.differential-toc-mcov {
text-align: right;
}
.differential-toc table td em {
color: #666666;
}
.differential-mcoverage-loading {
color: #888888;
}

View file

@ -9,7 +9,16 @@
JX.behavior('differential-populate', function(config) { JX.behavior('differential-populate', function(config) {
function onresponse(target, response) { function onresponse(target, response) {
JX.DOM.replace(JX.$(target), JX.$H(response)); JX.DOM.replace(JX.$(target), JX.$H(response.changeset));
if (response.coverage) {
for (var k in response.coverage) {
try {
JX.DOM.replace(JX.$(k), JX.$H(response.coverage[k]));
} catch (ignored) {
// Not terribly important.
}
}
}
} }
for (var k in config.registry) { for (var k in config.registry) {