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

[FEATURE] Indiquer dans les PRs qu'elles sont en cours de merge #501

Merged
merged 3 commits into from
Dec 31, 2024
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
6 changes: 3 additions & 3 deletions build/repositories/pull-request-repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ async function isAtLeastOneMergeInProgress(repositoryName) {
return Boolean(isAtLeastOneMergeInProgress);
}

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

async function update({ number, repositoryName, isMerging }) {
Expand All @@ -24,4 +24,4 @@ async function remove({ number, repositoryName }) {
await knex('pull_requests').where({ number, repositoryName }).delete();
}

export { save, isAtLeastOneMergeInProgress, getOldest, update, remove };
export { save, isAtLeastOneMergeInProgress, findNotMerged, update, remove };
25 changes: 21 additions & 4 deletions build/services/merge-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ export class MergeQueue {
return;
}

const pr = await this.#pullRequestRepository.getOldest(repositoryName);
if (!pr) {
const pr = await this.#pullRequestRepository.findNotMerged(repositoryName);
if (pr.length === 0) {
return;
}

const nextMergingPr = pr[0];
await this.#pullRequestRepository.update({
...pr,
...nextMergingPr,
isMerging: true,
});
await this.#githubService.triggerWorkflow({
Expand All @@ -34,9 +35,25 @@ export class MergeQueue {
ref: config.github.automerge.workflowRef,
},
inputs: {
pullRequest: `${pr.repositoryName}/${pr.number}`,
pullRequest: `${nextMergingPr.repositoryName}/${nextMergingPr.number}`,
},
});

await this.#githubService.setMergeQueueStatus({
status: 'pending',
description: 'En cours de merge',
repositoryFullName: repositoryName,
prNumber: pr[0].number,
});

for (let i = 1; i < pr.length; i++) {
await this.#githubService.setMergeQueueStatus({
status: 'pending',
description: `${i + 1}/${pr.length} dans la file d'attente`,
repositoryFullName: repositoryName,
prNumber: pr[i].number,
});
}
}

