Update status/checks retrieval from Github, move validation of CLA to use status instead of the label, move other validators to common location

.ng-dev/pull-request.ts

@@ -3,7 +3,6 @@ import {PullRequestConfig} from '../ng-dev/pr/config';
 /** Configuration for interacting with pull requests in the repo. */
 export const pullRequest: PullRequestConfig = {
   githubApiMerge: false,
-  claSignedLabel: 'cla: yes',
   mergeReadyLabel: 'action: merge',
   caretakerNoteLabel: 'merge note',
   commitMessageFixupLabel: 'needs commit fixup',

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,16 +6,32 @@
  * 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 {getPendingPrs, getPr} from '../../utils/github';
+import {params, types as graphqlTypes, onUnion} from 'typed-graphqlify';
+import {
+  MergeableState,
+  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,
+  mergeable: graphqlTypes.custom<MergeableState>(),
+  updatedAt: graphqlTypes.string,
   // 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.
   commits: params(
@@ -25,8 +41,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 +101,93 @@ 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);
 }
+
+/** Fetches a pull request from Github. Returns null if an error occurred. */
+export async function fetchPendingPullRequestsFromGithub(
+  git: AuthenticatedGitClient,
+): Promise<PullRequestFromGithub[] | null> {
+  return await getPendingPrs(PR_SCHEMA, 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,11 @@ 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 {
+  getStatusesForPullRequest,
+  PullRequestFromGithub,
+  PullRequestStatus,
+} from '../fetch-pull-request';
 
 /**
  * Assert the commits provided are allowed to merge to the provided target label,
@@ -91,7 +95,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();
   }
@@ -102,3 +106,50 @@ export function assertPendingState(pullRequest: RawPullRequest) {
       throw PullRequestFailure.isMerged();
   }
 }
+
+/**
+ * Assert the pull request has all necessary CLAs signed.
+ * @throws {PullRequestFailure} if the pull request is missing a necessary CLA signature.
+ */
+export function assertSignedCla(pullRequest: PullRequestFromGithub) {
+  const passing = getStatusesForPullRequest(pullRequest).statuses.some(({name, status}) => {
+    return name === 'cla/google' && status === PullRequestStatus.PASSING;
+  });
+
+  if (passing) {
+    return;
+  }
+
+  throw PullRequestFailure.claUnsigned();
+}
+
+/**
+ * Assert the pull request has been marked ready for merge by the author.
+ * @throws {PullRequestFailure} if the pull request is missing the merge ready label.
+ */
+export function assertMergeReady(pullRequest: PullRequestFromGithub, config: PullRequestConfig) {
+  if (pullRequest.labels.nodes.some(({name}) => matchesPattern(name, config.mergeReadyLabel))) {
+    return true;
+  }
+  throw PullRequestFailure.notMergeReady();
+}
+
+/**
+ * Assert the pull request has been marked ready for merge by the author.
+ * @throws {PullRequestFailure} if the pull request is missing the merge ready label.
+ */
+export function assertPassingCi(pullRequest: PullRequestFromGithub) {
+  const {combinedStatus} = getStatusesForPullRequest(pullRequest);
+  if (combinedStatus === PullRequestStatus.PENDING) {
+    throw PullRequestFailure.pendingCiJobs();
+  }
+  if (combinedStatus === PullRequestStatus.FAILING) {
+    throw PullRequestFailure.failingCiJobs();
+  }
+}
+
+// TODO: Remove need to export this pattern matching utility.
+/** Checks whether the specified value matches the given pattern. */
+export function matchesPattern(value: string, pattern: RegExp | string): boolean {
+  return typeof pattern === 'string' ? value === pattern : pattern.test(value);
+}

ng-dev/pr/config/index.ts

@@ -33,8 +33,6 @@ export interface PullRequestConfig {
   noTargetLabeling?: boolean;
   /** Required base commits for given branches. */
   requiredBaseCommits?: {[branchName: string]: string};
-  /** Pattern that matches labels which imply a signed CLA. */
-  claSignedLabel: string | RegExp;
   /** Pattern that matches labels which imply a merge ready pull request. */
   mergeReadyLabel: string | RegExp;
   /** Label that is applied when special attention from the caretaker is required. */
@@ -66,9 +64,6 @@ export function assertValidPullRequestConfig<T>(
     );
   }
 
-  if (!config.pullRequest.claSignedLabel) {
-    errors.push('No CLA signed label configured.');
-  }
   if (!config.pullRequest.mergeReadyLabel) {
     errors.push('No merge ready label configured.');
   }

ng-dev/pr/discover-new-conflicts/BUILD.bazel

@@ -8,10 +8,10 @@ ts_library(
     ],
     visibility = ["//ng-dev:__subpackages__"],
     deps = [
+        "//ng-dev/pr/common",
         "//ng-dev/utils",
         "@npm//@types/cli-progress",
         "@npm//@types/node",
         "@npm//@types/yargs",
-        "@npm//typed-graphqlify",
     ],
 )

ng-dev/pr/discover-new-conflicts/index.ts

@@ -7,45 +7,14 @@
  */
 
 import {Bar} from 'cli-progress';
-import {types as graphqlTypes} from 'typed-graphqlify';
 
 import {error, info} from '../../utils/console';
 import {AuthenticatedGitClient} from '../../utils/git/authenticated-git-client';
 import {GitCommandError} from '../../utils/git/git-client';
-import {getPendingPrs} from '../../utils/github';
-
-/* Graphql schema for the response body for each pending PR. */
-const PR_SCHEMA = {
-  headRef: {
-    name: graphqlTypes.string,
-    repository: {
-      url: graphqlTypes.string,
-      nameWithOwner: graphqlTypes.string,
-    },
-  },
-  baseRef: {
-    name: graphqlTypes.string,
-    repository: {
-      url: graphqlTypes.string,
-      nameWithOwner: graphqlTypes.string,
-    },
-  },
-  updatedAt: graphqlTypes.string,
-  number: graphqlTypes.number,
-  mergeable: graphqlTypes.string,
-  title: graphqlTypes.string,
-};
-
-/* Pull Request response from Github Graphql query */
-type RawPullRequest = typeof PR_SCHEMA;
-
-/** Convert raw Pull Request response from Github to usable Pull Request object. */
-function processPr(pr: RawPullRequest) {
-  return {...pr, updatedAt: new Date(pr.updatedAt).getTime()};
-}
-
-/* Pull Request object after processing, derived from the return type of the processPr function. */
-type PullRequest = ReturnType<typeof processPr>;
+import {
+  fetchPendingPullRequestsFromGithub,
+  PullRequestFromGithub,
+} from '../common/fetch-pull-request';
 
 /** Name of a temporary local branch that is used for checking conflicts. **/
 const tempWorkingBranch = '__NgDevRepoBaseAfterChange__';
@@ -66,11 +35,17 @@ export async function discoverNewConflictsForPr(newPrNumber: number, updatedAfte
   /* Progress bar to indicate progress. */
   const progressBar = new Bar({format: `[{bar}] ETA: {eta}s | {value}/{total}`});
   /* PRs which were found to be conflicting. */
-  const conflicts: Array<PullRequest> = [];
+  const conflicts: Array<PullRequestFromGithub> = [];
 
   info(`Requesting pending PRs from Github`);
   /** List of PRs from github currently known as mergable. */
-  const allPendingPRs = (await getPendingPrs(PR_SCHEMA, git)).map(processPr);
+  const allPendingPRs = await fetchPendingPullRequestsFromGithub(git);
+
+  if (allPendingPRs === null) {
+    error('Unable to find any pending PRs in the repository');
+    process.exit(1);
+  }
+
   /** The PR which is being checked against. */
   const requestedPr = allPendingPRs.find((pr) => pr.number === newPrNumber);
   if (requestedPr === undefined) {
@@ -88,7 +63,7 @@ export async function discoverNewConflictsForPr(newPrNumber: number, updatedAfte
       // PRs which either have not been processed or are determined as mergable by Github
       pr.mergeable !== 'CONFLICTING' &&
       // PRs updated after the provided date
-      pr.updatedAt >= updatedAfter
+      new Date(pr.updatedAt).getTime() >= updatedAfter
     );
   });
   info(`Retrieved ${allPendingPRs.length} total pending PRs`);