fix(manage-merge-queue): improve jump queue handling (#650)

ExpediaGroup · Aug 7, 2024 · e61057f · e61057f
1 parent e16535a
commit e61057f
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 13 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/src/utils/update-merge-queue.ts b/src/utils/update-merge-queue.ts
@@ -29,27 +29,40 @@ const sortPrsByQueuePosition = (queuedPrs: PullRequestList) =>
   queuedPrs
     .map(pr => {
       const label = pr.labels.find(label => label.name?.startsWith(QUEUED_FOR_MERGE_PREFIX))?.name;
-      const isJumpingTheQueue = Boolean(pr.labels.find(label => label.name === JUMP_THE_QUEUE_PR_LABEL));
-      const queuePosition = isJumpingTheQueue ? 0 : Number(label?.split('#')?.[1]);
+      const hasJumpTheQueueLabel = Boolean(pr.labels.find(label => label.name === JUMP_THE_QUEUE_PR_LABEL));
+      const queuePosition = Number(label?.split('#')?.[1]);
       return {
         number: pr.number,
         label,
+        hasJumpTheQueueLabel,
         queuePosition,
         sha: pr.head.sha
       };
     })
-    .sort((pr1, pr2) => pr1.queuePosition - pr2.queuePosition);
+    .sort((pr1, pr2) => {
+      if (pr1.hasJumpTheQueueLabel) {
+        return -1;
+      }
+      if (pr2.hasJumpTheQueueLabel) {
+        return 1;
+      }
+      return pr1.queuePosition - pr2.queuePosition;
+    });
 
 const updateQueuePosition = async (pr: ReturnType<typeof sortPrsByQueuePosition>[number], index: number) => {
-  const { number, label, queuePosition, sha } = pr;
+  const { number, label, queuePosition, sha, hasJumpTheQueueLabel } = pr;
   const newQueuePosition = index + 1;
   if (!label || isNaN(queuePosition) || queuePosition === newQueuePosition) {
     return;
   }
+  if (hasJumpTheQueueLabel) {
+    await removeLabelIfExists(JUMP_THE_QUEUE_PR_LABEL, number);
+  }
+
   const prIsNowFirstInQueue = newQueuePosition === 1;
   if (prIsNowFirstInQueue) {
     const { data: firstPrInQueue } = await octokit.pulls.get({ pull_number: number, ...context.repo });
-    await Promise.all([removeLabelIfExists(JUMP_THE_QUEUE_PR_LABEL, number), updatePrWithDefaultBranch(firstPrInQueue)]);
+    await updatePrWithDefaultBranch(firstPrInQueue);
     const {
       data: {
         head: { sha: updatedHeadSha }

diff --git a/test/utils/update-merge-queue.test.ts b/test/utils/update-merge-queue.test.ts
@@ -11,7 +11,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { JUMP_THE_QUEUE_PR_LABEL, MERGE_QUEUE_STATUS } from '../../src/constants';
+import { JUMP_THE_QUEUE_PR_LABEL, MERGE_QUEUE_STATUS, READY_FOR_MERGE_PR_LABEL } from '../../src/constants';
 import { Mocktokit } from '../types';
 import { PullRequestList } from '../../src/types/github';
 import { context } from '@actions/github';
@@ -40,7 +40,7 @@ jest.mock('@actions/github', () => ({
 }));
 (octokit.pulls.get as unknown as Mocktokit).mockImplementation(async input => ({
   data: {
-    head: { sha: input.pull_number === 123 ? 'sha123' : 'sha456' }
+    head: { sha: `sha${input.pull_number}` }
   }
 }));
 
@@ -247,4 +247,78 @@ describe('updateMergeQueue', () => {
       expect(updatePrWithDefaultBranch).toHaveBeenCalledWith({ head: { sha: 'sha123' } });
     });
   });
+
+  describe('should jump the queue with the pr most ahead in the queue when multiple prs have jump the queue label', () => {
+    const queuedPrs = [
+      {
+        head: { sha: 'sha123' },
+        number: 123,
+        labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #1' }]
+      },
+      {
+        head: { sha: 'sha456' },
+        number: 456,
+        labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: JUMP_THE_QUEUE_PR_LABEL }, { name: 'QUEUED FOR MERGE #3' }]
+      },
+      {
+        head: { sha: 'sha789' },
+        number: 789,
+        labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: JUMP_THE_QUEUE_PR_LABEL }, { name: 'QUEUED FOR MERGE #2' }]
+      }
+    ];
+    beforeEach(async () => {
+      await updateMergeQueue(queuedPrs as PullRequestList);
+    });
+
+    it('should call add labels with correct params', () => {
+      expect(octokit.issues.addLabels).toHaveBeenCalledWith({
+        labels: ['QUEUED FOR MERGE #1'],
+        issue_number: 789,
+        ...context.repo
+      });
+      expect(octokit.issues.addLabels).toHaveBeenCalledWith({
+        labels: ['QUEUED FOR MERGE #2'],
+        issue_number: 456,
+        ...context.repo
+      });
+      expect(octokit.issues.addLabels).toHaveBeenCalledWith({
+        labels: ['QUEUED FOR MERGE #3'],
+        issue_number: 123,
+        ...context.repo
+      });
+    });
+
+    it('should call remove label with correct params', () => {
+      expect(removeLabelIfExists).toHaveBeenCalledWith(JUMP_THE_QUEUE_PR_LABEL, 456);
+      expect(removeLabelIfExists).toHaveBeenCalledWith(JUMP_THE_QUEUE_PR_LABEL, 789);
+      expect(removeLabelIfExists).toHaveBeenCalledWith('QUEUED FOR MERGE #1', 123);
+      expect(removeLabelIfExists).toHaveBeenCalledWith('QUEUED FOR MERGE #3', 456);
+      expect(removeLabelIfExists).toHaveBeenCalledWith('QUEUED FOR MERGE #2', 789);
+    });
+
+    it('should set the correct commit statuses', () => {
+      expect(setCommitStatus).toHaveBeenCalledWith({
+        sha: 'sha789',
+        context: MERGE_QUEUE_STATUS,
+        state: 'success',
+        description: 'This PR is next to merge.'
+      });
+      expect(setCommitStatus).toHaveBeenCalledWith({
+        sha: 'sha456',
+        context: MERGE_QUEUE_STATUS,
+        state: 'pending',
+        description: 'This PR is in line to merge.'
+      });
+      expect(setCommitStatus).toHaveBeenCalledWith({
+        sha: 'sha123',
+        context: MERGE_QUEUE_STATUS,
+        state: 'pending',
+        description: 'This PR is in line to merge.'
+      });
+    });
+
+    it('should call updatePrWithDefaultBranch', () => {
+      expect(updatePrWithDefaultBranch).toHaveBeenCalledWith({ head: { sha: 'sha789' } });
+    });
+  });
 });