feat(manage-merge-queue): do not allow ready for merge when PR is not…

… core approved (#546) * do not allow ready for merge when PR is not core approved * update to check for approvals satisfied check * fixed tests * fixed tests * fixed tests * update comment body --------- Co-authored-by: Pooja Donekal <[email protected]> Co-authored-by: Dan Adajian <[email protected]>
ExpediaGroup · Feb 19, 2024 · d247648 · d247648
1 parent 8e29319
commit d247648
Show file tree

Hide file tree

Showing 6 changed files with 433 additions and 6 deletions.
diff --git a/dist/676.index.js b/dist/676.index.js
diff --git a/dist/676.index.js.map b/dist/676.index.js.map
diff --git a/dist/index.js b/dist/index.js
@@ -42180,13 +42180,15 @@ var map = {
 		7473,
 		832,
 		710,
+		445,
 		205,
 		676
 	],
 	"./manage-merge-queue.ts": [
 		7473,
 		832,
 		710,
+		445,
 		205,
 		676
 	],
@@ -42216,13 +42218,15 @@ var map = {
 		1004,
 		832,
 		710,
+		445,
 		205,
 		676
 	],
 	"./prepare-queued-pr-for-merge.ts": [
 		1004,
 		832,
 		710,
+		445,
 		205,
 		676
 	],

diff --git a/dist/index.js.map b/dist/index.js.map
diff --git a/src/helpers/manage-merge-queue.ts b/src/helpers/manage-merge-queue.ts
@@ -30,6 +30,8 @@ import { setCommitStatus } from './set-commit-status';
 import { updateMergeQueue } from '../utils/update-merge-queue';
 import { paginateAllOpenPullRequests } from '../utils/paginate-open-pull-requests';
 import { updatePrWithDefaultBranch } from './prepare-queued-pr-for-merge';
+import { approvalsSatisfied } from './approvals-satisfied';
+import { createPrComment } from './create-pr-comment';
 
 export class ManageMergeQueue extends HelperInputs {
   login?: string;
@@ -42,6 +44,11 @@ export const manageMergeQueue = async ({ login, slack_webhook_url }: ManageMerge
     core.info('This PR is not in the merge queue.');
     return removePrFromQueue(pullRequest);
   }
+  const prMeetsRequiredApprovals = await approvalsSatisfied();
+  if (!prMeetsRequiredApprovals) {
+    await createPrComment({ body: 'PRs must meet all required approvals before entering the merge queue.' });
+    return removePrFromQueue(pullRequest);
+  }
   const queuedPrs = await getQueuedPullRequests();
   const queuePosition = queuedPrs.length;
   if (pullRequest.labels.find(label => label.name === JUMP_THE_QUEUE_PR_LABEL)) {

diff --git a/test/helpers/manage-merge-queue.test.ts b/test/helpers/manage-merge-queue.test.ts
@@ -21,11 +21,15 @@ import { removeLabelIfExists } from '../../src/helpers/remove-label';
 import { setCommitStatus } from '../../src/helpers/set-commit-status';
 import { updateMergeQueue } from '../../src/utils/update-merge-queue';
 import { updatePrWithDefaultBranch } from '../../src/helpers/prepare-queued-pr-for-merge';
+import { approvalsSatisfied } from '../../src/helpers/approvals-satisfied';
+import { createPrComment } from '../../src/helpers/create-pr-comment';
 
 jest.mock('../../src/helpers/remove-label');
 jest.mock('../../src/helpers/set-commit-status');
 jest.mock('../../src/utils/notify-user');
 jest.mock('../../src/utils/update-merge-queue');
+jest.mock('../../src/helpers/approvals-satisfied');
+jest.mock('../../src/helpers/create-pr-comment');
 jest.mock('../../src/utils/../../src/helpers/prepare-queued-pr-for-merge');
 jest.mock('@actions/core');
 jest.mock('@actions/github', () => ({
@@ -69,6 +73,38 @@ describe('manageMergeQueue', () => {
     });
   });
 
+  describe('pr not core approved case', () => {
+    const queuedPrs = [{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }, { labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }];
+    beforeEach(async () => {
+      (octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
+        data: page === 1 ? queuedPrs : []
+      }));
+      (octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
+        data: {
+          merged: false,
+          head: { sha: 'sha' },
+          number: 123,
+          labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #2' }]
+        }
+      }));
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(false);
+      await manageMergeQueue();
+    });
+
+    it('should check for no commit status being published', () => {
+      expect(setCommitStatus).not.toHaveBeenCalledWith({
+        sha: 'sha',
+        context: MERGE_QUEUE_STATUS,
+        state: 'pending',
+        description: 'This PR is in line to merge.'
+      });
+    });
+
+    it('should add pr comment', () => {
+      expect(createPrComment).toHaveBeenCalled();
+    });
+  });
+
   describe('pr not ready for merge case', () => {
     const queuedPrs = [{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }, { labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }];
     beforeEach(async () => {
@@ -83,6 +119,7 @@ describe('manageMergeQueue', () => {
           labels: [{ name: 'QUEUED FOR MERGE #2' }]
         }
       }));
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue();
     });
 
