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

Migrated ciChecks Module from probot to github actions #300

Closed
wants to merge 7 commits into from
Closed
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
5 changes: 5 additions & 0 deletions actions/src/dispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const claCheckGithubActionModule = require('./pull_requests/claCheck');
const constants = require('../../constants');
const PRLabelsModule = require('./pull_requests/labelCheck');
const wipDraftModule = require('./pull_requests/checkWipDraftPR');
const ciCheckModule = require('./pull_requests/ciChecks');

module.exports = {
async dispatch(event, action) {
Expand Down Expand Up @@ -58,6 +59,10 @@ module.exports = {
core.info('WIP check triggered');
await wipDraftModule.checkWIP();
break;
case constants.ciFailureCheck:
core.info('CI check triggered');
await ciCheckModule.handleFailure();
break;
}
}
}
Expand Down
24 changes: 14 additions & 10 deletions lib/ciChecks.js → actions/src/pull_requests/ciChecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,24 @@
// See the License for the specific language governing permissions and
// limitations under the License.

const { pingAndAssignUsers } = require('./utils');

/**
*
* @fileoverview File to handle checks when a PR fails CI checks.
*/

const core = require('@actions/core');
const { context, GitHub } = require('@actions/github');
const { commentAndAssignUsers } = require('../utils');

