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

@@ -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',

src/applications/conduit/method/ConduitAPIMethod.php

@@ -1,18 +1,41 @@
 <?php
 
 /**
- * @task  status  Method Status
+ * @task info Method Information
- * @task  pager   Paging Results
+ * @task status Method Status
+ * @task pager Paging Results
  */
 abstract class ConduitAPIMethod
   extends Phobject
   implements PhabricatorPolicyInterface {
 
+
   const METHOD_STATUS_STABLE      = 'stable';
   const METHOD_STATUS_UNSTABLE    = 'unstable';
   const METHOD_STATUS_DEPRECATED  = 'deprecated';
 
+
+  /**
+   * Get a short, human-readable text summary of the method.
+   *
+   * @return string Short summary of method.
+   * @task info
+   */
+  public function getMethodSummary() {
+    return $this->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();
 

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:

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 '.
+      'Send a message about the status of a build target to Harbormaster, '.
-      'external system.');
+      '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<wild>',
-      'unit' => 'optional list<wild>',
       'type' => 'required '.$type_const,
+      'unit' => 'optional list<wild>',
+      'lint' => 'optional list<wild>',
     );
   }
 

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) {

src/applications/harbormaster/engine/HarbormasterMessageType.php

@@ -0,0 +1,44 @@
+<?php
+
+final class HarbormasterMessageType extends Phobject {
+
+  const MESSAGE_PASS = 'pass';
+  const MESSAGE_FAIL = 'fail';
+  const MESSAGE_WORK = 'work';
+
+  public static function getAllMessages() {
+    return array_keys(self::getMessageSpecifications());
+  }
+
+  public static function getMessageDescription($message) {
+    $spec = self::getMessageSpecification($message);
+    if (!$spec) {
+      return null;
+    }
+    return idx($spec, 'description');
+  }
+
+  private static function getMessageSpecification($message) {
+    $specs = self::getMessageSpecifications();
+    return idx($specs, $message);
+  }
+
+  private static function getMessageSpecifications() {
+    return array(
+      self::MESSAGE_PASS => 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.'),
+      ),
+    );
+  }
+
+}

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(
+    $spec = self::getParameterSpec();
-      'path' => 'string',
+    $spec = ipull($spec, 'type');
-      'line' => 'optional int',
-      'char' => 'optional int',
-      'code' => 'string',
-      'severity' => 'string',
-      'name' => 'string',
-      'description' => 'optional string',
-    );
 
     // We're just going to ignore extra keys for now, to make it easier to
     // add stuff here later on.

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<string, wild>',
+        'description' => pht(
+          'Coverage information for this test.'),
+      ),
+    );
+  }
+
   public static function newFromDictionary(
     HarbormasterBuildTarget $build_target,
     array $dict) {
 
     $obj = self::initializeNewUnitMessage($build_target);
 
-    $spec = array(
+    $spec = self::getParameterSpec();
-      'engine' => 'optional string',
+    $spec = ipull($spec, 'type');
-      'namespace' => 'optional string',
-      'name' => 'string',
-      'result' => 'string',
-      'duration' => 'optional float|int',
-      'path' => 'optional string',
-      'coverage' => 'optional map<string, wild>',
-    );
 
     // We're just going to ignore extra keys for now, to make it easier to
     // add stuff here later on.