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

Show coverage information in Differential

Summary:
Render coverage information in the right gutter, if available.

We could render some kind of summary report deal too but this seems like a good
start.

Test Plan:
  - Looked at diffs with coverage.
  - Looked at diffs without coverage.
  - Used inline comments, diff-of-diff, "show more", "show entire file", "show
generated file", "undo". Nothing seemed disrupted by the addition of a 5th
column.

Reviewers: btrahan, tuomaspelkonen, jungejason

Reviewed By: btrahan

CC: zeeg, aran, epriestley

Maniphest Tasks: T140

Differential Revision: https://secure.phabricator.com/D1527
This commit is contained in:
epriestley 2012-01-31 12:07:47 -08:00
parent 3d7a1d936c
commit 1e9492f8c2
6 changed files with 175 additions and 43 deletions

View file

@ -163,7 +163,7 @@ celerity_register_resource_map(array(
),
'differential-changeset-view-css' =>
array(
'uri' => '/res/bc78a228/rsrc/css/application/differential/changeset-view.css',
'uri' => '/res/2b0c9b6a/rsrc/css/application/differential/changeset-view.css',
'type' => 'css',
'requires' =>
array(
@ -213,7 +213,7 @@ celerity_register_resource_map(array(
),
'differential-revision-comment-css' =>
array(
'uri' => '/res/76b6d378/rsrc/css/application/differential/revision-comment.css',
'uri' => '/res/be9a95b4/rsrc/css/application/differential/revision-comment.css',
'type' => 'css',
'requires' =>
array(
@ -332,13 +332,14 @@ celerity_register_resource_map(array(
),
0 =>
array(
'uri' => '/res/14c48a9f/rsrc/js/javelin/lib/__tests__/behavior.js',
'uri' => '/res/b6096fdd/rsrc/js/javelin/lib/__tests__/URI.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-behavior',
0 => 'javelin-uri',
1 => 'javelin-php-serializer',
),
'disk' => '/rsrc/js/javelin/lib/__tests__/behavior.js',
'disk' => '/rsrc/js/javelin/lib/__tests__/URI.js',
),
'javelin-behavior-aphront-basic-tokenizer' =>
array(
@ -1762,6 +1763,27 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/540effd7/typeahead.pkg.js',
'type' => 'js',
),
'80580cea' =>
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/80580cea/differential.pkg.css',
'type' => 'css',
),
'a6562582' =>
array(
'name' => 'differential.pkg.js',
@ -1786,27 +1808,6 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/a6562582/differential.pkg.js',
'type' => 'js',
),
'ab397f85' =>
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/ab397f85/differential.pkg.css',
'type' => 'css',
),
'b164acea' =>
array(
'name' => 'javelin.pkg.js',
@ -1856,23 +1857,23 @@ celerity_register_resource_map(array(
'aphront-crumbs-view-css' => '16378540',
'aphront-dialog-view-css' => '16378540',
'aphront-form-view-css' => '16378540',
'aphront-headsup-action-list-view-css' => 'ab397f85',
'aphront-headsup-action-list-view-css' => '80580cea',
'aphront-list-filter-view-css' => '16378540',
'aphront-panel-view-css' => '16378540',
'aphront-side-nav-view-css' => '16378540',
'aphront-table-view-css' => '16378540',
'aphront-tokenizer-control-css' => '16378540',
'aphront-typeahead-control-css' => '16378540',
'differential-changeset-view-css' => 'ab397f85',
'differential-core-view-css' => 'ab397f85',
'differential-changeset-view-css' => '80580cea',
'differential-core-view-css' => '80580cea',
'differential-inline-comment-editor' => 'a6562582',
'differential-local-commits-view-css' => 'ab397f85',
'differential-revision-add-comment-css' => 'ab397f85',
'differential-revision-comment-css' => 'ab397f85',
'differential-revision-comment-list-css' => 'ab397f85',
'differential-revision-detail-css' => 'ab397f85',
'differential-revision-history-css' => 'ab397f85',
'differential-table-of-contents-css' => 'ab397f85',
'differential-local-commits-view-css' => '80580cea',
'differential-revision-add-comment-css' => '80580cea',
'differential-revision-comment-css' => '80580cea',
'differential-revision-comment-list-css' => '80580cea',
'differential-revision-detail-css' => '80580cea',
'differential-revision-history-css' => '80580cea',
'differential-table-of-contents-css' => '80580cea',
'diffusion-commit-view-css' => '03ef179e',
'javelin-behavior' => 'b164acea',
'javelin-behavior-aphront-basic-tokenizer' => '540effd7',
@ -1910,14 +1911,14 @@ celerity_register_resource_map(array(
'javelin-util' => 'b164acea',
'javelin-vector' => 'b164acea',
'javelin-workflow' => '46547a92',
'phabricator-content-source-view-css' => 'ab397f85',
'phabricator-content-source-view-css' => '80580cea',
'phabricator-core-buttons-css' => '16378540',
'phabricator-core-css' => '16378540',
'phabricator-directory-css' => '16378540',
'phabricator-drag-and-drop-file-upload' => 'a6562582',
'phabricator-keyboard-shortcut' => '46547a92',
'phabricator-keyboard-shortcut-manager' => '46547a92',
'phabricator-object-selector-css' => 'ab397f85',
'phabricator-object-selector-css' => '80580cea',
'phabricator-remarkup-css' => '16378540',
'phabricator-shaped-request' => 'a6562582',
'phabricator-standard-page-view' => '16378540',

View file

@ -125,11 +125,37 @@ class DifferentialChangesetViewController extends DifferentialController {
$changeset = $choice;
}
$coverage = null;
if ($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;
}
$coverage = ArcanistUnitTestResult::mergeCoverage($coverage);
}
}
$spec = $request->getStr('range');
list($range_s, $range_e, $mask) =
DifferentialChangesetParser::parseRangeSpecification($spec);
$parser = new DifferentialChangesetParser();
$parser->setCoverage($coverage);
$parser->setChangeset($changeset);
$parser->setRenderingReference($rendering_reference);
$parser->setRenderCacheKey($render_cache_key);

View file

@ -6,6 +6,8 @@
phutil_require_module('arcanist', 'unit/result');
phutil_require_module('phabricator', 'aphront/response/400');
phutil_require_module('phabricator', 'aphront/response/404');
phutil_require_module('phabricator', 'aphront/response/ajax');

View file

@ -53,6 +53,7 @@ class DifferentialChangesetParser {
private $lineWidth = 80;
private $isTopLevel;
private $coverage;
const CACHE_VERSION = 4;
@ -177,6 +178,11 @@ class DifferentialChangesetParser {
return $this;
}
public function setCoverage($coverage) {
$this->coverage = $coverage;
return $this;
}
public function parseHunk(DifferentialHunk $hunk) {
$lines = $hunk->getChanges();
@ -1120,7 +1126,7 @@ class DifferentialChangesetParser {
array(
'sigil' => 'context-target',
),
'<td class="differential-shield" colspan="4">'.
'<td class="differential-shield" colspan="5">'.
phutil_escape_html($message).
$more.
'</td>');
@ -1141,7 +1147,7 @@ class DifferentialChangesetParser {
array(
'sigil' => 'context-target',
),
'<td colspan="4" class="show-more">'.
'<td colspan="5" class="show-more">'.
'Context not available.'.
'</td>');
}
@ -1293,7 +1299,7 @@ class DifferentialChangesetParser {
array(
'sigil' => 'context-target',
),
'<td colspan="4" class="show-more">'.
'<td colspan="5" class="show-more">'.
implode(' &bull; ', $contents).
'</td>');
@ -1320,10 +1326,24 @@ class DifferentialChangesetParser {
$o_attr = null;
}
if (isset($this->new[$ii])) {
$n_num = $this->new[$ii]['line'];
$n_text = isset($this->newRender[$ii]) ? $this->newRender[$ii] : null;
$n_attr = null;
$cov_class = null;
if ($this->coverage !== null) {
if (empty($this->coverage[$n_num - 1])) {
$cov_class = 'N';
} else {
$cov_class = $this->coverage[$n_num - 1];
}
$cov_class = 'cov-'.$cov_class;
}
$n_cov = '<td class="cov '.$cov_class.'"></td>';
if ($this->new[$ii]['type']) {
if (empty($this->old[$ii])) {
$n_attr = ' class="new new-full"';
@ -1335,6 +1355,7 @@ class DifferentialChangesetParser {
$n_num = null;
$n_text = null;
$n_attr = null;
$n_cov = null;
}
@ -1363,6 +1384,7 @@ class DifferentialChangesetParser {
'<td'.$o_attr.'>'.$o_text.'</td>'.
'<th'.$n_id.'>'.$n_num.'</th>'.
'<td'.$n_attr.'>'.$n_text.'</td>'.
$n_cov.
'</tr>';
if ($context_not_available && ($ii == $rows - 1)) {
@ -1375,7 +1397,7 @@ class DifferentialChangesetParser {
$html[] =
'<tr class="inline"><th /><td>'.
$xhp.
'</td><th /><td /></tr>';
'</td><th /><td /><td /></tr>';
}
}
if ($n_num && isset($new_comments[$n_num])) {
@ -1384,7 +1406,7 @@ class DifferentialChangesetParser {
$html[] =
'<tr class="inline"><th /><td /><th /><td>'.
$xhp.
'</td></tr>';
'</td><td /></tr>';
}
}
}

View file

@ -0,0 +1,60 @@
@title Arcanist User Guide: Code Coverage
@group userguide
Explains code coverage features in Arcanist and Phabricator.
= Using Coverage Features =
If your project has unit tests with coverage integration (see below for
instructions on setting it up), you can use "arc" to show coverage reports.
For example:
arc unit --detailed-coverage src/some/file.php
Depending on how your test engine is configured, this will run tests relevant
to ##src/some/file.php## and give you a detailed coverage report.
If the test engine enables coverage by default, it will be uploaded to
Differential and displayed in the right gutter when viewing diffs.
= Enabling Coverage for libphutil, Arcanist and Phabricator =
If you're contributing, libphutil, Arcanist and Phabricator support coverage if
you install Xdebug:
http://xdebug.org/
It should be sufficient to correctly install Xdebug; coverage information will
be automatically enabled.
= Building Coverage Support =
To add coverage support to a unit test engine, just call ##setCoverage()## when
building @{class@arcanist:ArcanistUnitTestResult} objects. Provide a map of
file names (relative to the working copy root) to coverage report strings.
Coverage report strings look like this:
NNNNNCCCNNNNNNNNCCCCCCNNNUUUNNNNN
Each line in the file is represented by a character. Valid characters are:
- **N** Not executable. This is a comment or whitespace which should be
ignored when computing test coverage.
- **C** Covered. This line has test coverage.
- **U** Uncovered. This line is executable but has no test coverage.
- **X** Unreachable. If your coverage analysis can detect unreachable code,
you can report it here.
This format is intended to be as simple as possible. A valid coverage result
might look like this:
array(
'src/example.php' => 'NNCNNNCNUNNNUNUNUNUNUNC',
'src/other.php' => 'NNUNNNUNCNNNUNUNCNCNCNU',
);
You may also want to filter coverage information to the paths passed to the
unit test engine. See @{class@arcanist:ArcanistPhutilTestCase} and
@{class@arcanist:PhutilUnitTestEngine} for an example of coverage integration
in PHP using Xdebug.

View file

@ -75,6 +75,27 @@
background: #aaffaa;
}
.differential-diff td.cov {
width: 12px;
padding: 0;
}
td.cov-U {
background: #dd3300;
}
td.cov-C {
background: #0066ff;
}
td.cov-N {
background: #ddeeff;
}
td.cov-X {
background: #aa00aa;
}
.differential-diff td.show-more,
.differential-diff td.differential-shield {
background: #ffffee;