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.

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

fix(remove-pr-from-merge-queue): clean up stray PRs #378

Merged
merged 2 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 0 additions & 9 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
token: ${{ secrets.GH_PERSONAL_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}

- name: Validate package.json
uses: ExpediaGroup/package-json-validator@v1
Expand Down Expand Up @@ -55,10 +51,5 @@ jobs:
if [[ $(git status --porcelain) ]]; then
echo "Detected uncommitted changes after build. Please run npm run package and commit the changes!"
git status
git config user.name "${{ github.actor }}"
git config user.email "${{ github.actor }}@users.noreply.github.com"
git add dist/
git commit -m "chore: committing generated dist" --no-verify
git push
exit 1
fi
22 changes: 16 additions & 6 deletions dist/974.index.js

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

2 changes: 1 addition & 1 deletion dist/974.index.js.map

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20196,11 +20196,13 @@ var map = {
"./remove-pr-from-merge-queue": [
6974,
832,
710,
974
],
"./remove-pr-from-merge-queue.ts": [
6974,
832,
710,
974
],
"./rerun-pr-checks": [
Expand Down
2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

19 changes: 14 additions & 5 deletions src/helpers/remove-pr-from-merge-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,34 @@ limitations under the License.
*/

import * as core from '@actions/core';
import { FIRST_QUEUED_PR_LABEL, READY_FOR_MERGE_PR_LABEL } from '../constants';
import { FIRST_QUEUED_PR_LABEL, QUEUED_FOR_MERGE_PREFIX, READY_FOR_MERGE_PR_LABEL } from '../constants';
import { HelperInputs } from '../types/generated';
import { context } from '@actions/github';
import { octokit } from '../octokit';
import { removeLabelIfExists } from './remove-label';
import { map } from 'bluebird';

export class RemovePrFromMergeQueue extends HelperInputs {
seconds = '';
}

export const removePrFromMergeQueue = async ({ seconds }: RemovePrFromMergeQueue) => {
const { data: prData } = await octokit.pulls.list({
const { data: pullRequests } = await octokit.pulls.list({
state: 'open',
per_page: 100,
...context.repo
});
const firstQueuedPr = prData.find(pr => pr.labels.some(label => label.name === FIRST_QUEUED_PR_LABEL));
const firstQueuedPr = pullRequests.find(pr => pr.labels.some(label => label.name === FIRST_QUEUED_PR_LABEL));
if (!firstQueuedPr) {
core.info('No PR is first in the merge queue.');
return;

return map(pullRequests, pr => {
const queueLabel = pr.labels.find(label => label.name.startsWith(QUEUED_FOR_MERGE_PREFIX));
if (queueLabel?.name) {
core.info(`Cleaning up PR with queue label ${queueLabel.name}...`);
return Promise.all([removeLabelIfExists(READY_FOR_MERGE_PR_LABEL, pr.number), removeLabelIfExists(queueLabel.name, pr.number)]);
}
});
}

const {
Expand All @@ -44,7 +52,8 @@ export const removePrFromMergeQueue = async ({ seconds }: RemovePrFromMergeQueue
});
const failingStatus = data.find(status => status.state === 'failure');
if (failingStatus && timestampIsStale(failingStatus.created_at, seconds)) {
return removeLabelIfExists(READY_FOR_MERGE_PR_LABEL, number);
core.info('Removing stale PR from first queued position...');
return Promise.all([removeLabelIfExists(READY_FOR_MERGE_PR_LABEL, number), removeLabelIfExists(FIRST_QUEUED_PR_LABEL, number)]);
}
};

Expand Down
51 changes: 48 additions & 3 deletions test/helpers/remove-pr-from-merge-queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { FIRST_QUEUED_PR_LABEL, READY_FOR_MERGE_PR_LABEL } from '../../src/constants';
import { FIRST_QUEUED_PR_LABEL, QUEUED_FOR_MERGE_PREFIX, READY_FOR_MERGE_PR_LABEL } from '../../src/constants';
import { Mocktokit } from '../types';
import { context } from '@actions/github';
import { octokit } from '../../src/octokit';
Expand Down Expand Up @@ -91,6 +91,7 @@ describe('removePrFromMergeQueue', () => {

it('should call removeLabelIfExists', () => {
expect(removeLabelIfExists).toHaveBeenCalledWith(READY_FOR_MERGE_PR_LABEL, 12345);
expect(removeLabelIfExists).toHaveBeenCalledWith(FIRST_QUEUED_PR_LABEL, 12345);
});
});

Expand Down Expand Up @@ -136,7 +137,7 @@ describe('removePrFromMergeQueue', () => {
});

describe('should not remove pr case with no failure status', () => {
beforeEach(() => {
beforeEach(async () => {
(octokit.repos.listCommitStatusesForRef as unknown as Mocktokit).mockImplementation(async () => ({
data: [
{
Expand All @@ -153,7 +154,7 @@ describe('removePrFromMergeQueue', () => {
}
]
}));
removePrFromMergeQueue({ seconds });
await removePrFromMergeQueue({ seconds });
});

it('should call pulls.list with correct params', () => {
Expand All @@ -175,4 +176,48 @@ describe('removePrFromMergeQueue', () => {
expect(removeLabelIfExists).not.toHaveBeenCalled();
});
});

describe('should remove stray PRs in the queue', () => {
beforeEach(async () => {
(octokit.pulls.list as unknown as Mocktokit).mockImplementation(async () => ({
data: [
{
head: { sha: 'wrong sha' },
number: 456,
labels: [{ name: 'test label' }]
},
{
number: 12345,
head: { sha: 'correct sha' },
labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: `${QUEUED_FOR_MERGE_PREFIX} #2` }]
},
{
number: 678,
head: { sha: 'correct sha' },
labels: [{ name: READY_FOR_MERGE_PR_LABEL }, { name: `${QUEUED_FOR_MERGE_PREFIX} #3` }]
}
]
}));
await removePrFromMergeQueue({ seconds });
});

it('should call pulls.list with correct params', () => {
expect(octokit.pulls.list).toHaveBeenCalledWith({
state: 'open',
per_page: 100,
...context.repo
});
});

it('should call listCommitStatusesForRef with correct params', () => {
expect(octokit.repos.listCommitStatusesForRef).not.toHaveBeenCalled();
});

it('should call removeLabelIfExists', () => {
expect(removeLabelIfExists).toHaveBeenCalledWith(READY_FOR_MERGE_PR_LABEL, 12345);
expect(removeLabelIfExists).toHaveBeenCalledWith(`${QUEUED_FOR_MERGE_PREFIX} #2`, 12345);
expect(removeLabelIfExists).toHaveBeenCalledWith(READY_FOR_MERGE_PR_LABEL, 678);
expect(removeLabelIfExists).toHaveBeenCalledWith(`${QUEUED_FOR_MERGE_PREFIX} #3`, 678);
});
});
});