revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-19 21:32:43 +01:00
Code Issues Releases Wiki Activity

Remove some obsolete lint and unit support

Browse source 
Summary:
Ref T8096.

Long ago, support for "postponed" lint and unit tests got hacked in. `arc` would publish a bunch of ghost results, and then something else would fill the results in later.

This was always a hack. It is not nearly as powerful or flexible as having a real build system, and is obsolete with Harbormaster, which supports these operations in a more reasonable and straightforward way.

This was used (only? almost only?) at Facebook.

  - Remove `differential.finishpostponedlinters`. This only served to update postponed linters.
  - Remove lint magic in `differential.setdiffproperty`. This magic only made sense in the context of postponed linters.
  - Remove `differential.updateunitresults`. The only made sense for postponed unit tests.

And one minor change: when a diff contains >100 affected files, we hide the content by default, but show content for files with inline comments. Previously, we'd do this for lint inlines, too. I don't tink this is too useful, and it's much simpler to just remove it. We could add it back at some point, but I think large changes often trigger a lot of lint and no one actually cares. The behavior for actual human inlines is retained.

Test Plan: `grep`

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8096

Differential Revision: https://secure.phabricator.com/D13848
This commit is contained in:
epriestley 2015-08-10 14:16:25 -07:00
parent 2b13da88ce
commit 60a529b9d1
5 changed files with 0 additions and 346 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/__phutil_library_map__.php
View file

