Report queued builds as "queued", drive one check run through its full lifecycle

master/github_app_check_push.py

@@ -51,9 +51,10 @@ def _fetch(self):
 
 class GitHubAppCheckPush(GitHubStatusPush):
     """Like GitHubStatusPush, but reports through the Checks API instead of the legacy Statuses
-    API, so a build in progress shows GitHub's spinner instead of a static pending dot. Requires
-    a GitHub App: pass an AppInstallationToken as `token=` (the Statuses API's PAT-based token
-    doesn't work here; only the Checks API used by this class requires App auth).
+    API, so a build shows GitHub's queued/in-progress states (and eventual spinner) instead of a
+    single static pending dot. Requires a GitHub App: pass an AppInstallationToken as `token=`
+    (the Statuses API's PAT-based token doesn't work here; only the Checks API used by this class
+    requires App auth).
     """
 
     @defer.inlineCallbacks
@@ -70,40 +71,40 @@ def createStatus(
         output = {"title": context, "summary": description or ""}
 
         if state == "pending":
-            payload = {
-                "name": context,
-                "head_sha": sha,
-                "status": "in_progress",
-                "details_url": target_url,
-                "output": output,
-                "external_id": issue,
-            }
-            return (yield self._http.post(base, json=payload, headers=headers))
+            # GitHubStatusPush's default generators report both a build being merely queued
+            # (BuildRequestGenerator, before any worker picks it up) and a build actually
+            # starting (BuildStartEndStatusGenerator) with state == "pending" -- neither is
+            # "complete" yet. Only the queued report's target_url points at a buildrequest page
+            # rather than an actual build, so use that to tell the two apart.
+            status = "queued" if target_url and "/buildrequests/" in target_url else "in_progress"
+            payload = {"status": status, "details_url": target_url, "output": output}
+        else:
+            # GitHubStatusPush.sendMessage() already collapsed several build results into
+            # "error"; both "failure" and "error" map to the same GitHub conclusion.
+            conclusion = "success" if state == "success" else "failure"
+            payload = {"status": "completed", "conclusion": conclusion, "details_url": target_url, "output": output}
 
-        # The check run's id isn't threaded through from the "pending" call above, so look it up
-        # by name instead of tracking build-run state; one extra GET, but no persisted state.
+        # The check run's id isn't threaded through between calls, so look it up by name instead
+        # of tracking build-run state; one extra GET, but no persisted state. A build's queued,
+        # started, and completed reports all update the same check run this way.
         resp = yield self._http.get(
             f"/repos/{repo_user}/{repo_name}/commits/{sha}/check-runs",
             params={"check_name": context},
             headers=headers,
         )
         runs = (yield resp.json())["check_runs"]
-        if not runs:
-            return None
-
-        # GitHubStatusPush.sendMessage() already collapsed several build results into "error";
-        # both "failure" and "error" map to the same GitHub conclusion.
-        conclusion = "success" if state == "success" else "failure"
-        payload = {
-            "status": "completed",
-            "conclusion": conclusion,
-            "details_url": target_url,
-            "output": output,
-        }
-        # HTTPSession has no patch() wrapper (only get/put/post/delete); the Checks API update
-        # endpoint is PATCH-only, so fall through to the generic dispatcher it's built on.
-        return (
-            yield self._http.http._do_request(
-                self._http, "patch", f"{base}/{runs[0]['id']}", json=payload, headers=headers
+        if runs:
+            run_id = max(runs, key=lambda r: r["id"])["id"]
+            # HTTPSession has no patch() wrapper (only get/put/post/delete); the Checks API
+            # update endpoint is PATCH-only, so fall through to the generic dispatcher it's
+            # built on.
+            return (
+                yield self._http.http._do_request(
+                    self._http, "patch", f"{base}/{run_id}", json=payload, headers=headers
+                )
             )
-        )
+
+        # No existing check run (this is the first report for this build, or GitHub is still
+        # processing the previous write) -- create one from scratch.
+        payload = {**payload, "name": context, "head_sha": sha, "external_id": issue}
+        return (yield self._http.post(base, json=payload, headers=headers))