fix(ng-dev/pr): correctly retrieve both github checks and statuses an…

…d normalize them together (#242)

Previously we only retrieved the status for a pull request rather than also retrieving the results
for github check runs on commits.  It is more accurate to retrieve both and normalize them together
to check all statuses during validation.

PR Close #242

ng-dev/pr/common/BUILD.bazel

@@ -10,6 +10,7 @@ ts_library(
         "//ng-dev/release/config",
         "//ng-dev/release/versioning",
         "//ng-dev/utils",
+        "@npm//@octokit/graphql-schema",
         "@npm//@types/node",
         "@npm//@types/semver",
         "@npm//semver",

ng-dev/pr/common/fetch-pull-request.ts

@@ -6,15 +6,28 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {params, types as graphqlTypes} from 'typed-graphqlify';
 import {AuthenticatedGitClient} from '../../utils/git/authenticated-git-client';
 import {getPr} from '../../utils/github';
+import {params, types as graphqlTypes, onUnion} from 'typed-graphqlify';
+import {
+  CheckConclusionState,
+  StatusState,
+  PullRequestState,
+  CheckStatusState,
+} from '@octokit/graphql-schema';
+
+/** A status for a pull request status or check. */
+export enum PullRequestStatus {
+  PASSING,
+  FAILING,
+  PENDING,
+}
 
 /** Graphql schema for the response body the requested pull request. */
-const PR_SCHEMA = {
+export const PR_SCHEMA = {
   url: graphqlTypes.string,
   isDraft: graphqlTypes.boolean,
-  state: graphqlTypes.oneOf(['OPEN', 'MERGED', 'CLOSED'] as const),
+  state: graphqlTypes.custom<PullRequestState>(),
   number: graphqlTypes.number,
   // Only the last 100 commits from a pull request are obtained as we likely will never see a pull
   // requests with more than 100 commits.
@@ -25,8 +38,28 @@ const PR_SCHEMA = {
       nodes: [
         {
           commit: {
-            status: {
-              state: graphqlTypes.oneOf(['FAILURE', 'PENDING', 'SUCCESS'] as const),
+            statusCheckRollup: {
+              state: graphqlTypes.custom<StatusState>(),
+              contexts: params(
+                {last: 100},
+                {
+                  nodes: [
+                    onUnion({
+                      CheckRun: {
+                        __typename: graphqlTypes.constant('CheckRun'),
+                        status: graphqlTypes.custom<CheckStatusState>(),
+                        conclusion: graphqlTypes.custom<CheckConclusionState>(),
+                        name: graphqlTypes.string,
+                      },
+                      StatusContext: {
+                        __typename: graphqlTypes.constant('StatusContext'),
+                        state: graphqlTypes.custom<StatusState>(),
+                        context: graphqlTypes.string,
+                      },
+                    }),
+                  ],
+                },
+              ),
             },
             message: graphqlTypes.string,
           },
@@ -65,13 +98,86 @@ const PR_SCHEMA = {
   ),
 };
 
-/** A pull request retrieved from github via the graphql API. */
-export type RawPullRequest = typeof PR_SCHEMA;
+export type PullRequestFromGithub = typeof PR_SCHEMA;
 
 /** Fetches a pull request from Github. Returns null if an error occurred. */
 export async function fetchPullRequestFromGithub(
   git: AuthenticatedGitClient,
   prNumber: number,
-): Promise<RawPullRequest | null> {
+): Promise<PullRequestFromGithub | null> {
   return await getPr(PR_SCHEMA, prNumber, git);
 }
+
+/**
+ * Gets the statuses for a commit from a pull requeste, using a consistent interface for both
+ * status and checks results.
+ */
+export function getStatusesForPullRequest(pullRequest: PullRequestFromGithub) {
+  const nodes = pullRequest.commits.nodes;
+  /** The combined github status and github checks object. */
+  const {statusCheckRollup} = nodes[nodes.length - 1].commit;
+
+  const statuses = statusCheckRollup.contexts.nodes.map((context) => {
+    switch (context.__typename) {
+      case 'CheckRun':
+        return {
+          type: 'check',
+          name: context.name,
+          status: normalizeGithubCheckState(context.conclusion, context.status),
+        };
+      case 'StatusContext':
+        return {
+          type: 'status',
+          name: context.context,
+          status: normalizeGithubStatusState(context.state),
+        };
+    }
+  });
+
+  return {
+    combinedStatus: normalizeGithubStatusState(statusCheckRollup.state),
+    statuses,
+  };
+}
+
+/** Retrieve the normalized PullRequestStatus for the provided github status state. */
+function normalizeGithubStatusState(state: StatusState) {
+  switch (state) {
+    case 'FAILURE':
+    case 'ERROR':
+      return PullRequestStatus.FAILING;
+    case 'PENDING':
+      return PullRequestStatus.PENDING;
+    case 'SUCCESS':
+    case 'EXPECTED':
+      return PullRequestStatus.PASSING;
+  }
+}
+
+/** Retrieve the normalized PullRequestStatus for the provided github check state. */
+function normalizeGithubCheckState(conclusion: CheckConclusionState, status: CheckStatusState) {
+  switch (status) {
+    case 'COMPLETED':
+      break;
+    case 'QUEUED':
+    case 'IN_PROGRESS':
+    case 'WAITING':
+    case 'PENDING':
+    case 'REQUESTED':
+      return PullRequestStatus.PENDING;
+  }
+
+  switch (conclusion) {
+    case 'ACTION_REQUIRED':
+    case 'TIMED_OUT':
+    case 'CANCELLED':
+    case 'FAILURE':
+    case 'SKIPPED':
+    case 'STALE':
+    case 'STARTUP_FAILURE':
+      return PullRequestStatus.FAILING;
+    case 'SUCCESS':
+    case 'NEUTRAL':
+      return PullRequestStatus.PASSING;
+  }
+}

ng-dev/pr/common/validation/validations.ts

@@ -11,7 +11,7 @@ import {TargetLabel, TargetLabelName} from '../targeting/target-label';
 import {breakingChangeLabel, PullRequestConfig} from '../../config';
 import {PullRequestFailure} from './failures';
 import {red, warn} from '../../../utils/console';
-import {RawPullRequest} from '../fetch-pull-request';
+import {PullRequestFromGithub} from '../fetch-pull-request';
 
 /**
  * Assert the commits provided are allowed to merge to the provided target label,
@@ -91,7 +91,7 @@ export function assertCorrectBreakingChangeLabeling(
  * Assert the pull request is pending, not closed, merged or in draft.
  * @throws {PullRequestFailure} if the pull request is not pending.
  */
-export function assertPendingState(pullRequest: RawPullRequest) {
+export function assertPendingState(pullRequest: PullRequestFromGithub) {
   if (pullRequest.isDraft) {
     throw PullRequestFailure.isDraft();
   }

ng-dev/pr/merge/pull-request.ts

@@ -15,7 +15,11 @@ import {
   assertCorrectBreakingChangeLabeling,
   assertPendingState,
 } from '../common/validation/validations';
-import {fetchPullRequestFromGithub} from '../common/fetch-pull-request';
+import {
+  fetchPullRequestFromGithub,
+  getStatusesForPullRequest,
+  PullRequestStatus,
+} from '../common/fetch-pull-request';
 
 /** Interface that describes a pull request. */
 export interface PullRequest {
@@ -90,11 +94,11 @@ export async function loadAndValidatePullRequest(
   }
 
   /** The combined status of the latest commit in the pull request. */
-  const state = prData.commits.nodes.slice(-1)[0].commit.status.state;
-  if (state === 'FAILURE' && !ignoreNonFatalFailures) {
+  const {combinedStatus} = getStatusesForPullRequest(prData);
+  if (combinedStatus === PullRequestStatus.FAILING && !ignoreNonFatalFailures) {
     return PullRequestFailure.failingCiJobs();
   }
-  if (state === 'PENDING' && !ignoreNonFatalFailures) {
+  if (combinedStatus === PullRequestStatus.PENDING && !ignoreNonFatalFailures) {
     return PullRequestFailure.pendingCiJobs();
   }
 

package.json

@@ -63,6 +63,7 @@
     "yargs": "^17.0.0"
   },
   "devDependencies": {
+    "@octokit/graphql-schema": "^10.72.0",
     "@types/cli-progress": "^3.9.1",
     "@types/conventional-commits-parser": "^3.0.1",
     "@types/ejs": "^3.0.6",

yarn.lock

@@ -269,6 +269,14 @@
     is-plain-object "^5.0.0"
     universal-user-agent "^6.0.0"
 
+"@octokit/graphql-schema@^10.72.0":
+  version "10.72.0"
+  resolved "https://registry.yarnpkg.com/@octokit/graphql-schema/-/graphql-schema-10.72.0.tgz#39f6aa1be17afdd26c3b0c44475052168547f4b6"
+  integrity sha512-Ofz/29THb+HgOzwS6t9CGhRJplhmBlMhWIJKd9ynJzDj+QJuqnQqN7prYo9AJP+XzGs8fW+fvsNNrkFtHjvVcw==
+  dependencies:
+    graphql "^15.0.0"
+    graphql-tag "^2.10.3"
+
 "@octokit/graphql@^4.5.8", "@octokit/graphql@^4.8.0":
   version "4.8.0"
   resolved "https://registry.yarnpkg.com/@octokit/graphql/-/graphql-4.8.0.tgz#664d9b11c0e12112cbf78e10f49a05959aa22cc3"
@@ -1546,6 +1554,18 @@ graceful-fs@^4.1.2, graceful-fs@^4.1.6, graceful-fs@^4.1.9:
   resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.8.tgz#e412b8d33f5e006593cbd3cee6df9f2cebbe802a"
   integrity sha512-qkIilPUYcNhJpd33n0GBXTB1MMPp14TxEsEs0pTrsSVucApsYzW5V+Q8Qxhik6KU3evy+qkAAowTByymK0avdg==
 
+graphql-tag@^2.10.3:
+  version "2.12.5"
+  resolved "https://registry.yarnpkg.com/graphql-tag/-/graphql-tag-2.12.5.tgz#5cff974a67b417747d05c8d9f5f3cb4495d0db8f"
+  integrity sha512-5xNhP4063d16Pz3HBtKprutsPrmHZi5IdUGOWRxA2B6VF7BIRGOHZ5WQvDmJXZuPcBg7rYwaFxvQYjqkSdR3TQ==
+  dependencies:
+    tslib "^2.1.0"
+
+graphql@^15.0.0:
+  version "15.6.0"
+  resolved "https://registry.yarnpkg.com/graphql/-/graphql-15.6.0.tgz#e69323c6a9780a1a4b9ddf7e35ca8904bb04df02"
+  integrity sha512-WJR872Zlc9hckiEPhXgyUftXH48jp2EjO5tgBBOyNMRJZ9fviL2mJBD6CAysk6N5S0r9BTs09Qk39nnJBkvOXQ==
+
 har-schema@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/har-schema/-/har-schema-2.0.0.tgz#a94c2224ebcac04782a0d9035521f24735b7ec92"
@@ -3080,7 +3100,7 @@ tslib@^1.13.0, tslib@^1.8.1:
   resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.14.1.tgz#cf2d38bdc34a134bcaf1091c41f6619e2f672d00"
   integrity sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==
 
-tslib@^2.0.0, tslib@^2.3.0:
+tslib@^2.0.0, tslib@^2.1.0, tslib@^2.3.0:
   version "2.3.1"
   resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.3.1.tgz#e8a335add5ceae51aa261d32a490158ef042ef01"
   integrity sha512-77EbyPPpMz+FRFRuAFlWMtmgUWGe9UOG2Z25NqCwiIjRhOf5iKGuzSe5P2w1laq+FkRy4p+PCuVkJSGkzTEKVw==