mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-18 21:02:41 +01:00
Provide a rough, unstable API for reporting coverage into Diffusion
Summary: Ref T4994. This stuff works: - You can dump a blob of coverage information into `diffusion.updatecoverage`. This wipes existing coverage information and replaces it. - It shows up when viewing files. - It shows up when viewing commits. This stuff does not work: - When viewing files, the Javascript hover interaction isn't tied in yet. - We always show this information, even if you're behind the commit where it was generated. - You can't do incremental updates. - There's no aggregation at the file (this file has 90% coverage), diff (the changes in this commit are 90% covered), or directory (the code in this directory has 90% coverage) levels yet. - This is probably not the final form of the UI, storage, or API, so you should expect occasional changes over time. I've marked the method as "Unstable" for now. Test Plan: - Ran `save_lint.php` to check for collateral damage; it worked fine. - Ran `save_lint.php` on a new branch to check creation. - Published some fake coverage information. - Viewed an affected commit. - Viewed an affected file. {F151915} {F151916} Reviewers: chad, btrahan Reviewed By: btrahan Subscribers: jhurwitz, epriestley, zeeg Maniphest Tasks: T5044, T4994 Differential Revision: https://secure.phabricator.com/D9022
This commit is contained in:
parent
baa6441668
commit
a74545c9da
10 changed files with 226 additions and 21 deletions
|
@ -10,7 +10,7 @@ return array(
|
|||
'core.pkg.css' => '59ea1706',
|
||||
'core.pkg.js' => 'b2ed04a2',
|
||||
'darkconsole.pkg.js' => 'ca8671ce',
|
||||
'differential.pkg.css' => '4b8686e3',
|
||||
'differential.pkg.css' => '382ca868',
|
||||
'differential.pkg.js' => '36bec171',
|
||||
'diffusion.pkg.css' => '3783278d',
|
||||
'diffusion.pkg.js' => '077e3ad0',
|
||||
|
@ -55,7 +55,7 @@ return array(
|
|||
'rsrc/css/application/dashboard/dashboard.css' => '2b41640b',
|
||||
'rsrc/css/application/diff/inline-comment-summary.css' => '8cfd34e8',
|
||||
'rsrc/css/application/differential/add-comment.css' => 'c478bcaa',
|
||||
'rsrc/css/application/differential/changeset-view.css' => '1570a1ff',
|
||||
'rsrc/css/application/differential/changeset-view.css' => 'c45747f0',
|
||||
'rsrc/css/application/differential/core.css' => '7ac3cabc',
|
||||
'rsrc/css/application/differential/results-table.css' => '239924f9',
|
||||
'rsrc/css/application/differential/revision-comment.css' => '48186045',
|
||||
|
@ -514,7 +514,7 @@ return array(
|
|||
'conpherence-notification-css' => '403cf598',
|
||||
'conpherence-update-css' => '1099a660',
|
||||
'conpherence-widget-pane-css' => 'bf275a6c',
|
||||
'differential-changeset-view-css' => '1570a1ff',
|
||||
'differential-changeset-view-css' => 'c45747f0',
|
||||
'differential-core-view-css' => '7ac3cabc',
|
||||
'differential-inline-comment-editor' => 'f2441746',
|
||||
'differential-results-table-css' => '239924f9',
|
||||
|
|
8
resources/sql/autopatches/20140509.coverage.1.sql
Normal file
8
resources/sql/autopatches/20140509.coverage.1.sql
Normal file
|
@ -0,0 +1,8 @@
|
|||
CREATE TABLE {$NAMESPACE}_repository.repository_coverage (
|
||||
id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
|
||||
branchID INT UNSIGNED NOT NULL,
|
||||
commitID INT UNSIGNED NOT NULL,
|
||||
pathID INT UNSIGNED NOT NULL,
|
||||
coverage LONGTEXT NOT NULL COLLATE latin1_bin,
|
||||
KEY `key_path` (branchID, pathID, commitID)
|
||||
) ENGINE=InnoDB, COLLATE utf8_general_ci;
|
|
@ -178,6 +178,7 @@ phutil_register_library_map(array(
|
|||
'ConduitAPI_diffusion_resolverefs_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_resolverefs_Method.php',
|
||||
'ConduitAPI_diffusion_searchquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_searchquery_Method.php',
|
||||
'ConduitAPI_diffusion_tagsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php',
|
||||
'ConduitAPI_diffusion_updatecoverage_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_updatecoverage_Method.php',
|
||||
'ConduitAPI_feed_Method' => 'applications/feed/conduit/ConduitAPI_feed_Method.php',
|
||||
'ConduitAPI_feed_publish_Method' => 'applications/feed/conduit/ConduitAPI_feed_publish_Method.php',
|
||||
'ConduitAPI_feed_query_Method' => 'applications/feed/conduit/ConduitAPI_feed_query_Method.php',
|
||||
|
@ -2838,6 +2839,7 @@ phutil_register_library_map(array(
|
|||
'ConduitAPI_diffusion_resolverefs_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
|
||||
'ConduitAPI_diffusion_searchquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
|
||||
'ConduitAPI_diffusion_tagsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
|
||||
'ConduitAPI_diffusion_updatecoverage_Method' => 'ConduitAPI_diffusion_Method',
|
||||
'ConduitAPI_feed_Method' => 'ConduitAPIMethod',
|
||||
'ConduitAPI_feed_publish_Method' => 'ConduitAPI_feed_Method',
|
||||
'ConduitAPI_feed_query_Method' => 'ConduitAPI_feed_Method',
|
||||
|
|
|
@ -66,20 +66,14 @@ final class DiffusionLintSaveRunner {
|
|||
}
|
||||
|
||||
$branch_name = $api->getBranchName();
|
||||
$this->branch = new PhabricatorRepositoryBranch();
|
||||
$this->conn = $this->branch->establishConnection('w');
|
||||
$this->branch = $this->branch->loadOneWhere(
|
||||
'repositoryID = %d AND name = %s',
|
||||
|
||||
$this->branch = PhabricatorRepositoryBranch::loadOrCreateBranch(
|
||||
$project->getRepositoryID(),
|
||||
$branch_name);
|
||||
$this->conn = $this->branch->establishConnection('w');
|
||||
|
||||
$this->lintCommit = null;
|
||||
if (!$this->branch) {
|
||||
$this->branch = id(new PhabricatorRepositoryBranch())
|
||||
->setRepositoryID($project->getRepositoryID())
|
||||
->setName($branch_name)
|
||||
->save();
|
||||
} else if (!$this->all) {
|
||||
if (!$this->all) {
|
||||
$this->lintCommit = $this->branch->getLintCommit();
|
||||
}
|
||||
|
||||
|
@ -95,6 +89,7 @@ final class DiffusionLintSaveRunner {
|
|||
}
|
||||
}
|
||||
|
||||
|
||||
if (!$this->lintCommit) {
|
||||
$where = ($this->svnRoot
|
||||
? qsprintf($this->conn, 'AND path LIKE %>', $this->svnRoot.'/')
|
||||
|
|
|
@ -0,0 +1,97 @@
|
|||
<?php
|
||||
|
||||
final class ConduitAPI_diffusion_updatecoverage_Method
|
||||
extends ConduitAPI_diffusion_Method {
|
||||
|
||||
public function getMethodStatus() {
|
||||
return self::METHOD_STATUS_UNSTABLE;
|
||||
}
|
||||
|
||||
public function getMethodDescription() {
|
||||
return pht('Publish coverage information for a repository.');
|
||||
}
|
||||
|
||||
public function defineReturnType() {
|
||||
return 'void';
|
||||
}
|
||||
|
||||
public function defineParamTypes() {
|
||||
return array(
|
||||
'repositoryPHID' => 'required phid',
|
||||
'branch' => 'required string',
|
||||
'commit' => 'required string',
|
||||
'coverage' => 'required map<string, string>',
|
||||
);
|
||||
}
|
||||
|
||||
public function defineErrorTypes() {
|
||||
return array();
|
||||
}
|
||||
|
||||
protected function execute(ConduitAPIRequest $request) {
|
||||
$viewer = $request->getUser();
|
||||
|
||||
$repository_phid = $request->getValue('repositoryPHID');
|
||||
$repository = id(new PhabricatorRepositoryQuery())
|
||||
->setViewer($viewer)
|
||||
->withPHIDs(array($repository_phid))
|
||||
->executeOne();
|
||||
|
||||
if (!$repository) {
|
||||
throw new Exception(
|
||||
pht('No repository exists with PHID "%s".', $repository_phid));
|
||||
}
|
||||
|
||||
$commit_name = $request->getValue('commit');
|
||||
$commit = id(new DiffusionCommitQuery())
|
||||
->setViewer($viewer)
|
||||
->withRepository($repository)
|
||||
->withIdentifiers(array($commit_name))
|
||||
->executeOne();
|
||||
if (!$commit) {
|
||||
throw new Exception(
|
||||
pht('No commit exists with identifier "%s".', $commit_name));
|
||||
}
|
||||
|
||||
$branch = PhabricatorRepositoryBranch::loadOrCreateBranch(
|
||||
$repository->getID(),
|
||||
$request->getValue('branch'));
|
||||
|
||||
$coverage = $request->getValue('coverage');
|
||||
$path_map = id(new DiffusionPathIDQuery(array_keys($coverage)))
|
||||
->loadPathIDs();
|
||||
|
||||
$conn = $repository->establishConnection('w');
|
||||
|
||||
$sql = array();
|
||||
foreach ($coverage as $path => $coverage_info) {
|
||||
$sql[] = qsprintf(
|
||||
$conn,
|
||||
'(%d, %d, %d, %s)',
|
||||
$branch->getID(),
|
||||
$path_map[$path],
|
||||
$commit->getID(),
|
||||
$coverage_info);
|
||||
}
|
||||
|
||||
$table_name = 'repository_coverage';
|
||||
|
||||
$conn->openTransaction();
|
||||
queryfx(
|
||||
$conn,
|
||||
'DELETE FROM %T WHERE branchID = %d',
|
||||
$table_name,
|
||||
$branch->getID());
|
||||
|
||||
foreach (PhabricatorLiskDAO::chunkSQL($sql) as $chunk) {
|
||||
queryfx(
|
||||
$conn,
|
||||
'INSERT INTO %T (branchID, pathID, commitID, coverage) VALUES %Q',
|
||||
$table_name,
|
||||
$chunk);
|
||||
}
|
||||
$conn->saveTransaction();
|
||||
}
|
||||
|
||||
|
||||
}
|
|
@ -4,6 +4,7 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
|
|||
|
||||
private $lintCommit;
|
||||
private $lintMessages;
|
||||
private $coverage;
|
||||
|
||||
public function processRequest() {
|
||||
$request = $this->getRequest();
|
||||
|
@ -68,6 +69,7 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
|
|||
}
|
||||
|
||||
$this->loadLintMessages();
|
||||
$this->coverage = $drequest->loadCoverage();
|
||||
|
||||
$binary_uri = null;
|
||||
if (ArcanistDiffUtils::isHeuristicBinaryFile($data)) {
|
||||
|
@ -625,7 +627,8 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
|
|||
|
||||
$rows = $this->renderInlines(
|
||||
idx($inlines, 0, array()),
|
||||
($show_blame),
|
||||
$show_blame,
|
||||
(bool)$this->coverage,
|
||||
$engine);
|
||||
|
||||
foreach ($display as $line) {
|
||||
|
@ -789,6 +792,24 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
|
|||
phutil_safe_html(str_replace("\t", ' ', $line['data'])),
|
||||
));
|
||||
|
||||
if ($this->coverage) {
|
||||
require_celerity_resource('differential-changeset-view-css');
|
||||
$cov_index = $line['line'] - 1;
|
||||
|
||||
if (isset($this->coverage[$cov_index])) {
|
||||
$cov_class = $this->coverage[$cov_index];
|
||||
} else {
|
||||
$cov_class = 'N';
|
||||
}
|
||||
|
||||
$blame[] = phutil_tag(
|
||||
'td',
|
||||
array(
|
||||
'class' => 'cov cov-'.$cov_class,
|
||||
),
|
||||
'');
|
||||
}
|
||||
|
||||
$rows[] = phutil_tag(
|
||||
'tr',
|
||||
array(
|
||||
|
@ -800,7 +821,8 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
|
|||
|
||||
$cur_inlines = $this->renderInlines(
|
||||
idx($inlines, $line['line'], array()),
|
||||
($show_blame),
|
||||
$show_blame,
|
||||
$this->coverage,
|
||||
$engine);
|
||||
foreach ($cur_inlines as $cur_inline) {
|
||||
$rows[] = $cur_inline;
|
||||
|
@ -810,17 +832,34 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
|
|||
return $rows;
|
||||
}
|
||||
|
||||
private function renderInlines(array $inlines, $needs_blame, $engine) {
|
||||
private function renderInlines(
|
||||
array $inlines,
|
||||
$needs_blame,
|
||||
$has_coverage,
|
||||
$engine) {
|
||||
|
||||
$rows = array();
|
||||
foreach ($inlines as $inline) {
|
||||
$inline_view = id(new DifferentialInlineCommentView())
|
||||
->setMarkupEngine($engine)
|
||||
->setInlineComment($inline)
|
||||
->render();
|
||||
$row = array_fill(0, ($needs_blame ? 5 : 1), phutil_tag('th'));
|
||||
|
||||
$row = array_fill(0, ($needs_blame ? 3 : 1), phutil_tag('th'));
|
||||
|
||||
$row[] = phutil_tag('td', array(), $inline_view);
|
||||
|
||||
if ($has_coverage) {
|
||||
$row[] = phutil_tag(
|
||||
'td',
|
||||
array(
|
||||
'class' => 'cov cov-I',
|
||||
));
|
||||
}
|
||||
|
||||
$rows[] = phutil_tag('tr', array('class' => 'inline'), $row);
|
||||
}
|
||||
|
||||
return $rows;
|
||||
}
|
||||
|
||||
|
|
|
@ -68,6 +68,11 @@ final class DiffusionDiffController extends DiffusionController {
|
|||
array(
|
||||
'action' => 'rendering-ref')));
|
||||
|
||||
$coverage = $drequest->loadCoverage();
|
||||
if ($coverage) {
|
||||
$parser->setCoverage($coverage);
|
||||
}
|
||||
|
||||
$pquery = new DiffusionPathIDQuery(array($changeset->getFilename()));
|
||||
$ids = $pquery->loadPathIDs();
|
||||
$path_id = $ids[$changeset->getFilename()];
|
||||
|
|
|
@ -26,6 +26,7 @@ abstract class DiffusionRequest {
|
|||
|
||||
private $initFromConduit = true;
|
||||
private $user;
|
||||
private $branchObject = false;
|
||||
|
||||
abstract protected function getSupportsBranches();
|
||||
abstract protected function isStableCommit($symbol);
|
||||
|
@ -338,12 +339,43 @@ abstract class DiffusionRequest {
|
|||
}
|
||||
|
||||
public function loadBranch() {
|
||||
return id(new PhabricatorRepositoryBranch())->loadOneWhere(
|
||||
'repositoryID = %d AND name = %s',
|
||||
$this->getRepository()->getID(),
|
||||
$this->getArcanistBranch());
|
||||
// TODO: Get rid of this and do real Queries on real objects.
|
||||
|
||||
if ($this->branchObject === false) {
|
||||
$this->branchObject = PhabricatorRepositoryBranch::loadBranch(
|
||||
$this->getRepository()->getID(),
|
||||
$this->getArcanistBranch());
|
||||
}
|
||||
|
||||
return $this->branchObject;
|
||||
}
|
||||
|
||||
public function loadCoverage() {
|
||||
// TODO: This should also die.
|
||||
$branch = $this->loadBranch();
|
||||
if (!$branch) {
|
||||
return;
|
||||
}
|
||||
|
||||
$path = $this->getPath();
|
||||
$path_map = id(new DiffusionPathIDQuery(array($path)))->loadPathIDs();
|
||||
|
||||
$coverage_row = queryfx_one(
|
||||
id(new PhabricatorRepository())->establishConnection('r'),
|
||||
'SELECT * FROM %T WHERE branchID = %d AND pathID = %d
|
||||
ORDER BY commitID DESC LIMIT 1',
|
||||
'repository_coverage',
|
||||
$branch->getID(),
|
||||
$path_map[$path]);
|
||||
|
||||
if (!$coverage_row) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return idx($coverage_row, 'coverage');
|
||||
}
|
||||
|
||||
|
||||
public function loadCommit() {
|
||||
if (empty($this->repositoryCommit)) {
|
||||
$repository = $this->getRepository();
|
||||
|
|
|
@ -6,4 +6,23 @@ final class PhabricatorRepositoryBranch extends PhabricatorRepositoryDAO {
|
|||
protected $name;
|
||||
protected $lintCommit;
|
||||
|
||||
public static function loadBranch($repository_id, $branch_name) {
|
||||
return id(new PhabricatorRepositoryBranch())->loadOneWhere(
|
||||
'repositoryID = %d AND name = %s',
|
||||
$repository_id,
|
||||
$branch_name);
|
||||
}
|
||||
|
||||
public static function loadOrCreateBranch($repository_id, $branch_name) {
|
||||
$branch = self::loadBranch($repository_id, $branch_name);
|
||||
if ($branch) {
|
||||
return $branch;
|
||||
}
|
||||
|
||||
return id(new PhabricatorRepositoryBranch())
|
||||
->setRepositoryID($repository_id)
|
||||
->setName($branch_name)
|
||||
->save();
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -136,6 +136,10 @@
|
|||
padding: 0;
|
||||
}
|
||||
|
||||
.diffusion-source td.cov {
|
||||
padding: 0 8px;
|
||||
}
|
||||
|
||||
td.cov-U {
|
||||
background: #dd8866;
|
||||
}
|
||||
|
@ -152,6 +156,10 @@ td.cov-X {
|
|||
background: #aa00aa;
|
||||
}
|
||||
|
||||
td.cov-I {
|
||||
background: {$lightgreybackground};
|
||||
}
|
||||
|
||||
.differential-diff td.source-cov-C,
|
||||
.differential-diff td.source-cov-C span.bright {
|
||||
background: #cceeff;
|
||||
|
|
Loading…
Reference in a new issue