Skip to content

Commit

Permalink
Merge pull request #9643 from Expensify/Rory-IsPullRequestMergeablePR…
Browse files Browse the repository at this point in the history
…Checks
  • Loading branch information
roryabraham authored Jun 30, 2022
2 parents 8f0af89 + b083ef9 commit 6bc73a6
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 73 deletions.
87 changes: 59 additions & 28 deletions .github/actions/isPullRequestMergeable/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,43 +23,74 @@ const run = function () {
let isMergeable = false;
let mergeabilityResolved = false;
console.log(`Checking the mergeability of PR #${pullRequestNumber}`);
return promiseWhile(
() => !mergeabilityResolved && retryCount < MAX_RETRIES,
_.throttle(() => GithubUtils.octokit.pulls.get({
owner: GithubUtils.GITHUB_OWNER,
repo: GithubUtils.APP_REPO,
pull_number: pullRequestNumber,
})
.then(({data}) => {
if (_.isNull(data.mergeable)) {
console.log('Pull request mergeability is not yet resolved...');
retryCount++;
return;
}
return GithubUtils.octokit.pulls.get({
owner: GithubUtils.GITHUB_OWNER,
repo: GithubUtils.APP_REPO,
pull_number: pullRequestNumber,
})
.then(({data}) => data.head.sha)
.then(headRef => promiseWhile(
() => !mergeabilityResolved && retryCount < MAX_RETRIES,
_.throttle(
() => Promise.all([
GithubUtils.octokit.pulls.get({
owner: GithubUtils.GITHUB_OWNER,
repo: GithubUtils.APP_REPO,
pull_number: pullRequestNumber,
}),
GithubUtils.octokit.checks.listForRef({
owner: GithubUtils.GITHUB_OWNER,
repo: GithubUtils.APP_REPO,
ref: headRef,
}),
])
.then(([prResponse, checksResponse]) => {
const mergeable = prResponse.data.mergeable;
const mergeableState = prResponse.data.mergeable_state;
const areChecksComplete = _.every(checksResponse.data.check_runs, check => check.status === 'completed');

if (_.isNull(mergeable)) {
console.log('Pull request mergeability is not yet resolved...');
retryCount++;
return;
}

if (_.isEmpty(data.mergeable_state)) {
console.log('Pull request mergeable_state is not yet resolved...');
retryCount++;
return;
}
if (_.isEmpty(mergeableState)) {
console.log('Pull request mergeable_state is not yet resolved...');
retryCount++;
return;
}

mergeabilityResolved = true;
console.log(`Merge information for #${pullRequestNumber} - mergeable: ${data.mergeable}, mergeable_state: ${data.mergeable_state}`);
isMergeable = data.mergeable && data.mergeable_state.toUpperCase() !== 'BLOCKED';
})
.catch((githubError) => {
mergeabilityResolved = true;
console.error(`An error occurred fetching the PR from Github: ${JSON.stringify(githubError)}`);
core.setFailed(githubError);
}), THROTTLE_DURATION),
)
if (!areChecksComplete) {
console.log('Pull request checks are not yet complete...');
retryCount++;
return;
}

mergeabilityResolved = true;
console.log(`Merge information for #${pullRequestNumber} - mergeable: ${mergeable}, mergeable_state: ${mergeableState}`);
isMergeable = mergeable && mergeableState !== 'blocked';
})
.catch((githubError) => {
mergeabilityResolved = true;
console.error(`An error occurred fetching the PR from Github: ${JSON.stringify(githubError)}`);
core.setFailed(githubError);
}),
THROTTLE_DURATION,
),
))
.then(() => {
if (retryCount >= MAX_RETRIES) {
console.error('Maximum retries reached, mergeability is undetermined, defaulting to false');
} else {
console.log(`Pull request #${pullRequestNumber} is ${isMergeable ? '' : 'not '}mergeable`);
}
core.setOutput('IS_MERGEABLE', isMergeable);
})
.catch((githubError) => {
mergeabilityResolved = true;
console.error(`An error occurred fetching the PR from Github: ${JSON.stringify(githubError)}`);
core.setFailed(githubError);
});
};

