From 853b992ef0f206409492655c83f687ba78278cac Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Sun, 23 Sep 2018 18:30:29 +0200 Subject: [PATCH 01/16] test that PRs in forks should not be updated --- test/pull-request-handler.test.ts | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/pull-request-handler.test.ts b/test/pull-request-handler.test.ts index 7af484ad2..f1cdb4f5f 100644 --- a/test/pull-request-handler.test.ts +++ b/test/pull-request-handler.test.ts @@ -100,6 +100,7 @@ describe('getPullRequestActions', () => { ) expect(actions).toEqual(['update_branch']) }) + it('not update branch when pull request is up-to-date and update-branch is disabled', async () => { const actions = getPullRequestActions( createHandlerContext({ @@ -131,6 +132,44 @@ describe('getPullRequestActions', () => { ) expect(actions).toEqual(['merge']) }) + + it('not update branch when pull request is is based in fork', async () => { + const actions = getPullRequestActions( + createHandlerContext({ + config: createConfig({ + updateBranch: true + }) + }), + createPullRequestInfo({ + baseRef: defaultBaseRef, + headRef: { + name: 'pr-my-branch', + repository: { + ...defaultBaseRef.repository, + owner: { + login: 'some-contributor' + } + }, + target: { + oid: '2' + } + }, + baseRefOid: '3', + repository: { + protectedBranches: { + nodes: [{ + name: 'master', + hasRestrictedPushes: true, + hasStrictRequiredStatusChecks: true + }] + } + } + }), + createPullRequestStatus() + ) + expect(actions).toEqual([]) + }) + it('delete branch when status is ready_for_merge and delete-branch-after-merge is enabled and branch resides in same repository', async () => { const actions = getPullRequestActions( createHandlerContext({ From 84982ffb457e6e4aba09e5a371686f8256fbe7be Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Thu, 4 Oct 2018 17:48:39 +0000 Subject: [PATCH 02/16] chore(package): update @types/debug to version 0.0.31 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e2f355622..6653dbb55 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,7 @@ "raven": "^2.6.4" }, "devDependencies": { - "@types/debug": "0.0.30", + "@types/debug": "0.0.31", "@types/jest": "^23.3.2", "@types/node": "^10.11.3", "@types/p-queue": "^2.3.1", From 374d4c5e9d91b8283bfd56c4fbb43531976638e8 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Thu, 4 Oct 2018 17:48:43 +0000 Subject: [PATCH 03/16] chore(package): update lockfile package-lock.json --- package-lock.json | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/package-lock.json b/package-lock.json index cdd092f07..92204424c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -78,9 +78,9 @@ } }, "@types/debug": { - "version": "0.0.30", - "resolved": "https://registry.npmjs.org/@types/debug/-/debug-0.0.30.tgz", - "integrity": "sha512-orGL5LXERPYsLov6CWs3Fh6203+dXzJkR7OnddIr2514Hsecwc8xRpzCapshBbKFImCsvS/mk6+FWiN5LyZJAQ==", + "version": "0.0.31", + "resolved": "https://registry.npmjs.org/@types/debug/-/debug-0.0.31.tgz", + "integrity": "sha512-LS1MCPaQKqspg7FvexuhmDbWUhE2yIJ+4AgVIyObfc06/UKZ8REgxGNjZc82wPLWmbeOm7S+gSsLgo75TanG4A==", "dev": true }, "@types/events": { @@ -207,6 +207,7 @@ "version": "0.1.4", "resolved": "https://registry.npmjs.org/align-text/-/align-text-0.1.4.tgz", "integrity": "sha1-DNkKVhCT810KmSVsIrcGlDP60Rc=", + "optional": true, "requires": { "kind-of": "^3.0.2", "longest": "^1.0.1", @@ -2975,12 +2976,14 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -2995,17 +2998,20 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -3122,7 +3128,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -3134,6 +3141,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -3148,6 +3156,7 @@ "version": "3.0.4", "bundled": true, "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -3155,12 +3164,14 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.2.4", "bundled": true, "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -3179,6 +3190,7 @@ "version": "0.5.1", "bundled": true, "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -3259,7 +3271,8 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -3271,6 +3284,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -3392,6 +3406,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -5174,7 +5189,8 @@ "longest": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/longest/-/longest-1.0.1.tgz", - "integrity": "sha1-MKCy2jj3N3DoKUoNIuZiXtd9AJc=" + "integrity": "sha1-MKCy2jj3N3DoKUoNIuZiXtd9AJc=", + "optional": true }, "loose-envify": { "version": "1.4.0", @@ -6374,7 +6390,7 @@ "dependencies": { "minimist": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true } @@ -8765,7 +8781,7 @@ }, "strip-ansi": { "version": "3.0.1", - "resolved": "http://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "dev": true, "requires": { From beaad3e0880b853b8ef4898560050c0569151f92 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Thu, 4 Oct 2018 20:57:10 +0200 Subject: [PATCH 04/16] validate query result for PR headRef --- src/pull-request-query.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pull-request-query.ts b/src/pull-request-query.ts index 9a71e257d..83e31c839 100644 --- a/src/pull-request-query.ts +++ b/src/pull-request-query.ts @@ -74,12 +74,13 @@ export async function queryPullRequest (github: Context['github'], { owner, repo if (!response) { throw new Error(`Could not query pull request ${owner}/${repo}#${pullRequestNumber}`) } - if (!response.repository) { - const error: any = new Error(`Query result does not have repository`) + if (!response.repository || !response.repository.pullRequest || !response.repository.pullRequest.headRef) { + const error: any = new Error(`Query result is not complete`) error.pullRequest = `${owner}/${repo}#${pullRequestNumber}` error.response = response throw error } + const queryResult = response as PullRequestQueryResult const checks = result<{ check_runs: CheckRun[] }>(await github.checks.listForRef({ From edaaba33aa14e3f93b9018461bf2b1ddd61a6b60 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Thu, 4 Oct 2018 22:03:16 +0200 Subject: [PATCH 05/16] throw specific error when no permission detected --- src/pull-request-query.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pull-request-query.ts b/src/pull-request-query.ts index 83e31c839..61d702b69 100644 --- a/src/pull-request-query.ts +++ b/src/pull-request-query.ts @@ -74,10 +74,9 @@ export async function queryPullRequest (github: Context['github'], { owner, repo if (!response) { throw new Error(`Could not query pull request ${owner}/${repo}#${pullRequestNumber}`) } - if (!response.repository || !response.repository.pullRequest || !response.repository.pullRequest.headRef) { - const error: any = new Error(`Query result is not complete`) + if (!response.repository.pullRequest.headRef && !response.repository.mergeable) { + const error: any = new Error(`No permission to source repository of pull request`) error.pullRequest = `${owner}/${repo}#${pullRequestNumber}` - error.response = response throw error } From 27ba1449ac9021659332ae4267cc36691e50f872 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Thu, 4 Oct 2018 22:13:20 +0200 Subject: [PATCH 06/16] test whether no permission error is thrown for no permission --- test/integration.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/integration.test.ts b/test/integration.test.ts index 74249e575..cf52b3fc5 100644 --- a/test/integration.test.ts +++ b/test/integration.test.ts @@ -294,3 +294,41 @@ it('to report error when processing pull request results in error', async () => expect(captureException).toHaveBeenCalled() expect(consoleError).toHaveBeenCalled() }) + +it('when no permission to source repository throw a no permission error', async () => { + const Raven = require('raven') + const captureException = jest.fn() + Raven.captureException = captureException + + const config = ` + minApprovals: + OWNER: 1 + ` + + const pullRequestInfo = createPullRequestInfo({ + headRef: undefined, + mergeable: undefined + }) + + const github = createGithubApiFromPullRequestInfo({ + pullRequestInfo, + config + }) + + const app = createApplication({ + appFn: require('../src/index'), + logger: createEmptyLogger(), + github + }) + + await app.receive( + createPullRequestOpenedEvent({ + owner: pullRequestInfo.baseRef.repository.owner.login, + repo: pullRequestInfo.baseRef.repository.name, + number: 1 + }) + ) + + expect(captureException).toHaveBeenCalled() + expect(captureException.mock.calls[0][0].message).toContain('No permission') +}) From ec8487c50bdf99392a287843f2d5eaa8e3a6692e Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Thu, 4 Oct 2018 22:35:31 +0200 Subject: [PATCH 07/16] add pullRequestInfo to reported errors --- src/pull-request-handler.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/pull-request-handler.ts b/src/pull-request-handler.ts index 820ec672a..8ffe24c0d 100644 --- a/src/pull-request-handler.ts +++ b/src/pull-request-handler.ts @@ -1,5 +1,5 @@ -import { conditions } from './conditions/index' import Raven from 'raven' +import { conditions } from './conditions/index' import { HandlerContext, PullRequestReference, PullRequestInfo } from './models' import { result } from './utils' import { getPullRequestStatus, PullRequestStatus } from './pull-request-status' @@ -20,7 +20,12 @@ export async function handlePullRequest ( context.github, pullRequestReference ) - context.log.debug('pullRequestInfo:', pullRequestInfo) + + Raven.mergeContext({ + extra: { + pullRequestInfo + } + }) const pullRequestStatus = getPullRequestStatus( context, From 1ad13e2ea211b070854155eeee4d3ae8e8a56f86 Mon Sep 17 00:00:00 2001 From: luoxi Date: Fri, 21 Sep 2018 18:08:23 +0900 Subject: [PATCH 08/16] add conditions, checking title automatic merges will be blocked if there is a match between the regular expression and title --- auto-merge.example.yml | 3 +++ src/conditions/blockingTitle.ts | 20 ++++++++++++++ src/conditions/index.ts | 4 ++- src/config.ts | 10 ++++--- src/github-models.ts | 1 + src/pull-request-query.ts | 1 + test/conditions/blockingTitle.test.ts | 38 +++++++++++++++++++++++++++ test/mock.ts | 3 ++- 8 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 src/conditions/blockingTitle.ts create mode 100644 test/conditions/blockingTitle.test.ts diff --git a/auto-merge.example.yml b/auto-merge.example.yml index 4ed6f7059..375b2b557 100644 --- a/auto-merge.example.yml +++ b/auto-merge.example.yml @@ -46,3 +46,6 @@ blockingLabels: # all of these labels are attached to a pull request. requiredLabels: - merge + +# Automatic merges will be blocked if there is a match between the regular expression and title +blockingTitleRegex: 'wip' \ No newline at end of file diff --git a/src/conditions/blockingTitle.ts b/src/conditions/blockingTitle.ts new file mode 100644 index 000000000..9bc3d4636 --- /dev/null +++ b/src/conditions/blockingTitle.ts @@ -0,0 +1,20 @@ +import { ConditionConfig } from './../config' +import { PullRequestInfo } from '../models' +import { ConditionResult } from '../condition' + +export default function doesNotHaveBlockingTitle ( + config: ConditionConfig, + pullRequestInfo: PullRequestInfo +): ConditionResult { + const regexObj = new RegExp(config.blockingTitleRegex, 'i') + + if (regexObj.test(pullRequestInfo.title)) { + return { + status: 'fail', + message: `Blocking words are found (${ pullRequestInfo.title })` + } + } + return { + status: 'success' + } +} diff --git a/src/conditions/index.ts b/src/conditions/index.ts index fc1d60c82..c8ce40044 100644 --- a/src/conditions/index.ts +++ b/src/conditions/index.ts @@ -6,6 +6,7 @@ import blockingLabels from './blockingLabels' import blockingChecks from './blockingChecks' import minimumApprovals from './minimumApprovals' import maximumChangesRequested from './maximumChangesRequested' +import blockingTitle from './blockingTitle' import { keysOf } from '../utils' export const conditions = { @@ -15,7 +16,8 @@ export const conditions = { blockingLabels, minimumApprovals, maximumChangesRequested, - blockingChecks + blockingChecks, + blockingTitle } export type Conditions = typeof conditions diff --git a/src/config.ts b/src/config.ts index d3eb826ed..de18f5536 100644 --- a/src/config.ts +++ b/src/config.ts @@ -29,7 +29,8 @@ export type ConditionConfig = { minApprovals: { [key in CommentAuthorAssociation]?: number }, maxRequestedChanges: { [key in CommentAuthorAssociation]?: number }, requiredLabels: string[], - blockingLabels: string[] + blockingLabels: string[], + blockingTitleRegex: string } export type Config = { @@ -46,7 +47,8 @@ export const defaultRuleConfig: ConditionConfig = { NONE: 0 }, blockingLabels: [], - requiredLabels: [] + requiredLabels: [], + blockingTitleRegex: 'wip' } export const defaultConfig: Config = { @@ -71,7 +73,8 @@ const conditionConfigDecoder: Decoder = object({ minApprovals: reviewConfigDecover, maxRequestedChanges: reviewConfigDecover, requiredLabels: array(string()), - blockingLabels: array(string()) + blockingLabels: array(string()), + blockingTitleRegex: string() }) const configDecoder: Decoder = object({ @@ -80,6 +83,7 @@ const configDecoder: Decoder = object({ maxRequestedChanges: reviewConfigDecover, requiredLabels: array(string()), blockingLabels: array(string()), + blockingTitleRegex: string(), updateBranch: boolean(), deleteBranchAfterMerge: boolean(), mergeMethod: oneOf( diff --git a/src/github-models.ts b/src/github-models.ts index 20eff2f45..0ec04b288 100644 --- a/src/github-models.ts +++ b/src/github-models.ts @@ -45,6 +45,7 @@ export interface PullRequestQueryResult { name: string }> }, + title: string, authorAssociation: CommentAuthorAssociation, baseRef: { repository: { diff --git a/src/pull-request-query.ts b/src/pull-request-query.ts index 61d702b69..be8e447ab 100644 --- a/src/pull-request-query.ts +++ b/src/pull-request-query.ts @@ -27,6 +27,7 @@ export async function queryPullRequest (github: Context['github'], { owner, repo name } } + title authorAssociation baseRef { repository { diff --git a/test/conditions/blockingTitle.test.ts b/test/conditions/blockingTitle.test.ts new file mode 100644 index 000000000..9451e192a --- /dev/null +++ b/test/conditions/blockingTitle.test.ts @@ -0,0 +1,38 @@ +import blockingTitle from '../../src/conditions/blockingTitle' +import { createPullRequestInfo, createConditionConfig } from '../mock' + +describe('blockingTitle', () => { + it('returns fail if there is not a match in title(with default configuration);', async () => { + const result = blockingTitle( + createConditionConfig(), + createPullRequestInfo({ + title: '[WIP] help needed' + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns success if there is a match in title;', async () => { + const result = blockingTitle( + createConditionConfig({ + blockingTitleRegex: 'wip' + }), + createPullRequestInfo({ + title: 'mergeable' + }) + ) + expect(result.status).toBe('success') + }) + + it('returns fail if there is not a match in title;', async () => { + const result = blockingTitle( + createConditionConfig({ + blockingTitleRegex: 'wip' + }), + createPullRequestInfo({ + title: '[WIP] help needed' + }) + ) + expect(result.status).toBe('fail') + }) +}) diff --git a/test/mock.ts b/test/mock.ts index 1422a4c0d..f79bc2d9f 100644 --- a/test/mock.ts +++ b/test/mock.ts @@ -54,7 +54,8 @@ export const defaultPullRequestInfo: PullRequestInfo = { nodes: [] } }, - checkRuns: [] + checkRuns: [], + title: 'mergeable title' } export function createPullRequestInfo (pullRequestInfo?: Partial): PullRequestInfo { From f780edee81c73ac753dacba4eb8d4f24d63a23e3 Mon Sep 17 00:00:00 2001 From: luoxi Date: Thu, 11 Oct 2018 14:42:39 +0900 Subject: [PATCH 09/16] fix for code review --- auto-merge.example.yml | 2 +- src/conditions/blockingTitle.ts | 8 +++++++- src/config.ts | 8 ++++---- test/conditions/blockingTitle.test.ts | 10 +++++----- test/mock.ts | 2 +- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/auto-merge.example.yml b/auto-merge.example.yml index 375b2b557..6da217b50 100644 --- a/auto-merge.example.yml +++ b/auto-merge.example.yml @@ -48,4 +48,4 @@ requiredLabels: - merge # Automatic merges will be blocked if there is a match between the regular expression and title -blockingTitleRegex: 'wip' \ No newline at end of file +blockingTitleRegex: '\bwip\b' \ No newline at end of file diff --git a/src/conditions/blockingTitle.ts b/src/conditions/blockingTitle.ts index 9bc3d4636..e7c17a632 100644 --- a/src/conditions/blockingTitle.ts +++ b/src/conditions/blockingTitle.ts @@ -6,12 +6,18 @@ export default function doesNotHaveBlockingTitle ( config: ConditionConfig, pullRequestInfo: PullRequestInfo ): ConditionResult { + if (config.blockingTitleRegex === undefined) { + return { + status: 'success' + } + } + const regexObj = new RegExp(config.blockingTitleRegex, 'i') if (regexObj.test(pullRequestInfo.title)) { return { status: 'fail', - message: `Blocking words are found (${ pullRequestInfo.title })` + message: `Blocking words are found in title (${ pullRequestInfo.title })` } } return { diff --git a/src/config.ts b/src/config.ts index de18f5536..ee4cc6c21 100644 --- a/src/config.ts +++ b/src/config.ts @@ -30,7 +30,7 @@ export type ConditionConfig = { maxRequestedChanges: { [key in CommentAuthorAssociation]?: number }, requiredLabels: string[], blockingLabels: string[], - blockingTitleRegex: string + blockingTitleRegex: string | undefined } export type Config = { @@ -48,7 +48,7 @@ export const defaultRuleConfig: ConditionConfig = { }, blockingLabels: [], requiredLabels: [], - blockingTitleRegex: 'wip' + blockingTitleRegex: undefined } export const defaultConfig: Config = { @@ -74,7 +74,7 @@ const conditionConfigDecoder: Decoder = object({ maxRequestedChanges: reviewConfigDecover, requiredLabels: array(string()), blockingLabels: array(string()), - blockingTitleRegex: string() + blockingTitleRegex: optional(string()) }) const configDecoder: Decoder = object({ @@ -83,7 +83,7 @@ const configDecoder: Decoder = object({ maxRequestedChanges: reviewConfigDecover, requiredLabels: array(string()), blockingLabels: array(string()), - blockingTitleRegex: string(), + blockingTitleRegex: optional(string()), updateBranch: boolean(), deleteBranchAfterMerge: boolean(), mergeMethod: oneOf( diff --git a/test/conditions/blockingTitle.test.ts b/test/conditions/blockingTitle.test.ts index 9451e192a..fe4beceb0 100644 --- a/test/conditions/blockingTitle.test.ts +++ b/test/conditions/blockingTitle.test.ts @@ -2,29 +2,29 @@ import blockingTitle from '../../src/conditions/blockingTitle' import { createPullRequestInfo, createConditionConfig } from '../mock' describe('blockingTitle', () => { - it('returns fail if there is not a match in title(with default configuration);', async () => { + it('returns success if with default configuration(undefined)', async () => { const result = blockingTitle( createConditionConfig(), createPullRequestInfo({ title: '[WIP] help needed' }) ) - expect(result.status).toBe('fail') + expect(result.status).toBe('success') }) - it('returns success if there is a match in title;', async () => { + it('returns success if there is a match in title', async () => { const result = blockingTitle( createConditionConfig({ blockingTitleRegex: 'wip' }), createPullRequestInfo({ - title: 'mergeable' + title: 'Add some feature' }) ) expect(result.status).toBe('success') }) - it('returns fail if there is not a match in title;', async () => { + it('returns fail if there is not a match in title', async () => { const result = blockingTitle( createConditionConfig({ blockingTitleRegex: 'wip' diff --git a/test/mock.ts b/test/mock.ts index f79bc2d9f..96008af78 100644 --- a/test/mock.ts +++ b/test/mock.ts @@ -55,7 +55,7 @@ export const defaultPullRequestInfo: PullRequestInfo = { } }, checkRuns: [], - title: 'mergeable title' + title: 'Add some feature' } export function createPullRequestInfo (pullRequestInfo?: Partial): PullRequestInfo { From cdd41a8c9397cb541f11ff3b71e030297d5fb182 Mon Sep 17 00:00:00 2001 From: luoxi Date: Thu, 11 Oct 2018 14:52:09 +0900 Subject: [PATCH 10/16] rm trailing whitespace --- src/conditions/blockingTitle.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conditions/blockingTitle.ts b/src/conditions/blockingTitle.ts index e7c17a632..fc0a4649c 100644 --- a/src/conditions/blockingTitle.ts +++ b/src/conditions/blockingTitle.ts @@ -6,10 +6,10 @@ export default function doesNotHaveBlockingTitle ( config: ConditionConfig, pullRequestInfo: PullRequestInfo ): ConditionResult { - if (config.blockingTitleRegex === undefined) { + if (config.blockingTitleRegex === undefined) { return { status: 'success' - } + } } const regexObj = new RegExp(config.blockingTitleRegex, 'i') From 1cfadb0af3b69c103d4ffd5e303de83267037acf Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Thu, 11 Oct 2018 20:25:19 +0200 Subject: [PATCH 11/16] always throw on unhandledRejection, resulting in exit code 1 --- package.json | 3 +++ test/jest-setup.ts | 3 +++ 2 files changed, 6 insertions(+) create mode 100644 test/jest-setup.ts diff --git a/package.json b/package.json index 6653dbb55..8cc807b8a 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,9 @@ ] }, "jest": { + "setupFiles": [ + "./test/jest-setup.ts" + ], "transform": { "^.+\\.tsx?$": "ts-jest" }, diff --git a/test/jest-setup.ts b/test/jest-setup.ts new file mode 100644 index 000000000..60d1c9d70 --- /dev/null +++ b/test/jest-setup.ts @@ -0,0 +1,3 @@ +process.on('unhandledRejection', reason => { + throw reason +}) From 66c28d38db8219a792e604076b0eef91ba2deb55 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Thu, 11 Oct 2018 20:44:53 +0200 Subject: [PATCH 12/16] properly handle rejections in query test --- test/pull-request-query.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/pull-request-query.test.ts b/test/pull-request-query.test.ts index ad5540b85..46b0a9a88 100644 --- a/test/pull-request-query.test.ts +++ b/test/pull-request-query.test.ts @@ -36,7 +36,7 @@ describe('queryPullRequest', () => { query }), { owner: 'bobvanderlinden', repo: 'probot-auto-merge', number: 1 } - )).rejects.toThrow('Could not query') + )).rejects.toThrowError('Could not query pull request') }) it('should throw error when empty query response', async () => { @@ -47,6 +47,6 @@ describe('queryPullRequest', () => { query }), { owner: 'bobvanderlinden', repo: 'probot-auto-merge', number: 1 } - )).rejects.toThrow('Query result does not have repository') + )).rejects.toThrowError('Query result does not have repository') }) }) From b0f828ceaa929305c06b8087b0424647029bf36f Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Thu, 11 Oct 2018 20:45:18 +0200 Subject: [PATCH 13/16] more cleanly implement the query result assertions --- src/pull-request-query.ts | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/pull-request-query.ts b/src/pull-request-query.ts index 61d702b69..2e76ef450 100644 --- a/src/pull-request-query.ts +++ b/src/pull-request-query.ts @@ -2,6 +2,14 @@ import { PullRequestReference, CheckRun, PullRequestInfo, PullRequestQueryResult import { Context } from 'probot' import { result } from './utils' +function assertPullRequest(pullRequest: PullRequestReference, condition: boolean, errorMessage: string) { + if (!condition) { + const error: any = new Error(errorMessage) + error.pullRequest = `${pullRequest.owner}/${pullRequest.repo}#${pullRequest.number}` + throw error + } +} + export async function queryPullRequest (github: Context['github'], { owner, repo, number: pullRequestNumber }: PullRequestReference): Promise { const response = await github.query(` query PullRequestQuery($owner:String!, $repo:String!, $pullRequestNumber:Int!) { @@ -71,14 +79,16 @@ export async function queryPullRequest (github: Context['github'], { owner, repo 'repo': repo, 'pullRequestNumber': pullRequestNumber }) as any - if (!response) { - throw new Error(`Could not query pull request ${owner}/${repo}#${pullRequestNumber}`) - } - if (!response.repository.pullRequest.headRef && !response.repository.mergeable) { - const error: any = new Error(`No permission to source repository of pull request`) - error.pullRequest = `${owner}/${repo}#${pullRequestNumber}` - throw error - } + + const assert = assertPullRequest.bind(null, { + owner, + repo, + number: pullRequestNumber + }) + + assert(response, 'Could not query pull request') + assert(response.repository, 'Query result does not have repository') + assert(response.repository.pullRequest.headRef && response.repository.pullRequest.mergeable, 'No permission to source repository of pull request') const queryResult = response as PullRequestQueryResult From 1f236ec8a5b06bdb42384af78c534a1992d26c26 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Thu, 11 Oct 2018 20:45:43 +0200 Subject: [PATCH 14/16] add test for query without permissions error --- test/pull-request-query.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/pull-request-query.test.ts b/test/pull-request-query.test.ts index 46b0a9a88..a5acfef69 100644 --- a/test/pull-request-query.test.ts +++ b/test/pull-request-query.test.ts @@ -49,4 +49,21 @@ describe('queryPullRequest', () => { { owner: 'bobvanderlinden', repo: 'probot-auto-merge', number: 1 } )).rejects.toThrowError('Query result does not have repository') }) + + it('should throw error when no headRef and not mergeable', async () => { + const query = jest.fn(() => createPullRequestInfo({ + repository: { + pullRequest: { + headRef: undefined, + mergeable: undefined + } + } + } as any)) + expect(queryPullRequest( + createGithubApi({ + query + }), + { owner: 'bobvanderlinden', repo: 'probot-auto-merge', number: 1 } + )).rejects.toThrowError('No permission to source repository of pull request') + }) }) From a47a9544f32030bfe9c6ef7b4f1338ee1a6a96b0 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Thu, 11 Oct 2018 20:46:50 +0200 Subject: [PATCH 15/16] fix linting errors --- src/pull-request-query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pull-request-query.ts b/src/pull-request-query.ts index 2e76ef450..07494d849 100644 --- a/src/pull-request-query.ts +++ b/src/pull-request-query.ts @@ -2,7 +2,7 @@ import { PullRequestReference, CheckRun, PullRequestInfo, PullRequestQueryResult import { Context } from 'probot' import { result } from './utils' -function assertPullRequest(pullRequest: PullRequestReference, condition: boolean, errorMessage: string) { +function assertPullRequest (pullRequest: PullRequestReference, condition: boolean, errorMessage: string) { if (!condition) { const error: any = new Error(errorMessage) error.pullRequest = `${pullRequest.owner}/${pullRequest.repo}#${pullRequest.number}` From 8c0820114f0a07d4e1b4f20adbcae2ccee5c1707 Mon Sep 17 00:00:00 2001 From: luoxi Date: Fri, 12 Oct 2018 10:05:19 +0900 Subject: [PATCH 16/16] insert final newline --- auto-merge.example.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auto-merge.example.yml b/auto-merge.example.yml index 6da217b50..7bb7dcec1 100644 --- a/auto-merge.example.yml +++ b/auto-merge.example.yml @@ -48,4 +48,4 @@ requiredLabels: - merge # Automatic merges will be blocked if there is a match between the regular expression and title -blockingTitleRegex: '\bwip\b' \ No newline at end of file +blockingTitleRegex: '\bwip\b'