Skip to content

Commit 43e43f5

Browse files
authored
fix(manage-merge-queue): don't remove queued pr if queue is full (#698)
1 parent 3e6f052 commit 43e43f5

File tree

4 files changed

+45
-7
lines changed

4 files changed

+45
-7
lines changed

dist/284.index.js

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/284.index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/helpers/manage-merge-queue.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,15 @@ export const manageMergeQueue = async ({
7777

7878
const queuedPrs = await getQueuedPullRequests();
7979
const queuePosition = queuedPrs.length + 1;
80+
const prAttemptingToJoinQueue = pullRequest.labels.every(label => !label.name?.startsWith(QUEUED_FOR_MERGE_PREFIX));
8081

81-
if (queuePosition > Number(max_queue_size)) {
82+
if (prAttemptingToJoinQueue && queuePosition > Number(max_queue_size)) {
8283
await createPrComment({
8384
body: `The merge queue is full! Only ${max_queue_size} PRs are allowed in the queue at a time.\n\nIf you would like to merge your PR, please monitor the PRs in the queue and make sure the authors are around to merge them.`
8485
});
8586
return removePrFromQueue(pullRequest);
8687
}
88+
8789
if (pullRequest.labels.find(label => label.name === JUMP_THE_QUEUE_PR_LABEL)) {
8890
if (allow_only_for_maintainers === 'true') {
8991
core.info(`Checking if user ${login} is a maintainer...`);
@@ -99,8 +101,7 @@ export const manageMergeQueue = async ({
99101
return updateMergeQueue(queuedPrs);
100102
}
101103

102-
const prIsAlreadyInTheQueue = pullRequest.labels.find(label => label.name?.startsWith(QUEUED_FOR_MERGE_PREFIX));
103-
if (!prIsAlreadyInTheQueue) {
104+
if (prAttemptingToJoinQueue) {
104105
await addPrToQueue(pullRequest, queuePosition, skip_auto_merge);
105106
}
106107

test/helpers/manage-merge-queue.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,43 @@ describe('manageMergeQueue', () => {
710710
});
711711
});
712712

713+
describe('should not remove queued pr when queue is maxed out', () => {
714+
const prUnderTest = {
715+
number: 2,
716+
merged: false,
717+
head: { sha: 'sha' },
718+
labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #2' }]
719+
};
720+
const openPrs = [
721+
prUnderTest,
722+
{ number: 1, labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #1' }] },
723+
{ number: 2, labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #2' }] },
724+
{ number: 3, labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #3' }] }
725+
];
726+
beforeEach(async () => {
727+
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
728+
data: page === 1 ? openPrs : []
729+
}));
730+
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
731+
data: prUnderTest
732+
}));
733+
(approvalsSatisfied as jest.Mock).mockResolvedValue(true);
734+
await manageMergeQueue({
735+
max_queue_size: '3'
736+
});
737+
});
738+
739+
it('should not remove queued pr when queue is full', () => {
740+
expect(createPrComment).not.toHaveBeenCalled();
741+
expect(setCommitStatus).toHaveBeenCalledWith({
742+
sha: 'sha',
743+
context: MERGE_QUEUE_STATUS,
744+
state: 'pending',
745+
description: 'This PR is in line to merge.'
746+
});
747+
});
748+
});
749+
713750
describe('fewer than max prs in the queue', () => {
714751
const prUnderTest = {
715752
number: 1,

0 commit comments

Comments
 (0)