Expand Down
91 changes: 61 additions & 30 deletions .github/actions/isPullRequestMergeable/isPullRequestMergeable.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,43 +13,74 @@ const run = function () {
let isMergeable = false;
let mergeabilityResolved = false;
console.log(`Checking the mergeability of PR #${pullRequestNumber}`);
return promiseWhile(
() => !mergeabilityResolved && retryCount < MAX_RETRIES,
_.throttle(() => GithubUtils.octokit.pulls.get({
owner: GithubUtils.GITHUB_OWNER,
repo: GithubUtils.APP_REPO,
pull_number: pullRequestNumber,
})
.then(({data}) => {
if (_.isNull(data.mergeable)) {
console.log('Pull request mergeability is not yet resolved...');
retryCount++;
return;
}

if (_.isEmpty(data.mergeable_state)) {
console.log('Pull request mergeable_state is not yet resolved...');
retryCount++;
return;
}

mergeabilityResolved = true;
console.log(`Merge information for #${pullRequestNumber} - mergeable: ${data.mergeable}, mergeable_state: ${data.mergeable_state}`);
isMergeable = data.mergeable && data.mergeable_state.toUpperCase() !== 'BLOCKED';
})
.catch((githubError) => {
mergeabilityResolved = true;
console.error(`An error occurred fetching the PR from Github: ${JSON.stringify(githubError)}`);
core.setFailed(githubError);
}), THROTTLE_DURATION),
)
return GithubUtils.octokit.pulls.get({
owner: GithubUtils.GITHUB_OWNER,
repo: GithubUtils.APP_REPO,
pull_number: pullRequestNumber,
})
.then(({data}) => data.head.sha)
.then(headRef => promiseWhile(
() => !mergeabilityResolved && retryCount < MAX_RETRIES,
_.throttle(
() => Promise.all([
GithubUtils.octokit.pulls.get({
owner: GithubUtils.GITHUB_OWNER,
repo: GithubUtils.APP_REPO,
pull_number: pullRequestNumber,
}),
GithubUtils.octokit.checks.listForRef({
owner: GithubUtils.GITHUB_OWNER,
repo: GithubUtils.APP_REPO,
ref: headRef,
}),
])
.then(([prResponse, checksResponse]) => {
const mergeable = prResponse.data.mergeable;
const mergeableState = prResponse.data.mergeable_state;
const areChecksComplete = _.every(checksResponse.data.check_runs, check => check.status === 'completed');

if (_.isNull(mergeable)) {
console.log('Pull request mergeability is not yet resolved...');
retryCount++;
return;
}

if (_.isEmpty(mergeableState)) {
console.log('Pull request mergeable_state is not yet resolved...');
retryCount++;
return;
}

if (!areChecksComplete) {
console.log('Pull request checks are not yet complete...');
retryCount++;
return;
}

mergeabilityResolved = true;
console.log(`Merge information for #${pullRequestNumber} - mergeable: ${mergeable}, mergeable_state: ${mergeableState}`);
isMergeable = mergeable && mergeableState !== 'blocked';
})
.catch((githubError) => {
mergeabilityResolved = true;
console.error(`An error occurred fetching the PR from Github: ${JSON.stringify(githubError)}`);
core.setFailed(githubError);
}),
THROTTLE_DURATION,
),
))
.then(() => {
if (retryCount >= MAX_RETRIES) {
console.error('Maximum retries reached, mergeability is undetermined, defaulting to false');
} else {
console.log(`Pull request #${pullRequestNumber} is ${isMergeable ? '' : 'not '}mergeable`);
}
core.setOutput('IS_MERGEABLE', isMergeable);
})
.catch((githubError) => {
mergeabilityResolved = true;
console.error(`An error occurred fetching the PR from Github: ${JSON.stringify(githubError)}`);
core.setFailed(githubError);
});
};

Expand Down
55 changes: 40 additions & 15 deletions tests/unit/isPullRequestMergeableTest.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

