Skip to content

Commit

Permalink
feat: handle one merge queue by repository
Browse files Browse the repository at this point in the history
  • Loading branch information
VincentHardouin committed Dec 18, 2024
1 parent a072a28 commit 07c63f7
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 46 deletions.
17 changes: 10 additions & 7 deletions build/controllers/github.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,12 @@ async function processWebhook(
return handleRA(request);
}
if (request.payload.action === 'closed') {
const repositoryName = request.payload.repository.full_name;
await pullRequestRepository.remove({
number: request.payload.number,
repositoryName: request.payload.repository.full_name,
repositoryName,
});
await mergeQueue();
await mergeQueue({ repositoryName });
return handleCloseRA(request);
}
if (request.payload.action === 'labeled' && request.payload.label.name == 'no-review-app') {
Expand All @@ -283,24 +284,26 @@ async function processWebhook(
number: request.payload.number,
repositoryName,
});
await mergeQueue();
await mergeQueue({ repositoryName });
}
}
if (request.payload.action === 'unlabeled' && request.payload.label.name === ':rocket: Ready to Merge') {
const repositoryName = request.payload.repository.full_name;
await pullRequestRepository.remove({
number: request.payload.number,
repositoryName: request.payload.repository.full_name,
repositoryName,
});
await mergeQueue();
await mergeQueue({ repositoryName });
}
return `Ignoring ${request.payload.action} action`;
} else if (eventName === 'check_suite') {
if (request.payload.action === 'completed' && request.payload.check_suite.conclusion !== 'success') {
const repositoryName = request.payload.repository.full_name;
await pullRequestRepository.remove({
number: request.payload.pull_requests[0].number,
repositoryName: request.payload.repository.full_name,
repositoryName,
});
await mergeQueue();
await mergeQueue({ repositoryName });
}
} else {
return `Ignoring ${eventName} event`;
Expand Down
2 changes: 1 addition & 1 deletion build/controllers/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const mergeController = {
const [organisation, repository, pullRequestNumber] = pullRequest.split('/');
const repositoryName = `${organisation}/${repository}`;
await dependencies.pullRequestRepository.remove({ number: Number(pullRequestNumber), repositoryName });
await dependencies.mergeQueue();
await dependencies.mergeQueue({ repositoryName });
return h.response().code(204);
},
};
Expand Down
8 changes: 4 additions & 4 deletions build/repositories/pull-request-repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ async function save({ number, repositoryName }) {
return knex('pull_requests').insert({ number, repositoryName });
}

