feat(status-check): allow custom required status checks

The optional `requiredStatusChecks` argument may be specified to customise the list of Status Checks that the PR must pass in order to be considered 'buildable'. If no argument is given, then all status checks must pass (ie, the current behaviour).
fountainhead · Sep 5, 2018 · 982e1b6 · 982e1b6
1 parent 1768716
commit 982e1b6
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 48 deletions.
diff --git a/src/index.ts b/src/index.ts
@@ -1,6 +1,4 @@
 import * as env from 'env-var';
-import { join } from 'path';
-import { tap } from 'ramda';
 import untildify from 'untildify';
 import yargs from 'yargs';
 
@@ -16,6 +14,7 @@ type Args = {
   owner: string;
   apply: boolean;
   mapper?: string;
+  requiredStatusChecks?: string[];
 };
 
 const args = yargs(process.argv)
@@ -39,14 +38,20 @@ const args = yargs(process.argv)
       describe: 'By default, this tool will display proposed changes to a Helmfile but will not modify it. '
         + 'To apply changes to a Helmfile, specify this flag.',
       default: false
+    },
+    requiredStatusChecks: {
+      describe: 'An optional list of Status Check names that must a Pull Request must pass in order for a Release to '
+        + 'be created or updated. If omitted, a Pull Request must pass all applicable Status Checks.'
     }
   })