async managePullRequest({ repositoryName, number }) {
Expand Down
35 changes: 28 additions & 7 deletions common/services/github.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,28 +359,48 @@ async function _getPullRequestsDetailsByCommitShas({ repoOwner, repoName, commit
return pullRequestsForCommitShaFilteredDetails;
}

async function addRADeploymentCheck({ repository, prNumber, status }) {
const checkName = 'check-ra-deployment';
async function createCommitStatus({ context, repo, pull_number, description, state }) {
const owner = '1024pix';
const octokit = _createOctokit();

let response = await octokit.pulls.get({ owner: '1024pix', repo: repository, pull_number: prNumber });
let response = await octokit.pulls.get({ owner, repo, pull_number });
const sha = response.data.head.sha;

response = await octokit.repos.createCommitStatus({
owner,
context: checkName,
repo: repository,
context,
description,
repo,
sha,
state: status,
state,
});

const startedAt = response.data.started_at;
logger.info(`Check ${checkName} added for PR ${prNumber} in ${repository} with status ${status}`);
logger.info(`Check ${context} added for PR ${pull_number} in ${repo} with status ${state}`);

return startedAt;
}

async function setMergeQueueStatus({ repositoryFullName, prNumber, status, description }) {
const repository = repositoryFullName.split('/')[1];
return createCommitStatus({
context: 'merge-queue-status',
repo: repository,
pull_number: prNumber,
state: status,
description,
});
}

async function addRADeploymentCheck({ repository, prNumber, status }) {
return createCommitStatus({
context: 'check-ra-deployment',
repo: repository,
pull_number: prNumber,
state: status,
});
}

const github = {
async getPullRequests(label) {
const pullRequests = await _getPullReviewsFromGithub(label);
Expand Down Expand Up @@ -480,6 +500,7 @@ const github = {
},
commentPullRequest,
addRADeploymentCheck,
setMergeQueueStatus,

async checkUserBelongsToPix(username) {
const octokit = _createOctokit();
Expand Down
21 changes: 21 additions & 0 deletions test/acceptance/build/github_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,15 @@ describe('Acceptance | Build | Github', function () {
})
.reply(200, {});

const prHeadCommit = 'aaaa';
const getPullRequestNock = nock('https://api.github.com')
.get(`/repos/1024pix/pix/pulls/123`)
.reply(200, { head: { sha: prHeadCommit } });

const createCheckNock = nock('https://api.github.com')
.post(`/repos/${body.repository.full_name}/statuses/${prHeadCommit}`)
.reply(201, { started_at: '2024-01-01' });

const response = await server.inject({
method: 'POST',
url: '/github/webhook',
Expand All @@ -694,6 +703,8 @@ describe('Acceptance | Build | Github', function () {

expect(checkUserBelongsToPixNock.isDone()).to.be.true;
expect(callGitHubAction.isDone()).to.be.true;
expect(getPullRequestNock.isDone()).to.be.true;
expect(createCheckNock.isDone()).to.be.true;
expect(response.statusCode).equal(200);
});

Expand Down Expand Up @@ -740,6 +751,16 @@ describe('Acceptance | Build | Github', function () {
})
.reply(200, {});

const prHeadCommit = 'aaaa';
nock('https://api.github.com')
.get(`/repos/1024pix/pix/pulls/123`)
.reply(200, { head: { sha: prHeadCommit } })
.persist();

nock('https://api.github.com')
.post(`/repos/${body.repository.full_name}/statuses/${prHeadCommit}`)
.reply(201, { started_at: '2024-01-01' });

const secondPRForSameRepo = 567;
const shouldNotBeCalled = nock('https://api.github.com/')
.post(`/repos/${workflowRepoName}/actions/workflows/${workflowId}/dispatches`, {
Expand Down
32 changes: 15 additions & 17 deletions test/integration/build/repositories/pull-request-repository_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,16 @@ describe('PullRequestRepository', function () {
});
});

describe('#getOldest', function () {
describe('#findNotMerged', function () {
context('when there is at least one pr', function () {
it('should return oldest pull request', async function () {
it('should return all pull requests', async function () {
const repositoryName = '1024pix/pix-sample-repo';
await knex('pull_requests').insert({
number: 789,
repositoryName,
isMerging: false,
createdAt: new Date('2024-02-02'),
});
await knex('pull_requests').insert({
number: 123,
repositoryName,
Expand All @@ -74,32 +80,24 @@ describe('PullRequestRepository', function () {
isMerging: false,
createdAt: new Date('2024-01-02'),
});
await knex('pull_requests').insert({
number: 789,
repositoryName,
isMerging: false,
createdAt: new Date('2024-02-02'),
});

const oldestPR = await pullRequestRepository.getOldest(repositoryName);

expect(oldestPR.number).to.be.equal(456);
const notMergedPullRequests = await pullRequestRepository.findNotMerged(repositoryName);
expect(notMergedPullRequests.length).to.be.equal(2);
expect(notMergedPullRequests[0].number).to.equal(456);
});
});

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

const oldestPR = await pullRequestRepository.getOldest(repositoryWithoutPRInMergeQueue);

expect(oldestPR).to.be.undefined;
const notMergedPullRequests = await pullRequestRepository.findNotMerged('1024pix/another-repo');
expect(notMergedPullRequests).to.be.empty;
});
});
});
Expand Down
39 changes: 39 additions & 0 deletions test/unit/build/services/github_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,45 @@ describe('Unit | Build | github-test', function () {
});
});

describe('#setMergeQueueStatus', function () {
describe('when everything is OK', function () {
it('should create the merge queue status', async function () {
// given
const repositoryFullName = '1024pix/my-repository';
const sha = 'my-sha';
const prNumber = 1234;
const startedAt = new Date();

const getPullRequestNock = nock('https://api.github.com')
.get(`/repos/${repositoryFullName}/pulls/${prNumber}`)
.reply(200, { head: { sha } });

const body = {
context: 'merge-queue-status',
state: 'pending',
description: 'fuu',
};

const createCheckNock = nock('https://api.github.com')
.post(`/repos/${repositoryFullName}/statuses/${sha}`, body)
.reply(201, { started_at: startedAt });

// when
const result = await githubService.setMergeQueueStatus({
repositoryFullName,
prNumber,
status: 'pending',
description: 'fuu',
});

// then
expect(result).to.equal(startedAt.toISOString());
expect(getPullRequestNock.isDone()).to.be.true;
expect(createCheckNock.isDone()).to.be.true;
});
});
});

describe('#addRADeploymentCheck', function () {
describe('when everything is OK', function () {
it('should create the check run', async function () {
Expand Down
51 changes: 48 additions & 3 deletions test/unit/run/services/merge-queue_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('Unit | Build | Services | merge-queue', function () {
});

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 () {
it('should find not merged pull requests, mark latest as currently merging and trigger auto-merge', async function () {
const repositoryName = 'foo/pix-project';
const pr = {
number: 1,
Expand All @@ -28,14 +28,15 @@ describe('Unit | Build | Services | merge-queue', function () {
};
const pullRequestRepository = {
isAtLeastOneMergeInProgress: sinon.stub(),
getOldest: sinon.stub(),
findNotMerged: sinon.stub(),
update: sinon.stub(),
};
pullRequestRepository.isAtLeastOneMergeInProgress.withArgs(repositoryName).resolves(false);
pullRequestRepository.getOldest.withArgs(repositoryName).resolves(pr);
pullRequestRepository.findNotMerged.withArgs(repositoryName).resolves([pr]);

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

await new MergeQueue({ pullRequestRepository, githubService }).manage({ repositoryName });
Expand All @@ -55,6 +56,50 @@ describe('Unit | Build | Services | merge-queue', function () {
inputs: { pullRequest: 'foo/pix-project/1' },
});
});

it('should create commit status for each pending pull request', async function () {
const repositoryName = 'foo/pix-project';
const prs = [
{
number: 1,
repositoryName,
isMerging: false,
},
{
number: 2,
repositoryName,
isMerging: false,
},
];
const pullRequestRepository = {
isAtLeastOneMergeInProgress: sinon.stub(),
findNotMerged: sinon.stub(),
update: sinon.stub(),
};
pullRequestRepository.isAtLeastOneMergeInProgress.withArgs(repositoryName).resolves(false);
pullRequestRepository.findNotMerged.withArgs(repositoryName).resolves(prs);

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

await new MergeQueue({ pullRequestRepository, githubService }).manage({ repositoryName });

expect(githubService.setMergeQueueStatus).to.have.been.calledTwice;
expect(githubService.setMergeQueueStatus.firstCall).to.have.been.calledWithExactly({
status: 'pending',
description: 'En cours de merge',
repositoryFullName: repositoryName,
prNumber: 1,
});
expect(githubService.setMergeQueueStatus.secondCall).to.have.been.calledWithExactly({
status: 'pending',
description: "2/2 dans la file d'attente",
repositoryFullName: repositoryName,
prNumber: 2,
});
});
});
});

Expand Down
Loading