From 25216210743ada26ec55566bae86fb77881832b9 Mon Sep 17 00:00:00 2001 From: tuomaspelkonen Date: Wed, 8 Jun 2011 16:16:59 -0700 Subject: [PATCH] Added a conduit call to update arc unit results for a postponed test. Summary: It was not possible before to update arc unit results for a postponed test. This change makes it possible. Also the number of postponed tests are shown in differential. Let me know if this looks too Facebook specific. Test Plan: Tested the conduit call manually from Conduit Console and updated test results for a diff that had 20 postponed tests. Reviewed By: jungejason Reviewers: epriestley, jungejason Commenters: epriestley CC: slawekbiel, aran, tuomaspelkonen, jungejason, epriestley Differential Revision: 416 --- src/__phutil_library_map__.php | 3 + ...duitAPI_differential_creatediff_Method.php | 3 + ..._differential_updateunitresults_Method.php | 129 ++++++++++++++++++ .../updateunitresults/__init__.php | 19 +++ .../unitstatus/DifferentialUnitStatus.php | 1 + .../DifferentialUnitTestResult.php | 28 ++++ .../constants/unittestresult/__init__.php | 10 ++ .../DifferentialRevisionViewController.php | 46 ++++--- .../controller/revisionview/__init__.php | 2 + .../DifferentialRevisionUpdateHistoryView.php | 3 + 10 files changed, 227 insertions(+), 17 deletions(-) create mode 100644 src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php create mode 100644 src/applications/conduit/method/differential/updateunitresults/__init__.php create mode 100644 src/applications/differential/constants/unittestresult/DifferentialUnitTestResult.php create mode 100644 src/applications/differential/constants/unittestresult/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cff72691d4..195693c8b5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -97,6 +97,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_setdiffproperty_Method' => 'applications/conduit/method/differential/setdiffproperty', 'ConduitAPI_differential_updaterevision_Method' => 'applications/conduit/method/differential/updaterevision', 'ConduitAPI_differential_updatetaskrevisionassoc_Method' => 'applications/conduit/method/differential/updatetaskrevisionassoc', + 'ConduitAPI_differential_updateunitresults_Method' => 'applications/conduit/method/differential/updateunitresults', 'ConduitAPI_diffusion_getcommits_Method' => 'applications/conduit/method/diffusion/getcommits', 'ConduitAPI_diffusion_getrecentcommitsbypath_Method' => 'applications/conduit/method/diffusion/getrecentcommitsbypath', 'ConduitAPI_file_download_Method' => 'applications/conduit/method/file/download', @@ -170,6 +171,7 @@ phutil_register_library_map(array( 'DifferentialSubscribeController' => 'applications/differential/controller/subscribe', 'DifferentialTasksAttacher' => 'applications/differential/tasks', 'DifferentialUnitStatus' => 'applications/differential/constants/unitstatus', + 'DifferentialUnitTestResult' => 'applications/differential/constants/unittestresult', 'DifferentialViewTime' => 'applications/differential/storage/viewtime', 'DiffusionBranchInformation' => 'applications/diffusion/data/branch', 'DiffusionBranchQuery' => 'applications/diffusion/query/branch/base', @@ -610,6 +612,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_setdiffproperty_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_updaterevision_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_updatetaskrevisionassoc_Method' => 'ConduitAPIMethod', + 'ConduitAPI_differential_updateunitresults_Method' => 'ConduitAPIMethod', 'ConduitAPI_diffusion_getcommits_Method' => 'ConduitAPIMethod', 'ConduitAPI_diffusion_getrecentcommitsbypath_Method' => 'ConduitAPIMethod', 'ConduitAPI_file_download_Method' => 'ConduitAPIMethod', diff --git a/src/applications/conduit/method/differential/creatediff/ConduitAPI_differential_creatediff_Method.php b/src/applications/conduit/method/differential/creatediff/ConduitAPI_differential_creatediff_Method.php index 077f245233..324cb40d03 100644 --- a/src/applications/conduit/method/differential/creatediff/ConduitAPI_differential_creatediff_Method.php +++ b/src/applications/conduit/method/differential/creatediff/ConduitAPI_differential_creatediff_Method.php @@ -134,6 +134,9 @@ class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod { case 'fail': $diff->setUnitStatus(DifferentialUnitStatus::UNIT_FAIL); break; + case 'postponed': + $diff->setUnitStatus(DifferentialUnitStatus::UNIT_POSTPONED); + break; case 'none': default: $diff->setUnitStatus(DifferentialUnitStatus::UNIT_NONE); diff --git a/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php b/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php new file mode 100644 index 0000000000..ae4f5c343e --- /dev/null +++ b/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php @@ -0,0 +1,129 @@ + 'required diff_id', + 'file' => 'required string', + 'name' => 'required string', + 'result' => 'required string', + 'message' => 'required string', + ); + } + + public function defineReturnType() { + return 'void'; + } + + public function defineErrorTypes() { + return array( + 'ERR_BAD_DIFF' => 'Bad diff ID.', + 'ERR_NO_RESULTS' => 'Could not find the postponed test', + 'ERR_BAD_FILE' => 'No results for given file', + ); + } + + 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'); + $message = $request->getValue('message'); + $result = $request->getValue('result'); + + $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 DifferentialDiff())->load($diff_id); + $unit_results = $diff_property->getData(); + $postponed_count = 0; + $unit_status = null; + + foreach ($unit_results as &$unit_result) { + // Update the results for the test that has the same path. + if (($unit_result['name'] === $file || + $unit_result['name'] === $diff->getSourcePath().$file) && + $unit_result['result'] === + DifferentialUnitTestResult::RESULT_POSTPONED) { + $unit_result['name'] = $name; + $unit_result['result'] = $result; + $unit_result['userdata'] = $message; + $unit_status = $result; + break; + } + } + unset($unit_result); + + if (!$unit_status) { + throw new ConduitException('ERR_BAD_FILE'); + } + + $diff_property->setData($unit_results); + $diff_property->save(); + + foreach ($unit_results as $unit_result) { + if ($unit_result['result'] == + DifferentialUnitTestResult::RESULT_POSTPONED) { + $postponed_count++; + } + } + + $status_codes = + array( + DifferentialUnitTestResult::RESULT_PASS => + DifferentialUnitStatus::UNIT_OKAY, + DifferentialUnitTestResult::RESULT_UNSOUND => + DifferentialUnitStatus::UNIT_WARN, + DifferentialUnitTestResult::RESULT_FAIL => + DifferentialUnitStatus::UNIT_FAIL, + DifferentialUnitTestResult::RESULT_SKIP => + DifferentialUnitStatus::UNIT_SKIP, + DifferentialUnitTestResult::RESULT_POSTPONED => + DifferentialUnitStatus::UNIT_POSTPONED); + + if ($diff->getUnitStatus() == DifferentialUnitStatus::UNIT_POSTPONED) { + if ($postponed_count == 0 || + $unit_status != DifferentialUnitTestResult::RESULT_PASS) { + $diff->setUnitStatus( + idx($status_codes, $unit_status, DifferentialUnitStatus::UNIT_NONE)); + $diff->save(); + } + } + + return; + } + +} diff --git a/src/applications/conduit/method/differential/updateunitresults/__init__.php b/src/applications/conduit/method/differential/updateunitresults/__init__.php new file mode 100644 index 0000000000..d0e80055ee --- /dev/null +++ b/src/applications/conduit/method/differential/updateunitresults/__init__.php @@ -0,0 +1,19 @@ + 256) { - $userdata = substr($userdata, 0, 256).'...'; - } - $userdata = str_replace("\n", '
', $userdata); - $unit_messages[] = - ''. + + if ($result != DifferentialUnitTestResult::RESULT_POSTPONED && + $result != DifferentialUnitTestResult::RESULT_PASS) { + $userdata = phutil_escape_html(idx($test, 'userdata')); + if (strlen($userdata) > 256) { + $userdata = substr($userdata, 0, 256).'...'; + } + $userdata = str_replace("\n", '
', $userdata); + $unit_messages[] = + ''. ''.$name.''. ''. - '
'. - strtoupper($result). - '
'. + '
'. + strtoupper($result). + '
'. ''. ''.$userdata.''. - ''; - } + ''; - $utail = - '
'. - ''. + $utail = + '
'. + '
'. implode("\n", $unit_messages). - '
'. - '
'; + ''. + ''; + } else if ($result == DifferentialUnitTestResult::RESULT_POSTPONED) { + $postponed_count++; + } + } + } + + if ($postponed_count > 0 && + $diff->getUnitStatus() == DifferentialUnitStatus::UNIT_POSTPONED) { + $umsg = $postponed_count.' '.$umsg; } $properties['Unit'] = $ustar.' '.$umsg.$utail; diff --git a/src/applications/differential/controller/revisionview/__init__.php b/src/applications/differential/controller/revisionview/__init__.php index dac3a7fe73..cbc1f3627c 100644 --- a/src/applications/differential/controller/revisionview/__init__.php +++ b/src/applications/differential/controller/revisionview/__init__.php @@ -9,6 +9,8 @@ phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'applications/differential/constants/action'); phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); +phutil_require_module('phabricator', 'applications/differential/constants/unitstatus'); +phutil_require_module('phabricator', 'applications/differential/constants/unittestresult'); phutil_require_module('phabricator', 'applications/differential/controller/base'); phutil_require_module('phabricator', 'applications/differential/parser/changeset'); phutil_require_module('phabricator', 'applications/differential/storage/changeset'); diff --git a/src/applications/differential/view/revisionupdatehistory/DifferentialRevisionUpdateHistoryView.php b/src/applications/differential/view/revisionupdatehistory/DifferentialRevisionUpdateHistoryView.php index 56fc368878..9d71845ad7 100644 --- a/src/applications/differential/view/revisionupdatehistory/DifferentialRevisionUpdateHistoryView.php +++ b/src/applications/differential/view/revisionupdatehistory/DifferentialRevisionUpdateHistoryView.php @@ -235,6 +235,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView { DifferentialUnitStatus::UNIT_WARN => self::STAR_WARN, DifferentialUnitStatus::UNIT_FAIL => self::STAR_FAIL, DifferentialUnitStatus::UNIT_SKIP => self::STAR_SKIP, + DifferentialUnitStatus::UNIT_POSTPONED => self::STAR_SKIP, ); $star = idx($map, $diff->getUnitStatus(), self::STAR_FAIL); @@ -270,6 +271,8 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView { return 'Unit Test Errors'; case DifferentialUnitStatus::UNIT_SKIP: return 'Unit Tests Skipped'; + case DifferentialUnitStatus::UNIT_POSTPONED: + return 'Unit Tests Postponed'; } return '???'; }