diff --git a/constants.js b/constants.js index 9c14a4f5..eecf2ed0 100644 --- a/constants.js +++ b/constants.js @@ -8,8 +8,9 @@ const editEvent = 'edited'; const issuesLabelEvent = 'issues_labeled'; const issuesAssignedEvent = 'issues_assigned'; const pushEvent = 'push'; -const pullRequestReviewEvent = 'pr-review'; +const pullRequestReviewEvent = 'pr_review'; const checkCompletedEvent = 'check_completed'; +const issueCommentCreatedEvent = 'comment_created'; const claCheck = 'cla-check'; const changelogCheck = 'changelog-check'; @@ -32,6 +33,7 @@ const pullRequestReviewCheck = 'pr-review-check'; const codeOwnerCheck = 'code-owner-check' const ciFailureCheck = 'ci-failure-check'; const updateWithDevelopCheck = 'update-with-develop-check'; +const respondToReviewCheck = 'respond-to-review-check'; const checksWhitelist = { 'oppia-android': { @@ -75,7 +77,8 @@ const checksWhitelist = { [unlabelEvent]: [datastoreLabelCheck], [pushEvent]: [forcePushCheck], [pullRequestReviewEvent]: [pullRequestReviewCheck], - [checkCompletedEvent]: [ciFailureCheck] + [checkCompletedEvent]: [ciFailureCheck], + [issueCommentCreatedEvent]: [respondToReviewCheck] }, 'oppiabot': { [openEvent]: [claCheck], @@ -103,6 +106,7 @@ module.exports.issuesAssignedEvent = issuesAssignedEvent; module.exports.pushEvent = pushEvent; module.exports.pullRequestReviewEvent = pullRequestReviewEvent; module.exports.checkCompletedEvent = checkCompletedEvent; +module.exports.issueCommentCreatedEvent = issueCommentCreatedEvent; module.exports.claCheck = claCheck; module.exports.changelogCheck = changelogCheck; @@ -123,6 +127,7 @@ module.exports.pullRequestReviewCheck = pullRequestReviewCheck; module.exports.codeOwnerCheck = codeOwnerCheck; module.exports.ciFailureCheck = ciFailureCheck; module.exports.updateWithDevelopCheck = updateWithDevelopCheck; +module.exports.respondToReviewCheck = respondToReviewCheck module.exports.getBlacklistedAuthors = function() { return blacklistedAuthors; diff --git a/fixtures/pullRequestComment.json b/fixtures/pullRequestComment.json new file mode 100644 index 00000000..a67694b8 --- /dev/null +++ b/fixtures/pullRequestComment.json @@ -0,0 +1,238 @@ +{ + "name": "issue_comment", + "payload": { + "action": "created", + "issue": { + "url": "https://api.github.com/repos/oppia/oppia/issues/39", + "repository_url": "https://api.github.com/repos/oppia/oppia", + "labels_url": "https://api.github.com/repos/oppia/oppia/issues/39/labels{/name}", + "comments_url": "https://api.github.com/repos/oppia/oppia/issues/39/comments", + "events_url": "https://api.github.com/repos/oppia/oppia/issues/39/events", + "html_url": "https://github.com/oppia/oppia/pull/39", + "id": 676178626, + "node_id": "MDExOlB1bGxSZXF1ZXN0NDY1NTM1MjI4", + "number": 39, + "title": "Changes", + "user": { + "login": "testuser", + "id": 31052489, + "node_id": "MDQ6VXNlcjMxMDUyNDg5", + "avatar_url": "https://avatars1.githubusercontent.com/u/31052489?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/testuser", + "html_url": "https://github.com/testuser", + "followers_url": "https://api.github.com/users/testuser/followers", + "following_url": "https://api.github.com/users/testuser/following{/other_user}", + "gists_url": "https://api.github.com/users/testuser/gists{/gist_id}", + "starred_url": "https://api.github.com/users/testuser/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/testuser/subscriptions", + "organizations_url": "https://api.github.com/users/testuser/orgs", + "repos_url": "https://api.github.com/users/testuser/repos", + "events_url": "https://api.github.com/users/testuser/events{/privacy}", + "received_events_url": "https://api.github.com/users/testuser/received_events", + "type": "User", + "site_admin": false + }, + "labels": [ + { + "id": 2140127429, + "node_id": "MDU6TGFiZWwyMTQwMTI3NDI5", + "url": "https://api.github.com/repos/testuser/oppia/labels/PR%20CHANGELOG:%20code%20health%20--%20@testuser", + "name": "PR CHANGELOG: code health -- @testuser", + "color": "c5ccf9", + "default": false, + "description": "Optional" + } + ], + "state": "open", + "locked": false, + "assignee": null, + "assignees": [], + "milestone": null, + "comments": 20, + "created_at": "2020-08-10T14:26:14Z", + "updated_at": "2020-08-26T14:44:35Z", + "closed_at": null, + "author_association": "OWNER", + "active_lock_reason": null, + "pull_request": { + "url": "https://api.github.com/repos/oppia/oppia/pulls/39", + "html_url": "https://github.com/oppia/oppia/pull/39", + "diff_url": "https://github.com/oppia/oppia/pull/39.diff", + "patch_url": "https://github.com/oppia/oppia/pull/39.patch" + }, + "body": "## Overview\r\n\r\n\r\n1. This PR fixes or fixes part of #[fill_in_number_here].\r\n2. This PR does the following: [Explain here what your PR does and why]\r\n\r\n## Essential Checklist\r\n\r\n- [ ] The PR title starts with \"Fix #bugnum: \", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with \"Fix part of #bugnum: ...\".)\r\n- [ ] The linter/Karma presubmit checks have passed locally on your machine.\r\n- [ ] \"Allow edits from maintainers\" is checked. (See [here](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork) for instructions on how to enable it.)\r\n - This lets reviewers restart your CircleCI tests for you.\r\n- [ ] The PR is made from a branch that's **not** called \"develop\".\r\n\r\n## PR Pointers\r\n\r\n- Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on [Gitter](https://gitter.im/oppia/oppia-chat).\r\n- For what code owners will expect, see the [Code Owner's wiki page](https://github.com/oppia/oppia/wiki/Oppia%27s-code-owners-and-checks-to-be-carried-out-by-developers).\r\n- Make sure your PR follows conventions in the [style guide](https://github.com/oppia/oppia/wiki/Coding-style-guide), otherwise this will lead to review delays\r\n- Never force push. If you do, your PR will be closed.\r\n", + "performed_via_github_app": null + }, + "comment": { + "url": "https://api.github.com/repos/oppia/oppia/issues/comments/680925201", + "html_url": "https://github.com/oppia/oppia/pull/39#issuecomment-680925201", + "issue_url": "https://api.github.com/repos/oppia/oppia/issues/39", + "id": 680925201, + "node_id": "MDEyOklzc3VlQ29tbWVudDY4MDkyNTIwMQ==", + "user": { + "login": "testuser", + "id": 31052489, + "node_id": "MDQ6VXNlcjMxMDUyNDg5", + "avatar_url": "https://avatars1.githubusercontent.com/u/31052489?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/testuser", + "html_url": "https://github.com/testuser", + "followers_url": "https://api.github.com/users/testuser/followers", + "following_url": "https://api.github.com/users/testuser/following{/other_user}", + "gists_url": "https://api.github.com/users/testuser/gists{/gist_id}", + "starred_url": "https://api.github.com/users/testuser/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/testuser/subscriptions", + "organizations_url": "https://api.github.com/users/testuser/orgs", + "repos_url": "https://api.github.com/users/testuser/repos", + "events_url": "https://api.github.com/users/testuser/events{/privacy}", + "received_events_url": "https://api.github.com/users/testuser/received_events", + "type": "User", + "site_admin": false + }, + "created_at": "2020-08-26T14:44:35Z", + "updated_at": "2020-08-26T14:44:35Z", + "author_association": "OWNER", + "body": "Hi @reviewer1, @reviewer2 PTAL. Thanks!", + "performed_via_github_app": null + }, + "repository": { + "id": 173004022, + "node_id": "MDEwOlJlcG9zaXRvcnkxNzMwMDQwMjI=", + "name": "oppia", + "full_name": "oppia/oppia", + "private": false, + "owner": { + "login": "oppia", + "id": 31052489, + "node_id": "MDQ6VXNlcjMxMDUyNDg5", + "avatar_url": "https://avatars1.githubusercontent.com/u/31052489?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/oppia", + "html_url": "https://github.com/oppia", + "followers_url": "https://api.github.com/users/oppia/followers", + "following_url": "https://api.github.com/users/oppia/following{/other_user}", + "gists_url": "https://api.github.com/users/oppia/gists{/gist_id}", + "starred_url": "https://api.github.com/users/oppia/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/oppia/subscriptions", + "organizations_url": "https://api.github.com/users/oppia/orgs", + "repos_url": "https://api.github.com/users/oppia/repos", + "events_url": "https://api.github.com/users/oppia/events{/privacy}", + "received_events_url": "https://api.github.com/users/oppia/received_events", + "type": "User", + "site_admin": false + }, + "html_url": "https://github.com/oppia/oppia", + "description": "Tool for collaboratively building interactive lessons.", + "fork": true, + "url": "https://api.github.com/repos/oppia/oppia", + "forks_url": "https://api.github.com/repos/oppia/oppia/forks", + "keys_url": "https://api.github.com/repos/oppia/oppia/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/oppia/oppia/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/oppia/oppia/teams", + "hooks_url": "https://api.github.com/repos/oppia/oppia/hooks", + "issue_events_url": "https://api.github.com/repos/oppia/oppia/issues/events{/number}", + "events_url": "https://api.github.com/repos/oppia/oppia/events", + "assignees_url": "https://api.github.com/repos/oppia/oppia/assignees{/user}", + "branches_url": "https://api.github.com/repos/oppia/oppia/branches{/branch}", + "tags_url": "https://api.github.com/repos/oppia/oppia/tags", + "blobs_url": "https://api.github.com/repos/oppia/oppia/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/oppia/oppia/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/oppia/oppia/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/oppia/oppia/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/oppia/oppia/statuses/{sha}", + "languages_url": "https://api.github.com/repos/oppia/oppia/languages", + "stargazers_url": "https://api.github.com/repos/oppia/oppia/stargazers", + "contributors_url": "https://api.github.com/repos/oppia/oppia/contributors", + "subscribers_url": "https://api.github.com/repos/oppia/oppia/subscribers", + "subscription_url": "https://api.github.com/repos/oppia/oppia/subscription", + "commits_url": "https://api.github.com/repos/oppia/oppia/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/oppia/oppia/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/oppia/oppia/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/oppia/oppia/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/oppia/oppia/contents/{+path}", + "compare_url": "https://api.github.com/repos/oppia/oppia/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/oppia/oppia/merges", + "archive_url": "https://api.github.com/repos/oppia/oppia/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/oppia/oppia/downloads", + "issues_url": "https://api.github.com/repos/oppia/oppia/issues{/number}", + "pulls_url": "https://api.github.com/repos/oppia/oppia/pulls{/number}", + "milestones_url": "https://api.github.com/repos/oppia/oppia/milestones{/number}", + "notifications_url": "https://api.github.com/repos/oppia/oppia/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/oppia/oppia/labels{/name}", + "releases_url": "https://api.github.com/repos/oppia/oppia/releases{/id}", + "deployments_url": "https://api.github.com/repos/oppia/oppia/deployments", + "created_at": "2019-02-27T23:03:20Z", + "updated_at": "2020-08-10T14:17:19Z", + "pushed_at": "2020-08-11T20:02:08Z", + "git_url": "git://github.com/oppia/oppia.git", + "ssh_url": "git@github.com:oppia/oppia.git", + "clone_url": "https://github.com/oppia/oppia.git", + "svn_url": "https://github.com/oppia/oppia", + "homepage": "https://www.oppia.org", + "size": 147378, + "stargazers_count": 2, + "watchers_count": 2, + "language": "Python", + "has_issues": false, + "has_projects": true, + "has_downloads": false, + "has_wiki": true, + "has_pages": false, + "forks_count": 0, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 1, + "license": { + "key": "apache-2.0", + "name": "Apache License 2.0", + "spdx_id": "Apache-2.0", + "url": "https://api.github.com/licenses/apache-2.0", + "node_id": "MDc6TGljZW5zZTI=" + }, + "forks": 0, + "open_issues": 1, + "watchers": 2, + "default_branch": "develop" + }, + "organization": { + "login": "oppia", + "id": 11620230, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjExNjIwMjMw", + "url": "https://api.github.com/orgs/oppia", + "repos_url": "https://api.github.com/orgs/oppia/repos", + "events_url": "https://api.github.com/orgs/oppia/events", + "hooks_url": "https://api.github.com/orgs/oppia/hooks", + "issues_url": "https://api.github.com/orgs/oppia/issues", + "members_url": "https://api.github.com/orgs/oppia/members{/member}", + "public_members_url": "https://api.github.com/orgs/oppia/public_members{/member}", + "avatar_url": "https://avatars1.githubusercontent.com/u/11620230?v=4", + "description": "" + }, + "sender": { + "login": "testuser", + "id": 31052489, + "node_id": "MDQ6VXNlcjMxMDUyNDg5", + "avatar_url": "https://avatars1.githubusercontent.com/u/31052489?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/testuser", + "html_url": "https://github.com/testuser", + "followers_url": "https://api.github.com/users/testuser/followers", + "following_url": "https://api.github.com/users/testuser/following{/other_user}", + "gists_url": "https://api.github.com/users/testuser/gists{/gist_id}", + "starred_url": "https://api.github.com/users/testuser/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/testuser/subscriptions", + "organizations_url": "https://api.github.com/users/testuser/orgs", + "repos_url": "https://api.github.com/users/testuser/repos", + "events_url": "https://api.github.com/users/testuser/events{/privacy}", + "received_events_url": "https://api.github.com/users/testuser/received_events", + "type": "User", + "site_admin": false + }, + "installation": { + "id": 6909030, + "node_id": "MDIzOkludGVncmF0aW9uSW5zdGFsbGF0aW9uNjkwOTAzMA==" + } + } +} diff --git a/index.js b/index.js index 641aacf3..8b74615b 100644 --- a/index.js +++ b/index.js @@ -97,6 +97,9 @@ const runChecks = async (context, checkEvent) => { case constants.updateWithDevelopCheck: await checkMergeConflictsModule.pingAllPullRequestsToMergeFromDevelop(context); break; + case constants.respondToReviewCheck: + await checkPullRequestReviewModule.handleResponseToReview(context); + break; } } } @@ -140,6 +143,14 @@ module.exports = (oppiabot) => { } }); + oppiabot.on('issue_comment.created', async (context) => { + if (checkWhitelistedAccounts(context)) { + // eslint-disable-next-line no-console + console.log('COMMENT CREATED ON ISSUE OR PULL REQUEST...'); + await runChecks(context, constants.issueCommentCreatedEvent); + } + }) + oppiabot.on('pull_request.opened', async (context) => { // The oppiabot runs only for repositories belonging to certain // whitelisted accounts. The whitelisted accounts are stored as an diff --git a/lib/checkPullRequestReview.js b/lib/checkPullRequestReview.js index c47af2aa..a7e8799a 100644 --- a/lib/checkPullRequestReview.js +++ b/lib/checkPullRequestReview.js @@ -21,6 +21,7 @@ const COMMENTED = 'commented'; const LGTM_LABEL = 'PR: LGTM'; const THREE_MINUTES = 60 * 1000 * 3; const CLOSED_STATE = 'closed'; +const PTAL_COMMENTS = ['PTAL', 'Please take a look']; /** * This functions handles a "changes requested" review. @@ -287,6 +288,105 @@ const handlePullRequestReview = async (context) => { } }; +/** + * This function handles the case when a PR author comments on the pull + * request. If pr author comments on the PR asking reviewers to please + * take a look (PTAL). + * The function checks if the reviewers have been assigned and if not, + * assigns them. + * + * @param {import('probot').Context} context + */ +const handleResponseToReview = async (context) => { + /** + * @type {import('probot').Octokit.IssuesGetResponse} + */ + const pullRequest = context.payload.issue; + const pullRequestProp = 'pull_request'; + const isPullRequest = pullRequestProp in pullRequest; + if (!isPullRequest) { + return; + } + + /** + * @type {import('probot').Octokit.IssuesGetCommentResponse} + */ + const comment = context.payload.comment; + const commenter = comment.user.login; + const commenterIsAuthor = commenter === pullRequest.user.login; + if (!commenterIsAuthor) { + return; + } + + const containsPtalComment = PTAL_COMMENTS.some((item) => + comment.body.toLowerCase().includes(item.toLowerCase()) + ); + if (!containsPtalComment) { + return; + } + + // Pause for 3 minutes in case author wants to perform + // the required actions. + await utilityModule.sleep(THREE_MINUTES); + + // Fetch the pull request in case there has been any changes since + // the comment was made. + const pullRequestResponse = await context.github.pulls.get( + context.repo({ + pull_number: pullRequest.number, + }) + ); + const updatedPullRequest = pullRequestResponse.data; + + const usersInComment = utilityModule.getUsernamesFromText(comment.body); + // Check if assignment was carried out by author. + const assignees = updatedPullRequest.assignees.map( + (assignee) => assignee.login + ); + const unassignedUsers = usersInComment.filter( + (user) => !assignees.includes(user) + ); + + if (unassignedUsers.length === 0) { + // All users in comment have been assigned. + return; + } + + // Assign users in comment + context.github.issues.addAssignees( + context.repo({ + issue_number: updatedPullRequest.number, + assignees: unassignedUsers, + }) + ); + + // Unassign author if assigned. + const authorIsAssigned = updatedPullRequest.assignees.some( + (assignee) => assignee.login === updatedPullRequest.user.login + ); + if (authorIsAssigned) { + context.github.issues.removeAssignees( + context.repo({ + issue_number: updatedPullRequest.number, + assignees: [updatedPullRequest.user.login], + }) + ); + + context.github.issues.createComment( + context.repo({ + issue_number: updatedPullRequest.number, + body: + 'Unassigning @' + + updatedPullRequest.user.login + + ' since a re-review was requested. @' + + updatedPullRequest.user.login + + ', please make sure you have addressed all review comments. Thanks!', + }) + ); + } +}; + module.exports = { handlePullRequestReview, + handleResponseToReview, }; diff --git a/lib/utils.js b/lib/utils.js index 06fa3886..32f1adc7 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -177,6 +177,20 @@ const isChangelogLabel = (label) => { return label.trim().toUpperCase().startsWith(CHANGELOG_START); } +/** + * This function retrieves github usernames from a text. + * + * @param {string} text + * @returns {string[]} + */ +const getUsernamesFromText = (text) => { + const regex = /(?@)(?\w+)/gm; + const matches = [...text.matchAll(regex)]; + const usernames = matches.map((match) => match.groups.username); + return usernames; +}; + + module.exports = { LABELS_EXCLUDED_FROM_CODEOWNER_ASSIGNMENT, DATASTORE_LABEL, @@ -188,5 +202,6 @@ module.exports = { getChangelogLabelFromPullRequest, getProjectOwnerFromLabel, getMainCodeOwnerfile, - isChangelogLabel + isChangelogLabel, + getUsernamesFromText }; diff --git a/spec/checkPullRequestReviewSpec.js b/spec/checkPullRequestReviewSpec.js index 8e7d33cc..ada36e53 100644 --- a/spec/checkPullRequestReviewSpec.js +++ b/spec/checkPullRequestReviewSpec.js @@ -21,7 +21,8 @@ const { createProbot } = require('probot'); const oppiaBot = require('../index'); const scheduler = require('../lib/scheduler'); const pullRequestReviewModule = require('../lib/checkPullRequestReview'); -const payloadData = require('../fixtures/pullRequestReview.json'); +const reviewPayloadData = require('../fixtures/pullRequestReview.json'); +const commentPayloadData = require('../fixtures/pullRequestComment.json'); const utilityModule = require('../lib/utils'); describe('Pull Request Review Module', () => { @@ -53,11 +54,6 @@ describe('Pull Request Review Module', () => { .and.returnValue({}), addLabels: jasmine.createSpy('addLabels').and.returnValue({}), }, - pulls: { - get: jasmine.createSpy('get').and.resolveTo({ - data: payloadData.payload.pull_request, - }), - }, }; robot = createProbot({ @@ -69,13 +65,22 @@ describe('Pull Request Review Module', () => { app = robot.load(oppiaBot); spyOn(app, 'auth').and.resolveTo(github); spyOn(pullRequestReviewModule, 'handlePullRequestReview').and.callThrough(); + spyOn(pullRequestReviewModule, 'handleResponseToReview').and.callThrough(); spyOn(utilityModule, 'sleep').and.callFake(() => {}); }); describe('A reviewer requests changes to the PR', () => { + beforeEach(() => { + github.pulls = { + get: jasmine.createSpy('get').and.resolveTo({ + data: reviewPayloadData.payload.pull_request, + }), + }; + }); + describe('when reviewer is assigned and pr author is not assigned', () => { beforeEach(async () => { - await robot.receive(payloadData); + await robot.receive(reviewPayloadData); }); it('should check type of review', () => { @@ -92,20 +97,20 @@ describe('Pull Request Review Module', () => { it('should unassign reviewer', async () => { expect(github.issues.removeAssignees).toHaveBeenCalled(); expect(github.issues.removeAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.review.user.login], + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.review.user.login], }); expect(github.issues.createComment).toHaveBeenCalled(); expect(github.issues.createComment).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, body: 'Unassigning @' + - payloadData.payload.review.user.login + + reviewPayloadData.payload.review.user.login + ' since the review is done.', }); }); @@ -113,22 +118,22 @@ describe('Pull Request Review Module', () => { it('should assign pr author', () => { expect(github.issues.addAssignees).toHaveBeenCalled(); expect(github.issues.addAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.pull_request.user.login], + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.pull_request.user.login], }); expect(github.issues.createComment).toHaveBeenCalled(); expect(github.issues.createComment).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, body: 'Assigning @' + - payloadData.payload.pull_request.user.login + + reviewPayloadData.payload.pull_request.user.login + ' to respond to reviews from @' + - payloadData.payload.review.user.login + + reviewPayloadData.payload.review.user.login + '. Thanks!', }); }); @@ -136,15 +141,15 @@ describe('Pull Request Review Module', () => { describe('when reviewer is assigned and pr author is assigned', () => { beforeAll(() => { - payloadData.payload.pull_request.assignees = [ - ...payloadData.payload.pull_request.assignees, + reviewPayloadData.payload.pull_request.assignees = [ + ...reviewPayloadData.payload.pull_request.assignees, { login: 'testuser', }, ]; }); beforeEach(async () => { - await robot.receive(payloadData); + await robot.receive(reviewPayloadData); }); it('should check type of review', () => { @@ -161,20 +166,20 @@ describe('Pull Request Review Module', () => { it('should unassign reviewer', async () => { expect(github.issues.removeAssignees).toHaveBeenCalled(); expect(github.issues.removeAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.review.user.login], + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.review.user.login], }); expect(github.issues.createComment).toHaveBeenCalled(); expect(github.issues.createComment).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, body: 'Unassigning @' + - payloadData.payload.review.user.login + + reviewPayloadData.payload.review.user.login + ' since the review is done.', }); }); @@ -185,30 +190,30 @@ describe('Pull Request Review Module', () => { it('should not ping pr author', () => { expect(github.issues.createComment).not.toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, body: 'Assigning @' + - payloadData.payload.pull_request.user.login + + reviewPayloadData.payload.pull_request.user.login + ' to respond to reviews from @' + - payloadData.payload.review.user.login + + reviewPayloadData.payload.review.user.login + '. Thanks!', }); }); afterAll(() => { - payloadData.payload.pull_request.assignees.pop(); + reviewPayloadData.payload.pull_request.assignees.pop(); }); }); describe('when reviewer and author are not assigned', () => { - const assignees = [...payloadData.payload.pull_request.assignees]; + const assignees = [...reviewPayloadData.payload.pull_request.assignees]; beforeAll(() => { - payloadData.payload.pull_request.assignees = []; + reviewPayloadData.payload.pull_request.assignees = []; }); beforeEach(async () => { - await robot.receive(payloadData); + await robot.receive(reviewPayloadData); }); it('should check type of review', () => { @@ -229,42 +234,42 @@ describe('Pull Request Review Module', () => { it('should assign pr author', () => { expect(github.issues.addAssignees).toHaveBeenCalled(); expect(github.issues.addAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.pull_request.user.login], + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.pull_request.user.login], }); expect(github.issues.createComment).toHaveBeenCalled(); expect(github.issues.createComment).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, body: 'Assigning @' + - payloadData.payload.pull_request.user.login + + reviewPayloadData.payload.pull_request.user.login + ' to respond to reviews from @' + - payloadData.payload.review.user.login + + reviewPayloadData.payload.review.user.login + '. Thanks!', }); }); afterAll(() => { - payloadData.payload.pull_request.assignees = assignees; + reviewPayloadData.payload.pull_request.assignees = assignees; }); }); describe('when reviewer is not assigned but author is assigned', () => { - const assignees = [...payloadData.payload.pull_request.assignees]; + const assignees = [...reviewPayloadData.payload.pull_request.assignees]; beforeAll(() => { - payloadData.payload.pull_request.assignees = [ + reviewPayloadData.payload.pull_request.assignees = [ { login: 'testuser', }, ]; }); beforeEach(async () => { - await robot.receive(payloadData); + await robot.receive(reviewPayloadData); }); it('should check type of review', () => { @@ -288,157 +293,172 @@ describe('Pull Request Review Module', () => { }); afterAll(() => { - payloadData.payload.pull_request.assignees = assignees; + reviewPayloadData.payload.pull_request.assignees = assignees; }); }); - describe('when the pull request gets closed before 3 minutes delay is over', () => { - const initalState = payloadData.payload.pull_request.state; - beforeAll(() => { - payloadData.payload.pull_request.state = 'closed'; - }); - beforeEach(async () => { - await robot.receive(payloadData); - }); + describe( + 'when the pull request gets closed before 3 minutes delay is ' + + 'over', () => { + const initalState = reviewPayloadData.payload.pull_request.state; + beforeAll(() => { + reviewPayloadData.payload.pull_request.state = 'closed'; + }); + beforeEach(async () => { + await robot.receive(reviewPayloadData); + }); - it('should check type of review', () => { - expect( - pullRequestReviewModule.handlePullRequestReview - ).toHaveBeenCalled(); - }); + it('should check type of review', () => { + expect( + pullRequestReviewModule.handlePullRequestReview + ).toHaveBeenCalled(); + }); - it('should wait for 3 minutes before performing any action', () => { - expect(utilityModule.sleep).toHaveBeenCalled(); - expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); - }); + it('should wait for 3 minutes before performing any action', () => { + expect(utilityModule.sleep).toHaveBeenCalled(); + expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); + }); - it('should not unassign reviewer', async () => { - expect(github.issues.removeAssignees).not.toHaveBeenCalled(); - }); + it('should not unassign reviewer', async () => { + expect(github.issues.removeAssignees).not.toHaveBeenCalled(); + }); - it('should not assign pr author', () => { - expect(github.issues.addAssignees).not.toHaveBeenCalled(); - }); + it('should not assign pr author', () => { + expect(github.issues.addAssignees).not.toHaveBeenCalled(); + }); - it('should not ping pr author', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); + it('should not ping pr author', () => { + expect(github.issues.createComment).not.toHaveBeenCalled(); + }); - afterAll(() => { - payloadData.payload.pull_request.state = initalState; - }); + afterAll(() => { + reviewPayloadData.payload.pull_request.state = initalState; + }); }); }); describe('A reviewer approves the PR', () => { beforeAll(() => { - payloadData.payload.review.state = 'approved'; + reviewPayloadData.payload.review.state = 'approved'; + }); + beforeEach(() => { + github.pulls = { + get: jasmine.createSpy('get').and.resolveTo({ + data: reviewPayloadData.payload.pull_request, + }), + }; }); describe( 'when other reviewers are yet to review and are not assigned', () => { - const initialReviewers = [ - ...payloadData.payload.pull_request.requested_reviewers, - ]; - beforeAll(() => { - payloadData.payload.pull_request.requested_reviewers = [ - ...payloadData.payload.pull_request.requested_reviewers, - { login: 'reviewer2' }, - { login: 'reviewer3' }, + const initialReviewers = [ + ...reviewPayloadData.payload.pull_request.requested_reviewers, ]; - }); - beforeEach(async () => { - github.search = { - issuesAndPullRequests: jasmine - .createSpy('issuesAndPullRequests') - .and.resolveTo({ - data: { - items: [], - }, - }), - }; - await robot.receive(payloadData); - }); + beforeAll(() => { + reviewPayloadData.payload.pull_request.requested_reviewers = [ + ...reviewPayloadData.payload.pull_request.requested_reviewers, + { login: 'reviewer2' }, + { login: 'reviewer3' }, + ]; + }); + beforeEach(async () => { + github.search = { + issuesAndPullRequests: jasmine + .createSpy('issuesAndPullRequests') + .and.resolveTo({ + data: { + items: [], + }, + }), + }; + await robot.receive(reviewPayloadData); + }); - it('should check type of review', () => { - expect( - pullRequestReviewModule.handlePullRequestReview - ).toHaveBeenCalled(); - }); + it('should check type of review', () => { + expect( + pullRequestReviewModule.handlePullRequestReview + ).toHaveBeenCalled(); + }); - it('should wait for 3 minutes before performing any action', () => { - expect(utilityModule.sleep).toHaveBeenCalled(); - expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); - }); + it('should wait for 3 minutes before performing any action', () => { + expect(utilityModule.sleep).toHaveBeenCalled(); + expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); + }); - it('should unassign reviewer', async () => { - expect(github.issues.removeAssignees).toHaveBeenCalled(); - expect(github.issues.removeAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.review.user.login], + it('should unassign reviewer', async () => { + expect(github.issues.removeAssignees).toHaveBeenCalled(); + expect(github.issues.removeAssignees).toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.review.user.login], + }); }); - }); - it('should check if all reviewers have approved the PR', () => { - expect(github.search.issuesAndPullRequests).toHaveBeenCalled(); - expect(github.search.issuesAndPullRequests).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - q: `repo:oppia/oppia review:approved ${payloadData.payload.pull_request.number}`, + it('should check if all reviewers have approved the PR', () => { + expect(github.search.issuesAndPullRequests).toHaveBeenCalled(); + expect(github.search.issuesAndPullRequests).toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + q: + 'repo:oppia/oppia review:approved ' + + reviewPayloadData.payload.pull_request.number, + }); }); - }); - it('should assign remaining reviewers', () => { - expect(github.issues.addAssignees).toHaveBeenCalled(); - expect(github.issues.addAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: ['reviewer2', 'reviewer3'], + it('should assign remaining reviewers', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: ['reviewer2', 'reviewer3'], + }); }); - }); - it('should ping remaining reviewers', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - expect(github.issues.createComment).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - body: - 'Assigning @reviewer2, @reviewer3 for code owner reviews' + - ', Thanks!', + it('should ping remaining reviewers', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + expect(github.issues.createComment).toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + body: + 'Assigning @reviewer2, @reviewer3 for code owner reviews' + + ', Thanks!', + }); }); - }); - it('should not assign pr author', () => { - expect(github.issues.addAssignees).not.toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.pull_request.user.login], + it('should not assign pr author', () => { + expect(github.issues.addAssignees).not.toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.pull_request.user.login], + }); }); - }); - afterAll(() => { - payloadData.payload.pull_request.requested_reviewers = initialReviewers; - }); + afterAll(() => { + reviewPayloadData.payload.pull_request.requested_reviewers = ( + initialReviewers + ); + }); }); describe('when other reviewers are yet to review and are assigned', () => { const initialReviewers = [ - ...payloadData.payload.pull_request.requested_reviewers, + ...reviewPayloadData.payload.pull_request.requested_reviewers, + ]; + const initialAssignees = [ + ...reviewPayloadData.payload.pull_request.assignees, ]; - const initialAssignees = [...payloadData.payload.pull_request.assignees]; beforeAll(() => { - payloadData.payload.pull_request.requested_reviewers = [ - ...payloadData.payload.pull_request.requested_reviewers, + reviewPayloadData.payload.pull_request.requested_reviewers = [ + ...reviewPayloadData.payload.pull_request.requested_reviewers, { login: 'reviewer2' }, { login: 'reviewer3' }, ]; - payloadData.payload.pull_request.assignees = [ - ...payloadData.payload.pull_request.assignees, + reviewPayloadData.payload.pull_request.assignees = [ + ...reviewPayloadData.payload.pull_request.assignees, { login: 'reviewer2' }, { login: 'reviewer3' }, ]; @@ -453,7 +473,7 @@ describe('Pull Request Review Module', () => { }, }), }; - await robot.receive(payloadData); + await robot.receive(reviewPayloadData); }); it('should check type of review', () => { @@ -470,20 +490,20 @@ describe('Pull Request Review Module', () => { it('should unassign reviewer', async () => { expect(github.issues.removeAssignees).toHaveBeenCalled(); expect(github.issues.removeAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.review.user.login], + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.review.user.login], }); expect(github.issues.createComment).toHaveBeenCalled(); expect(github.issues.createComment).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, body: 'Unassigning @' + - payloadData.payload.review.user.login + + reviewPayloadData.payload.review.user.login + ' since the PR is approved.', }); }); @@ -491,9 +511,11 @@ describe('Pull Request Review Module', () => { it('should check if all reviewers have approved the PR', () => { expect(github.search.issuesAndPullRequests).toHaveBeenCalled(); expect(github.search.issuesAndPullRequests).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - q: `repo:oppia/oppia review:approved ${payloadData.payload.pull_request.number}`, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + q: + 'repo:oppia/oppia review:approved ' + + reviewPayloadData.payload.pull_request.number, }); }); @@ -505,113 +527,121 @@ describe('Pull Request Review Module', () => { it('should not assign pr author', () => { expect(github.issues.addAssignees).not.toHaveBeenCalled(); expect(github.issues.addAssignees).not.toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.pull_request.user.login] + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.pull_request.user.login], }); }); afterAll(() => { - payloadData.payload.pull_request.assignees = initialAssignees; - payloadData.payload.pull_request.requested_reviewers = initialReviewers; + reviewPayloadData.payload.pull_request.assignees = initialAssignees; + reviewPayloadData.payload.pull_request.requested_reviewers = ( + initialReviewers + ); }); }); describe( 'when other reviewers are yet to review and some are assigned', () => { - const initialAssignees = [...payloadData.payload.pull_request.assignees]; - const initialReviewers = [ - ...payloadData.payload.pull_request.requested_reviewers, - ]; - beforeAll(() => { - payloadData.payload.pull_request.requested_reviewers = [ - ...payloadData.payload.pull_request.requested_reviewers, - { login: 'reviewer2' }, - { login: 'reviewer3' }, + const initialAssignees = [ + ...reviewPayloadData.payload.pull_request.assignees, ]; - - payloadData.payload.pull_request.assignees = [ - ...payloadData.payload.pull_request.assignees, - { login: 'reviewer2' }, + const initialReviewers = [ + ...reviewPayloadData.payload.pull_request.requested_reviewers, ]; - }); - beforeEach(async () => { - github.search = { - issuesAndPullRequests: jasmine - .createSpy('issuesAndPullRequests') - .and.resolveTo({ - data: { - items: [], - }, - }), - }; - await robot.receive(payloadData); - }); + beforeAll(() => { + reviewPayloadData.payload.pull_request.requested_reviewers = [ + ...reviewPayloadData.payload.pull_request.requested_reviewers, + { login: 'reviewer2' }, + { login: 'reviewer3' }, + ]; + + reviewPayloadData.payload.pull_request.assignees = [ + ...reviewPayloadData.payload.pull_request.assignees, + { login: 'reviewer2' }, + ]; + }); + beforeEach(async () => { + github.search = { + issuesAndPullRequests: jasmine + .createSpy('issuesAndPullRequests') + .and.resolveTo({ + data: { + items: [], + }, + }), + }; + await robot.receive(reviewPayloadData); + }); - it('should check type of review', () => { - expect( - pullRequestReviewModule.handlePullRequestReview - ).toHaveBeenCalled(); - }); + it('should check type of review', () => { + expect( + pullRequestReviewModule.handlePullRequestReview + ).toHaveBeenCalled(); + }); - it('should wait for 3 minutes before performing any action', () => { - expect(utilityModule.sleep).toHaveBeenCalled(); - expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); - }); + it('should wait for 3 minutes before performing any action', () => { + expect(utilityModule.sleep).toHaveBeenCalled(); + expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); + }); - it('should unassign reviewer', async () => { - expect(github.issues.removeAssignees).toHaveBeenCalled(); - expect(github.issues.removeAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.review.user.login], + it('should unassign reviewer', async () => { + expect(github.issues.removeAssignees).toHaveBeenCalled(); + expect(github.issues.removeAssignees).toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.review.user.login], + }); }); - }); - it('should check if all reviewers have approved the PR', () => { - expect(github.search.issuesAndPullRequests).toHaveBeenCalled(); - expect(github.search.issuesAndPullRequests).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - q: `repo:oppia/oppia review:approved ${payloadData.payload.pull_request.number}`, + it('should check if all reviewers have approved the PR', () => { + expect(github.search.issuesAndPullRequests).toHaveBeenCalled(); + expect(github.search.issuesAndPullRequests).toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + q: + 'repo:oppia/oppia review:approved ' + + reviewPayloadData.payload.pull_request.number, + }); }); - }); - it('should assign remaining reviewer', () => { - expect(github.issues.addAssignees).toHaveBeenCalled(); - expect(github.issues.addAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: ['reviewer3'], + it('should assign remaining reviewers', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: ['reviewer3'], + }); }); - }); - it('should ping remaining reviewers', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - expect(github.issues.createComment).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - body: 'Assigning @reviewer3 for code owner reviews' + ', Thanks!', + it('should ping remaining reviewers', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + expect(github.issues.createComment).toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + body: 'Assigning @reviewer3 for code owner reviews' + ', Thanks!', + }); }); - }); - it('should not assign pr author', () => { - expect(github.issues.addAssignees).not.toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.pull_request.user.login], + it('should not assign pr author', () => { + expect(github.issues.addAssignees).not.toHaveBeenCalledWith({ + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.pull_request.user.login], + }); }); - }); - afterAll(() => { - payloadData.payload.pull_request.requested_reviewers = initialReviewers; - payloadData.payload.pull_request.assignees = initialAssignees; - }); + afterAll(() => { + reviewPayloadData.payload.pull_request.requested_reviewers = ( + initialReviewers + ); + reviewPayloadData.payload.pull_request.assignees = initialAssignees; + }); }); describe( @@ -620,19 +650,21 @@ describe('Pull Request Review Module', () => { // Note that when the last reviewer approves, the list of // requested reviewers will be empty. const initialReviewers = [ - ...payloadData.payload.pull_request.requested_reviewers, + ...reviewPayloadData.payload.pull_request.requested_reviewers, ]; const initialLabels = [ - ...payloadData.payload.pull_request.labels + ...reviewPayloadData.payload.pull_request.labels ]; const changelogLabel = { name: 'PR CHANGELOG: Server Errors -- @kevintab95' }; - const initialAssignees = [...payloadData.payload.pull_request.assignees]; + const initialAssignees = [ + ...reviewPayloadData.payload.pull_request.assignees + ]; beforeAll(() => { - payloadData.payload.pull_request.requested_reviewers = []; - payloadData.payload.pull_request.assignees = []; - payloadData.payload.pull_request.labels = [changelogLabel]; + reviewPayloadData.payload.pull_request.requested_reviewers = []; + reviewPayloadData.payload.pull_request.assignees = []; + reviewPayloadData.payload.pull_request.labels = [changelogLabel]; }); beforeEach(async () => { github.search = { @@ -640,7 +672,7 @@ describe('Pull Request Review Module', () => { .createSpy('issuesAndPullRequests') .and.resolveTo({ data: { - items: [payloadData.payload.pull_request], + items: [reviewPayloadData.payload.pull_request], }, }), }; @@ -649,7 +681,7 @@ describe('Pull Request Review Module', () => { status: 404, }), }; - await robot.receive(payloadData); + await robot.receive(reviewPayloadData); }); it('should check type of review', () => { @@ -670,19 +702,20 @@ describe('Pull Request Review Module', () => { it('should check if all reviewers have approved the PR', () => { expect(github.search.issuesAndPullRequests).toHaveBeenCalled(); expect(github.search.issuesAndPullRequests).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - q: 'repo:oppia/oppia review:approved ' + - payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + q: + 'repo:oppia/oppia review:approved ' + + reviewPayloadData.payload.pull_request.number, }); }); it('should add LGTM label', () => { expect(github.issues.addLabels).toHaveBeenCalled(); expect(github.issues.addLabels).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, labels: ['PR: LGTM'], }); }); @@ -690,38 +723,41 @@ describe('Pull Request Review Module', () => { it('should check if author can merge', () => { expect(github.orgs.checkMembership).toHaveBeenCalled(); expect(github.orgs.checkMembership).toHaveBeenCalledWith({ - org: payloadData.payload.organization.login, - username: payloadData.payload.pull_request.user.login, + org: reviewPayloadData.payload.organization.login, + username: reviewPayloadData.payload.pull_request.user.login, }); }); - it('should assign project owner', () => { + it('should assign one of the reviewers', () => { expect(github.issues.addAssignees).toHaveBeenCalled(); expect(github.issues.addAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, assignees: ['kevintab95'], }); }); - it('should ping project owner to merge', () => { + it('should ping one of the reviewers to merge', () => { expect(github.issues.createComment).toHaveBeenCalled(); expect(github.issues.createComment).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, body: - 'Hi @kevintab95, this PR is ready to be merged. We are assigning you ' + - 'since the author does not have merging rights. Please make ' + - 'sure there are no pending comments from the ' + + 'Hi @kevintab95, this PR is ready to be merged. We are ' + + 'assigning you since the author does not have merging rights. ' + + 'Please make sure there are no pending comments from the ' + "author's end before merge. Thanks!", }); }); afterAll(() => { - payloadData.payload.pull_request.requested_reviewers = initialReviewers; - payloadData.payload.pull_request.assignees = initialAssignees; + reviewPayloadData.payload.pull_request.requested_reviewers = ( + initialReviewers + ); + reviewPayloadData.payload.pull_request.assignees = initialAssignees; + reviewPayloadData.payload.pull_request.labels = initialLabels; }); }); @@ -730,10 +766,10 @@ describe('Pull Request Review Module', () => { // Note that when the reviewer is the last reviewer, the list of // requested reviewers will be empty. const initialReviewers = [ - ...payloadData.payload.pull_request.requested_reviewers, + ...reviewPayloadData.payload.pull_request.requested_reviewers, ]; beforeAll(() => { - payloadData.payload.pull_request.requested_reviewers = []; + reviewPayloadData.payload.pull_request.requested_reviewers = []; }); beforeEach(async () => { github.search = { @@ -741,7 +777,7 @@ describe('Pull Request Review Module', () => { .createSpy('issuesAndPullRequests') .and.resolveTo({ data: { - items: [payloadData.payload.pull_request], + items: [reviewPayloadData.payload.pull_request], }, }), }; @@ -750,7 +786,7 @@ describe('Pull Request Review Module', () => { status: 204, }), }; - await robot.receive(payloadData); + await robot.receive(reviewPayloadData); }); it('should check type of review', () => { @@ -767,30 +803,30 @@ describe('Pull Request Review Module', () => { it('should unassign reviewer', async () => { expect(github.issues.removeAssignees).toHaveBeenCalled(); expect(github.issues.removeAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.review.user.login], + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.review.user.login], }); }); it('should check if all reviewers have approved the PR', () => { expect(github.search.issuesAndPullRequests).toHaveBeenCalled(); expect(github.search.issuesAndPullRequests).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, q: 'repo:oppia/oppia review:approved ' + - payloadData.payload.pull_request.number, + reviewPayloadData.payload.pull_request.number, }); }); it('should add LGTM label', () => { expect(github.issues.addLabels).toHaveBeenCalled(); expect(github.issues.addLabels).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, labels: ['PR: LGTM'], }); }); @@ -798,53 +834,54 @@ describe('Pull Request Review Module', () => { it('should check if author can merge', () => { expect(github.orgs.checkMembership).toHaveBeenCalled(); expect(github.orgs.checkMembership).toHaveBeenCalledWith({ - org: payloadData.payload.organization.login, - username: payloadData.payload.pull_request.user.login, + org: reviewPayloadData.payload.organization.login, + username: reviewPayloadData.payload.pull_request.user.login, }); }); it('should assign pr author', () => { expect(github.issues.addAssignees).toHaveBeenCalled(); expect(github.issues.addAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.pull_request.user.login], + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.pull_request.user.login], }); }); it('should ping pr author', () => { expect(github.issues.createComment).toHaveBeenCalled(); expect(github.issues.createComment).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, body: 'Hi @' + - payloadData.payload.pull_request.user.login + + reviewPayloadData.payload.pull_request.user.login + ', this PR is ready to be merged. Please make sure there are ' + 'no pending comments before merge. Thanks!', }); }); afterAll(() => { - payloadData.payload.pull_request.requested_reviewers = initialReviewers; + reviewPayloadData.payload.pull_request.requested_reviewers = ( + initialReviewers + ); }); }); - describe( 'when reviewer is last reviewer and already adds the lgtm label', () => { // Note that when the reviewer is the last reviewer, the list of // requested reviewers will be empty. const initialReviewers = [ - ...payloadData.payload.pull_request.requested_reviewers, + ...reviewPayloadData.payload.pull_request.requested_reviewers, ]; - const initialLabels = [...payloadData.payload.pull_request.labels]; + const initialLabels = [...reviewPayloadData.payload.pull_request.labels]; beforeAll(() => { - payloadData.payload.pull_request.requested_reviewers = []; - payloadData.payload.pull_request.labels = [ - ...payloadData.payload.pull_request.labels, + reviewPayloadData.payload.pull_request.requested_reviewers = []; + reviewPayloadData.payload.pull_request.labels = [ + ...reviewPayloadData.payload.pull_request.labels, { name: 'PR: LGTM' }, ]; }); @@ -855,7 +892,7 @@ describe('Pull Request Review Module', () => { .createSpy('issuesAndPullRequests') .and.resolveTo({ data: { - items: [payloadData.payload.pull_request], + items: [reviewPayloadData.payload.pull_request], }, }), }; @@ -864,7 +901,7 @@ describe('Pull Request Review Module', () => { status: 204, }), }; - await robot.receive(payloadData); + await robot.receive(reviewPayloadData); }); it('should check type of review', () => { @@ -881,21 +918,21 @@ describe('Pull Request Review Module', () => { it('should unassign reviewer', async () => { expect(github.issues.removeAssignees).toHaveBeenCalled(); expect(github.issues.removeAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.review.user.login], + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.review.user.login], }); }); it('should check if all reviewers have approved the PR', () => { expect(github.search.issuesAndPullRequests).toHaveBeenCalled(); expect(github.search.issuesAndPullRequests).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, q: 'repo:oppia/oppia review:approved ' + - payloadData.payload.pull_request.number, + reviewPayloadData.payload.pull_request.number, }); }); @@ -906,75 +943,85 @@ describe('Pull Request Review Module', () => { it('should check if author can merge', () => { expect(github.orgs.checkMembership).toHaveBeenCalled(); expect(github.orgs.checkMembership).toHaveBeenCalledWith({ - org: payloadData.payload.organization.login, - username: payloadData.payload.pull_request.user.login, + org: reviewPayloadData.payload.organization.login, + username: reviewPayloadData.payload.pull_request.user.login, }); }); it('should assign pr author', () => { expect(github.issues.addAssignees).toHaveBeenCalled(); expect(github.issues.addAssignees).toHaveBeenCalledWith({ - owner: payloadData.payload.repository.owner.login, - repo: payloadData.payload.repository.name, - issue_number: payloadData.payload.pull_request.number, - assignees: [payloadData.payload.pull_request.user.login], + owner: reviewPayloadData.payload.repository.owner.login, + repo: reviewPayloadData.payload.repository.name, + issue_number: reviewPayloadData.payload.pull_request.number, + assignees: [reviewPayloadData.payload.pull_request.user.login], }); }); afterAll(() => { - payloadData.payload.pull_request.requested_reviewers = initialReviewers; - payloadData.payload.pull_request.labels = initialLabels; + reviewPayloadData.payload.pull_request.requested_reviewers = ( + initialReviewers + ); + reviewPayloadData.payload.pull_request.labels = initialLabels; }); }); describe( - 'when the pull request gets closed / merged before 3 minutes delay '+ - 'is over', () => { - const initalState = payloadData.payload.pull_request.state; - beforeAll(() => { - payloadData.payload.pull_request.state = 'closed'; - }); - beforeEach(async () => { - await robot.receive(payloadData); - }); + 'when the pull request gets closed / merged before 3 minutes delay ' + + 'is over', + () => { + const initalState = reviewPayloadData.payload.pull_request.state; + beforeAll(() => { + reviewPayloadData.payload.pull_request.state = 'closed'; + }); + beforeEach(async () => { + await robot.receive(reviewPayloadData); + }); - it('should check type of review', () => { - expect( - pullRequestReviewModule.handlePullRequestReview - ).toHaveBeenCalled(); - }); + it('should check type of review', () => { + expect( + pullRequestReviewModule.handlePullRequestReview + ).toHaveBeenCalled(); + }); - it('should wait for 3 minutes before performing any action', () => { - expect(utilityModule.sleep).toHaveBeenCalled(); - expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); - }); + it('should wait for 3 minutes before performing any action', () => { + expect(utilityModule.sleep).toHaveBeenCalled(); + expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); + }); - it('should not unassign reviewer', async () => { - expect(github.issues.removeAssignees).not.toHaveBeenCalled(); - }); + it('should not unassign reviewer', async () => { + expect(github.issues.removeAssignees).not.toHaveBeenCalled(); + }); - it('should not add LGTM label', () => { - expect(github.issues.addLabels).not.toHaveBeenCalled(); - }); + it('should not add LGTM label', () => { + expect(github.issues.addLabels).not.toHaveBeenCalled(); + }); - it('should not assign pr author', () => { - expect(github.issues.addAssignees).not.toHaveBeenCalled(); - }); + it('should not assign pr author', () => { + expect(github.issues.addAssignees).not.toHaveBeenCalled(); + }); - afterAll(() => { - payloadData.payload.pull_request.state = initalState; - }); - }); + afterAll(() => { + reviewPayloadData.payload.pull_request.state = initalState; + }); + } + ); }); describe('A reviewer comments on the PR', () => { - const initalState = payloadData.payload.review.state; + const initalState = reviewPayloadData.payload.review.state; beforeAll(() => { - payloadData.payload.review.state = 'commented'; + reviewPayloadData.payload.review.state = 'commented'; }); beforeEach(async () => { - await robot.receive(payloadData); + github.pulls = { + get: jasmine.createSpy('get').and.resolveTo({ + data: reviewPayloadData.payload.pull_request, + }), + }; + + await robot.receive(reviewPayloadData); }); it('should check type of review', () => { @@ -1000,7 +1047,347 @@ describe('Pull Request Review Module', () => { }); afterAll(() => { - payloadData.payload.pull_request.state = initalState; + reviewPayloadData.payload.pull_request.state = initalState; + }); + }); + + describe('Pull request author comments on PR', () => { + beforeEach(() => { + github.pulls = { + get: jasmine.createSpy('get').and.resolveTo({ + data: commentPayloadData.payload.issue, + }), + }; }); + + describe( + 'when author asks reviewer to ptal and does not assign them and author ' + + 'is assigned', () => { + const initialAssignees = commentPayloadData.payload.issue.assignees; + beforeAll(() => { + commentPayloadData.payload.issue.assignees = [ + { + login: commentPayloadData.payload.issue.user.login + } + ]; + }); + + beforeEach(async () => { + await robot.receive(commentPayloadData); + }); + + it('should check call response to review method', () => { + expect( + pullRequestReviewModule.handleResponseToReview + ).toHaveBeenCalled(); + }); + + it('should wait for 3 minutes before performing any action', () => { + expect(utilityModule.sleep).toHaveBeenCalled(); + expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); + }); + + it('should get the updated version of the pull request', () => { + expect(github.pulls.get).toHaveBeenCalled(); + expect(github.pulls.get).toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + pull_number: commentPayloadData.payload.issue.number, + }); + }); + + it('should assign reviewers', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + issue_number: commentPayloadData.payload.issue.number, + assignees: ['reviewer1', 'reviewer2'], + }); + }); + + it('should unassign author', () => { + expect(github.issues.removeAssignees).toHaveBeenCalled(); + expect(github.issues.removeAssignees).toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + issue_number: commentPayloadData.payload.issue.number, + assignees: [commentPayloadData.payload.issue.user.login], + }); + + expect(github.issues.createComment).toHaveBeenCalled(); + expect(github.issues.createComment).toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + issue_number: commentPayloadData.payload.issue.number, + body: + 'Unassigning @testuser since a re-review was requested. ' + + '@testuser, please make sure you have addressed all review ' + + 'comments. Thanks!', + }); + }) + + afterAll(() => { + commentPayloadData.payload.issue.assignees = initialAssignees; + }); + } + ); + + describe( + 'when author asks reviewer to ptal and assigns some of them ' + + 'already', () => { + const initialAssignees = commentPayloadData.payload.issue.assignees; + beforeAll(() => { + commentPayloadData.payload.issue.assignees = [ + { + login: 'reviewer1', + }, + ]; + }); + beforeEach(async () => { + await robot.receive(commentPayloadData); + }); + + it('should check call response to review method', () => { + expect( + pullRequestReviewModule.handleResponseToReview + ).toHaveBeenCalled(); + }); + + it('should wait for 3 minutes before performing any action', () => { + expect(utilityModule.sleep).toHaveBeenCalled(); + expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); + }); + + it('should get the updated version of the pull request', () => { + expect(github.pulls.get).toHaveBeenCalled(); + expect(github.pulls.get).toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + pull_number: commentPayloadData.payload.issue.number, + }); + }); + + it('should assign remaining reviewers', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + issue_number: commentPayloadData.payload.issue.number, + assignees: ['reviewer2'], + }); + }); + + afterAll(() => { + commentPayloadData.payload.issue.assignees = initialAssignees; + }); + } + ); + + describe( + 'when author asks reviewer to ptal and assigns them already', () => { + const initialAssignees = commentPayloadData.payload.issue.assignees; + beforeAll(() => { + commentPayloadData.payload.issue.assignees = [ + { + login: 'reviewer1', + }, + { + login: 'reviewer2', + }, + ]; + }); + beforeEach(async () => { + await robot.receive(commentPayloadData); + }); + + it('should check call response to review method', () => { + expect( + pullRequestReviewModule.handleResponseToReview + ).toHaveBeenCalled(); + }); + + it('should wait for 3 minutes before performing any action', () => { + expect(utilityModule.sleep).toHaveBeenCalled(); + expect(utilityModule.sleep).toHaveBeenCalledWith(THREE_MINUTES); + }); + + it('should get the updated version of the pull request', () => { + expect(github.pulls.get).toHaveBeenCalled(); + expect(github.pulls.get).toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + pull_number: commentPayloadData.payload.issue.number, + }); + }); + + it('should not assign reviewers', () => { + expect(github.issues.addAssignees).not.toHaveBeenCalled(); + expect(github.issues.addAssignees).not.toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + issue_number: commentPayloadData.payload.issue.number, + assignees: ['reviewer1', 'reviewer2'], + }); + }); + + afterAll(() => { + commentPayloadData.payload.issue.assignees = initialAssignees; + }); + } + ); + + describe('when author does not ask reviewer to ptal', () => { + const initialAssignees = commentPayloadData.payload.issue.assignees; + const initalCommentBody = commentPayloadData.payload.comment.body; + beforeAll(() => { + commentPayloadData.payload.issue.assignees = []; + commentPayloadData.payload.comment.body = + 'Hi @reviewer, I wanted you to know that I can create pull ' + + 'requests. Thanks!'; + }); + beforeEach(async () => { + await robot.receive(commentPayloadData); + }); + + it('should check call response to review method', () => { + expect( + pullRequestReviewModule.handleResponseToReview + ).toHaveBeenCalled(); + }); + + it('should not wait for 3 minutes', () => { + expect(utilityModule.sleep).not.toHaveBeenCalled(); + expect(utilityModule.sleep).not.toHaveBeenCalledWith(THREE_MINUTES); + }); + + it('should not get the updated version of the pull request', () => { + expect(github.pulls.get).not.toHaveBeenCalled(); + expect(github.pulls.get).not.toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + pull_number: commentPayloadData.payload.issue.number, + }); + }); + + it('should not assign reviewers', () => { + expect(github.issues.addAssignees).not.toHaveBeenCalled(); + expect(github.issues.addAssignees).not.toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + issue_number: commentPayloadData.payload.issue.number, + assignees: ['reviewer1', 'reviewer2'], + }); + }); + + afterAll(() => { + commentPayloadData.payload.issue.assignees = initialAssignees; + commentPayloadData.payload.comment.body = initalCommentBody; + }); + }); + + describe('when another user comments on pr', () => { + const initialAssignees = commentPayloadData.payload.issue.assignees; + const initalCommentBody = commentPayloadData.payload.comment.body; + const initialCommenter = { ...commentPayloadData.payload.comment.user }; + const initialSender = { ...commentPayloadData.payload.sender }; + beforeAll(() => { + commentPayloadData.payload.issue.assignees = []; + commentPayloadData.payload.comment.body = + 'Hi @author, I will be reviewing this PR in a few minutes.'; + commentPayloadData.payload.sender = { + login: 'sample_sender', + }; + commentPayloadData.payload.comment.user = { + login: 'sample_sender', + }; + }); + beforeEach(async () => { + await robot.receive(commentPayloadData); + }); + + it('should check call response to review method', () => { + expect( + pullRequestReviewModule.handleResponseToReview + ).toHaveBeenCalled(); + }); + + it('should not wait for 3 minutes', () => { + expect(utilityModule.sleep).not.toHaveBeenCalled(); + expect(utilityModule.sleep).not.toHaveBeenCalledWith(THREE_MINUTES); + }); + + it('should not get the updated version of the pull request', () => { + expect(github.pulls.get).not.toHaveBeenCalled(); + expect(github.pulls.get).not.toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + pull_number: commentPayloadData.payload.issue.number, + }); + }); + + it('should not assign reviewers', () => { + expect(github.issues.addAssignees).not.toHaveBeenCalled(); + expect(github.issues.addAssignees).not.toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + issue_number: commentPayloadData.payload.issue.number, + assignees: ['reviewer1', 'reviewer2'], + }); + }); + + afterAll(() => { + commentPayloadData.payload.issue.assignees = initialAssignees; + commentPayloadData.payload.comment.body = initalCommentBody; + commentPayloadData.payload.sender = initialSender; + commentPayloadData.payload.comment.user = initialCommenter; + }); + }); + + describe( + 'when the comment is on an issue and not a pull request', () => { + const pullRequestInfo = commentPayloadData.payload.issue.pull_request; + beforeAll(() => { + delete commentPayloadData.payload.issue.pull_request; + }); + beforeEach(async () => { + await robot.receive(commentPayloadData); + }); + + it('should check call response to review method', () => { + expect( + pullRequestReviewModule.handleResponseToReview + ).toHaveBeenCalled(); + }); + + it('should not wait for 3 minutes', () => { + expect(utilityModule.sleep).not.toHaveBeenCalled(); + expect(utilityModule.sleep).not.toHaveBeenCalledWith(THREE_MINUTES); + }); + + it('should not get the updated version of the pull request', () => { + expect(github.pulls.get).not.toHaveBeenCalled(); + expect(github.pulls.get).not.toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + pull_number: commentPayloadData.payload.issue.number, + }); + }); + + it('should not assign reviewers', () => { + expect(github.issues.addAssignees).not.toHaveBeenCalled(); + expect(github.issues.addAssignees).not.toHaveBeenCalledWith({ + repo: commentPayloadData.payload.repository.name, + owner: commentPayloadData.payload.repository.owner.login, + issue_number: commentPayloadData.payload.issue.number, + assignees: ['reviewer1', 'reviewer2'], + }); + }); + + afterAll(() => { + commentPayloadData.payload.issue.pull_request = pullRequestInfo; + }); + } + ); }); }); diff --git a/spec/utilsSpec.js b/spec/utilsSpec.js index 9f9473cb..4a007549 100644 --- a/spec/utilsSpec.js +++ b/spec/utilsSpec.js @@ -283,4 +283,48 @@ describe('Utility module tests', () => { ); expect(response).toBe(false); }); + + it('should get github usernames from text', () => { + let text = '@U8NWXD PTAL!'; + let usernames = utilityModule.getUsernamesFromText(text); + expect(usernames.length).toBe(1); + expect(usernames[0]).toBe('U8NWXD'); + + text = '@aks681 @seanlip PTAL!'; + usernames = utilityModule.getUsernamesFromText(text); + expect(usernames.length).toBe(2); + expect(usernames[0]).toBe('aks681'); + expect(usernames[1]).toBe('seanlip'); + + text = `@DubeySandeep, done I've created the issue(#10419 ) and addressed your comments. + @seanlip @aks681 PTAL`; + usernames = utilityModule.getUsernamesFromText(text); + expect(usernames.length).toBe(3); + expect(usernames[0]).toBe('DubeySandeep'); + expect(usernames[1]).toBe('seanlip'); + expect(usernames[2]).toBe('aks681'); + + text = 'Hi @U8NWXD, please take a look, thanks!'; + usernames = utilityModule.getUsernamesFromText(text); + expect(usernames.length).toBe(1); + expect(usernames[0]).toBe('U8NWXD'); + + text = "@aks681 @seanlip I've addressed all comments, please take a look.!"; + usernames = utilityModule.getUsernamesFromText(text); + expect(usernames.length).toBe(2); + expect(usernames[0]).toBe('aks681'); + expect(usernames[1]).toBe('seanlip'); + + text = `@DubeySandeep, done I've created the issue(#10419 ) and addressed your comments. + @seanlip @aks681 please take a look`; + usernames = utilityModule.getUsernamesFromText(text); + expect(usernames.length).toBe(3); + expect(usernames[0]).toBe('DubeySandeep'); + expect(usernames[1]).toBe('seanlip'); + expect(usernames[2]).toBe('aks681'); + + text = 'A random text containing no users'; + usernames = utilityModule.getUsernamesFromText(text); + expect(usernames.length).toBe(0); + }); });