@@ -118,6 +155,7 @@ describe('manageMergeQueue', () => {
           labels: [{ name: READY_FOR_MERGE_PR_LABEL }]
         }
       }));
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue();
     });
 
@@ -161,6 +199,7 @@ describe('manageMergeQueue', () => {
       (octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
         data: pullRequest
       }));
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue();
     });
 
@@ -204,6 +243,7 @@ describe('manageMergeQueue', () => {
         }
       }));
       (octokitGraphql as unknown as jest.Mock).mockRejectedValue(new Error('Auto merge is not allowed for this repo'));
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue();
     });
 
@@ -248,6 +288,7 @@ describe('manageMergeQueue', () => {
           labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #5' }]
         }
       }));
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue();
     });
 
@@ -277,6 +318,7 @@ describe('manageMergeQueue', () => {
           labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: JUMP_THE_QUEUE_PR_LABEL }, { name: 'QUEUED FOR MERGE #5' }]
         }
       }));
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue();
     });
 
@@ -301,7 +343,7 @@ describe('manageMergeQueue', () => {
           labels: [{ name: READY_FOR_MERGE_PR_LABEL }]
         }
       }));
-
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue({ login, slack_webhook_url });
 
       expect(notifyUser).toHaveBeenCalled();
@@ -325,6 +367,7 @@ describe('manageMergeQueue', () => {
           labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #5' }]
         }
       }));
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue({ login, slack_webhook_url });
 
       expect(notifyUser).not.toHaveBeenCalled();
@@ -342,7 +385,7 @@ describe('manageMergeQueue', () => {
           labels: [{ name: READY_FOR_MERGE_PR_LABEL }]
         }
       }));
-
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue({ login });
 
       expect(notifyUser).not.toHaveBeenCalled();
@@ -360,7 +403,7 @@ describe('manageMergeQueue', () => {
           labels: [{ name: READY_FOR_MERGE_PR_LABEL }]
         }
       }));
-
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue({ slack_webhook_url });
 
       expect(notifyUser).not.toHaveBeenCalled();
@@ -383,6 +426,7 @@ describe('manageMergeQueue', () => {
           labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #1' }]
         }
       }));
+      (approvalsSatisfied as jest.Mock).mockResolvedValue(true);
       await manageMergeQueue();
     });
-Original file line number
+Diff line change
@@ Expand Up / @@ -42180,13 +42180,15 @@ var map = { @@
 ,
 ,
 ,
+,
 ,
     	],
     	"./manage-merge-queue.ts": [
 ,
 ,
 ,
+,
 ,
     	],
@@ Expand Down Expand Up / @@ -42216,13 +42218,15 @@ var map = { @@
 ,
 ,
 ,
+,
 ,
     	],
     	"./prepare-queued-pr-for-merge.ts": [
 ,
 ,
 ,
+,
 ,
     	],
@@ Expand Down @@