async function isAtLeastOneMergeInProgress() {
async function isAtLeastOneMergeInProgress(repositoryName) {
const isAtLeastOneMergeInProgress = await knex('pull_requests')
.select('isMerging')
.where({ isMerging: true })
.where({ isMerging: true, repositoryName })
.first();
return Boolean(isAtLeastOneMergeInProgress);
}

async function getOldest() {
return knex('pull_requests').where({ isMerging: false }).orderBy('createdAt', 'asc').first();
async function getOldest(repositoryName) {
return knex('pull_requests').where({ isMerging: false, repositoryName }).orderBy('createdAt', 'asc').first();
}

async function update({ number, repositoryName, isMerging }) {
Expand Down
5 changes: 3 additions & 2 deletions build/services/merge-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ import * as _pullRequestRepository from '../repositories/pull-request-repository
import _githubService from '../../common/services/github.js';

export async function mergeQueue({
repositoryName,
pullRequestRepository = _pullRequestRepository,
githubService = _githubService,
} = {}) {
const isAtLeastOneMergeInProgress = await pullRequestRepository.isAtLeastOneMergeInProgress();
const isAtLeastOneMergeInProgress = await pullRequestRepository.isAtLeastOneMergeInProgress(repositoryName);
if (isAtLeastOneMergeInProgress) {
return;
}

const pr = await pullRequestRepository.getOldest();
const pr = await pullRequestRepository.getOldest(repositoryName);
if (!pr) {
return;
}
Expand Down
36 changes: 24 additions & 12 deletions test/integration/build/repositories/pull-request-repository_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,24 @@ describe('PullRequestRepository', function () {
});

describe('#isAtLeastOneMergeInProgress', function () {
context('when there is at least one pr in merging', function () {
context('when there is at least one pr in merging for specify repository', function () {
it('should return true', async function () {
await knex('pull_requests').insert({ number: 123, repositoryName: 'pix-sample-repo', isMerging: true });
const repositoryName = '1024pix/pix-sample-repo';
await knex('pull_requests').insert({ number: 123, repositoryName, isMerging: true });

const isMerging = await pullRequestRepository.isAtLeastOneMergeInProgress();
const isMerging = await pullRequestRepository.isAtLeastOneMergeInProgress(repositoryName);

expect(isMerging).to.be.true;
});
});

context('when no pr are currently in progress', function () {
context('when no pr are currently in progress for specify repository', function () {
it('should return false', async function () {
await knex('pull_requests').insert({ number: 123, repositoryName: 'pix-sample-repo', isMerging: false });
const repositoryName = '1024pix/pix-sample-repo';
await knex('pull_requests').insert({ number: 123, repositoryName, isMerging: false });
await knex('pull_requests').insert({ number: 123, repositoryName: '1024pix/another-repo', isMerging: true });

const isMerging = await pullRequestRepository.isAtLeastOneMergeInProgress();
const isMerging = await pullRequestRepository.isAtLeastOneMergeInProgress(repositoryName);

expect(isMerging).to.be.false;
});
Expand All @@ -50,34 +53,43 @@ describe('PullRequestRepository', function () {
describe('#getOldest', function () {
context('when there is at least one pr', function () {
it('should return oldest pull request', async function () {
const repositoryName = '1024pix/pix-sample-repo';
await knex('pull_requests').insert({
number: 123,
repositoryName: 'pix-sample-repo',
repositoryName,
isMerging: true,
createdAt: new Date('2024-01-01'),
});
await knex('pull_requests').insert({
number: 456,
repositoryName: 'pix-sample-repo',
repositoryName,
isMerging: false,
createdAt: new Date('2024-01-02'),
});
await knex('pull_requests').insert({
number: 789,
repositoryName: 'pix-sample-repo',
repositoryName,
isMerging: false,
createdAt: new Date('2024-02-02'),
});

const oldestPR = await pullRequestRepository.getOldest();
const oldestPR = await pullRequestRepository.getOldest(repositoryName);

expect(oldestPR.number).to.be.equal(456);
});
});

context('when there are no pr', function () {
context('when there are no pr for given repository', function () {
it('should return undefined', async function () {
const oldestPR = await pullRequestRepository.getOldest();
const repositoryWithoutPRInMergeQueue = '1024pix/repo-under-test';
await knex('pull_requests').insert({
number: 123,
repositoryName: '1024pix/another-repo',
isMerging: false,
createdAt: new Date('2024-01-01'),
});

const oldestPR = await pullRequestRepository.getOldest(repositoryWithoutPRInMergeQueue);

expect(oldestPR).to.be.undefined;
});
Expand Down
30 changes: 17 additions & 13 deletions test/unit/build/controllers/github_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,15 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
describe('when repo is allowed', function () {
it('should call pullRequestRepository.save() method', async function () {
// given
const repositoryName = '1024pix/pix-sample-repo';
sinon.stub(config.github, 'automerge').value({
allowedRepositories: ['1024pix/pix-sample-repo'],
allowedRepositories: [repositoryName],
});
sinon.stub(request, 'payload').value({
action: 'labeled',
number: 123,
label: { name: ':rocket: Ready to Merge' },
repository: { full_name: '1024pix/pix-sample-repo' },
repository: { full_name: repositoryName },
sender: {
login: 'ouiiiii',
},
Expand All @@ -205,10 +206,10 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
// then
expect(pullRequestRepository.save).to.be.calledOnceWithExactly({
number: 123,
repositoryName: '1024pix/pix-sample-repo',
repositoryName,
});

expect(mergeQueue).to.be.calledOnce;
expect(mergeQueue).to.be.calledOnceWithExactly({ repositoryName });
});
});

Expand Down Expand Up @@ -247,11 +248,12 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
describe('when label is Ready-to-merge', function () {
it('should call pullRequestRepository.remove() method', async function () {
// given
const repositoryName = '1024pix/pix-sample-repo';
sinon.stub(request, 'payload').value({
action: 'unlabeled',
number: 123,
label: { name: ':rocket: Ready to Merge' },
repository: { full_name: 'pix-sample-repo' },
repository: { full_name: repositoryName },
});

const mergeQueue = sinon.stub();
Expand All @@ -263,21 +265,22 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
// then
expect(pullRequestRepository.remove).to.be.calledOnceWithExactly({
number: 123,
repositoryName: 'pix-sample-repo',
repositoryName: repositoryName,
});

expect(mergeQueue).to.be.calledOnce;
expect(mergeQueue).to.be.calledOnceWithExactly({ repositoryName });
});
});
});

describe('when action is `closed`', function () {
it('should call pullRequestRepository.remove() method', async function () {
// given
const repositoryName = '1024pix/pix-sample-repo';
sinon.stub(request, 'payload').value({
action: 'closed',
number: 123,
repository: { full_name: '1024pix/pix-sample-repo' },
repository: { full_name: repositoryName },
});

const handleCloseRA = sinon.stub();
Expand All @@ -290,10 +293,10 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
// then
expect(pullRequestRepository.remove).to.be.calledOnceWithExactly({
number: 123,
repositoryName: '1024pix/pix-sample-repo',
repositoryName,
});
expect(handleCloseRA.calledOnceWithExactly(request)).to.be.true;
expect(mergeQueue).to.be.calledOnce;
expect(mergeQueue).to.be.calledOnceWithExactly({ repositoryName });
});
});

Expand All @@ -312,14 +315,15 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
describe('when event is check_suite', function () {
describe("when action is 'completed' and conclusion is not 'success'", function () {
it('should call pullRequestRepository.remove() method', async function () {
const repositoryName = '1024pix/pix-sample-repo';
const request = {
headers: {
'x-github-event': 'check_suite',
},
payload: {
action: 'completed',
pull_requests: [{ number: 123 }],
repository: { full_name: 'pix-sample-repo' },
repository: { full_name: repositoryName },
check_suite: { conclusion: 'failure' },
},
};
Expand All @@ -333,9 +337,9 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
// then
expect(pullRequestRepository.remove).to.be.calledOnceWithExactly({
number: 123,
repositoryName: 'pix-sample-repo',
repositoryName,
});
expect(mergeQueue).to.be.calledOnce;
expect(mergeQueue).to.be.calledOnceWithExactly({ repositoryName });
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/unit/build/controllers/merge_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('Unit | Controller | Merge', function () {
repositoryName: '1024pix/pix',
});

expect(mergeQueue).to.be.calledOnce;
expect(mergeQueue).to.be.calledOnceWithExactly({ repositoryName: '1024pix/pix'});
expect(hStub.response).to.be.calledOnce;
});
});
Expand Down
14 changes: 8 additions & 6 deletions test/unit/run/services/merge-queue_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,39 @@ import { config } from '../../../../config.js';
describe('Unit | Build | Services | merge-queue', function () {
context('when there is PR in merging', function () {
it('should do nothing', async function () {
const repositoryName = Symbol('repository-name');
const pullRequestRepository = {
isAtLeastOneMergeInProgress: sinon.stub(),
};
pullRequestRepository.isAtLeastOneMergeInProgress.resolves(true);
pullRequestRepository.isAtLeastOneMergeInProgress.withArgs(repositoryName).resolves(true);

await mergeQueue({ pullRequestRepository });
await mergeQueue({ repositoryName, pullRequestRepository });

expect(pullRequestRepository.isAtLeastOneMergeInProgress).to.have.been.called;
});
});

context('when there is no PR in merging', function () {
it('should get oldest pull requests, mark as currently merging and trigger auto-merge', async function () {
const repositoryName = 'foo/pix-project';
const pr = {
number: 1,
repositoryName: 'foo/pix-project',
repositoryName,
isMerging: false,
};
const pullRequestRepository = {
isAtLeastOneMergeInProgress: sinon.stub(),
getOldest: sinon.stub(),
update: sinon.stub(),
};
pullRequestRepository.isAtLeastOneMergeInProgress.resolves(false);
pullRequestRepository.getOldest.resolves(pr);
pullRequestRepository.isAtLeastOneMergeInProgress.withArgs(repositoryName).resolves(false);
pullRequestRepository.getOldest.withArgs(repositoryName).resolves(pr);

const githubService = {
triggerWorkflow: sinon.stub(),
};

await mergeQueue({ pullRequestRepository, githubService });
await mergeQueue({ repositoryName, pullRequestRepository, githubService });

expect(pullRequestRepository.isAtLeastOneMergeInProgress).to.have.been.called;
expect(pullRequestRepository.update).to.have.been.calledWithExactly({
Expand Down

0 comments on commit 07c63f7

Please sign in to comment.