@ -378,7 +378,6 @@ phutil_register_library_map(array(
'DifferentialFieldParseException' => 'applications/differential/exception/DifferentialFieldParseException.php',
'DifferentialFieldValidationException' => 'applications/differential/exception/DifferentialFieldValidationException.php',
'DifferentialFindConduitAPIMethod' => 'applications/differential/conduit/DifferentialFindConduitAPIMethod.php',
'DifferentialFinishPostponedLintersConduitAPIMethod' => 'applications/differential/conduit/DifferentialFinishPostponedLintersConduitAPIMethod.php',
'DifferentialGetAllDiffsConduitAPIMethod' => 'applications/differential/conduit/DifferentialGetAllDiffsConduitAPIMethod.php',
'DifferentialGetCommitMessageConduitAPIMethod' => 'applications/differential/conduit/DifferentialGetCommitMessageConduitAPIMethod.php',
'DifferentialGetCommitPathsConduitAPIMethod' => 'applications/differential/conduit/DifferentialGetCommitPathsConduitAPIMethod.php',
@ -495,7 +494,6 @@ phutil_register_library_map(array(
'DifferentialUnitStatus' => 'applications/differential/constants/DifferentialUnitStatus.php',
'DifferentialUnitTestResult' => 'applications/differential/constants/DifferentialUnitTestResult.php',
'DifferentialUpdateRevisionConduitAPIMethod' => 'applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php',
'DifferentialUpdateUnitResultsConduitAPIMethod' => 'applications/differential/conduit/DifferentialUpdateUnitResultsConduitAPIMethod.php',
'DifferentialViewPolicyField' => 'applications/differential/customfield/DifferentialViewPolicyField.php',
'DiffusionAuditorDatasource' => 'applications/diffusion/typeahead/DiffusionAuditorDatasource.php',
'DiffusionAuditorsAddAuditorsHeraldAction' => 'applications/diffusion/herald/DiffusionAuditorsAddAuditorsHeraldAction.php',
@ -3999,7 +3997,6 @@ phutil_register_library_map(array(
'DifferentialFieldParseException' => 'Exception',
'DifferentialFieldValidationException' => 'Exception',
'DifferentialFindConduitAPIMethod' => 'DifferentialConduitAPIMethod',
'DifferentialFinishPostponedLintersConduitAPIMethod' => 'DifferentialConduitAPIMethod',
'DifferentialGetAllDiffsConduitAPIMethod' => 'DifferentialConduitAPIMethod',
'DifferentialGetCommitMessageConduitAPIMethod' => 'DifferentialConduitAPIMethod',
'DifferentialGetCommitPathsConduitAPIMethod' => 'DifferentialConduitAPIMethod',
@ -4136,7 +4133,6 @@ phutil_register_library_map(array(
'DifferentialUnitStatus' => 'Phobject',
'DifferentialUnitTestResult' => 'Phobject',
'DifferentialUpdateRevisionConduitAPIMethod' => 'DifferentialConduitAPIMethod',
'DifferentialUpdateUnitResultsConduitAPIMethod' => 'DifferentialConduitAPIMethod',
'DifferentialViewPolicyField' => 'DifferentialCoreCustomField',
'DiffusionAuditorDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
'DiffusionAuditorsAddAuditorsHeraldAction' => 'DiffusionAuditorsHeraldAction',

118
src/applications/differential/conduit/DifferentialFinishPostponedLintersConduitAPIMethod.php
View file

@ -1,118 +0,0 @@
<?php
final class DifferentialFinishPostponedLintersConduitAPIMethod
extends DifferentialConduitAPIMethod {
public function getAPIMethodName() {
return 'differential.finishpostponedlinters';
}
public function getMethodDescription() {
return pht(
'Update diff with new lint messages and mark postponed '.
'linters as finished.');
}
protected function defineParamTypes() {
return array(
'diffID' => 'required diffID',
'linters' => 'required dict',
);
}
protected function defineReturnType() {
return 'void';
}
protected function defineErrorTypes() {
return array(
'ERR-BAD-DIFF' => pht('Bad diff ID.'),
'ERR-BAD-LINTER' => pht('No postponed linter by the given name.'),
'ERR-NO-LINT' => pht('No postponed lint field available in diff.'),
);
}
protected function execute(ConduitAPIRequest $request) {
$diff_id = $request->getValue('diffID');
$linter_map = $request->getValue('linters');
$diff = id(new DifferentialDiffQuery())
->setViewer($request->getUser())
->withIDs(array($diff_id))
->executeOne();
if (!$diff) {
throw new ConduitException('ERR-BAD-DIFF');
}
// Extract the finished linters and messages from the linter map.
$finished_linters = array_keys($linter_map);
$new_messages = array();
foreach ($linter_map as $linter => $messages) {
$new_messages = array_merge($new_messages, $messages);
}
// Load the postponed linters attached to this diff.
$postponed_linters_property = id(
new DifferentialDiffProperty())->loadOneWhere(
'diffID = %d AND name = %s',
$diff_id,
'arc:lint-postponed');
if ($postponed_linters_property) {
$postponed_linters = $postponed_linters_property->getData();
} else {
$postponed_linters = array();
}
foreach ($finished_linters as $linter) {
if (!in_array($linter, $postponed_linters)) {
throw new ConduitException('ERR-BAD-LINTER');
}
}
foreach ($postponed_linters as $idx => $linter) {
if (in_array($linter, $finished_linters)) {
unset($postponed_linters[$idx]);
}
}
// Load the lint messages currenty attached to the diff. If this
// diff property doesn't exist, create it.
$messages_property = id(new DifferentialDiffProperty())->loadOneWhere(
'diffID = %d AND name = %s',
$diff_id,
'arc:lint');
if ($messages_property) {
$messages = $messages_property->getData();
} else {
$messages = array();
}
// Add new lint messages, removing duplicates.
foreach ($new_messages as $new_message) {
if (!in_array($new_message, $messages)) {
$messages[] = $new_message;
}
}
// Use setdiffproperty to update the postponed linters and messages,
// as these will also update the lint status correctly.
$call = new ConduitCall(
'differential.setdiffproperty',
array(
'diff_id' => $diff_id,
'name' => 'arc:lint',
'data' => json_encode($messages),
));
$call->setUser($request->getUser());
$call->execute();
$call = new ConduitCall(
'differential.setdiffproperty',
array(
'diff_id' => $diff_id,
'name' => 'arc:lint-postponed',
'data' => json_encode($postponed_linters),
));
$call->setUser($request->getUser());
$call->execute();
}
}

60
src/applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php
View file

@ -29,72 +29,12 @@ final class DifferentialSetDiffPropertyConduitAPIMethod
);
}
private static function updateLintStatus($diff_id) {
$diff = id(new DifferentialDiff())->load($diff_id);
if (!$diff) {
throw new ConduitException('ERR_NOT_FOUND');
}
// Load the postponed linters attached to this diff.
$postponed_linters_property = id(
new DifferentialDiffProperty())->loadOneWhere(
'diffID = %d AND name = %s',
$diff_id,
'arc:lint-postponed');
if ($postponed_linters_property) {
$postponed_linters = $postponed_linters_property->getData();
} else {
$postponed_linters = array();
}
// Load the lint messages currenty attached to the diff
$messages_property = id(new DifferentialDiffProperty())->loadOneWhere(
'diffID = %d AND name = %s',
$diff_id,
'arc:lint');
if ($messages_property) {
$results = $messages_property->getData();
} else {
$results = array();
}
$has_error = false;
$has_warning = false;
foreach ($results as $result) {
if ($result['severity'] === ArcanistLintSeverity::SEVERITY_ERROR) {
$has_error = true;
break;
} else if ($result['severity'] ===
ArcanistLintSeverity::SEVERITY_WARNING) {
$has_warning = true;
}
}
if ($has_error) {
$diff->setLintStatus(DifferentialLintStatus::LINT_FAIL);
} else if ($has_warning) {
$diff->setLintStatus(DifferentialLintStatus::LINT_WARN);
} else if (!empty($postponed_linters)) {
$diff->setLintStatus(DifferentialLintStatus::LINT_POSTPONED);
} else if ($diff->getLintStatus() != DifferentialLintStatus::LINT_SKIP) {
$diff->setLintStatus(DifferentialLintStatus::LINT_OKAY);
}
$diff->save();
}
protected function execute(ConduitAPIRequest $request) {
$diff_id = $request->getValue('diff_id');
$name = $request->getValue('name');
$data = json_decode($request->getValue('data'), true);
self::updateDiffProperty($diff_id, $name, $data);
if ($name === 'arc:lint' || $name == 'arc:lint-postponed') {
self::updateLintStatus($diff_id);
}
return;
}
private static function updateDiffProperty($diff_id, $name, $data) {

154
src/applications/differential/conduit/DifferentialUpdateUnitResultsConduitAPIMethod.php
View file

@ -1,154 +0,0 @@
<?php
final class DifferentialUpdateUnitResultsConduitAPIMethod
extends DifferentialConduitAPIMethod {
public function getAPIMethodName() {
return 'differential.updateunitresults';
}
public function getMethodDescription() {
return pht('Update arc unit results for a postponed test.');
}
protected function defineParamTypes() {
return array(
'diff_id' => 'required diff_id',
'file' => 'required string',
'name' => 'required string',
'link' => 'optional string',
'result' => 'required string',
'message' => 'required string',
'coverage' => 'optional map<string, string>',
);
}
protected function defineReturnType() {
return 'void';
}
protected function defineErrorTypes() {
return array(
'ERR_BAD_DIFF' => pht('Bad diff ID.'),
'ERR_NO_RESULTS' => pht('Could not find the postponed test'),
);
}
protected function execute(ConduitAPIRequest $request) {
$diff_id = $request->getValue('diff_id');
if (!$diff_id) {
throw new ConduitException('ERR_BAD_DIFF');
}
$file = $request->getValue('file');
$name = $request->getValue('name');
$link = $request->getValue('link');
$message = $request->getValue('message');
$result = $request->getValue('result');
$coverage = $request->getValue('coverage', array());
$diff_property = id(new DifferentialDiffProperty())->loadOneWhere(
'diffID = %d AND name = %s',
$diff_id,
'arc:unit');
if (!$diff_property) {
throw new ConduitException('ERR_NO_RESULTS');
}
$diff = id(new DifferentialDiffQuery())
->setViewer($request->getUser())
->withIDs(array($diff_id))
->executeOne();
$unit_results = $diff_property->getData();
$postponed_count = 0;
$unit_status = null;
// If the test result already exists, then update it with
// the new info.
foreach ($unit_results as &$unit_result) {
if ($unit_result['name'] === $name ||
$unit_result['name'] === $file ||
$unit_result['name'] === $diff->getSourcePath().$file) {
$unit_result['name'] = $name;
$unit_result['link'] = $link;
$unit_result['file'] = $file;
$unit_result['result'] = $result;
$unit_result['userdata'] = $message;
$unit_result['coverage'] = $coverage;
$unit_status = $result;
break;
}
}
unset($unit_result);
// If the test result doesn't exist, just add it.
if (!$unit_status) {
$unit_result = array();
$unit_result['file'] = $file;
$unit_result['name'] = $name;
$unit_result['link'] = $link;
$unit_result['result'] = $result;
$unit_result['userdata'] = $message;
$unit_result['coverage'] = $coverage;
$unit_status = $result;
$unit_results[] = $unit_result;
}
unset($unit_result);
$diff_property->setData($unit_results);
$diff_property->save();
// Map external unit test status to internal overall diff status
$status_codes =
array(
DifferentialUnitTestResult::RESULT_PASS =>
DifferentialUnitStatus::UNIT_OKAY,
DifferentialUnitTestResult::RESULT_UNSOUND =>
DifferentialUnitStatus::UNIT_WARN,
DifferentialUnitTestResult::RESULT_FAIL =>
DifferentialUnitStatus::UNIT_FAIL,
DifferentialUnitTestResult::RESULT_BROKEN =>
DifferentialUnitStatus::UNIT_FAIL,
DifferentialUnitTestResult::RESULT_SKIP =>
DifferentialUnitStatus::UNIT_OKAY,
DifferentialUnitTestResult::RESULT_POSTPONED =>
DifferentialUnitStatus::UNIT_POSTPONED,
);
// These are the relative priorities for the unit test results
$status_codes_priority =
array(
DifferentialUnitStatus::UNIT_OKAY => 1,
DifferentialUnitStatus::UNIT_WARN => 2,
DifferentialUnitStatus::UNIT_POSTPONED => 3,
DifferentialUnitStatus::UNIT_FAIL => 4,
);
// Walk the now-current list of status codes to find the overall diff
// status
$final_diff_status = DifferentialUnitStatus::UNIT_NONE;
foreach ($unit_results as $unit_result) {
// Convert the text result into a diff unit status value
$status_code = idx($status_codes,
$unit_result['result'],
DifferentialUnitStatus::UNIT_NONE);
// Convert the unit status into a relative value
$diff_status_priority = idx($status_codes_priority, $status_code, 0);
// If the relative value of this result is "more bad" than previous
// results, use it as the new final diff status
if ($diff_status_priority > idx($status_codes_priority,
$final_diff_status, 0)) {
$final_diff_status = $status_code;
}
}
// Update our unit test result status with the final value
$diff->setUnitStatus($final_diff_status);
$diff->save();
}
}

10
src/applications/differential/controller/DifferentialRevisionViewController.php
View file

@ -191,16 +191,6 @@ final class DifferentialRevisionViewController extends DifferentialController {
$visible_changesets[$changeset_id] = $changesets[$changeset_id];
}
}
if (!empty($props['arc:lint'])) {
$changeset_paths = mpull($changesets, null, 'getFilename');
foreach ($props['arc:lint'] as $lint) {
$changeset = idx($changeset_paths, $lint['path']);
if ($changeset) {
$visible_changesets[$changeset->getID()] = $changeset;
}
}
}
} else {
$warning = null;
$visible_changesets = $changesets;