From 59ff01a6dcadd315b7bf2186e71459bf38aacd20 Mon Sep 17 00:00:00 2001 From: Fabian Wollert Date: Sat, 23 Feb 2019 16:16:25 +0100 Subject: [PATCH 1/4] basic implemantion --- server/checks/PullRequestSize.js | 75 +++++++++++++++++++++++++++++ server/checks/index.js | 5 +- test/server/pullrequestsize.test.js | 15 ++++++ 3 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 server/checks/PullRequestSize.js create mode 100644 test/server/pullrequestsize.test.js diff --git a/server/checks/PullRequestSize.js b/server/checks/PullRequestSize.js new file mode 100644 index 00000000..edcaefbc --- /dev/null +++ b/server/checks/PullRequestSize.js @@ -0,0 +1,75 @@ +import Check, { getPayloadFn } from './Check' +import { PULL_REQUEST } from '../model/GithubEvents' +import { logger } from '../../common/debug' + +const CHECK_TYPE = 'pullrequestsize' +const CONTEXT = 'zappr/pr/size' +const info = logger(CHECK_TYPE, 'info') +const error = logger(CHECK_TYPE, 'error') +const debug = logger(CHECK_TYPE) +const createStatePayload = getPayloadFn(CONTEXT) + + +export default class PullRequestTasks extends Check { + static TYPE = CHECK_TYPE; + static CONTEXT = CONTEXT; + static HOOK_EVENTS = [PULL_REQUEST]; + static NAME = 'Pull request size check' + + constructor(github) { + super() + this.github = github; + } + + async checkPRSizeAndSetStatus({pull_request, repository, token, config}) { + const {fullName, name, owner} = repository + const {number, additions, deletions, changed_files} = pull_request + const max_additions = getIn(config, ['pull-request', 'size', 'max', 'additions'], []) + const max_deletions = getIn(config, ['pull-request', 'size', 'max', 'deletions'], []) + const max_changed_files = getIn(config, ['pull-request', 'size', 'max', 'changed_files'], []) + + const checkEnabled = max_additions.length > 0 ? max_deletions.length > 0 ? max_changed_files.length > 0 : false : false + let msg, status; + + if (checkEnabled) { + if (additions > max_additions) { + info(`${fullName}#${number}: Failed the PR due to more additions than allowed.`); + status = createStatePayload(`PR adds too much lines.`, 'failure'); + } else if (deletions > max_deletions) { + info(`${fullName}#${number}: Failed the PR due to more deletions than allowed.`); + status = createStatePayload(`PR deletes too much lines.`, 'failure'); + } else if (changed_files > max_changed_files) { + info(`${fullName}#${number}: Failed the PR due to more files changed than allowed.`); + status = createStatePayload(`PR changes too much files.`, 'failure'); + } else { + msg = `PR Size restrictions are met.`; + info(`${fullName}#${number}: ${msg}`); + status = createStatePayload(msg); + } + } else { + info(`${fullName}#${number}: No PR size settings set.`); + status = createStatePayload(`No PR size settings set.`); + } + + await this.github.setCommitStatus(owner.login, name, pull_request.head.sha, status, token); + info(`${fullName}#${number}: Set status to ${status.state}.`) + } + + async execute(config, hookPayload, token) { + const {action, repository, number, pull_request} = hookPayload + const repoOwner = repository.owner.login + const repoName = repository.name + const fullName = repository.full_name + let status; + + try { + if (pull_request.state === 'open' && ['opened', 'edited', 'reopened', 'synchronize'].indexOf(action) !== -1) { + await this.checkPRSizeAndSetStatus({pull_request, repository, token, config}) + } + } catch (e) { + error(`${fullName}#${number}: Could not execute ${NAME}`, e) + status = createStatePayload(`Error: ${e.message}`, 'error') + await this.github.setCommitStatus(repoOwner, repoName, pull_request.head.sha, status, token); + } + } +} diff --git a/server/checks/index.js b/server/checks/index.js index 57f26850..f2e3ca73 100644 --- a/server/checks/index.js +++ b/server/checks/index.js @@ -6,6 +6,7 @@ import PullRequestMilestone from './PullRequestMilestone' import PullRequestLabels from './PullRequestLabels' import PullRequestMergeCommit from './PullRequestMergeCommit' import PullRequestTasks from './PullRequestTasks' +import PullRequestSize from './PullRequestSize' const CHECKS = { [Approval.TYPE]: Approval, @@ -16,6 +17,7 @@ const CHECKS = { [PullRequestLabels.TYPE]: PullRequestLabels, [PullRequestMergeCommit.TYPE]: PullRequestMergeCommit, [PullRequestTasks.TYPE]: PullRequestTasks + [PullRequestSize.TYPE]: PullRequestSize } export const CHECK_NAMES = { @@ -27,6 +29,7 @@ export const CHECK_NAMES = { [PullRequestLabels.TYPE]: PullRequestLabels.NAME, [PullRequestMergeCommit.TYPE]: PullRequestMergeCommit.NAME, [PullRequestTasks.TYPE]: PullRequestTasks.NAME + [PullRequestSize.TYPE]: PullRequestSize.NAME } export const CHECK_TYPES = Object.keys(CHECKS) @@ -35,4 +38,4 @@ export function getCheckByType(type) { return CHECKS[type] } -export { Approval, Autobranch, CommitMessage, Specification, PullRequestMilestone, PullRequestLabels, PullRequestMergeCommit, PullRequestTasks } +export { Approval, Autobranch, CommitMessage, Specification, PullRequestMilestone, PullRequestLabels, PullRequestMergeCommit, PullRequestTasks, PullRequestSize } diff --git a/test/server/pullrequestsize.test.js b/test/server/pullrequestsize.test.js new file mode 100644 index 00000000..889a0dac --- /dev/null +++ b/test/server/pullrequestsize.test.js @@ -0,0 +1,15 @@ +import sinon from 'sinon' +import { expect } from 'chai' +import { GithubService } from '../../server/service/GithubService' +import PullRequestSize, { generateStatus } from '../../server/checks/PullRequestSize' + + +describe('Pull Request Size', () => { + describe('#generateStatus', () => { + it('generates failure when there are redundant labels and additional = false', () => { + const status = generateStatus(labels, {size: {max: {additions: 10}}}) + expect(status.description).to.equal(`PR has redundant labels: work-in-progress.`) + expect(status.state).to.equal('failure') + }) + }) +}) From d48c4afe8c05ede7b93571439a60bd5b0aaa2890 Mon Sep 17 00:00:00 2001 From: Fabian Wollert Date: Sat, 23 Feb 2019 16:17:35 +0100 Subject: [PATCH 2/4] fix class name --- server/checks/PullRequestSize.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/checks/PullRequestSize.js b/server/checks/PullRequestSize.js index edcaefbc..34ac8716 100644 --- a/server/checks/PullRequestSize.js +++ b/server/checks/PullRequestSize.js @@ -10,7 +10,7 @@ const debug = logger(CHECK_TYPE) const createStatePayload = getPayloadFn(CONTEXT) -export default class PullRequestTasks extends Check { +export default class PullRequestSize extends Check { static TYPE = CHECK_TYPE; static CONTEXT = CONTEXT; static HOOK_EVENTS = [PULL_REQUEST]; From 47c0f66080a7d03d68e449930f41dbffe698410f Mon Sep 17 00:00:00 2001 From: Fabian Wollert Date: Sat, 23 Feb 2019 16:20:39 +0100 Subject: [PATCH 3/4] add docs --- docs/setup.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/setup.md b/docs/setup.md index 636ab5bf..16e46776 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -200,3 +200,22 @@ Needs to be done: ~~~ Currently this check doesn't take configuration. + +### Pull Request Size + +With the Pull Request Size Restriction check you can block a pull request which is too big. You can restrict three different key metrics from the PR: + +- Lines added +- Lines removed +- Files changed + +It is configured under `pull-request.size` and supports the following options: + +~~~ yaml +pull-request: + size: + max: + additions: 500 + deletions: 500 + changed_files: 20 +~~~ From 96ea2fffa87b00ed9e85f25709cfe3923a0a610a Mon Sep 17 00:00:00 2001 From: Fabian Wollert Date: Sat, 23 Feb 2019 16:28:08 +0100 Subject: [PATCH 4/4] fix JS code --- server/checks/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/checks/index.js b/server/checks/index.js index f2e3ca73..230a6073 100644 --- a/server/checks/index.js +++ b/server/checks/index.js @@ -16,7 +16,7 @@ const CHECKS = { [PullRequestMilestone.TYPE]: PullRequestMilestone, [PullRequestLabels.TYPE]: PullRequestLabels, [PullRequestMergeCommit.TYPE]: PullRequestMergeCommit, - [PullRequestTasks.TYPE]: PullRequestTasks + [PullRequestTasks.TYPE]: PullRequestTasks, [PullRequestSize.TYPE]: PullRequestSize } @@ -28,7 +28,7 @@ export const CHECK_NAMES = { [PullRequestMilestone.TYPE]: PullRequestMilestone.NAME, [PullRequestLabels.TYPE]: PullRequestLabels.NAME, [PullRequestMergeCommit.TYPE]: PullRequestMergeCommit.NAME, - [PullRequestTasks.TYPE]: PullRequestTasks.NAME + [PullRequestTasks.TYPE]: PullRequestTasks.NAME, [PullRequestSize.TYPE]: PullRequestSize.NAME }