Skip to content

Commit

Permalink
refactor(github): unify all pull_request event with same intention
Browse files Browse the repository at this point in the history 
Co-authored-by: Guillaume Lagorce <guillaume.lagorce@pix.fr>
  • Loading branch information
@VincentHardouin @HEYGUL
VincentHardouin and HEYGUL committed Sep 24, 2024
1 parent 98b4a28 commit 3eb15f3
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 207 deletions.
57 changes: 10 additions & 47 deletions build/controllers/github.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,42 +60,7 @@ const _addMessageToPullRequest = async ({ repositoryName, pullRequestId, scaling
});
};

async function _pullRequestOpenedWebhook(
request,
scalingoClient = ScalingoClient,
addMessageToPullRequest = _addMessageToPullRequest,
githubService = commonGithubService,
) {
const payload = request.payload;
const prId = payload.number;
const ref = payload.pull_request.head.ref;
const repository = payload.pull_request.head.repo.name;
const reviewApps = repositoryToScalingoAppsReview[repository];

const { shouldContinue, message } = _handleNoRACase(request);
if (!shouldContinue) {
return message;
}

const deployedRA = await deployPullRequest(
scalingoClient,
reviewApps,
prId,
ref,
repository,
addMessageToPullRequest,
githubService,
);

logger.info({
event: 'review-app',
message: `Created RA for repo ${repository} PR ${prId}`,
});

return `Created RA on app ${deployedRA.join(', ')} with pr ${prId}`;
}