+  .array('requiredStatusChecks')
   .argv as any as Args;
 
 const gitHub = {
   token: env.get('GITHUB_TOKEN').required().asString(),
   repo: args.repo,
-  owner: args.owner
+  owner: args.owner,
+  requiredStatusChecks: args.requiredStatusChecks
 };
 
 const helmfile = {

diff --git a/src/pullRequestEnumerator.spec.ts b/src/pullRequestEnumerator.spec.ts
@@ -1,3 +1,5 @@
+import { GetCombinedStatusForRefResponse, Response } from '@octokit/rest';
+
 import * as prEnumerator from './pullRequestEnumerator';
 
 describe('extractLastPage', () => {
@@ -71,21 +73,57 @@ ingress: test.tld.dev
 });
 
 describe('getBuildSucceeded', () => {
-  it('returns `data[0].state === \'success\'`', () => {
-    expect(prEnumerator.getBuildSucceeded({
-      data: [{
+  const mixedResponse = {
+    data: {
+      statuses: [{
+        context: 'test: stageA',
+        state: 'success'
+      }, {
+        context: 'test: stageB',
         state: 'success'
+      }, {
+        context: 'test: stageC',
+        state: 'failure'
       }]
-    } as any)).toBe(true);
+    }
+  } as any as Response<GetCombinedStatusForRefResponse>;
 
-    expect(prEnumerator.getBuildSucceeded({
-      data: [{
-        state: 'failed'
+  const okResponse = {
+    data: {
+      statuses: [{
+        context: 'test: stageA',
+        state: 'success'
+      }, {
+        context: 'test: stageB',
+        state: 'success'
+      }]
+    }
+  } as any as Response<GetCombinedStatusForRefResponse>;
+
+  const failedResponse = {
+    data: {
+      statuses: [{
+        context: 'test: stageA',
+        state: 'failure'
+      }, {
+        context: 'test: stageB',
+        state: 'failure'
       }]
-    } as any)).toBe(false);
+    }
+  } as any as Response<GetCombinedStatusForRefResponse>;
+
+  it('returns `true` only when State for each name has `state` === `success`', () => {
+    expect(prEnumerator.getBuildSucceeded(['test: stageA'])(mixedResponse)).toEqual(true);
+    expect(prEnumerator.getBuildSucceeded(['test: stageB'])(mixedResponse)).toEqual(true);
+    expect(prEnumerator.getBuildSucceeded(['test: stageC'])(mixedResponse)).toEqual(false);
+
+    expect(prEnumerator.getBuildSucceeded(['test: stageA', 'test: stageB'])(mixedResponse)).toEqual(true);
+    expect(prEnumerator.getBuildSucceeded(['test: stageA', 'test: stageC'])(mixedResponse)).toEqual(false);
+  });
 
-    expect(prEnumerator.getBuildSucceeded({
-      data: []
-    } as any)).toBe(false);
+  it('returns `true` only when all Statues have `state` === `success` when list is omitted', () => {
+    expect(prEnumerator.getBuildSucceeded()(okResponse)).toEqual(true);
+    expect(prEnumerator.getBuildSucceeded()(mixedResponse)).toEqual(false);
+    expect(prEnumerator.getBuildSucceeded()(failedResponse)).toEqual(false);
   });
 });
diff --git a/src/pullRequestEnumerator.ts b/src/pullRequestEnumerator.ts
@@ -1,13 +1,11 @@
-import Octokit, { Auth, Response } from '@octokit/rest';
+import Octokit, { GetAllResponseItem, GetCombinedStatusForRefResponse, Response } from '@octokit/rest';
 import { Bar } from 'cli-progress';
 import { safeLoadAll } from 'js-yaml';
 import {
-  always, converge, defaultTo, head, identity, ifElse, is, map, merge, mergeAll, objOf, path, pick, pipe, prop, propEq,
-  tryCatch, unapply
+  all, allPass, always, any, converge, head, identity, ifElse, is, map, merge, mergeAll, objOf, path, pick, pipe, prop,
+  tryCatch, unapply, whereEq
 } from 'ramda';
 
-import { pullRequest } from './taskList/cellRenderers';
-
 export type PullRequest = {
   number: number;
   title: string;
@@ -25,7 +23,7 @@ export const extractLastPage = (link: string) => {
   return matches ? parseInt(matches[1], 10) : 1;
 };
 
-export const getPrData: (resp: Response<any[]>) => PullRequest[] = pipe(
+export const getPrData: (resp: Response<GetAllResponseItem[]>) => PullRequest[] = pipe(
   prop('data'),
   map(
     converge(unapply(mergeAll), [
@@ -59,12 +57,18 @@ export const getPrData: (resp: Response<any[]>) => PullRequest[] = pipe(
   )
 );
 
-export const getBuildSucceeded: (resp: Response<any[]>) => boolean = pipe(
-  prop('data'),
-  head,
-  defaultTo({}),
-  propEq('state', 'success')
-);
+export const getBuildSucceeded = (requiredStatusChecks?: string[]) =>
+  pipe(
+    path(['data', 'statuses']),
+    requiredStatusChecks ?
+      allPass(requiredStatusChecks.map(name => any(whereEq({
+        context: name,
+        state: 'success'
+      })))) as any :
+      all(whereEq({
+        state: 'success'
+      })) as any
+  ) as (response: Response<GetCombinedStatusForRefResponse>) => boolean;
 
 type FetchAllOptions = {
   client: Octokit;
@@ -108,44 +112,53 @@ type WithBuildStatusesOptions = {
   owner: string;
   repo: string;
   pullRequests: PullRequest[];
+  statusChecker: (response: Response<GetCombinedStatusForRefResponse>) => boolean;
 };
 
-export const withBuildStatuses = async ({ client, owner, repo, pullRequests }: WithBuildStatusesOptions) => {
-  console.log('Fetching build status for open PRs');
+export const withBuildStatuses =
+  async ({ client, owner, repo, pullRequests, statusChecker }: WithBuildStatusesOptions) => {
+    console.log('Fetching build status for open PRs');
 
-  const bar = new Bar({});
-  bar.start(pullRequests.length, 0);
+    const bar = new Bar({});
+    bar.start(pullRequests.length, 0);
 
-  const buildStatuses = await Promise.all(pullRequests.map(async pr => {
-    const response = await client.repos.getStatuses({
-      ref: pr.sha,
-      per_page: 1,
-      owner,
-      repo
-    });
+    const buildStatuses = await Promise.all(pullRequests.map(async pr => {
+      const response = await client.repos.getCombinedStatusForRef({
+        ref: pr.sha,
+        owner,
+        repo
+      });
 
-    bar.increment(1);
+      bar.increment(1);
 
-    return merge(pr, {
-      buildSucceeded: getBuildSucceeded(response)
-    });
-  }));
+      return merge(pr, {
+        buildSucceeded: statusChecker(response)
+      });
+    }));
 
-  bar.stop();
+    bar.stop();
 
-  return buildStatuses;
-};
+    return buildStatuses;
+  };
 
 type FetchOptions = {
   token: string;
   owner: string;
   repo: string;
+  requiredStatusChecks?: string[];
 };
 
-export const fetch = async ({ token, owner, repo }: FetchOptions) => {
+export const fetch = async ({ token, owner, repo, requiredStatusChecks }: FetchOptions) => {
   const client = new Octokit();
   client.authenticate({ type: 'token', token });
 
   const pullRequests = await fetchAll({ client, owner, repo });
-  return withBuildStatuses({ client, owner, repo, pullRequests });
+
+  return withBuildStatuses({
+    statusChecker: getBuildSucceeded(requiredStatusChecks),
+    client,
+    owner,
+    repo,
+    pullRequests
+  });
 };
diff --git a/tsconfig.json b/tsconfig.json
@@ -30,7 +30,7 @@
     // "alwaysStrict": true,                  /* Parse in strict mode and emit "use strict" for each source file. */
 
     /* Additional Checks */
-    // "noUnusedLocals": true,                /* Report errors on unused locals. */
+    "noUnusedLocals": true,                   /* Report errors on unused locals. */
     // "noUnusedParameters": true,            /* Report errors on unused parameters. */
     // "noImplicitReturns": true,             /* Report error when not all code paths in function return a value. */
     // "noFallthroughCasesInSwitch": true,    /* Report errors for fallthrough cases in switch statement. */