Skip to content

Commit

Permalink
feat: allow manage-merge-queue to paginate all PRs (#156)
Browse files Browse the repository at this point in the history 
* fix: allow manage-merge-queue to handle multiple pages (>100 PRs) of PR results

* chore: add test

* chore: test fix

* chore: add tests

* chore: comitting generated dist

* use recursion

* chore: comitting generated dist

Co-authored-by: aschwenn <[email protected]>
Co-authored-by: dadajian <[email protected]>
Co-authored-by: danadajian <[email protected]>
Co-authored-by: Dan Adajian <[email protected]>
  • Loading branch information
@aschwenn @danadajian
5 people authored Mar 30, 2022
1 parent c1f637b commit 4501c7a
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 24 deletions.
9 changes: 8 additions & 1 deletion dist/473.index.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/473.index.js.map
View file

Large diffs are not rendered by default.

13 changes: 11 additions & 2 deletions src/helpers/manage-merge-queue.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,21 @@ const addPrToQueue = async (pullRequest: PullRequest, queuePosition: number) =>
});

const getQueuedPullRequests = async (): Promise<PullRequestList> => {
const { data: openPullRequests } = await octokit.pulls.list({
const openPullRequests = await paginateAllOpenPullRequests();
return openPullRequests.filter(pr => pr.labels.some(label => label.name === READY_FOR_MERGE_PR_LABEL));
};

const paginateAllOpenPullRequests = async (page = 1): Promise<PullRequestList> => {
const response = await octokit.pulls.list({
state: 'open',
sort: 'updated',
direction: 'desc',
per_page: 100,
page,
...context.repo
});
return openPullRequests.filter(pr => pr.labels.some(label => label.name === READY_FOR_MERGE_PR_LABEL));
if (!response.data.length) {
return [];
}
return response.data.concat(await paginateAllOpenPullRequests(page + 1));
};
95 changes: 75 additions & 20 deletions test/helpers/manage-merge-queue.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ describe('manageMergeQueue', () => {
describe('pr merged 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 () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand All @@ -68,8 +68,8 @@ describe('manageMergeQueue', () => {
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 () => {
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand All @@ -94,8 +94,8 @@ describe('manageMergeQueue', () => {
describe('pr ready for merge 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 () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand Down Expand Up @@ -128,8 +128,8 @@ describe('manageMergeQueue', () => {
describe('pr ready for merge case queued #1', () => {
const queuedPrs = [{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }];
beforeEach(async () => {
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand Down Expand Up @@ -168,8 +168,8 @@ describe('manageMergeQueue', () => {
{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }
];
beforeEach(async () => {
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand All @@ -196,8 +196,8 @@ describe('manageMergeQueue', () => {
{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }
];
beforeEach(async () => {
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand All @@ -220,8 +220,8 @@ describe('manageMergeQueue', () => {
const queuedPrs = [{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }];

it('should notify user if queue position 1', async () => {
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand All @@ -244,8 +244,8 @@ describe('manageMergeQueue', () => {
{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] },
{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }
];
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand All @@ -261,8 +261,8 @@ describe('manageMergeQueue', () => {

it('should not notify user if slack_webhook_url not provided', async () => {
const queuedPrs = [{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }];
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand All @@ -279,8 +279,8 @@ describe('manageMergeQueue', () => {

it('should not notify user if login not provided', async () => {
const queuedPrs = [{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }];
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async () => ({
data: queuedPrs
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrs : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
Expand All @@ -295,4 +295,59 @@ describe('manageMergeQueue', () => {
expect(notifyUser).not.toHaveBeenCalled();
});
});

describe('multiple pages of prs', () => {
const queuedPrsPage1 = [{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }, { labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }];
const queuedPrsPage2 = [{ labels: [] }, { labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }];
const queuedPrs = queuedPrsPage1.concat(queuedPrsPage2).filter(pr => pr.labels.some(label => label.name === READY_FOR_MERGE_PR_LABEL));
beforeEach(async () => {
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({
data: page === 1 ? queuedPrsPage1 : page === 2 ? queuedPrsPage2 : []
}));
(octokit.pulls.get as unknown as Mocktokit).mockImplementation(async () => ({
data: {
merged: true,
number: 123,
labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: 'QUEUED FOR MERGE #1' }]
}
}));
await manageMergeQueue();
});

it('should call pulls.list for multiple pages', () => {
expect(octokit.pulls.list).toHaveBeenCalledWith({
state: 'open',
sort: 'updated',
direction: 'desc',
per_page: 100,
page: 1,
...context.repo
});
expect(octokit.pulls.list).toHaveBeenCalledWith({
state: 'open',
sort: 'updated',
direction: 'desc',
per_page: 100,
page: 2,
...context.repo
});
expect(octokit.pulls.list).toHaveBeenCalledWith({
state: 'open',
sort: 'updated',
direction: 'desc',
per_page: 100,
page: 3,
...context.repo
});
});

it('should call remove label with correct params', () => {
expect(removeLabelIfExists).toHaveBeenCalledWith(READY_FOR_MERGE_PR_LABEL, 123);
expect(removeLabelIfExists).toHaveBeenCalledWith('QUEUED FOR MERGE #1', 123);
});

it('should call updateMergeQueue with correct params', () => {
expect(updateMergeQueue).toHaveBeenCalledWith(queuedPrs);
});
});
});

0 comments on commit 4501c7a

Please sign in to comment.