const core = require('@actions/core');
const GithubUtils = require('../../.github/libs/GithubUtils');
const run = require('../../.github/actions/isPullRequestMergeable/isPullRequestMergeable');
Expand All @@ -18,6 +17,8 @@ const mockGetInput = jest.fn().mockImplementation((arg) => {
const mockSetOutput = jest.fn();
const mockSetFailed = jest.fn();
const mockGetPullRequest = jest.fn();
const mockListChecks = jest.fn();
const mockError = new Error('Some GitHub error');

beforeAll(() => {
// Mock core module
Expand All @@ -30,6 +31,9 @@ beforeAll(() => {
pulls: {
get: mockGetPullRequest,
},
checks: {
listForRef: mockListChecks,
},
};
GithubUtils.octokitInternal = mocktokit;
});
Expand All @@ -38,6 +42,7 @@ afterEach(() => {
mockSetOutput.mockClear();
mockSetFailed.mockClear();
mockGetPullRequest.mockClear();
mockListChecks.mockClear();
});

afterAll(() => {
Expand All @@ -46,54 +51,74 @@ afterAll(() => {

describe('isPullRequestMergeable', () => {
test('Pull request immediately mergeable', () => {
mockGetPullRequest.mockResolvedValue({data: {mergeable: true, mergeable_state: 'CLEAN'}});
mockGetPullRequest.mockResolvedValue({data: {mergeable: true, mergeable_state: 'clean', head: {sha: 'abcd'}}});
mockListChecks.mockResolvedValue({data: {check_runs: [{status: 'completed'}, {status: 'completed'}]}});
return run().then(() => {
expect(mockSetOutput).toHaveBeenCalledWith('IS_MERGEABLE', true);
});
});

test('Pull request immediately not mergeable', () => {
mockGetPullRequest.mockResolvedValue({data: {mergeable: false, mergeable_state: 'BLOCKED'}});
mockGetPullRequest.mockResolvedValue({data: {mergeable: false, mergeable_state: 'blocked', head: {sha: 'abcd'}}});
mockListChecks.mockResolvedValue({data: {check_runs: [{status: 'completed'}, {status: 'completed'}]}});
return run().then(() => {
expect(mockSetOutput).toHaveBeenCalledWith('IS_MERGEABLE', false);
});
});

test('Pull request mergeable after delay', () => {
mockGetPullRequest
.mockResolvedValue({data: {mergeable: true, mergeable_state: 'CLEAN'}})
.mockResolvedValueOnce({data: {mergeable: null}})
.mockResolvedValueOnce({data: {mergeable: null}});
.mockResolvedValue({data: {mergeable: true, mergeable_state: 'clean', head: {sha: 'abcd'}}})
.mockResolvedValueOnce({data: {mergeable: null, mergeable_state: 'blocked', head: {sha: 'abcd'}}}) // first response
.mockResolvedValueOnce({data: {mergeable: null, mergeable_state: 'blocked', head: {sha: 'abcd'}}}) // second response
.mockResolvedValueOnce({data: {mergeable: true, mergeable_state: 'blocked', head: {sha: 'abcd'}}}); // third response
mockListChecks
.mockResolvedValue({data: {check_runs: [{status: 'completed'}, {status: 'completed'}]}})
.mockResolvedValueOnce({data: {check_runs: [{status: 'in_progress'}, {status: 'in_progress'}]}}) // first response
.mockResolvedValueOnce({data: {check_runs: [{status: 'completed'}, {status: 'in_progress'}]}}); // second response
return run().then(() => {
expect(mockGetPullRequest).toHaveBeenCalledTimes(3);
expect(mockGetPullRequest).toHaveBeenCalledTimes(4);
expect(mockListChecks).toHaveBeenCalledTimes(3);
expect(mockSetOutput).toHaveBeenCalledWith('IS_MERGEABLE', true);
});
});

test('Pull request not mergeable after delay', () => {
mockGetPullRequest
.mockResolvedValue({data: {mergeable: false, mergeable_state: 'BLOCKED'}})
.mockResolvedValueOnce({data: {mergeable: null}})
.mockResolvedValueOnce({data: {mergeable: null}});
.mockResolvedValue({data: {mergeable: false, mergeable_state: 'blocked', head: {sha: 'abcd'}}})
.mockResolvedValueOnce({data: {mergeable: null, head: {sha: 'abcd'}}})
.mockResolvedValueOnce({data: {mergeable: null, head: {sha: 'abcd'}}});
mockListChecks.mockResolvedValue({data: {check_runs: [{status: 'completed'}]}});
return run().then(() => {
expect(mockGetPullRequest).toHaveBeenCalledTimes(3);
expect(mockListChecks).toHaveBeenCalledTimes(2);
expect(mockSetOutput).toHaveBeenCalledWith('IS_MERGEABLE', false);
});
});

test('Pull request mergeability never resolves', () => {
mockGetPullRequest
.mockResolvedValue({data: {mergeable: null}});
.mockResolvedValue({data: {mergeable: null, head: {sha: 'abcd'}}});
mockListChecks.mockResolvedValue({data: {check_runs: [{status: 'completed'}]}});
return run().then(() => {
expect(mockGetPullRequest).toHaveBeenCalledTimes(30);
expect(mockGetPullRequest).toHaveBeenCalledTimes(31);
expect(mockListChecks).toHaveBeenCalledTimes(30);
expect(mockSetOutput).toHaveBeenCalledWith('IS_MERGEABLE', false);
});
});

test('Github API error', () => {
mockGetPullRequest.mockRejectedValue(new Error('Some github error'));
test('Github API error on first request', () => {
mockGetPullRequest.mockRejectedValue(mockError);
return run().then(() => {
expect(mockSetFailed).toHaveBeenCalledWith(mockError);
});
});

test('GitHub API error on later request', () => {
mockGetPullRequest.mockResolvedValue({data: {mergeable: null, head: {sha: 'abcd'}}});
mockListChecks.mockRejectedValue(mockError);
return run().then(() => {
expect(mockSetFailed).toHaveBeenCalledWith(new Error('Some github error'));
expect(mockSetFailed).toHaveBeenCalledWith(mockError);
});
});
});

0 comments on commit 6bc73a6

Please sign in to comment.