Merge master into dependabot/npm_and_yarn/probot-config-tw-0.2.0

bobvanderlinden · Aug 9, 2018 · e3cb10b · e3cb10b
2 parents 7c555dd + eebf89d
commit e3cb10b
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 22 deletions.
diff --git a/.github/auto-merge.yml b/.github/auto-merge.yml
@@ -1,7 +1,7 @@
 # Configuration for probot-auto-merge - https://github.com/bobvanderlinden/probot-auto-merge
 
 minApprovals:
-  COLLABORATOR: 2
+  COLLABORATOR: 1
 maxRequestedChanges:
   NONE: 0
 updateBranch: true

diff --git a/package.json b/package.json
@@ -37,7 +37,7 @@
     "jest": "^22.4.3",
     "nodemon": "^1.17.2",
     "smee-client": "^1.0.2",
-    "standard": "^10.0.3",
+    "standard": "^11.0.1",
     "ts-jest": "^23.0.0",
     "typescript": "^2.9.2"
   },
@@ -61,6 +61,7 @@
       "jsx",
       "json",
       "node"
-    ]
+    ],
+    "testEnvironment": "node"
   }
 }
diff --git a/src/index.ts b/src/index.ts
@@ -15,10 +15,15 @@ async function getHandlerContext(options: {app: Application, context: Context}):
 export = (app: Application) => {
   app.on([
     'pull_request.opened',
+    'pull_request.edited',
+    'pull_request.reopened',
+    'pull_request.synchronize',
+    'pull_request.labeled',
+    'pull_request.unlabeled',
     'pull_request.reopened',
     'pull_request_review.submitted',
-    'pull_request_review.dismissed',
-    'pull_request.synchronize'
+    'pull_request_review.edited',
+    'pull_request_review.dismissed'
   ], async context => {
     const handlerContext = await getHandlerContext({ app, context })
     if (!handlerContext) { return }

diff --git a/src/pull-request-handler.ts b/src/pull-request-handler.ts
@@ -21,12 +21,16 @@ export function schedulePullRequestTrigger(
   context: HandlerContext,
   pullRequestInfo: PullRequestInfo
 ) {
-  const queueName = getPullRequestKey(pullRequestInfo);
+  const queueName = getRepositoryKey(pullRequestInfo);
   if (!taskScheduler.hasQueued(queueName)) {
     taskScheduler.queue(queueName, { context, pullRequestInfo });
   }
 }
 
+function getRepositoryKey({ owner, repo }: { owner: string, repo: string }) {
+  return `${owner}/${repo}`
+}
+
 function getPullRequestKey({ owner, repo, number }: PullRequestInfo) {
   return `${owner}/${repo}#${number}`;
 }

diff --git a/src/pull-request-status.ts b/src/pull-request-status.ts
@@ -243,19 +243,36 @@ async function getPullRequestStatusFromChecks(
   return null;
 }
 
+async function hasStrictBranchChecks(
+  context: HandlerContext,
+  { owner, repo, branch }: { owner: string, repo: string, branch: string }
+): Promise<boolean> {
+  const { github } = context
+  const branchProtectionResponse = await github.repos.getBranchProtection({
+    owner: owner,
+    repo: repo,
+    branch: branch
+  });
+  if (branchProtectionResponse.status === 404) {
+    return false;
+  } else {
+    const response = result<BranchProtection>(branchProtectionResponse);
+    return response.required_status_checks.strict;
+  }
+}
+
 async function getPullRequestStatusFromProtectedBranch(
   context: HandlerContext,
   pullRequest: PullRequest
 ): Promise<PullRequestStatus | null> {
   const { github, log } = context;
-  const branchProtection = result<BranchProtection>(
-    await github.repos.getBranchProtection({
-      owner: pullRequest.base.user.login,
-      repo: pullRequest.base.repo.name,
-      branch: pullRequest.base.ref
-    })
-  );
-  if (branchProtection.required_status_checks.strict) {
+
+  const strictBranchChecks = await hasStrictBranchChecks(context, {
+    owner: pullRequest.base.user.login,
+    repo: pullRequest.base.repo.name,
+    branch: pullRequest.base.ref
+  });
+  if (strictBranchChecks) {
     log(`baseRef: ${pullRequest.base.ref}`);
     const branch = result<Branch>(
       await github.repos.getBranch({

diff --git a/test/mock.ts b/test/mock.ts
@@ -10,16 +10,22 @@ import {
 import { Config, defaultConfig } from "../src/config";
 
 export function githubCallMock<T>(data: T) {
-  return jest.fn(async params => ({
+  return githubResponseMock({
+    status: 200,
     data: data
-  }));
+  });
+}
+
+export function githubResponseMock<T>(response: { status: number, data: T }) {
+  return jest.fn(async _params => response);
 }
 
 export function mockPullRequestContext(options?: {
   reviews?: Review[];
   checkRuns?: CheckRun[];
   branch?: Branch;
   branchProtection?: BranchProtection;
+  hasBranchProtection?: boolean,
   mergeable?: boolean;
   merged?: boolean;
   githubRepoMerge?: Function;
@@ -34,6 +40,7 @@ export function mockPullRequestContext(options?: {
   const log = jest.fn((...args) => {
     // console.log(...args)
   });
+  const hasBranchProtection = options.hasBranchProtection === undefined ? false : options.hasBranchProtection;
 
   const pullRequest: PullRequest = {
     base: {
@@ -87,13 +94,15 @@ export function mockPullRequestContext(options?: {
         },
         repos: {
           merge: options.githubRepoMerge,
-          getBranchProtection: githubCallMock(
-            options.branchProtection || {
-              required_status_checks: {
-                strict: false
+          getBranchProtection: hasBranchProtection
+            ? githubCallMock(
+              options.branchProtection || {
+                required_status_checks: {
+                  strict: false
+                }
               }
-            }
-          ),
+            )
+            : githubResponseMock({ status: 404, data: {} }),
           getBranch: githubCallMock<Branch>(
             options.branch || {
               commit: {

diff --git a/test/pull-request-status.test.ts b/test/pull-request-status.test.ts
@@ -103,6 +103,27 @@ describe("getPullRequestStatus", () => {
     expect(status.code).toBe("ready_for_merge");
   });
 
+  it("returns ready_for_merge when two members approved, but user requested changes", async () => {
+    const { context, pullRequestInfo } = mockPullRequestContext({
+      reviews: [
+        approvedReview({user:{login:"henk"}, author_association: 'OWNER'}),
+        approvedReview({user: {login:"sjaak"}, author_association: 'MEMBER'}),
+        changesRequestedReview({user: {login:"piet"}, author_association: 'NONE'})
+      ],
+      checkRuns: [successCheckRun],
+      config: {
+        minApprovals: {
+          MEMBER: 2
+        },
+        maxRequestedChanges: {
+          MEMBER: 0
+        }
+      }
+    });
+    const status = await getPullRequestStatus(context, pullRequestInfo);
+    expect(status.code).toBe("ready_for_merge");
+  });
+
   it("returns changes_requested when same reviewer approved and requested changes", async () => {
     const { context, pullRequestInfo } = mockPullRequestContext({
       reviews: [approvedReview({user:{login:"henk"}}), changesRequestedReview({user:{login:"henk"}})],
@@ -191,6 +212,7 @@ describe("getPullRequestStatus", () => {
   it("returns out_of_date_branch when pull request is based on strict protected branch", async () => {
     const { context, pullRequestInfo } = mockPullRequestContext({
       reviews: [approvedReview()],
+      hasBranchProtection: true,
       branchProtection: {
         required_status_checks: {
           strict: true