From 630fb06c428e674fe04be3ff6d9cfd05fd1e1d4f Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 4 Aug 2015 13:05:52 -0700 Subject: [PATCH] Document how to use `harbormaster.sendmessage` to report lint and unit results Summary: Fixes T7419. This doesn't really do anything, just adds documentation. Test Plan: - Read the documentation: {F688899} - Created a build plan which makes an HTTP request to `example.com` and waits for a result. - Ran that build plan manually. - Called `harbormaster.sendmessage` manually with the example lint/unit values to provide a result. - Saw the results report correctly and the message ("fail") process as expected: {F688902} Reviewers: chad Reviewed By: chad Maniphest Tasks: T7419 Differential Revision: https://secure.phabricator.com/D13789 --- src/__phutil_library_map__.php | 2 + .../conduit/method/ConduitAPIMethod.php | 27 ++- .../query/PhabricatorConduitSearchEngine.php | 2 +- ...arbormasterSendMessageConduitAPIMethod.php | 198 +++++++++++++++++- .../engine/HarbormasterBuildEngine.php | 9 +- .../engine/HarbormasterMessageType.php | 44 ++++ .../build/HarbormasterBuildLintMessage.php | 57 ++++- .../build/HarbormasterBuildUnitMessage.php | 56 ++++- 8 files changed, 365 insertions(+), 30 deletions(-) create mode 100644 src/applications/harbormaster/engine/HarbormasterMessageType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3927a010b0..779c48bd3c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -990,6 +990,7 @@ phutil_register_library_map(array( 'HarbormasterManagementBuildWorkflow' => 'applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php', 'HarbormasterManagementUpdateWorkflow' => 'applications/harbormaster/management/HarbormasterManagementUpdateWorkflow.php', 'HarbormasterManagementWorkflow' => 'applications/harbormaster/management/HarbormasterManagementWorkflow.php', + 'HarbormasterMessageType' => 'applications/harbormaster/engine/HarbormasterMessageType.php', 'HarbormasterObject' => 'applications/harbormaster/storage/HarbormasterObject.php', 'HarbormasterPlanController' => 'applications/harbormaster/controller/HarbormasterPlanController.php', 'HarbormasterPlanDisableController' => 'applications/harbormaster/controller/HarbormasterPlanDisableController.php', @@ -4702,6 +4703,7 @@ phutil_register_library_map(array( 'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow', 'HarbormasterManagementUpdateWorkflow' => 'HarbormasterManagementWorkflow', 'HarbormasterManagementWorkflow' => 'PhabricatorManagementWorkflow', + 'HarbormasterMessageType' => 'Phobject', 'HarbormasterObject' => 'HarbormasterDAO', 'HarbormasterPlanController' => 'HarbormasterController', 'HarbormasterPlanDisableController' => 'HarbormasterPlanController', diff --git a/src/applications/conduit/method/ConduitAPIMethod.php b/src/applications/conduit/method/ConduitAPIMethod.php index 4472f80864..39fa9b66f4 100644 --- a/src/applications/conduit/method/ConduitAPIMethod.php +++ b/src/applications/conduit/method/ConduitAPIMethod.php @@ -1,18 +1,41 @@ getMethodDescription(); + } + + + /** + * Get a detailed description of the method. + * + * This method should return remarkup. + * + * @return string Detailed description of the method. + * @task info + */ abstract public function getMethodDescription(); + abstract protected function defineParamTypes(); abstract protected function defineReturnType(); diff --git a/src/applications/conduit/query/PhabricatorConduitSearchEngine.php b/src/applications/conduit/query/PhabricatorConduitSearchEngine.php index 5e12cf7f99..6c067a8ff4 100644 --- a/src/applications/conduit/query/PhabricatorConduitSearchEngine.php +++ b/src/applications/conduit/query/PhabricatorConduitSearchEngine.php @@ -174,7 +174,7 @@ final class PhabricatorConduitSearchEngine $item = id(new PHUIObjectItemView()) ->setHeader($method_name) ->setHref($this->getApplicationURI('method/'.$method_name.'/')) - ->addAttribute($method->getMethodDescription()); + ->addAttribute($method->getMethodSummary()); switch ($method->getMethodStatus()) { case ConduitAPIMethod::METHOD_STATUS_STABLE: diff --git a/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php b/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php index 9590f90cec..3a2d808239 100644 --- a/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php +++ b/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php @@ -7,20 +7,206 @@ final class HarbormasterSendMessageConduitAPIMethod return 'harbormaster.sendmessage'; } - public function getMethodDescription() { + public function getMethodSummary() { return pht( - 'Send a message to a build target, notifying it of results in an '. - 'external system.'); + 'Send a message about the status of a build target to Harbormaster, '. + 'notifying the application of build results in an external system.'); + } + + public function getMethodDescription() { + $messages = HarbormasterMessageType::getAllMessages(); + + $head_type = pht('Constant'); + $head_desc = pht('Description'); + $head_key = pht('Key'); + $head_type = pht('Type'); + $head_name = pht('Name'); + + $rows = array(); + $rows[] = "| {$head_type} | {$head_desc} |"; + $rows[] = '|--------------|--------------|'; + foreach ($messages as $message) { + $description = HarbormasterMessageType::getMessageDescription($message); + $rows[] = "| `{$message}` | {$description} |"; + } + $message_table = implode("\n", $rows); + + $rows = array(); + $rows[] = "| {$head_key} | {$head_type} | {$head_desc} |"; + $rows[] = '|-------------|--------------|--------------|'; + $unit_spec = HarbormasterBuildUnitMessage::getParameterSpec(); + foreach ($unit_spec as $key => $parameter) { + $type = idx($parameter, 'type'); + $type = str_replace('|', pht(' or '), $type); + $description = idx($parameter, 'description'); + $rows[] = "| `{$key}` | //{$type}// | {$description} |"; + } + $unit_table = implode("\n", $rows); + + $rows = array(); + $rows[] = "| {$head_key} | {$head_name} | {$head_desc} |"; + $rows[] = '|-------------|--------------|--------------|'; + $results = ArcanistUnitTestResult::getAllResultCodes(); + foreach ($results as $result_code) { + $name = ArcanistUnitTestResult::getResultCodeName($result_code); + $description = ArcanistUnitTestResult::getResultCodeDescription( + $result_code); + $rows[] = "| `{$result_code}` | **{$name}** | {$description} |"; + } + $result_table = implode("\n", $rows); + + $rows = array(); + $rows[] = "| {$head_key} | {$head_type} | {$head_desc} |"; + $rows[] = '|-------------|--------------|--------------|'; + $lint_spec = HarbormasterBuildLintMessage::getParameterSpec(); + foreach ($lint_spec as $key => $parameter) { + $type = idx($parameter, 'type'); + $type = str_replace('|', pht(' or '), $type); + $description = idx($parameter, 'description'); + $rows[] = "| `{$key}` | //{$type}// | {$description} |"; + } + $lint_table = implode("\n", $rows); + + $rows = array(); + $rows[] = "| {$head_key} | {$head_name} |"; + $rows[] = '|-------------|--------------|'; + $severities = ArcanistLintSeverity::getLintSeverities(); + foreach ($severities as $key => $name) { + $rows[] = "| `{$key}` | **{$name}** |"; + } + $severity_table = implode("\n", $rows); + + $valid_unit = array( + array( + 'name' => 'PassingTest', + 'result' => ArcanistUnitTestResult::RESULT_PASS, + ), + array( + 'name' => 'FailingTest', + 'result' => ArcanistUnitTestResult::RESULT_FAIL, + ), + ); + + $valid_lint = array( + array( + 'name' => pht('Syntax Error'), + 'code' => 'EXAMPLE1', + 'severity' => ArcanistLintSeverity::SEVERITY_ERROR, + 'path' => 'path/to/example.c', + 'line' => 17, + 'char' => 3, + ), + array( + 'name' => pht('Not A Haiku'), + 'code' => 'EXAMPLE2', + 'severity' => ArcanistLintSeverity::SEVERITY_ERROR, + 'path' => 'path/to/source.cpp', + 'line' => 23, + 'char' => 1, + 'description' => pht( + 'This function definition is not a haiku.'), + ), + ); + + $json = new PhutilJSON(); + $valid_unit = $json->encodeAsList($valid_unit); + $valid_lint = $json->encodeAsList($valid_lint); + + return pht( + "Send a message about the status of a build target to Harbormaster, ". + "notifying the application of build results in an external system.". + "\n\n". + "Sending Messages\n". + "================\n". + "If you run external builds, you can use this method to publish build ". + "results back into Harbormaster after the external system finishes work ". + "or as it makes progress.". + "\n\n". + "The simplest way to use this method is to call it once after the ". + "build finishes with a `pass` or `fail` message. This will record the ". + "build result, and continue the next step in the build if the build was ". + "waiting for a result.". + "\n\n". + "When you send a status message about a build target, you can ". + "optionally include detailed `lint` or `unit` results alongside the ". + "message. See below for details.". + "\n\n". + "If you want to report intermediate results but a build hasn't ". + "completed yet, you can use the `work` message. This message doesn't ". + "have any direct effects, but allows you to send additional data to ". + "update the progress of the build target. The target will continue ". + "waiting for a completion message, but the UI will update to show the ". + "progress which has been made.". + "\n\n". + "Message Types\n". + "=============\n". + "When you send Harbormaster a message, you must include a `type`, ". + "which describes the overall state of the build. For example, use ". + "`pass` to tell Harbomaster that a build completed successfully.". + "\n\n". + "Supported message types are:". + "\n\n". + "%s". + "\n\n". + "Unit Results\n". + "============\n". + "You can report test results alongside a message. The simplest way to ". + "do this is to report all the results alongside a `pass` or `fail` ". + "message, but you can also send a `work` message to report intermediate ". + "results.\n\n". + "To provide unit test results, pass a list of results in the `unit` ". + "parameter. Each result shoud be a dictionary with these keys:". + "\n\n". + "%s". + "\n\n". + "The `result` parameter recognizes these test results:". + "\n\n". + "%s". + "\n\n". + "This is a simple, valid value for the `unit` parameter. It reports ". + "one passing test and one failing test:\n\n". + "\n\n". + "```lang=json\n". + "%s". + "```". + "\n\n". + "Lint Results\n". + "============\n". + "Like unit test results, you can report lint results alongside a ". + "message. The `lint` parameter should contain results as a list of ". + "dictionaries with these keys:". + "\n\n". + "%s". + "\n\n". + "The `severity` parameter recognizes these severity levels:". + "\n\n". + "%s". + "\n\n". + "This is a simple, valid value for the `lint` parameter. It reports one ". + "error and one warning:". + "\n\n". + "```lang=json\n". + "%s". + "```". + "\n\n", + $message_table, + $unit_table, + $result_table, + $valid_unit, + $lint_table, + $severity_table, + $valid_lint); } protected function defineParamTypes() { - $type_const = $this->formatStringConstants(array('pass', 'fail')); + $messages = HarbormasterMessageType::getAllMessages(); + $type_const = $this->formatStringConstants($messages); return array( 'buildTargetPHID' => 'required phid', - 'lint' => 'optional list', - 'unit' => 'optional list', 'type' => 'required '.$type_const, + 'unit' => 'optional list', + 'lint' => 'optional list', ); } diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php index 8d8c26818e..fd895979cd 100644 --- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php @@ -317,14 +317,17 @@ final class HarbormasterBuildEngine extends Phobject { foreach ($messages as $message) { $target = $waiting_targets[$message->getBuildTargetPHID()]; - $new_status = null; switch ($message->getType()) { - case 'pass': + case HarbormasterMessageType::MESSAGE_PASS: $new_status = HarbormasterBuildTarget::STATUS_PASSED; break; - case 'fail': + case HarbormasterMessageType::MESSAGE_FAIL: $new_status = HarbormasterBuildTarget::STATUS_FAILED; break; + case HarbormasterMessageType::MESSAGE_WORK: + default: + $new_status = null; + break; } if ($new_status !== null) { diff --git a/src/applications/harbormaster/engine/HarbormasterMessageType.php b/src/applications/harbormaster/engine/HarbormasterMessageType.php new file mode 100644 index 0000000000..5900817c69 --- /dev/null +++ b/src/applications/harbormaster/engine/HarbormasterMessageType.php @@ -0,0 +1,44 @@ + array( + 'description' => pht( + 'Report that the target is complete, and the target has passed.'), + ), + self::MESSAGE_FAIL => array( + 'description' => pht( + 'Report that the target is complete, and the target has failed.'), + ), + self::MESSAGE_WORK => array( + 'description' => pht( + 'Report that work on the target is ongoing. This message can be '. + 'used to report partial results during a build.'), + ), + ); + } + +} diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php b/src/applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php index c9957b0149..2bd37b2f79 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php @@ -20,21 +20,60 @@ final class HarbormasterBuildLintMessage ->setBuildTargetPHID($build_target->getPHID()); } + public static function getParameterSpec() { + return array( + 'name' => array( + 'type' => 'string', + 'description' => pht( + 'Short message name, like "Syntax Error".'), + ), + 'code' => array( + 'type' => 'string', + 'description' => pht( + 'Lint message code identifying the type of message, like "ERR123".'), + ), + 'severity' => array( + 'type' => 'string', + 'description' => pht( + 'Severity of the message.'), + ), + 'path' => array( + 'type' => 'string', + 'description' => pht( + 'Path to the file containing the lint message, from the project '. + 'root.'), + ), + 'line' => array( + 'type' => 'optional int', + 'description' => pht( + 'Line number in the file where the text which triggered the '. + 'message first appears. The first line of the file is line 1, '. + 'not line 0.'), + ), + 'char' => array( + 'type' => 'optional int', + 'description' => pht( + 'Byte position on the line where the text which triggered the '. + 'message starts. The first byte on the line is byte 1, not byte '. + '0. This position is byte-based (not character-based) because '. + 'not all lintable files have a valid character encoding.'), + ), + 'description' => array( + 'type' => 'optional string', + 'description' => pht( + 'Long explanation of the lint message.'), + ), + ); + } + public static function newFromDictionary( HarbormasterBuildTarget $build_target, array $dict) { $obj = self::initializeNewLintMessage($build_target); - $spec = array( - 'path' => 'string', - 'line' => 'optional int', - 'char' => 'optional int', - 'code' => 'string', - 'severity' => 'string', - 'name' => 'string', - 'description' => 'optional string', - ); + $spec = self::getParameterSpec(); + $spec = ipull($spec, 'type'); // We're just going to ignore extra keys for now, to make it easier to // add stuff here later on. diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php index 2b9108b035..1062b64a3a 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php @@ -19,21 +19,59 @@ final class HarbormasterBuildUnitMessage ->setBuildTargetPHID($build_target->getPHID()); } + public static function getParameterSpec() { + return array( + 'name' => array( + 'type' => 'string', + 'description' => pht( + 'Short test name, like "ExampleTest".'), + ), + 'result' => array( + 'type' => 'string', + 'description' => pht( + 'Result of the test.'), + ), + 'namespace' => array( + 'type' => 'optional string', + 'description' => pht( + 'Optional namespace for this test. This is organizational and '. + 'is often a class or module name, like "ExampleTestCase".'), + ), + 'engine' => array( + 'type' => 'optional string', + 'description' => pht( + 'Test engine running the test, like "JavascriptTestEngine". This '. + 'primarily prevents collisions between tests with the same name '. + 'in different test suites (for example, a Javascript test and a '. + 'Python test).'), + ), + 'duration' => array( + 'type' => 'optional float|int', + 'description' => pht( + 'Runtime duration of the test, in seconds.'), + ), + 'path' => array( + 'type' => 'optional string', + 'description' => pht( + 'Path to the file where the test is declared, relative to the '. + 'project root.'), + ), + 'coverage' => array( + 'type' => 'optional map', + 'description' => pht( + 'Coverage information for this test.'), + ), + ); + } + public static function newFromDictionary( HarbormasterBuildTarget $build_target, array $dict) { $obj = self::initializeNewUnitMessage($build_target); - $spec = array( - 'engine' => 'optional string', - 'namespace' => 'optional string', - 'name' => 'string', - 'result' => 'string', - 'duration' => 'optional float|int', - 'path' => 'optional string', - 'coverage' => 'optional map', - ); + $spec = self::getParameterSpec(); + $spec = ipull($spec, 'type'); // We're just going to ignore extra keys for now, to make it easier to // add stuff here later on.