async function _pullRequestSynchronizeWebhook(
async function _handleRA(
request,
scalingoClient = ScalingoClient,
addMessageToPullRequest = _addMessageToPullRequest,
Expand Down Expand Up @@ -181,6 +146,11 @@ async function deployPullRequest(
},
{ githubService },
);

logger.info({
event: 'review-app',
message: `Created RA for repo ${repository} PR ${prId}`,
});
}
logger.info({
event: 'review-app',
Expand Down Expand Up @@ -217,21 +187,15 @@ async function processWebhook(
request,
{
pushOnDefaultBranchWebhook = _pushOnDefaultBranchWebhook,
pullRequestOpenedWebhook = _pullRequestOpenedWebhook,
pullRequestSynchronizeWebhook = _pullRequestSynchronizeWebhook,
handleRA = _handleRA,
} = {},
) {
const eventName = request.headers['x-github-event'];
if (eventName === 'push') {
return pushOnDefaultBranchWebhook(request);
} else if (eventName === 'pull_request') {
switch (request.payload.action) {
case 'opened':
return pullRequestOpenedWebhook(request);
case 'reopened':
return pullRequestOpenedWebhook(request);
case 'synchronize':
return pullRequestSynchronizeWebhook(request);
if (['opened', 'reopened', 'synchronize'].includes(request.payload.action)) {
return handleRA(request);
}
return `Ignoring ${request.payload.action} action`;
} else {
Expand Down Expand Up @@ -267,8 +231,7 @@ export {
_addMessageToPullRequest as addMessageToPullRequest,
getMessage,
getMessageTemplate,
_handleRA as handleRA,
processWebhook,
_pullRequestOpenedWebhook as pullRequestOpenedWebhook,
_pullRequestSynchronizeWebhook as pullRequestSynchronizeWebhook,
_pushOnDefaultBranchWebhook as pushOnDefaultBranchWebhook,
};
2 changes: 1 addition & 1 deletion test/acceptance/build/github_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('Acceptance | Build | Github', function () {
});
expect(res.statusCode).to.equal(StatusCodes.OK);
expect(res.result).to.eql(
'Created RA on app pix-api-review, pix-audit-logger-review, pix-front-review with pr 2',
'Triggered deployment of RA on app pix-api-review, pix-audit-logger-review, pix-front-review with pr 2',
);
expect(scalingoAuth.isDone()).to.be.true;
expect(scalingoDeploy1.isDone()).to.be.true;
Expand Down
182 changes: 23 additions & 159 deletions test/unit/build/controllers/github_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,31 +133,31 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
payload: { action: 'nothing' },
};
['opened', 'reopened'].forEach((action) => {
it(`should call pullRequestOpenedWebhook() method on ${action} action`, async function () {
it(`should call handleRA() method on ${action} action`, async function () {
// given
sinon.stub(request, 'payload').value({ action });

const pullRequestOpenedWebhook = sinon.stub();
const handleRA = sinon.stub();

// when
await githubController.processWebhook(request, { pullRequestOpenedWebhook });
await githubController.processWebhook(request, { handleRA });

// then
expect(pullRequestOpenedWebhook.calledOnceWithExactly(request)).to.be.true;
expect(handleRA.calledOnceWithExactly(request)).to.be.true;
});
});

it('should call pullRequestSynchronizeWebhook() method on synchronize action', async function () {
it('should call handleRA() method on synchronize action', async function () {
// given
sinon.stub(request, 'payload').value({ action: 'synchronize' });

const pullRequestSynchronizeWebhook = sinon.stub();
const handleRA = sinon.stub();

// when
await githubController.processWebhook(request, { pullRequestSynchronizeWebhook });
await githubController.processWebhook(request, { handleRA });

// then
expect(pullRequestSynchronizeWebhook.calledOnceWithExactly(request)).to.be.true;
expect(handleRA.calledOnceWithExactly(request)).to.be.true;
});

it('should ignore the action', async function () {
Expand Down Expand Up @@ -282,7 +282,7 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
});
});

describe('#pullRequestOpenedWebhook', function () {
describe('#handleRA', function () {
const request = {
payload: {
number: 3,
Expand All @@ -307,7 +307,7 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
sinon.stub(request.payload.pull_request.head.repo, 'fork').value(true);

// when
const response = await githubController.pullRequestOpenedWebhook(request);
const response = await githubController.handleRA(request);

// then
expect(response).to.equal('No RA for a fork');
Expand All @@ -320,7 +320,7 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
sinon.stub(request.payload.pull_request.head.repo, 'name').value('no-ra-app-repo-name');

// when
const response = await githubController.pullRequestOpenedWebhook(request);
const response = await githubController.handleRA(request);

// then
expect(response).to.equal('No RA configured for this repository');
Expand All @@ -333,7 +333,7 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
sinon.stub(request.payload.pull_request, 'labels').value([{ name: 'no-review-app' }]);

// when
const response = await githubController.pullRequestOpenedWebhook(request);
const response = await githubController.handleRA(request);

// then
expect(response).to.equal('RA disabled for this PR');
Expand All @@ -346,7 +346,7 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
sinon.stub(request.payload.pull_request, 'state').value('closed');

// when
const response = await githubController.pullRequestOpenedWebhook(request);
const response = await githubController.handleRA(request);

// then
expect(response).to.equal('No RA for closed PR');
Expand All @@ -355,35 +355,28 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
});

describe('when the Review App creation conditions are met', function () {
it('should call scalingo to create and deploy the corresponding applications', async function () {
it('should call scalingo to deploy the corresponding applications', async function () {
// given
const scalingoClientStub = sinon.stub();
const deployUsingSCMStub = sinon.stub();
const deployReviewAppStub = sinon.stub();
const disableAutoDeployStub = sinon.stub();
const deployUsingSCMStub = sinon.stub();
const reviewAppExistsStub = sinon.stub();
const reviewAppExistsStub = sinon.stub().resolves(false);

scalingoClientStub.getInstance = sinon.stub().returns({
deployUsingSCM: deployUsingSCMStub,
deployReviewApp: deployReviewAppStub,
disableAutoDeploy: disableAutoDeployStub,
deployUsingSCM: deployUsingSCMStub,
reviewAppExists: reviewAppExistsStub,
});

const addMessageToPullRequestStub = sinon.stub();
const githubServiceStub = sinon.stub();
reviewAppExistsStub.resolves(false);

// when
const response = await githubController.pullRequestOpenedWebhook(
request,
scalingoClientStub,
addMessageToPullRequestStub,
githubServiceStub,
);
const response = await githubController.handleRA(request, scalingoClientStub, addMessageToPullRequestStub, githubServiceStub);

// then
expect(deployReviewAppStub.firstCall.calledWith('pix-api-review', 3)).to.be.true;
expect(disableAutoDeployStub.firstCall.calledWith('pix-api-review-pr3')).to.be.true;
expect(deployUsingSCMStub.firstCall.calledWith('pix-api-review-pr3', 'my-branch')).to.be.true;

expect(deployReviewAppStub.secondCall.calledWith('pix-audit-logger-review', 3)).to.be.true;
expect(disableAutoDeployStub.secondCall.calledWith('pix-audit-logger-review-pr3')).to.be.true;
Expand All @@ -393,142 +386,13 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
expect(disableAutoDeployStub.thirdCall.calledWith('pix-front-review-pr3')).to.be.true;
expect(deployUsingSCMStub.thirdCall.calledWith('pix-front-review-pr3', 'my-branch')).to.be.true;

expect(
addMessageToPullRequestStub.calledOnceWithExactly(
expect(addMessageToPullRequestStub).to.have.been.calledOnceWithExactly(
{
repositoryName: 'pix',
scalingoReviewApps: ['pix-api-review', 'pix-audit-logger-review', 'pix-front-review'],
pullRequestId: 3,
},
{ githubService: githubServiceStub },
),
).to.be.true;

expect(response).to.equal(
'Created RA on app pix-api-review, pix-audit-logger-review, pix-front-review with pr 3',
);
});

it('throws an error on scalingo deployment fails', async function () {
// given
const scalingoClientStub = sinon.stub();
const deployReviewAppStub = sinon.stub().rejects(new Error('Deployment error'));
const disableAutoDeployStub = sinon.stub();
const deployUsingSCMStub = sinon.stub();
scalingoClientStub.getInstance = sinon.stub().returns({
deployReviewApp: deployReviewAppStub,
disableAutoDeploy: disableAutoDeployStub,
deployUsingSCM: deployUsingSCMStub,
});
const addMessageToPullRequestStub = sinon.stub();

// when
const result = await catchErr(githubController.pullRequestOpenedWebhook)(
request,
scalingoClientStub,
addMessageToPullRequestStub,
);

// then
expect(result).to.be.instanceOf(Error);
expect(result.message).to.equal('No RA deployed for repository pix and pr3');
});
});
});

describe('#pullRequestSynchronizeWebhook', function () {
const request = {
payload: {
number: 3,
pull_request: {
state: 'open',
labels: [],
head: {
ref: 'my-branch',
repo: {
name: 'pix',
fork: false,
},
},
},
},
};

describe('when the Review App creation conditions are not met', function () {
describe('when the project is a fork', function () {
it('should not create a Review App', async function () {
// given
sinon.stub(request.payload.pull_request.head.repo, 'fork').value(true);

// when
const response = await githubController.pullRequestSynchronizeWebhook(request);

// then
expect(response).to.equal('No RA for a fork');
});
});

describe('when there is no RA configured for this repository', function () {
it('should not create a Review App', async function () {
// given
sinon.stub(request.payload.pull_request.head.repo, 'name').value('no-ra-app-repo-name');

// when
const response = await githubController.pullRequestSynchronizeWebhook(request);

// then
expect(response).to.equal('No RA configured for this repository');
});
});

describe('when there is a no-review-app label on the PR', function () {
it('should not create a Review App', async function () {
// given
sinon.stub(request.payload.pull_request, 'labels').value([{ name: 'no-review-app' }]);

// when
const response = await githubController.pullRequestSynchronizeWebhook(request);

// then
expect(response).to.equal('RA disabled for this PR');
});
});

describe('when the PR is not opened', function () {
it('should not create a Review App', async function () {
// given
sinon.stub(request.payload.pull_request, 'state').value('closed');

// when
const response = await githubController.pullRequestSynchronizeWebhook(request);

// then
expect(response).to.equal('No RA for closed PR');
});
});
});

describe('when the Review App creation conditions are met', function () {
it('should call scalingo to deploy the corresponding applications', async function () {
// given
const scalingoClientStub = sinon.stub();
const deployUsingSCMStub = sinon.stub();
const reviewAppExistsStub = sinon.stub().resolves(true);

scalingoClientStub.getInstance = sinon.stub().returns({
deployUsingSCM: deployUsingSCMStub,
reviewAppExists: reviewAppExistsStub,
});

// when
const response = await githubController.pullRequestSynchronizeWebhook(request, scalingoClientStub);

// then
expect(deployUsingSCMStub.firstCall.calledWith('pix-api-review-pr3', 'my-branch')).to.be.true;
expect(deployUsingSCMStub.secondCall.calledWith('pix-audit-logger-review-pr3', 'my-branch')).to.be.true;
expect(deployUsingSCMStub.thirdCall.calledWith('pix-front-review-pr3', 'my-branch')).to.be.true;
expect(response).to.equal(
'Triggered deployment of RA on app pix-api-review, pix-audit-logger-review, pix-front-review with pr 3',
);
});

Expand All @@ -543,7 +407,7 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
});

// when
const result = await catchErr(githubController.pullRequestSynchronizeWebhook)(request, scalingoClientStub);
const result = await catchErr(githubController.handleRA)(request, scalingoClientStub);

// then
expect(result).to.be.instanceOf(Error);
Expand All @@ -569,7 +433,7 @@ Les variables d'environnement seront accessibles sur scalingo https://dashboard.
});

// when
const response = await githubController.pullRequestSynchronizeWebhook(
const response = await githubController.handleRA(
request,
scalingoClientStub,
githubServiceStub,
Expand Down

0 comments on commit 3eb15f3

Please sign in to comment.