/**
* This function checks for CI test failure and pings PR author.
* @param {import('probot').Context} context
* @param {import('@octokit/rest').Octokit} octokit
* @returns{Promise<Boolean>}
*/
const handleFailure = async (context) => {
/**
* @type {import('probot').Octokit.ChecksGetSuiteResponse} checkSuite
*/
const handleFailure = async () => {

const octokit = new GitHub(core.getInput('repo-token'));
const checkSuite = context.payload.check_suite;
if (checkSuite.conclusion === 'failure') {
// Some checks fail in develop due to a PR being merged.
Expand All @@ -38,10 +41,11 @@ const handleFailure = async (context) => {
// pull request data, hence we need to fetch the pull request
// so we can get the author.
const pullRequestData = checkSuite.pull_requests[0];
const pullRequestResponse = await context.github.pulls.get(
context.repo({
const pullRequestResponse = await octokit.pulls.get(
{
pull_number: pullRequestData.number,
})
...context.repo
}
);
const pullRequest = pullRequestResponse.data;
const prAuthor = pullRequest.user.login;
Expand All @@ -52,7 +56,7 @@ const handleFailure = async (context) => {
', there are some failing CI checks in your latest push ' +
' If you think this is due to a flake, please file an issue ' +
'before restarting the tests. Thanks!';
await pingAndAssignUsers(context, pullRequest, [prAuthor], commentBody);
await commentAndAssignUsers(octokit, pullRequest, [prAuthor], commentBody);
}
}
};
Expand Down
4 changes: 0 additions & 4 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const checkCriticalPullRequestModule = require(
const checkBranchPushModule = require('./lib/checkBranchPush');
const checkPullRequestReviewModule = require('./lib/checkPullRequestReview');
const newCodeOwnerModule = require('./lib/checkForNewCodeowner');
const ciCheckModule = require('./lib/ciChecks');
const periodicCheckModule = require('./lib/periodicChecks');

const constants = require('./constants');
Expand Down Expand Up @@ -136,9 +135,6 @@ const runChecks = async (context, checkEvent) => {
case constants.codeOwnerCheck:
callable.push(newCodeOwnerModule.checkForNewCodeowner(context));
break;
case constants.ciFailureCheck:
callable.push(ciCheckModule.handleFailure(context));
break;
case constants.updateWithDevelopCheck:
callable.push(
checkMergeConflictsModule.pingAllPullRequestsToMergeFromDevelop(
Expand Down
85 changes: 36 additions & 49 deletions spec/ciChecksSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,19 @@
*/

require('dotenv').config();
const { createProbot } = require('probot');
const oppiaBot = require('../index');
const scheduler = require('../lib/scheduler');

const payloadData = require('../fixtures/checksuite.complete.json');
const ciCheckModule = require('../lib/ciChecks');
const github = require('@actions/github');
const core = require('@actions/core');
const ciCheckModule = require('../actions/src/pull_requests/ciChecks');
const dispatcher = require('../actions/src/dispatcher');

describe('CI Checks', () => {
/**
* @type {import('probot').Probot} robot
*/
let robot;

/**
* @type {import('probot').Octokit} github
*/
let github;

/**
* @type {import('probot').Application} app
* @type {import('@actions/github').GitHub} octokit
*/
let app;
let octokit;

/**
* @type {import('probot').Octokit.PullsGetResponse} pullRequest
*/
const pullRequest = {
number: 25,
body: 'Sample Pull Request',
Expand All @@ -50,15 +38,15 @@ describe('CI Checks', () => {
}
};

beforeEach(() => {
spyOn(scheduler, 'createScheduler').and.callFake(() => {});
beforeEach(async () => {
github.context.eventName = payloadData.name;
github.context.payload = payloadData.payload;
github.context.pull_request = pullRequest;

github = {
octokit = {
issues: {
createComment: jasmine
.createSpy('createComment')
.and.callFake(() => {}),
addAssignees: jasmine.createSpy('addAssignees').and.callFake(() => {}),
createComment: jasmine.createSpy('createComment').and.callFake(()=>{}),
addAssignees: jasmine.createSpy('addAssignees').and.callFake(()=>{}),
},
pulls: {
get: jasmine.createSpy('get').and.resolveTo({
Expand All @@ -67,40 +55,39 @@ describe('CI Checks', () => {
},
};

robot = createProbot({
id: 1,
cert: 'test',
githubToken: 'test',
spyOn(core, 'getInput').and.returnValue('sample-token');

// Mock GitHub API.
Object.setPrototypeOf(github.GitHub, function () {
return octokit;
});

app = robot.load(oppiaBot);
spyOn(app, 'auth').and.resolveTo(github);
spyOn(ciCheckModule, 'handleFailure').and.callThrough();
});

describe('When a PR fails the check suite', () => {
beforeEach(async () => {
await robot.receive(payloadData);
await dispatcher.dispatch('check', 'completed');
});

it('should call handle failure module', () => {
expect(ciCheckModule.handleFailure).toHaveBeenCalled();
});

it('should fetch pull request data', () => {
expect(github.pulls.get).toHaveBeenCalled();
expect(github.pulls.get).toHaveBeenCalledWith({
expect(octokit.pulls.get).toHaveBeenCalled();
expect(octokit.pulls.get).toHaveBeenCalledWith({
owner: payloadData.payload.repository.owner.login,
repo: payloadData.payload.repository.name,
pull_number: pullRequest.number,
});
});

it('should comment on pull request', () => {
expect(github.issues.createComment).toHaveBeenCalled();
expect(octokit.issues.createComment).toHaveBeenCalled();

const prAuthor = pullRequest.user.login;
expect(github.issues.createComment).toHaveBeenCalledWith({
expect(octokit.issues.createComment).toHaveBeenCalledWith({
owner: payloadData.payload.repository.owner.login,
repo: payloadData.payload.repository.name,
issue_number: pullRequest.number,
Expand All @@ -114,10 +101,10 @@ describe('CI Checks', () => {
});

it('should assign PR author', () => {
expect(github.issues.addAssignees).toHaveBeenCalled();
expect(octokit.issues.addAssignees).toHaveBeenCalled();

const prAuthor = pullRequest.user.login;
expect(github.issues.addAssignees).toHaveBeenCalledWith({
expect(octokit.issues.addAssignees).toHaveBeenCalledWith({
owner: payloadData.payload.repository.owner.login,
repo: payloadData.payload.repository.name,
issue_number: pullRequest.number,
Expand All @@ -129,7 +116,7 @@ describe('CI Checks', () => {
describe('When a check suite is successful', () => {
beforeEach(async () => {
payloadData.payload.check_suite.conclusion = 'success';
await robot.receive(payloadData);
await dispatcher.dispatch('check', 'completed');
});

it('should call handle failure module', () => {
Expand All @@ -138,18 +125,18 @@ describe('CI Checks', () => {


it('should not comment on pull request', () => {
expect(github.issues.createComment).not.toHaveBeenCalled();
expect(octokit.issues.createComment).not.toHaveBeenCalled();
});

it('should not assign PR author', () => {
expect(github.issues.addAssignees).not.toHaveBeenCalled();
expect(octokit.issues.addAssignees).not.toHaveBeenCalled();
});
});

describe('When a check suite is canceled', () => {
beforeEach(async () => {
payloadData.payload.check_suite.conclusion = 'canceled';
await robot.receive(payloadData);
await dispatcher.dispatch('check', 'completed');
});

it('should call handle failure module', () => {
Expand All @@ -158,35 +145,35 @@ describe('CI Checks', () => {


it('should not comment on pull request', () => {
expect(github.issues.createComment).not.toHaveBeenCalled();
expect(octokit.issues.createComment).not.toHaveBeenCalled();
});

it('should not assign PR author', () => {
expect(github.issues.addAssignees).not.toHaveBeenCalled();
expect(octokit.issues.addAssignees).not.toHaveBeenCalled();
});
});

describe('When a non PR check suite failes', () => {
beforeEach(async () => {
payloadData.payload.check_suite.conclusion = 'failure';
payloadData.payload.check_suite.pull_requests = [];
await robot.receive(payloadData);
await dispatcher.dispatch('check', 'completed');
});

it('should call handle failure module', () => {
expect(ciCheckModule.handleFailure).toHaveBeenCalled();
});

it('should not fetch pull request data', () => {
expect(github.pulls.get).not.toHaveBeenCalled();
expect(octokit.pulls.get).not.toHaveBeenCalled();
});

it('should not comment on pull request', () => {
expect(github.issues.createComment).not.toHaveBeenCalled();
expect(octokit.issues.createComment).not.toHaveBeenCalled();
});

it('should not assign PR author', () => {
expect(github.issues.addAssignees).not.toHaveBeenCalled();
expect(octokit.issues.addAssignees).not.toHaveBeenCalled();
});
});
});