Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[No QA] Wait for checks to complete before giving up due to mergeable_state blocked #9643

Merged
merged 2 commits into from
Jun 30, 2022
Merged

[No QA] Wait for checks to complete before giving up due to mergeable_state blocked #9643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fbed15b
Wait for checks to complete before giving up due to mergeable_state b…
roryabraham Jun 30, 2022
b083ef9
Recompile GH Actions
roryabraham Jun 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 59 additions & 28 deletions .github/actions/isPullRequestMergeable/index.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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);
});
});
});