From f461e8e9de5c60ca302f87bfadb63adfa89679a2 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Wed, 15 Aug 2018 20:50:00 +0200 Subject: [PATCH 1/9] split tests for not merging/updating into one per status --- test/pull-request-handler.test.ts | 73 ++++++++++++++++--------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/test/pull-request-handler.test.ts b/test/pull-request-handler.test.ts index ff1864136..b33f40ccc 100644 --- a/test/pull-request-handler.test.ts +++ b/test/pull-request-handler.test.ts @@ -58,25 +58,24 @@ describe("handlePullRequestStatus", () => { ); expect(merge).toHaveBeenCalledTimes(1); }); - it("does not merge on status other than ready_for_merge", async () => { + + const pullRequestStatusCodesOtherThanReadyForMerge = PullRequestStatusCodes + .filter(code => code !== "ready_for_merge") + test.each(pullRequestStatusCodesOtherThanReadyForMerge)("does not merge on status %s", async (code) => { const merge = jest.fn(() => ({ status: 200 })) - for (let code of PullRequestStatusCodes.filter( - code => code !== "ready_for_merge" && code !== "out_of_date_branch" - )) { - await handlePullRequestStatus( - createHandlerContext({ - github: createGithubApi({ - pullRequests: { - merge - } - }) - }), - createPullRequestInfo(), { - code, - message: "bogus" - } as any - ); - } + await handlePullRequestStatus( + createHandlerContext({ + github: createGithubApi({ + pullRequests: { + merge + } + }) + }), + createPullRequestInfo(), { + code, + message: "bogus" + } as any + ); expect(merge).toHaveBeenCalledTimes(0); }); it("schedules next run when status is pending_checks", async () => { @@ -88,25 +87,27 @@ describe("handlePullRequestStatus", () => { }); expect(setTimeout).toHaveBeenCalledTimes(1); }); - it("does not merge on status other than pending_checks", async () => { + + const pullRequestStatusCodesOtherThanOutOfDateBranch = PullRequestStatusCodes + .filter(code => code !== "out_of_date_branch") + test.each(pullRequestStatusCodesOtherThanOutOfDateBranch)("does not update branch on status %s", async (code) => { const merge = jest.fn(() => ({ status: 200 })) - for (let code of PullRequestStatusCodes.filter( - code => code !== "pending_checks" && code !== "out_of_date_branch" - )) { - await handlePullRequestStatus( - createHandlerContext({ - github: createGithubApi({ - pullRequests: { - merge - } - }) - }), - createPullRequestInfo(), { - code, - message: "bogus" - } as any); - } - expect(setTimeout).toHaveBeenCalledTimes(0); + await handlePullRequestStatus( + createHandlerContext({ + github: createGithubApi({ + pullRequests: { + merge: jest.fn(() => ({ status: 200 })) + }, + repos: { + merge + } + }) + }), + createPullRequestInfo(), { + code, + message: "bogus" + } as any); + expect(merge).toHaveBeenCalledTimes(0); }); it("update branch when status is out_of_date_branch", async () => { const merge = jest.fn(() => ({ status: 200 })); From 681832b7c1862c1465ede78d10e3dd428c525c7a Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Wed, 15 Aug 2018 20:50:50 +0200 Subject: [PATCH 2/9] fix casing for variable PullRequestReference --- src/pull-request-handler.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/pull-request-handler.ts b/src/pull-request-handler.ts index e3145f4fd..0d4dfff1b 100644 --- a/src/pull-request-handler.ts +++ b/src/pull-request-handler.ts @@ -8,7 +8,7 @@ const debug = require('debug')('pull-request-handler') interface PullRequestTask { context: HandlerContext - PullRequestReference: PullRequestReference + pullRequestReference: PullRequestReference } const taskScheduler = new TaskScheduler({ @@ -35,13 +35,13 @@ export function arePullRequestReferencesEqual (a: PullRequestReference, b: PullR export function schedulePullRequestTrigger ( context: HandlerContext, - PullRequestReference: PullRequestReference + pullRequestReference: PullRequestReference ) { - const queueName = getRepositoryKey(PullRequestReference) + const queueName = getRepositoryKey(pullRequestReference) const queueContainsTask = taskScheduler.getQueue(queueName) - .some(task => arePullRequestReferencesEqual(task.PullRequestReference, PullRequestReference)) + .some(task => arePullRequestReferencesEqual(task.pullRequestReference, pullRequestReference)) if (!queueContainsTask) { - taskScheduler.queue(queueName, { context, PullRequestReference }) + taskScheduler.queue(queueName, { context, pullRequestReference }) } } @@ -55,25 +55,25 @@ function getPullRequestKey (pullRequestReference: PullRequestReference) { async function pullRequestWorker ({ context, - PullRequestReference + pullRequestReference }: PullRequestTask) { await Raven.context({ tags: { - owner: PullRequestReference.owner, - repository: PullRequestReference.repo, - pullRequestNumber: PullRequestReference.number + owner: pullRequestReference.owner, + repository: pullRequestReference.repo, + pullRequestNumber: pullRequestReference.number } }, async () => { - await handlePullRequestTrigger(context, PullRequestReference) + await handlePullRequestTrigger(context, pullRequestReference) }) } async function handlePullRequestTrigger ( context: HandlerContext, - PullRequestReference: PullRequestReference + pullRequestReference: PullRequestReference ) { const { log: appLog } = context - const pullRequestKey = getPullRequestKey(PullRequestReference) + const pullRequestKey = getPullRequestKey(pullRequestReference) function log (msg: string) { appLog(`${pullRequestKey}: ${msg}`) @@ -87,17 +87,17 @@ async function handlePullRequestTrigger ( ...context, log } - await doPullRequestWork(pullRequestContext, PullRequestReference) + await doPullRequestWork(pullRequestContext, pullRequestReference) } async function doPullRequestWork ( context: HandlerContext, - PullRequestReference: PullRequestReference + pullRequestReference: PullRequestReference ) { const { log } = context const pullRequestInfo = await queryPullRequest( context.github, - PullRequestReference + pullRequestReference ) const pullRequestStatus = await getPullRequestStatus( From f7e1184669bc87c9cc60c0acca3d57c67f5c3d93 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Wed, 15 Aug 2018 20:55:57 +0200 Subject: [PATCH 3/9] apply standard linting rules --- src/global.d.ts | 4 +- test/mock.ts | 117 ++++++++-------- test/pull-request-handler.test.ts | 206 +++++++++++++-------------- test/pull-request-status.test.ts | 224 +++++++++++++++--------------- tsconfig.json | 3 +- 5 files changed, 272 insertions(+), 282 deletions(-) diff --git a/src/global.d.ts b/src/global.d.ts index 05113299a..761f2d303 100644 --- a/src/global.d.ts +++ b/src/global.d.ts @@ -1,3 +1,3 @@ -declare module "probot-config" { - export default function getConfig(context: TContext, fileName: String, defaultConfig: TConfig): Promise +declare module 'probot-config' { + export default function getConfig (context: TContext, fileName: String, defaultConfig: TConfig): Promise } diff --git a/test/mock.ts b/test/mock.ts index ddcc297e7..8f48e91e2 100644 --- a/test/mock.ts +++ b/test/mock.ts @@ -1,46 +1,46 @@ -import { Review, CheckRun, PullRequestReviewState } from './../src/github-models'; -import { DeepPartial } from './../src/utils'; -import { HandlerContext } from './../src/models'; +import { Review, CheckRun, PullRequestReviewState } from './../src/github-models' +import { DeepPartial } from './../src/utils' +import { HandlerContext } from './../src/models' import { - PullRequestInfo, -} from "../src/models"; -import { Config, defaultConfig } from "../src/config"; -import { GitHubAPI } from "probot/lib/github"; + PullRequestInfo +} from '../src/models' +import { Config, defaultConfig } from '../src/config' +import { GitHubAPI } from 'probot/lib/github' export const defaultPullRequestInfo: PullRequestInfo = { number: 1, - state: "OPEN", - mergeable: "MERGEABLE", + state: 'OPEN', + mergeable: 'MERGEABLE', potentialMergeCommit: { - oid: "da39a3ee5e6b4b0d3255bfef95601890afd80709" + oid: 'da39a3ee5e6b4b0d3255bfef95601890afd80709' }, baseRef: { name: 'master', repository: { - name: "probot-auto-merge", + name: 'probot-auto-merge', owner: { - login: "bobvanderlinden" + login: 'bobvanderlinden' } }, target: { - oid: "c00dbbc9dadfbe1e232e93a729dd4752fade0abf" + oid: 'c00dbbc9dadfbe1e232e93a729dd4752fade0abf' } }, - baseRefOid: "c00dbbc9dadfbe1e232e93a729dd4752fade0abf", + baseRefOid: 'c00dbbc9dadfbe1e232e93a729dd4752fade0abf', headRef: { - name: "pr-some-change", + name: 'pr-some-change', repository: { - name: "probot-auto-merge", + name: 'probot-auto-merge', owner: { - login: "bobvanderlinden" + login: 'bobvanderlinden' } }, target: { - oid: "c2a6b03f190dfb2b4aa91f8af8d477a9bc3401dc" + oid: 'c2a6b03f190dfb2b4aa91f8af8d477a9bc3401dc' } }, - headRefOid: "c2a6b03f190dfb2b4aa91f8af8d477a9bc3401dc", - authorAssociation: "OWNER", + headRefOid: 'c2a6b03f190dfb2b4aa91f8af8d477a9bc3401dc', + authorAssociation: 'OWNER', labels: { nodes: [] }, @@ -55,86 +55,85 @@ export const defaultPullRequestInfo: PullRequestInfo = { checkRuns: [] } - -export function createPullRequestInfo(pullRequestInfo?: Partial): PullRequestInfo { +export function createPullRequestInfo (pullRequestInfo?: Partial): PullRequestInfo { return { ...defaultPullRequestInfo, ...pullRequestInfo } } -export function createGithubApi(options?: DeepPartial): GitHubAPI { +export function createGithubApi (options?: DeepPartial): GitHubAPI { return { ...options - } as GitHubAPI + } as GitHubAPI } -export function createConfig(options?: Partial): Config { +export function createConfig (options?: Partial): Config { return { ...defaultConfig, minApprovals: { MEMBER: 1 }, - ...options, + ...options } } -export function createHandlerContext(options?: Partial): HandlerContext { +export function createHandlerContext (options?: Partial): HandlerContext { return { - log: () => {}, + log: () => undefined, github: options && options.github || createGithubApi(), config: options && options.config || createConfig(), ...options } } -export function review(options: Partial & { state: PullRequestReviewState }): Review { +export function review (options: Partial & { state: PullRequestReviewState }): Review { return { - submittedAt: "2018-07-15T20:53:17Z", + submittedAt: '2018-07-15T20:53:17Z', authorAssociation: 'MEMBER', author: { - login: "henk" + login: 'henk' }, ...options - }; + } } export const approvedReview = (options?: Partial) => review({ - state: "APPROVED", + state: 'APPROVED', ...options - }); + }) export const changesRequestedReview = (options?: Partial) => review({ - state: "CHANGES_REQUESTED", + state: 'CHANGES_REQUESTED', ...options - }); + }) export const successCheckRun: CheckRun = { - name: "checka", - status: "completed", - conclusion: "success", - head_sha: "12345", - external_id: "1" -}; + name: 'checka', + status: 'completed', + conclusion: 'success', + head_sha: '12345', + external_id: '1' +} export const queuedCheckRun: CheckRun = { - name: "checka", - status: "queued", + name: 'checka', + status: 'queued', conclusion: null, - head_sha: "12345", - external_id: "1" -}; + head_sha: '12345', + external_id: '1' +} export const failedCheckRun: CheckRun = { - name: "checka", - status: "completed", - conclusion: "failure", - head_sha: "12345", - external_id: "1" -}; + name: 'checka', + status: 'completed', + conclusion: 'failure', + head_sha: '12345', + external_id: '1' +} export const neutralCheckRun: CheckRun = { - name: "checka", - status: "completed", - conclusion: "neutral", - head_sha: "12345", - external_id: "1" -}; + name: 'checka', + status: 'completed', + conclusion: 'neutral', + head_sha: '12345', + external_id: '1' +} diff --git a/test/pull-request-handler.test.ts b/test/pull-request-handler.test.ts index b33f40ccc..bb756f214 100644 --- a/test/pull-request-handler.test.ts +++ b/test/pull-request-handler.test.ts @@ -1,47 +1,37 @@ -import { PullRequestInfo } from './../src/models'; -import { handlePullRequestStatus } from "../src/pull-request-handler"; -import { PullRequestStatusCodes } from "../src/pull-request-status"; -import { createHandlerContext, createPullRequestInfo, createGithubApi, createConfig, defaultPullRequestInfo } from "./mock"; +import { PullRequestInfo } from './../src/models' +import { handlePullRequestStatus } from '../src/pull-request-handler' +import { PullRequestStatusCodes } from '../src/pull-request-status' +import { createHandlerContext, createPullRequestInfo, createGithubApi, createConfig, defaultPullRequestInfo } from './mock' -const defaultBaseRef: PullRequestInfo["baseRef"] = { +const defaultBaseRef: PullRequestInfo['baseRef'] = { repository: { owner: { - login: "bobvanderlinden" + login: 'bobvanderlinden' }, - name: "probot-auto-merge" + name: 'probot-auto-merge' }, - name: "master", + name: 'master', target: { - oid: "0000000000000000000000000000000000000000" + oid: '0000000000000000000000000000000000000000' } } -const headRefInSameRepository: PullRequestInfo["headRef"] = { +const headRefInSameRepository: PullRequestInfo['headRef'] = { ...defaultBaseRef, - name: "pr-some-changes", + name: 'pr-some-changes', target: { - oid: "1111111111111111111111111111111111111111" + oid: '1111111111111111111111111111111111111111' } } -const headRefInFork: PullRequestInfo["headRef"] = { - ...headRefInSameRepository, - repository: { - ...headRefInSameRepository.repository, - owner: { - login: "someone-else" - } - } -} - -describe("handlePullRequestStatus", () => { +describe('handlePullRequestStatus', () => { beforeEach(() => { - jest.useFakeTimers(); - }); + jest.useFakeTimers() + }) afterEach(() => { - jest.clearAllTimers(); - }); - it("merges when status is ready_for_merge", async () => { + jest.clearAllTimers() + }) + it('merges when status is ready_for_merge', async () => { const merge = jest.fn(() => ({ status: 200 })) await handlePullRequestStatus( createHandlerContext({ @@ -52,16 +42,16 @@ describe("handlePullRequestStatus", () => { }) }), createPullRequestInfo(), { - code: "ready_for_merge", - message: "bogus" + code: 'ready_for_merge', + message: 'bogus' } - ); - expect(merge).toHaveBeenCalledTimes(1); - }); + ) + expect(merge).toHaveBeenCalledTimes(1) + }) const pullRequestStatusCodesOtherThanReadyForMerge = PullRequestStatusCodes - .filter(code => code !== "ready_for_merge") - test.each(pullRequestStatusCodesOtherThanReadyForMerge)("does not merge on status %s", async (code) => { + .filter(code => code !== 'ready_for_merge') + test.each(pullRequestStatusCodesOtherThanReadyForMerge)('does not merge on status %s', async (code) => { const merge = jest.fn(() => ({ status: 200 })) await handlePullRequestStatus( createHandlerContext({ @@ -73,24 +63,24 @@ describe("handlePullRequestStatus", () => { }), createPullRequestInfo(), { code, - message: "bogus" + message: 'bogus' } as any - ); - expect(merge).toHaveBeenCalledTimes(0); - }); - it("schedules next run when status is pending_checks", async () => { + ) + expect(merge).toHaveBeenCalledTimes(0) + }) + it('schedules next run when status is pending_checks', async () => { await handlePullRequestStatus( createHandlerContext(), createPullRequestInfo(), { - code: "pending_checks", - message: "bogus" - }); - expect(setTimeout).toHaveBeenCalledTimes(1); - }); + code: 'pending_checks', + message: 'bogus' + }) + expect(setTimeout).toHaveBeenCalledTimes(1) + }) const pullRequestStatusCodesOtherThanOutOfDateBranch = PullRequestStatusCodes - .filter(code => code !== "out_of_date_branch") - test.each(pullRequestStatusCodesOtherThanOutOfDateBranch)("does not update branch on status %s", async (code) => { + .filter(code => code !== 'out_of_date_branch') + test.each(pullRequestStatusCodesOtherThanOutOfDateBranch)('does not update branch on status %s', async (code) => { const merge = jest.fn(() => ({ status: 200 })) await handlePullRequestStatus( createHandlerContext({ @@ -104,13 +94,13 @@ describe("handlePullRequestStatus", () => { }) }), createPullRequestInfo(), { - code, - message: "bogus" - } as any); - expect(merge).toHaveBeenCalledTimes(0); - }); - it("update branch when status is out_of_date_branch", async () => { - const merge = jest.fn(() => ({ status: 200 })); + code, + message: 'bogus' + } as any) + expect(merge).toHaveBeenCalledTimes(0) + }) + it('update branch when status is out_of_date_branch', async () => { + const merge = jest.fn(() => ({ status: 200 })) await handlePullRequestStatus( createHandlerContext({ github: createGithubApi({ @@ -125,19 +115,19 @@ describe("handlePullRequestStatus", () => { createPullRequestInfo({ }), { - code: "out_of_date_branch", - message: "bogus" - }); - expect(merge).toHaveBeenCalledTimes(1); + code: 'out_of_date_branch', + message: 'bogus' + }) + expect(merge).toHaveBeenCalledTimes(1) expect(merge.mock.calls[0]).toEqual([{ - base: "pr-some-change", - head: "master", - owner: "bobvanderlinden", - repo: "probot-auto-merge" - }]); - }); - it("update branch when status is out_of_date_branch and update-branch is enabled", async () => { - const merge = jest.fn(() => ({ status: 200 })); + base: 'pr-some-change', + head: 'master', + owner: 'bobvanderlinden', + repo: 'probot-auto-merge' + }]) + }) + it('update branch when status is out_of_date_branch and update-branch is enabled', async () => { + const merge = jest.fn(() => ({ status: 200 })) await handlePullRequestStatus( createHandlerContext({ github: createGithubApi({ @@ -153,19 +143,19 @@ describe("handlePullRequestStatus", () => { baseRef: defaultBaseRef, headRef: headRefInSameRepository }), { - code: "out_of_date_branch", - message: "bogus" - }); - expect(merge).toHaveBeenCalledTimes(1); + code: 'out_of_date_branch', + message: 'bogus' + }) + expect(merge).toHaveBeenCalledTimes(1) expect(merge.mock.calls[0]).toEqual([{ - base: "pr-some-changes", - head: "master", - owner: "bobvanderlinden", - repo: "probot-auto-merge" - }]); - }); - it("not update branch when status is out_of_date_branch and update-branch is disabled", async () => { - const merge = jest.fn(() => ({ status: 200 })); + base: 'pr-some-changes', + head: 'master', + owner: 'bobvanderlinden', + repo: 'probot-auto-merge' + }]) + }) + it('not update branch when status is out_of_date_branch and update-branch is disabled', async () => { + const merge = jest.fn(() => ({ status: 200 })) await handlePullRequestStatus( createHandlerContext({ github: createGithubApi({ @@ -181,14 +171,14 @@ describe("handlePullRequestStatus", () => { baseRef: defaultBaseRef, headRef: headRefInSameRepository }), { - code: "out_of_date_branch", - message: "bogus" - }); - expect(merge).toHaveBeenCalledTimes(0); - }); - it("delete branch when status is ready_for_merge and delete-branch-after-merge is enabled and branch resides in same repository", async () => { - const merge = jest.fn(() => ({ status: 200 })); - const deleteReference = jest.fn(() => ({ status: 200 })); + code: 'out_of_date_branch', + message: 'bogus' + }) + expect(merge).toHaveBeenCalledTimes(0) + }) + it('delete branch when status is ready_for_merge and delete-branch-after-merge is enabled and branch resides in same repository', async () => { + const merge = jest.fn(() => ({ status: 200 })) + const deleteReference = jest.fn(() => ({ status: 200 })) await handlePullRequestStatus( createHandlerContext({ github: createGithubApi({ @@ -207,18 +197,18 @@ describe("handlePullRequestStatus", () => { baseRef: defaultBaseRef, headRef: headRefInSameRepository }), { - code: "ready_for_merge", - message: "bogus" + code: 'ready_for_merge', + message: 'bogus' } - ); - expect(deleteReference).toHaveBeenCalledTimes(1); + ) + expect(deleteReference).toHaveBeenCalledTimes(1) expect(deleteReference.mock.calls[0]).toEqual([ - { owner: "bobvanderlinden", ref: "heads/pr-some-changes", repo: "probot-auto-merge" } - ]); - }); - it("do not delete branch when status is ready_for_merge and delete-branch-after-merge is enabled, but branch resides in another repository", async () => { - const merge = jest.fn(() => ({ status: 200 })); - const deleteReference = jest.fn(() => ({ status: 200 })); + { owner: 'bobvanderlinden', ref: 'heads/pr-some-changes', repo: 'probot-auto-merge' } + ]) + }) + it('do not delete branch when status is ready_for_merge and delete-branch-after-merge is enabled, but branch resides in another repository', async () => { + const merge = jest.fn(() => ({ status: 200 })) + const deleteReference = jest.fn(() => ({ status: 200 })) await handlePullRequestStatus( createHandlerContext({ github: createGithubApi({ @@ -236,29 +226,29 @@ describe("handlePullRequestStatus", () => { createPullRequestInfo({ baseRef: { ...defaultPullRequestInfo.baseRef, - name: "master", + name: 'master', repository: { owner: { - login: "bobvanderlinden" + login: 'bobvanderlinden' }, - name: "probot-auto-merge" + name: 'probot-auto-merge' } }, headRef: { ...defaultPullRequestInfo.headRef, - name: "pr", + name: 'pr', repository: { owner: { - login: "someone-else" + login: 'someone-else' }, - name: "probot-auto-merge" + name: 'probot-auto-merge' } } }), { - code: "ready_for_merge", - message: "bogus" + code: 'ready_for_merge', + message: 'bogus' } - ); - expect(deleteReference).toHaveBeenCalledTimes(0); - }); -}); + ) + expect(deleteReference).toHaveBeenCalledTimes(0) + }) +}) diff --git a/test/pull-request-status.test.ts b/test/pull-request-status.test.ts index 4ff869901..7465faaef 100644 --- a/test/pull-request-status.test.ts +++ b/test/pull-request-status.test.ts @@ -1,4 +1,4 @@ -import { PullRequestState } from './../src/models'; +import { PullRequestState } from './../src/models' import { approvedReview, changesRequestedReview, @@ -9,22 +9,22 @@ import { createPullRequestInfo, createConfig, defaultPullRequestInfo -} from "./mock"; -import { getPullRequestStatus } from "../src/pull-request-status"; +} from './mock' +import { getPullRequestStatus } from '../src/pull-request-status' -describe("getPullRequestStatus", () => { - it("returns not_open when pull request state is a value that is undocumented", async () => { +describe('getPullRequestStatus', () => { + it('returns not_open when pull request state is a value that is undocumented', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ reviews: { nodes: [approvedReview()] }, - state: "this_value_is_undocumented" as PullRequestState + state: 'this_value_is_undocumented' as PullRequestState }) - ); - expect(status.code).toBe("not_open"); - }); + ) + expect(status.code).toBe('not_open') + }) - it("returns blocking_check when one check run failed", async () => { + it('returns blocking_check when one check run failed', async () => { const status = await getPullRequestStatus( createHandlerContext({ config: createConfig() @@ -33,11 +33,11 @@ describe("getPullRequestStatus", () => { reviews: { nodes: [approvedReview()] }, checkRuns: [successCheckRun, failedCheckRun] }) - ); - expect(status.code).toBe("blocking_check"); - }); + ) + expect(status.code).toBe('blocking_check') + }) - it("returns changes_requested when one reviewer requested changes", async () => { + it('returns changes_requested when one reviewer requested changes', async () => { const status = await getPullRequestStatus( createHandlerContext({ config: createConfig({ @@ -52,17 +52,17 @@ describe("getPullRequestStatus", () => { createPullRequestInfo({ reviews: { nodes: [ - approvedReview({ author: { login: "henk" }, authorAssociation: 'MEMBER' }), - changesRequestedReview({ author: { login: "sjaak" }, authorAssociation: 'MEMBER' }) + approvedReview({ author: { login: 'henk' }, authorAssociation: 'MEMBER' }), + changesRequestedReview({ author: { login: 'sjaak' }, authorAssociation: 'MEMBER' }) ] }, checkRuns: [successCheckRun] }) - ); - expect(status.code).toBe("changes_requested"); - }); + ) + expect(status.code).toBe('changes_requested') + }) - it("returns changes_requested when owner approved, but member requested changes", async () => { + it('returns changes_requested when owner approved, but member requested changes', async () => { const status = await getPullRequestStatus( createHandlerContext({ config: createConfig({ @@ -79,17 +79,17 @@ describe("getPullRequestStatus", () => { createPullRequestInfo({ reviews: { nodes: [ - approvedReview({ author: { login: "henk" }, authorAssociation: 'OWNER' }), - changesRequestedReview({ author: { login: "sjaak" }, authorAssociation: 'MEMBER' }) + approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }), + changesRequestedReview({ author: { login: 'sjaak' }, authorAssociation: 'MEMBER' }) ] }, - checkRuns: [successCheckRun], + checkRuns: [successCheckRun] }) - ); - expect(status.code).toBe("changes_requested"); - }); + ) + expect(status.code).toBe('changes_requested') + }) - it("returns ready_for_merge when owner approved, but user requested changes", async () => { + it('returns ready_for_merge when owner approved, but user requested changes', async () => { const status = await getPullRequestStatus( createHandlerContext({ config: createConfig({ @@ -101,22 +101,22 @@ describe("getPullRequestStatus", () => { OWNER: 0, MEMBER: 0 } - }), + }) }), createPullRequestInfo({ reviews: { nodes: [ - approvedReview({ author: { login: "henk" }, authorAssociation: 'OWNER' }), - changesRequestedReview({ author: { login: "sjaak" }, authorAssociation: 'NONE' }) + approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }), + changesRequestedReview({ author: { login: 'sjaak' }, authorAssociation: 'NONE' }) ] }, - checkRuns: [successCheckRun], + checkRuns: [successCheckRun] }) - ); - expect(status.code).toBe("ready_for_merge"); - }); + ) + expect(status.code).toBe('ready_for_merge') + }) - it("returns ready_for_merge when two members approved, but user requested changes", async () => { + it('returns ready_for_merge when two members approved, but user requested changes', async () => { const status = await getPullRequestStatus( createHandlerContext({ config: createConfig({ @@ -131,18 +131,18 @@ describe("getPullRequestStatus", () => { createPullRequestInfo({ reviews: { nodes: [ - approvedReview({ author: { login: "henk" }, authorAssociation: 'OWNER' }), - approvedReview({ author: { login: "sjaak" }, authorAssociation: 'MEMBER' }), - changesRequestedReview({ author: { login: "piet" }, authorAssociation: 'NONE' }) + approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }), + approvedReview({ author: { login: 'sjaak' }, authorAssociation: 'MEMBER' }), + changesRequestedReview({ author: { login: 'piet' }, authorAssociation: 'NONE' }) ] }, checkRuns: [successCheckRun] }) - ); - expect(status.code).toBe("ready_for_merge"); - }); + ) + expect(status.code).toBe('ready_for_merge') + }) - it("returns ready_for_merge when two members approved, but user requested changes", async () => { + it('returns ready_for_merge when two members approved, but user requested changes', async () => { const status = await getPullRequestStatus( createHandlerContext({ config: createConfig({ @@ -157,71 +157,71 @@ describe("getPullRequestStatus", () => { createPullRequestInfo({ reviews: { nodes: [ - approvedReview({ author: { login: "henk" }, authorAssociation: 'OWNER' }), - approvedReview({ author: { login: "sjaak" }, authorAssociation: 'MEMBER' }), - changesRequestedReview({ author: { login: "piet" }, authorAssociation: 'NONE' }) + approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }), + approvedReview({ author: { login: 'sjaak' }, authorAssociation: 'MEMBER' }), + changesRequestedReview({ author: { login: 'piet' }, authorAssociation: 'NONE' }) ] }, - checkRuns: [successCheckRun], + checkRuns: [successCheckRun] }) - ); - expect(status.code).toBe("ready_for_merge"); - }); + ) + expect(status.code).toBe('ready_for_merge') + }) - it("returns changes_requested when same reviewer approved and requested changes", async () => { + it('returns changes_requested when same reviewer approved and requested changes', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ - reviews: { nodes: [approvedReview({ author: { login: "henk" } }), changesRequestedReview({ author: { login: "henk" } })] }, + reviews: { nodes: [approvedReview({ author: { login: 'henk' } }), changesRequestedReview({ author: { login: 'henk' } })] }, checkRuns: [successCheckRun] }) - ); - expect(status.code).toBe("changes_requested"); - }); + ) + expect(status.code).toBe('changes_requested') + }) - it("returns pending_checks when check run is still queued", async () => { + it('returns pending_checks when check run is still queued', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ reviews: { nodes: [approvedReview()] }, checkRuns: [queuedCheckRun] }) - ); - expect(status.code).toBe("pending_checks"); - }); + ) + expect(status.code).toBe('pending_checks') + }) - it("returns ready_for_merge when reviewer requested changes and approved", async () => { + it('returns ready_for_merge when reviewer requested changes and approved', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ - reviews: { nodes: [changesRequestedReview({ author: { login: "henk" } }), approvedReview({ author: { login: "henk" } })] } + reviews: { nodes: [changesRequestedReview({ author: { login: 'henk' } }), approvedReview({ author: { login: 'henk' } })] } }) - ); - expect(status.code).toBe("ready_for_merge"); - }); + ) + expect(status.code).toBe('ready_for_merge') + }) - it("returns ready_for_merge when pull request is approved and check run succeeded", async () => { + it('returns ready_for_merge when pull request is approved and check run succeeded', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ reviews: { nodes: [approvedReview()] }, checkRuns: [successCheckRun] }) - ); - expect(status.code).toBe("ready_for_merge"); - }); + ) + expect(status.code).toBe('ready_for_merge') + }) - it("returns ready_for_merge when pull request is approved", async () => { + it('returns ready_for_merge when pull request is approved', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ reviews: { nodes: [approvedReview()] } }) - ); - expect(status.code).toBe("ready_for_merge"); - }); + ) + expect(status.code).toBe('ready_for_merge') + }) - it("returns closed when pull request is closed", async () => { + it('returns closed when pull request is closed', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ @@ -230,54 +230,54 @@ describe("getPullRequestStatus", () => { approvedReview() ] }, - state: "CLOSED" + state: 'CLOSED' }) - ); - expect(status.code).toBe("closed"); - }); + ) + expect(status.code).toBe('closed') + }) - it("returns conflicts when pull request is not mergeable", async () => { + it('returns conflicts when pull request is not mergeable', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ reviews: { nodes: [approvedReview()] }, - mergeable: "CONFLICTING" + mergeable: 'CONFLICTING' }) - ); - expect(status.code).toBe("conflicts"); - }); + ) + expect(status.code).toBe('conflicts') + }) - it("returns pending_mergeable when pull request is not mergeable", async () => { + it('returns pending_mergeable when pull request is not mergeable', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ reviews: { nodes: [approvedReview()] }, - mergeable: "UNKNOWN" + mergeable: 'UNKNOWN' }) - ); - expect(status.code).toBe("pending_mergeable"); - }); + ) + expect(status.code).toBe('pending_mergeable') + }) - it("returns merged when pull request is already merged", async () => { + it('returns merged when pull request is already merged', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ reviews: { nodes: [approvedReview()] }, - state: "MERGED" + state: 'MERGED' }) - ); - expect(status.code).toBe("merged"); - }); + ) + expect(status.code).toBe('merged') + }) - it("returns need_approvals when pull request is not reviewed", async () => { + it('returns need_approvals when pull request is not reviewed', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo() - ); - expect(status.code).toBe("need_approvals"); - }); + ) + expect(status.code).toBe('need_approvals') + }) - it("returns out_of_date_branch when pull request is based on strict protected branch", async () => { + it('returns out_of_date_branch when pull request is based on strict protected branch', async () => { const status = await getPullRequestStatus( createHandlerContext(), createPullRequestInfo({ @@ -288,27 +288,27 @@ describe("getPullRequestStatus", () => { }, baseRef: { ...defaultPullRequestInfo.baseRef, - name: "master", + name: 'master', target: { - oid: "1111111111111111111111111111111111111111", + oid: '1111111111111111111111111111111111111111' } }, - baseRefOid: "0000000000000000000000000000000000000000", + baseRefOid: '0000000000000000000000000000000000000000', repository: { protectedBranches: { nodes: [{ - name: "master", + name: 'master', hasRestrictedPushes: true, hasStrictRequiredStatusChecks: true }] } } }) - ); - expect(status.code).toBe("out_of_date_branch"); - }); + ) + expect(status.code).toBe('out_of_date_branch') + }) - it("returns requires_label when a required label is configured, but not set on pull request", async () => { + it('returns requires_label when a required label is configured, but not set on pull request', async () => { const status = await getPullRequestStatus( createHandlerContext({ config: createConfig({ @@ -320,11 +320,11 @@ describe("getPullRequestStatus", () => { createPullRequestInfo({ reviews: { nodes: [approvedReview()] } }) - ); - expect(status.code).toBe("requires_label"); - }); + ) + expect(status.code).toBe('requires_label') + }) - it("returns ready_for_merge when a required label is configured and it is set on pull request", async () => { + it('returns ready_for_merge when a required label is configured and it is set on pull request', async () => { const status = await getPullRequestStatus( createHandlerContext({ config: createConfig({ @@ -345,7 +345,7 @@ describe("getPullRequestStatus", () => { }] } }) - ); - expect(status.code).toBe("ready_for_merge"); - }); -}); + ) + expect(status.code).toBe('ready_for_merge') + }) +}) diff --git a/tsconfig.json b/tsconfig.json index e4c4618c7..fd1eeb25e 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -18,7 +18,8 @@ "declaration": true }, "include": [ - "src/**/*" + "src/**/*", + "test/**/*" ], "compileOnSave": false } From f3e6bb3195a21767c0e8353e84f6a41f395285b5 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Wed, 15 Aug 2018 21:00:25 +0200 Subject: [PATCH 4/9] checkruns may not have null conclusion --- test/mock.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mock.ts b/test/mock.ts index 8f48e91e2..bacec105a 100644 --- a/test/mock.ts +++ b/test/mock.ts @@ -119,7 +119,7 @@ export const successCheckRun: CheckRun = { export const queuedCheckRun: CheckRun = { name: 'checka', status: 'queued', - conclusion: null, + conclusion: 'neutral', head_sha: '12345', external_id: '1' } From da198460de6f460fc101cc7acf1b9c2fad9f660e Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Wed, 15 Aug 2018 21:10:56 +0200 Subject: [PATCH 5/9] do not use generated js files for testing --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c09a4f27f..3cf35dd50 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,7 @@ "transform": { "^.+\\.tsx?$": "ts-jest" }, - "testRegex": "(/__tests__/.*|(\\.|/)(test|spec))\\.(jsx?|tsx?)$", + "testRegex": "\\.test\\.tsx?$", "moduleFileExtensions": [ "ts", "tsx", From 61f9eee18a0fa60a8bb08ad238103b01335d5634 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Sat, 18 Aug 2018 20:52:40 +0200 Subject: [PATCH 6/9] split status conditions into separate functions --- src/pull-request-status.ts | 343 ++++++++++++++++++++++++------------- 1 file changed, 225 insertions(+), 118 deletions(-) diff --git a/src/pull-request-status.ts b/src/pull-request-status.ts index 8214d8895..64630a7d6 100644 --- a/src/pull-request-status.ts +++ b/src/pull-request-status.ts @@ -1,5 +1,7 @@ +import { HandlerContext, PullRequestInfo, PullRequestInfo } from './../lib/src/models.d'; +import { PullRequestInfo, PullRequestInfo, CommentAuthorAssociation, PullRequestReviewState } from './github-models'; import { HandlerContext, PullRequestInfo } from './models' -import { groupByLast, groupByLastMap } from './utils' +import { groupByLast, groupByLastMap, result } from './utils' import { associations, getAssociationPriority } from './association' export type PullRequestStatus = @@ -37,90 +39,142 @@ export const PullRequestStatusCodes: PullRequestStatusCode[] = [ 'ready_for_merge' ] -function getPullRequestStatusFromPullRequest ( - context: HandlerContext, - pullRequestInfo: PullRequestInfo -): PullRequestStatus | null { - switch (pullRequestInfo.state) { - case 'CLOSED': - return { - code: 'closed', - message: 'Pull request is closed' - } - case 'MERGED': - return { - code: 'merged', - message: 'Pull request was already merged' - } - default: - return { - code: 'not_open', - message: 'Pull request is not open' - } - case 'OPEN': - // Continue. - break - } +const conditions = { + open: isOpen, + mergeable: isMergeable, + requiredLabels: hasRequiredLabels, + blockingLabels: doesNotHaveBlockingLabels, + minimumApprovals: hasMinimumApprovals, + maximumChangesRequested: doesNotHaveMaximumChangesRequested, + blockingChecks: doesNotHaveBlockingChecks, + upToDateBranch: hasUpToDateBranch +} +type Condition = (context: HandlerContext, pullRequestInfo: PullRequestInfo) => ConditionResult +type ConditionName = keyof (typeof conditions) +type ConditionResult = { status: 'success' } | { status: 'fail', message: string } | { status: 'pending', message?: string } +type ConditionResults = { [key in ConditionName]: ConditionResult } + +function isOpen (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { + return pullRequestInfo.state === 'OPEN' + ? { + status: 'success' + } + : { + status: 'fail', + message: `State of pull request is ${pullRequestInfo.state}` + } +} + +function isMergeable (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { switch (pullRequestInfo.mergeable) { + case 'MERGEABLE': + return { + status: 'success' + } case 'CONFLICTING': return { - code: 'conflicts', - message: 'Could not merge pull request due to conflicts' + status: 'fail', + message: 'Pull request has conflicts' } case 'UNKNOWN': return { - code: 'pending_mergeable', - message: 'Mergeablity of pull request could not yet be determined' + status: 'pending', + message: 'Github could not yet determine the mergeable status of the pull request' } - case 'MERGEABLE': - // Continue. - break + default: + throw new Error(`Invalid mergeable state for pull request. mergeable=${pullRequestInfo.mergeable}`) } - return null } -function getPullRequestStatusFromLabels ( +function hasRequiredLabels ( context: HandlerContext, pullRequestInfo: PullRequestInfo -): PullRequestStatus | null { +): ConditionResult { const { config } = context - function hasLabel (labelName: string): boolean { - return pullRequestInfo.labels.nodes.some(label => label.name === labelName) - } + const pullRequestLabels = new Set(pullRequestInfo.labels.nodes.map(label => label.name)) + + const missingRequiredLabels = config.requiredLabels + .filter(requiredLabel => !pullRequestLabels.has(requiredLabel)) - const missingRequiredLabels = config.requiredLabels.filter( - requiredLabel => !hasLabel(requiredLabel) - ) if (missingRequiredLabels.length > 0) { return { - code: 'requires_label', - message: `Required labels are missing (${missingRequiredLabels.join( - ', ' - )})` + status: 'fail', + message: `Required labels are missing (${ + missingRequiredLabels.join(', ') + })` } } + return { + status: 'success' + } +} - const matchingBlockingLabels = config.blockingLabels.filter( - blockingLabel => hasLabel(blockingLabel) - ) - if (matchingBlockingLabels.length > 0) { +function doesNotHaveBlockingLabels ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo +): ConditionResult { + const { config } = context + const pullRequestLabels = new Set(pullRequestInfo.labels.nodes.map(label => label.name)) + const foundBlockingLabels = config.blockingLabels + .filter(blockingLabel => pullRequestLabels.has(blockingLabel)) + + if (foundBlockingLabels.length > 0) { return { - code: 'blocking_label', - message: `Blocking labels were added to the pull request (${ - matchingBlockingLabels.join(', ') + status: 'fail', + message: `Blocking labels are missing (${ + foundBlockingLabels.join(', ') })` } } + return { + status: 'success' + } +} - return null +function merge (a: A, b: B): A & B { + return Object.assign({}, a, b) as any } -function getPullRequestStatusFromReviews ( - context: HandlerContext, - pullRequestInfo: PullRequestInfo -): PullRequestStatus | null { - const { config, log } = context +function arrayToMap ( + arr: Array, + keyFn: (item: TItem) => TKey, + valueFn: (item: TItem) => TValue +): { [key in TKey]?: TValue } { + return arr.reduce((result: { [key in TKey]?: TValue }, item) => merge(result, { + [keyFn(item)]: valueFn(item) + }), {}) +} + +function or (optional: TValue | undefined, defaultValue: TValue): TValue { + return optional === undefined + ? defaultValue + : optional +} + +function groupByCount ( + arr: Array, + keyFn: (item: TItem) => TKey +): { [key in TKey]?: number } { + return arr.reduce((result: { [key in TKey]?: number }, item) => { + const key = keyFn(item) + const previousValue = result[key] + const newValue = or(previousValue, 0) + 1 + return merge(result, { + [key]: newValue + 1 + }) + }, {}) +} + +function mapToArray (map: { [key in TKey]?: TValue }) { + return Object.entries(map) +} + +function get (obj: { [key in TKey]: TValue }, key: TKey): TValue { + return obj[key] +} + +function getLatestReviews (pullRequestInfo: PullRequestInfo) { const sortedReviews = pullRequestInfo.reviews.nodes.sort( (a, b) => new Date(a.submittedAt).getTime() - new Date(b.submittedAt).getTime() @@ -130,65 +184,69 @@ function getPullRequestStatusFromReviews ( sortedReviews ) - const reviewSummary = Object.entries(latestReviewsByUser) - .map(([user, review]) => `${user}: ${review.state}`) - .join('\n') - - log(`\nReviews:\n${reviewSummary}\n\n`) - const latestReviews = Object.values(latestReviewsByUser) + return latestReviews +} - let approvedByOneAssociation = false - for (let association of associations) { - const associationReviews = latestReviews.filter(review => getAssociationPriority(review.authorAssociation) >= getAssociationPriority(association)) +function doesNotHaveMaximumChangesRequested (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { + const { config, log } = context - const changesRequestedCount = associationReviews.filter(review => review.state === 'CHANGES_REQUESTED').length - const maxRequestedChanges = config.maxRequestedChanges[association] - if (maxRequestedChanges !== undefined && changesRequestedCount > maxRequestedChanges) { - return { - code: 'changes_requested', - message: `There are changes requested by a reviewer.` - } - } + const latestReviews = getLatestReviews(pullRequestInfo) + const changesRequestedCountByAssociation = + arrayToMap(associations, + (association) => association, + (association) => latestReviews + .filter(review => getAssociationPriority(review.authorAssociation) >= getAssociationPriority(association)) + .filter(review => review.state === 'CHANGES_REQUESTED') + .length + ) - const approvalCount = associationReviews.filter(review => review.state === 'APPROVED').length - const minApprovals = config.minApprovals[association] - if (minApprovals !== undefined && approvalCount >= minApprovals) { - approvedByOneAssociation = true + return mapToArray(config.maxRequestedChanges) + .some(([association, maxRequestedChanges]) => or(get(changesRequestedCountByAssociation, association), 0) > maxRequestedChanges) + ? { + status: 'fail', + message: `There are changes requested by a reviewer.` } - } + : { + status: 'success' + } +} - if (approvedByOneAssociation) { - return null - } else { - return { - code: 'need_approvals', - message: `There are not enough approvals by reviewers` +function hasMinimumApprovals (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { + const { config } = context + + const latestReviews = getLatestReviews(pullRequestInfo) + const approvalCountByAssociation = + arrayToMap(associations, + (association) => association, + (association) => latestReviews + .filter(review => getAssociationPriority(review.authorAssociation) >= getAssociationPriority(association)) + .filter(review => review.state === 'APPROVED') + .length + ) + + return mapToArray(config.minApprovals) + .some(([association, minApproval]) => or(get(approvalCountByAssociation, association), 0) < minApproval) + ? { + status: 'fail', + message: 'There are not enough approvals by reviewers' + } + : { + status: 'success' } - } } -function getPullRequestStatusFromChecks ( +function doesNotHaveBlockingChecks ( context: HandlerContext, pullRequestInfo: PullRequestInfo -): PullRequestStatus | null { - const { log } = context +): ConditionResult { const checkRuns = pullRequestInfo.checkRuns - // log('checks: ' + JSON.stringify(checks)) - const checksSummary = checkRuns - .map( - checkRun => `${checkRun.name}: ${checkRun.status}: ${checkRun.conclusion}` - ) - .join('\n') - - log(`\nChecks:\n${checksSummary}\n\n`) - const allChecksCompleted = checkRuns.every( checkRun => checkRun.status === 'completed' ) if (!allChecksCompleted) { return { - code: 'pending_checks', + status: 'pending', message: 'There are still pending checks' } } @@ -197,7 +255,6 @@ function getPullRequestStatusFromChecks ( _ => true, checkRuns ) - log('conclusions: ' + JSON.stringify(checkConclusions)) const checksBlocking = checkConclusions.failure || checkConclusions.cancelled || @@ -205,18 +262,19 @@ function getPullRequestStatusFromChecks ( checkConclusions.action_required if (checksBlocking) { return { - code: 'blocking_check', + status: 'fail', message: 'There are blocking checks' } } - - return null + return { + status: 'success' + } } -function getPullRequestStatusFromProtectedBranch ( +function hasUpToDateBranch ( context: HandlerContext, pullRequestInfo: PullRequestInfo -): PullRequestStatus | null { +): ConditionResult { const protectedBranch = pullRequestInfo.repository.protectedBranches.nodes .filter(protectedBranch => protectedBranch.name === pullRequestInfo.baseRef.name)[0] @@ -224,7 +282,7 @@ function getPullRequestStatusFromProtectedBranch ( && protectedBranch.hasStrictRequiredStatusChecks && pullRequestInfo.baseRef.target.oid !== pullRequestInfo.baseRefOid) { return { - code: 'out_of_date_branch', + status: 'fail', message: `Pull request is based on a strict protected branch (${ pullRequestInfo.baseRef.name }) and base sha of pull request (${ @@ -233,21 +291,70 @@ function getPullRequestStatusFromProtectedBranch ( } } - return null + return { + status: 'success' + } +} + +function mapObject (obj: { [key in TKey]: TValue }, mapper: (value: TValue) => TMappedValue): { [key in TKey]: TMappedValue } { + const result: any = {} + for (let key in obj) { + result[key] = mapper(obj[key]) + } + return result +} + +export function getConditionResults ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo +): ConditionResults { + return mapObject( + conditions, + (condition: Condition) => condition(context, pullRequestInfo) + ) +} + +function isFail (conditionResult: ConditionResult): boolean { + return conditionResult.status === 'fail' +} + +function getPullRequestStatusCode (results: ConditionResults): PullRequestStatusCode { + if (isFail(results.open)) { + return 'not_open' + } else if (isFail(results.mergeable)) { + return 'conflicts' + } else if (results.mergeable.status === 'pending') { + return 'pending_mergeable' + } else if (isFail(results.requiredLabels)) { + return 'requires_label' + } else if (isFail(results.blockingLabels)) { + return 'blocking_label' + } else if (isFail(results.maximumChangesRequested)) { + return 'changes_requested' + } else if (isFail(results.minimumApprovals)) { + return 'need_approvals' + } else if (isFail(results.blockingChecks)) { + return 'blocking_check' + } else if (results.blockingChecks.status === 'pending') { + return 'pending_checks' + } else if (isFail(results.upToDateBranch)) { + return 'out_of_date_branch' + } else { + return 'ready_for_merge' + } } export async function getPullRequestStatus ( context: HandlerContext, pullRequestInfo: PullRequestInfo ): Promise { - return ( - getPullRequestStatusFromPullRequest(context, pullRequestInfo) || - getPullRequestStatusFromLabels(context, pullRequestInfo) || - getPullRequestStatusFromReviews(context, pullRequestInfo) || - getPullRequestStatusFromChecks(context, pullRequestInfo) || - getPullRequestStatusFromProtectedBranch(context, pullRequestInfo) || { - code: 'ready_for_merge', - message: 'Pull request successfully merged' - } - ) + const results = getConditionResults(context, pullRequestInfo) + return { + code: pullRequestInfo.state === 'CLOSED' + ? 'closed' + : pullRequestInfo.state === 'MERGED' + ? 'merged' + : getPullRequestStatusCode(results), + message: '' + } } From f5ae116cd86c792fe8e02a8aa267b2cf0263b36a Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Sat, 18 Aug 2018 21:29:58 +0200 Subject: [PATCH 7/9] move conditions to separate files --- src/condition.ts | 4 + src/conditions/blockingChecks.ts | 38 +++ src/conditions/blockingLabels.ts | 24 ++ src/conditions/maximumChangesRequested.ts | 28 +++ src/conditions/mergeable.ts | 23 ++ src/conditions/minimumApprovals.ts | 28 +++ src/conditions/open.ts | 13 ++ src/conditions/requiredLabels.ts | 25 ++ src/conditions/upToDateBranch.ts | 27 +++ src/pull-request-status.ts | 272 ++-------------------- src/utils.ts | 65 +++++- 11 files changed, 288 insertions(+), 259 deletions(-) create mode 100644 src/condition.ts create mode 100644 src/conditions/blockingChecks.ts create mode 100644 src/conditions/blockingLabels.ts create mode 100644 src/conditions/maximumChangesRequested.ts create mode 100644 src/conditions/mergeable.ts create mode 100644 src/conditions/minimumApprovals.ts create mode 100644 src/conditions/open.ts create mode 100644 src/conditions/requiredLabels.ts create mode 100644 src/conditions/upToDateBranch.ts diff --git a/src/condition.ts b/src/condition.ts new file mode 100644 index 000000000..b8bcd3adc --- /dev/null +++ b/src/condition.ts @@ -0,0 +1,4 @@ +import { HandlerContext, PullRequestInfo } from './models' + +export type Condition = (context: HandlerContext, pullRequestInfo: PullRequestInfo) => ConditionResult +export type ConditionResult = { status: 'success' } | { status: 'fail', message: string } | { status: 'pending', message?: string } diff --git a/src/conditions/blockingChecks.ts b/src/conditions/blockingChecks.ts new file mode 100644 index 000000000..76a120d4e --- /dev/null +++ b/src/conditions/blockingChecks.ts @@ -0,0 +1,38 @@ +import { HandlerContext, PullRequestInfo } from '../models' +import { ConditionResult } from '../condition' +import { groupByLastMap } from '../utils' + +export default function doesNotHaveBlockingChecks ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo +): ConditionResult { + const checkRuns = pullRequestInfo.checkRuns + const allChecksCompleted = checkRuns.every( + checkRun => checkRun.status === 'completed' + ) + if (!allChecksCompleted) { + return { + status: 'pending', + message: 'There are still pending checks' + } + } + const checkConclusions = groupByLastMap( + checkRun => checkRun.conclusion, + _ => true, + checkRuns + ) + const checksBlocking = + checkConclusions.failure || + checkConclusions.cancelled || + checkConclusions.timed_out || + checkConclusions.action_required + if (checksBlocking) { + return { + status: 'fail', + message: 'There are blocking checks' + } + } + return { + status: 'success' + } +} diff --git a/src/conditions/blockingLabels.ts b/src/conditions/blockingLabels.ts new file mode 100644 index 000000000..ffda040b5 --- /dev/null +++ b/src/conditions/blockingLabels.ts @@ -0,0 +1,24 @@ +import { HandlerContext, PullRequestInfo } from '../models' +import { ConditionResult } from '../condition' + +export default function doesNotHaveBlockingLabels ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo +): ConditionResult { + const { config } = context + const pullRequestLabels = new Set(pullRequestInfo.labels.nodes.map(label => label.name)) + const foundBlockingLabels = config.blockingLabels + .filter(blockingLabel => pullRequestLabels.has(blockingLabel)) + + if (foundBlockingLabels.length > 0) { + return { + status: 'fail', + message: `Blocking labels are missing (${ + foundBlockingLabels.join(', ') + })` + } + } + return { + status: 'success' + } +} diff --git a/src/conditions/maximumChangesRequested.ts b/src/conditions/maximumChangesRequested.ts new file mode 100644 index 000000000..8a9ce6868 --- /dev/null +++ b/src/conditions/maximumChangesRequested.ts @@ -0,0 +1,28 @@ +import { HandlerContext, PullRequestInfo } from '../models' +import { ConditionResult } from '../condition' +import { getLatestReviews, arrayToMap, mapToArray, or, get } from '../utils' +import { associations, getAssociationPriority } from '../association' + +export default function doesNotHaveMaximumChangesRequested (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { + const { config } = context + + const latestReviews = getLatestReviews(pullRequestInfo) + const changesRequestedCountByAssociation = + arrayToMap(associations, + (association) => association, + (association) => latestReviews + .filter(review => getAssociationPriority(review.authorAssociation) >= getAssociationPriority(association)) + .filter(review => review.state === 'CHANGES_REQUESTED') + .length + ) + + return mapToArray(config.maxRequestedChanges) + .some(([association, maxRequestedChanges]) => or(get(changesRequestedCountByAssociation, association), 0) > maxRequestedChanges) + ? { + status: 'fail', + message: `There are changes requested by a reviewer.` + } + : { + status: 'success' + } +} diff --git a/src/conditions/mergeable.ts b/src/conditions/mergeable.ts new file mode 100644 index 000000000..3e255e672 --- /dev/null +++ b/src/conditions/mergeable.ts @@ -0,0 +1,23 @@ +import { HandlerContext, PullRequestInfo } from '../models' +import { ConditionResult } from '../condition' + +export default function isMergeable (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { + switch (pullRequestInfo.mergeable) { + case 'MERGEABLE': + return { + status: 'success' + } + case 'CONFLICTING': + return { + status: 'fail', + message: 'Pull request has conflicts' + } + case 'UNKNOWN': + return { + status: 'pending', + message: 'Github could not yet determine the mergeable status of the pull request' + } + default: + throw new Error(`Invalid mergeable state for pull request. mergeable=${pullRequestInfo.mergeable}`) + } +} diff --git a/src/conditions/minimumApprovals.ts b/src/conditions/minimumApprovals.ts new file mode 100644 index 000000000..f61ac4ee7 --- /dev/null +++ b/src/conditions/minimumApprovals.ts @@ -0,0 +1,28 @@ +import { HandlerContext, PullRequestInfo } from '../models' +import { ConditionResult } from '../condition' +import { getLatestReviews, arrayToMap, or, get, mapToArray } from '../utils' +import { associations, getAssociationPriority } from '../association' + +export default function hasMinimumApprovals (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { + const { config } = context + + const latestReviews = getLatestReviews(pullRequestInfo) + const approvalCountByAssociation = + arrayToMap(associations, + (association) => association, + (association) => latestReviews + .filter(review => getAssociationPriority(review.authorAssociation) >= getAssociationPriority(association)) + .filter(review => review.state === 'APPROVED') + .length + ) + + return mapToArray(config.minApprovals) + .some(([association, minApproval]) => or(get(approvalCountByAssociation, association), 0) < minApproval) + ? { + status: 'fail', + message: 'There are not enough approvals by reviewers' + } + : { + status: 'success' + } +} diff --git a/src/conditions/open.ts b/src/conditions/open.ts new file mode 100644 index 000000000..2b7f4b4b2 --- /dev/null +++ b/src/conditions/open.ts @@ -0,0 +1,13 @@ +import { HandlerContext, PullRequestInfo } from '../models' +import { ConditionResult } from '../condition' + +export default function isOpen (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { + return pullRequestInfo.state === 'OPEN' + ? { + status: 'success' + } + : { + status: 'fail', + message: `State of pull request is ${pullRequestInfo.state}` + } +} diff --git a/src/conditions/requiredLabels.ts b/src/conditions/requiredLabels.ts new file mode 100644 index 000000000..d36370f5f --- /dev/null +++ b/src/conditions/requiredLabels.ts @@ -0,0 +1,25 @@ +import { HandlerContext, PullRequestInfo } from '../models' +import { ConditionResult } from '../condition' + +export default function hasRequiredLabels ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo +): ConditionResult { + const { config } = context + const pullRequestLabels = new Set(pullRequestInfo.labels.nodes.map(label => label.name)) + + const missingRequiredLabels = config.requiredLabels + .filter(requiredLabel => !pullRequestLabels.has(requiredLabel)) + + if (missingRequiredLabels.length > 0) { + return { + status: 'fail', + message: `Required labels are missing (${ + missingRequiredLabels.join(', ') + })` + } + } + return { + status: 'success' + } +} diff --git a/src/conditions/upToDateBranch.ts b/src/conditions/upToDateBranch.ts new file mode 100644 index 000000000..6b12d88a9 --- /dev/null +++ b/src/conditions/upToDateBranch.ts @@ -0,0 +1,27 @@ +import { HandlerContext, PullRequestInfo } from '../models' +import { ConditionResult } from '../condition' + +export default function hasUpToDateBranch ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo +): ConditionResult { + const protectedBranch = pullRequestInfo.repository.protectedBranches.nodes + .filter(protectedBranch => protectedBranch.name === pullRequestInfo.baseRef.name)[0] + + if (protectedBranch + && protectedBranch.hasStrictRequiredStatusChecks + && pullRequestInfo.baseRef.target.oid !== pullRequestInfo.baseRefOid) { + return { + status: 'fail', + message: `Pull request is based on a strict protected branch (${ + pullRequestInfo.baseRef.name + }) and base sha of pull request (${ + pullRequestInfo.baseRefOid + }) differs from sha of branch (${pullRequestInfo.baseRef.target.oid})` + } + } + + return { + status: 'success' + } +} diff --git a/src/pull-request-status.ts b/src/pull-request-status.ts index 64630a7d6..b7b5a7e8a 100644 --- a/src/pull-request-status.ts +++ b/src/pull-request-status.ts @@ -1,8 +1,13 @@ -import { HandlerContext, PullRequestInfo, PullRequestInfo } from './../lib/src/models.d'; -import { PullRequestInfo, PullRequestInfo, CommentAuthorAssociation, PullRequestReviewState } from './github-models'; import { HandlerContext, PullRequestInfo } from './models' -import { groupByLast, groupByLastMap, result } from './utils' -import { associations, getAssociationPriority } from './association' +import { ConditionResult, Condition } from './condition' +import open from './conditions/open' +import mergeable from './conditions/mergeable' +import requiredLabels from './conditions/requiredLabels' +import blockingLabels from './conditions/blockingLabels' +import blockingChecks from './conditions/blockingChecks' +import minimumApprovals from './conditions/minimumApprovals' +import maximumChangesRequested from './conditions/maximumChangesRequested' +import upToDateBranch from './conditions/upToDateBranch' export type PullRequestStatus = | { @@ -40,262 +45,19 @@ export const PullRequestStatusCodes: PullRequestStatusCode[] = [ ] const conditions = { - open: isOpen, - mergeable: isMergeable, - requiredLabels: hasRequiredLabels, - blockingLabels: doesNotHaveBlockingLabels, - minimumApprovals: hasMinimumApprovals, - maximumChangesRequested: doesNotHaveMaximumChangesRequested, - blockingChecks: doesNotHaveBlockingChecks, - upToDateBranch: hasUpToDateBranch + open, + mergeable, + requiredLabels, + blockingLabels, + minimumApprovals, + maximumChangesRequested, + blockingChecks, + upToDateBranch } -type Condition = (context: HandlerContext, pullRequestInfo: PullRequestInfo) => ConditionResult type ConditionName = keyof (typeof conditions) -type ConditionResult = { status: 'success' } | { status: 'fail', message: string } | { status: 'pending', message?: string } type ConditionResults = { [key in ConditionName]: ConditionResult } -function isOpen (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { - return pullRequestInfo.state === 'OPEN' - ? { - status: 'success' - } - : { - status: 'fail', - message: `State of pull request is ${pullRequestInfo.state}` - } -} - -function isMergeable (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { - switch (pullRequestInfo.mergeable) { - case 'MERGEABLE': - return { - status: 'success' - } - case 'CONFLICTING': - return { - status: 'fail', - message: 'Pull request has conflicts' - } - case 'UNKNOWN': - return { - status: 'pending', - message: 'Github could not yet determine the mergeable status of the pull request' - } - default: - throw new Error(`Invalid mergeable state for pull request. mergeable=${pullRequestInfo.mergeable}`) - } -} - -function hasRequiredLabels ( - context: HandlerContext, - pullRequestInfo: PullRequestInfo -): ConditionResult { - const { config } = context - const pullRequestLabels = new Set(pullRequestInfo.labels.nodes.map(label => label.name)) - - const missingRequiredLabels = config.requiredLabels - .filter(requiredLabel => !pullRequestLabels.has(requiredLabel)) - - if (missingRequiredLabels.length > 0) { - return { - status: 'fail', - message: `Required labels are missing (${ - missingRequiredLabels.join(', ') - })` - } - } - return { - status: 'success' - } -} - -function doesNotHaveBlockingLabels ( - context: HandlerContext, - pullRequestInfo: PullRequestInfo -): ConditionResult { - const { config } = context - const pullRequestLabels = new Set(pullRequestInfo.labels.nodes.map(label => label.name)) - const foundBlockingLabels = config.blockingLabels - .filter(blockingLabel => pullRequestLabels.has(blockingLabel)) - - if (foundBlockingLabels.length > 0) { - return { - status: 'fail', - message: `Blocking labels are missing (${ - foundBlockingLabels.join(', ') - })` - } - } - return { - status: 'success' - } -} - -function merge (a: A, b: B): A & B { - return Object.assign({}, a, b) as any -} - -function arrayToMap ( - arr: Array, - keyFn: (item: TItem) => TKey, - valueFn: (item: TItem) => TValue -): { [key in TKey]?: TValue } { - return arr.reduce((result: { [key in TKey]?: TValue }, item) => merge(result, { - [keyFn(item)]: valueFn(item) - }), {}) -} - -function or (optional: TValue | undefined, defaultValue: TValue): TValue { - return optional === undefined - ? defaultValue - : optional -} - -function groupByCount ( - arr: Array, - keyFn: (item: TItem) => TKey -): { [key in TKey]?: number } { - return arr.reduce((result: { [key in TKey]?: number }, item) => { - const key = keyFn(item) - const previousValue = result[key] - const newValue = or(previousValue, 0) + 1 - return merge(result, { - [key]: newValue + 1 - }) - }, {}) -} - -function mapToArray (map: { [key in TKey]?: TValue }) { - return Object.entries(map) -} - -function get (obj: { [key in TKey]: TValue }, key: TKey): TValue { - return obj[key] -} - -function getLatestReviews (pullRequestInfo: PullRequestInfo) { - const sortedReviews = pullRequestInfo.reviews.nodes.sort( - (a, b) => - new Date(a.submittedAt).getTime() - new Date(b.submittedAt).getTime() - ) - const latestReviewsByUser = groupByLast( - review => review.author.login, - sortedReviews - ) - - const latestReviews = Object.values(latestReviewsByUser) - return latestReviews -} - -function doesNotHaveMaximumChangesRequested (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { - const { config, log } = context - - const latestReviews = getLatestReviews(pullRequestInfo) - const changesRequestedCountByAssociation = - arrayToMap(associations, - (association) => association, - (association) => latestReviews - .filter(review => getAssociationPriority(review.authorAssociation) >= getAssociationPriority(association)) - .filter(review => review.state === 'CHANGES_REQUESTED') - .length - ) - - return mapToArray(config.maxRequestedChanges) - .some(([association, maxRequestedChanges]) => or(get(changesRequestedCountByAssociation, association), 0) > maxRequestedChanges) - ? { - status: 'fail', - message: `There are changes requested by a reviewer.` - } - : { - status: 'success' - } -} - -function hasMinimumApprovals (context: HandlerContext, pullRequestInfo: PullRequestInfo): ConditionResult { - const { config } = context - - const latestReviews = getLatestReviews(pullRequestInfo) - const approvalCountByAssociation = - arrayToMap(associations, - (association) => association, - (association) => latestReviews - .filter(review => getAssociationPriority(review.authorAssociation) >= getAssociationPriority(association)) - .filter(review => review.state === 'APPROVED') - .length - ) - - return mapToArray(config.minApprovals) - .some(([association, minApproval]) => or(get(approvalCountByAssociation, association), 0) < minApproval) - ? { - status: 'fail', - message: 'There are not enough approvals by reviewers' - } - : { - status: 'success' - } -} - -function doesNotHaveBlockingChecks ( - context: HandlerContext, - pullRequestInfo: PullRequestInfo -): ConditionResult { - const checkRuns = pullRequestInfo.checkRuns - const allChecksCompleted = checkRuns.every( - checkRun => checkRun.status === 'completed' - ) - if (!allChecksCompleted) { - return { - status: 'pending', - message: 'There are still pending checks' - } - } - const checkConclusions = groupByLastMap( - checkRun => checkRun.conclusion, - _ => true, - checkRuns - ) - const checksBlocking = - checkConclusions.failure || - checkConclusions.cancelled || - checkConclusions.timed_out || - checkConclusions.action_required - if (checksBlocking) { - return { - status: 'fail', - message: 'There are blocking checks' - } - } - return { - status: 'success' - } -} - -function hasUpToDateBranch ( - context: HandlerContext, - pullRequestInfo: PullRequestInfo -): ConditionResult { - const protectedBranch = pullRequestInfo.repository.protectedBranches.nodes - .filter(protectedBranch => protectedBranch.name === pullRequestInfo.baseRef.name)[0] - - if (protectedBranch - && protectedBranch.hasStrictRequiredStatusChecks - && pullRequestInfo.baseRef.target.oid !== pullRequestInfo.baseRefOid) { - return { - status: 'fail', - message: `Pull request is based on a strict protected branch (${ - pullRequestInfo.baseRef.name - }) and base sha of pull request (${ - pullRequestInfo.baseRefOid - }) differs from sha of branch (${pullRequestInfo.baseRef.target.oid})` - } - } - - return { - status: 'success' - } -} - function mapObject (obj: { [key in TKey]: TValue }, mapper: (value: TValue) => TMappedValue): { [key in TKey]: TMappedValue } { const result: any = {} for (let key in obj) { diff --git a/src/utils.ts b/src/utils.ts index eb32b4bcb..93d861b49 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,4 +1,5 @@ import { AnyResponse } from '@octokit/rest' +import { PullRequestInfo } from './models' export type DeepPartial = { [Key in keyof T]?: DeepPartial; } export type ElementOf = TArray extends Array ? TElement : never @@ -27,15 +28,15 @@ export function groupByLast ( return groupByLastMap(keyFn, identity, list) } -export function groupByLastMap ( - keyFn: (item: TItem) => string, +export function groupByLastMap ( + keyFn: (item: TItem) => TKey, valueFn: (item: TItem) => TValue, list: TItem[] -): { [key: string]: TValue } { +): { [key in TKey]: TValue } { return list.reduce((result, item) => ({ ...result, [keyFn(item)]: valueFn(item) - }), {}) + }), {} as any) } /** @@ -49,3 +50,59 @@ export function result (response: AnyResponse): TResult { } return response.data } + +export function merge (a: A, b: B): A & B { + return Object.assign({}, a, b) as any +} + +export function arrayToMap ( + arr: Array, + keyFn: (item: TItem) => TKey, + valueFn: (item: TItem) => TValue +): { [key in TKey]?: TValue } { + return arr.reduce((result: { [key in TKey]?: TValue }, item) => merge(result, { + [keyFn(item)]: valueFn(item) + }), {}) +} + +export function or (optional: TValue | undefined, defaultValue: TValue): TValue { + return optional === undefined + ? defaultValue + : optional +} + +export function groupByCount ( + arr: Array, + keyFn: (item: TItem) => TKey +): { [key in TKey]?: number } { + return arr.reduce((result: { [key in TKey]?: number }, item) => { + const key = keyFn(item) + const previousValue = result[key] + const newValue = or(previousValue, 0) + 1 + return merge(result, { + [key]: newValue + 1 + }) + }, {}) +} + +export function mapToArray (map: { [key in TKey]?: TValue }) { + return Object.entries(map) +} + +export function get (obj: { [key in TKey]: TValue }, key: TKey): TValue { + return obj[key] +} + +export function getLatestReviews (pullRequestInfo: PullRequestInfo) { + const sortedReviews = pullRequestInfo.reviews.nodes.sort( + (a, b) => + new Date(a.submittedAt).getTime() - new Date(b.submittedAt).getTime() + ) + const latestReviewsByUser = groupByLast( + review => review.author.login, + sortedReviews + ) + + const latestReviews = Object.values(latestReviewsByUser) + return latestReviews +} From ed2665983a0cb34d95b82445457af62498729393 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Sun, 19 Aug 2018 11:17:13 +0200 Subject: [PATCH 8/9] add tests for individual conditions --- test/conditions/blockingChecks.test.ts | 103 ++++++++++++++++ test/conditions/blockingLabels.test.ts | 74 ++++++++++++ .../maximumChangesRequested.test.ts | 80 +++++++++++++ test/conditions/mergeable.test.ts | 34 ++++++ test/conditions/minimumApprovals.test.ts | 100 ++++++++++++++++ test/conditions/open.test.ts | 27 +++++ test/conditions/requiredLabels.test.ts | 112 ++++++++++++++++++ test/conditions/upToDateBranch.test.ts | 82 +++++++++++++ 8 files changed, 612 insertions(+) create mode 100644 test/conditions/blockingChecks.test.ts create mode 100644 test/conditions/blockingLabels.test.ts create mode 100644 test/conditions/maximumChangesRequested.test.ts create mode 100644 test/conditions/mergeable.test.ts create mode 100644 test/conditions/minimumApprovals.test.ts create mode 100644 test/conditions/open.test.ts create mode 100644 test/conditions/requiredLabels.test.ts create mode 100644 test/conditions/upToDateBranch.test.ts diff --git a/test/conditions/blockingChecks.test.ts b/test/conditions/blockingChecks.test.ts new file mode 100644 index 000000000..f191c49dc --- /dev/null +++ b/test/conditions/blockingChecks.test.ts @@ -0,0 +1,103 @@ +import blockingChecks from '../../src/conditions/blockingChecks' +import { createHandlerContext, createPullRequestInfo, successCheckRun, failedCheckRun } from '../mock' + +describe('blockingChecks', () => { + it('returns success pull request has succeeding check', async () => { + const result = blockingChecks( + createHandlerContext(), + createPullRequestInfo({ + checkRuns: [successCheckRun] + }) + ) + expect(result.status).toBe('success') + }) + + it('returns pending pull request has in_progress check', async () => { + const result = blockingChecks( + createHandlerContext(), + createPullRequestInfo({ + checkRuns: [{ + ...successCheckRun, + status: 'in_progress' + }] + }) + ) + expect(result.status).toBe('pending') + }) + + it('returns pending pull request has queued check', async () => { + const result = blockingChecks( + createHandlerContext(), + createPullRequestInfo({ + checkRuns: [{ + ...successCheckRun, + status: 'queued' + }] + }) + ) + expect(result.status).toBe('pending') + }) + + it('returns fail pull request has failed check', async () => { + const result = blockingChecks( + createHandlerContext(), + createPullRequestInfo({ + checkRuns: [failedCheckRun] + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns fail pull request has timed_out check', async () => { + const result = blockingChecks( + createHandlerContext(), + createPullRequestInfo({ + checkRuns: [{ + ...failedCheckRun, + conclusion: 'timed_out' + }] + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns fail pull request has cancelled check', async () => { + const result = blockingChecks( + createHandlerContext(), + createPullRequestInfo({ + checkRuns: [{ + ...failedCheckRun, + conclusion: 'cancelled' + }] + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns fail pull request has action_required check', async () => { + const result = blockingChecks( + createHandlerContext(), + createPullRequestInfo({ + checkRuns: [{ + ...failedCheckRun, + conclusion: 'action_required' + }] + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns success pull request has neutral check', async () => { + const result = blockingChecks( + createHandlerContext(), + createPullRequestInfo({ + checkRuns: [{ + ...failedCheckRun, + conclusion: 'neutral' + }] + }) + ) + expect(result.status).toBe('success') + }) + +}) diff --git a/test/conditions/blockingLabels.test.ts b/test/conditions/blockingLabels.test.ts new file mode 100644 index 000000000..e48b7814e --- /dev/null +++ b/test/conditions/blockingLabels.test.ts @@ -0,0 +1,74 @@ +import blockingLabels from '../../src/conditions/blockingLabels' +import { createHandlerContext, createPullRequestInfo, createConfig } from '../mock' + +describe('blockingLabels', () => { + it('returns success with no labels and no configuration', async () => { + const result = blockingLabels( + createHandlerContext({ + config: createConfig() + }), + createPullRequestInfo({ + labels: { + nodes: [] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns success with label not in configuration', async () => { + const result = blockingLabels( + createHandlerContext({ + config: createConfig({ + blockingLabels: ['blocking label'] + }) + }), + createPullRequestInfo({ + labels: { + nodes: [{ + name: 'non blocking label' + }] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns fail with label in configuration', async () => { + const result = blockingLabels( + createHandlerContext({ + config: createConfig({ + blockingLabels: ['blocking label'] + }) + }), + createPullRequestInfo({ + labels: { + nodes: [{ + name: 'blocking label' + }] + } + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns fail with label, among others, in configuration', async () => { + const result = blockingLabels( + createHandlerContext({ + config: createConfig({ + blockingLabels: ['blocking label'] + }) + }), + createPullRequestInfo({ + labels: { + nodes: [{ + name: 'blocking label' + }, { + name: 'non blocking label' + }] + } + }) + ) + expect(result.status).toBe('fail') + }) +}) diff --git a/test/conditions/maximumChangesRequested.test.ts b/test/conditions/maximumChangesRequested.test.ts new file mode 100644 index 000000000..07aff73cd --- /dev/null +++ b/test/conditions/maximumChangesRequested.test.ts @@ -0,0 +1,80 @@ +import doesNotHaveMaximumChangesRequested from '../../src/conditions/maximumChangesRequested' +import { createHandlerContext, createConfig, createPullRequestInfo, approvedReview, changesRequestedReview } from '../mock' + +describe('maximumChangesRequested', () => { + it('returns success when owner approved and nothing was configured', async () => { + const result = doesNotHaveMaximumChangesRequested( + createHandlerContext({ + config: createConfig() + }), + createPullRequestInfo({ + reviews: { + nodes: [ + approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }) + ] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns fail when owner requested changes and none role has 0 maximum requested changes', async () => { + const result = doesNotHaveMaximumChangesRequested( + createHandlerContext({ + config: createConfig({ + maxRequestedChanges: { + NONE: 0 + } + }) + }), + createPullRequestInfo({ + reviews: { + nodes: [ + changesRequestedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }) + ] + } + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns fail when owner requested changes and owner role has 0 maximum requested changes', async () => { + const result = doesNotHaveMaximumChangesRequested( + createHandlerContext({ + config: createConfig({ + maxRequestedChanges: { + OWNER: 0 + } + }) + }), + createPullRequestInfo({ + reviews: { + nodes: [ + changesRequestedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }) + ] + } + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns success when member requested changes but owner role has 0 maximum requested changes', async () => { + const result = doesNotHaveMaximumChangesRequested( + createHandlerContext({ + config: createConfig({ + maxRequestedChanges: { + OWNER: 0 + } + }) + }), + createPullRequestInfo({ + reviews: { + nodes: [ + changesRequestedReview({ author: { login: 'henk' }, authorAssociation: 'MEMBER' }) + ] + } + }) + ) + expect(result.status).toBe('success') + }) +}) diff --git a/test/conditions/mergeable.test.ts b/test/conditions/mergeable.test.ts new file mode 100644 index 000000000..2a326dac4 --- /dev/null +++ b/test/conditions/mergeable.test.ts @@ -0,0 +1,34 @@ +import mergeable from '../../src/conditions/mergeable' +import { createHandlerContext, createPullRequestInfo } from '../mock' + +describe('mergeable', () => { + it('returns success pull request state is MERGEABLE', async () => { + const result = mergeable( + createHandlerContext(), + createPullRequestInfo({ + mergeable: 'MERGEABLE' + }) + ) + expect(result.status).toBe('success') + }) + + it('returns fail pull request state is CONFLICTING', async () => { + const result = mergeable( + createHandlerContext(), + createPullRequestInfo({ + mergeable: 'CONFLICTING' + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns fail pull request state is UNKNOWN', async () => { + const result = mergeable( + createHandlerContext(), + createPullRequestInfo({ + mergeable: 'UNKNOWN' + }) + ) + expect(result.status).toBe('pending') + }) +}) diff --git a/test/conditions/minimumApprovals.test.ts b/test/conditions/minimumApprovals.test.ts new file mode 100644 index 000000000..dd09682a8 --- /dev/null +++ b/test/conditions/minimumApprovals.test.ts @@ -0,0 +1,100 @@ +import hasMinimumApprovals from '../../src/conditions/minimumApprovals' +import { createHandlerContext, createConfig, createPullRequestInfo, approvedReview } from '../mock' + +describe('minimumApprovals', () => { + it('returns success when owner approved and owner was configured', async () => { + const result = hasMinimumApprovals( + createHandlerContext({ + config: createConfig({ + minApprovals: { + OWNER: 1 + } + }) + }), + createPullRequestInfo({ + reviews: { + nodes: [ + approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }) + ] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns fail when member approved but owner was configured', async () => { + const result = hasMinimumApprovals( + createHandlerContext({ + config: createConfig({ + minApprovals: { + OWNER: 1 + } + }) + }), + createPullRequestInfo({ + reviews: { + nodes: [ + approvedReview({ author: { login: 'henk' }, authorAssociation: 'MEMBER' }) + ] + } + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns success when owner approved and member was configured', async () => { + const result = hasMinimumApprovals( + createHandlerContext({ + config: createConfig({ + minApprovals: { + MEMBER: 1 + } + }) + }), + createPullRequestInfo({ + reviews: { + nodes: [ + approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }) + ] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns success when owner approved but nothing was configured', async () => { + const result = hasMinimumApprovals( + createHandlerContext({ + config: createConfig() + }), + createPullRequestInfo({ + reviews: { + nodes: [ + approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }) + ] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns fail when no-one approved but member was configured', async () => { + const result = hasMinimumApprovals( + createHandlerContext({ + config: createConfig({ + minApprovals: { + MEMBER: 1 + } + }) + }), + createPullRequestInfo({ + reviews: { + nodes: [ + ] + } + }) + ) + expect(result.status).toBe('fail') + }) + +}) diff --git a/test/conditions/open.test.ts b/test/conditions/open.test.ts new file mode 100644 index 000000000..507214663 --- /dev/null +++ b/test/conditions/open.test.ts @@ -0,0 +1,27 @@ +import open from '../../src/conditions/open' +import { createHandlerContext, createPullRequestInfo } from '../mock' + +describe('open', () => { + it('returns success pull request state is open', async () => { + const result = open( + createHandlerContext(), + createPullRequestInfo({ + state: 'OPEN' + }) + ) + expect(result.status).toBe('success') + }) + + test.each([ + 'CLOSED', + 'MERGED' + ])('returns fail pull request state is not %s', async (state) => { + const result = open( + createHandlerContext(), + createPullRequestInfo({ + state + }) + ) + expect(result.status).toBe('fail') + }) +}) diff --git a/test/conditions/requiredLabels.test.ts b/test/conditions/requiredLabels.test.ts new file mode 100644 index 000000000..3f452d05b --- /dev/null +++ b/test/conditions/requiredLabels.test.ts @@ -0,0 +1,112 @@ +import requiredLabels from '../../src/conditions/requiredLabels' +import { createHandlerContext, createPullRequestInfo, createConfig } from '../mock' + +describe('open', () => { + it('returns success with no labels and no configuration', async () => { + const result = requiredLabels( + createHandlerContext({ + config: createConfig() + }), + createPullRequestInfo({ + labels: { + nodes: [] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns fail with label not in configuration', async () => { + const result = requiredLabels( + createHandlerContext({ + config: createConfig({ + requiredLabels: ['required label'] + }) + }), + createPullRequestInfo({ + labels: { + nodes: [{ + name: 'non required label' + }] + } + }) + ) + expect(result.status).toBe('fail') + }) + + it('returns success with label in configuration', async () => { + const result = requiredLabels( + createHandlerContext({ + config: createConfig({ + requiredLabels: ['required label'] + }) + }), + createPullRequestInfo({ + labels: { + nodes: [{ + name: 'required label' + }] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns success with multiple labels in pull request has required label in configuration', async () => { + const result = requiredLabels( + createHandlerContext({ + config: createConfig({ + requiredLabels: ['required label'] + }) + }), + createPullRequestInfo({ + labels: { + nodes: [{ + name: 'required label' + }, { + name: 'non required label' + }] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns success with labels in pull request also in configuration', async () => { + const result = requiredLabels( + createHandlerContext({ + config: createConfig({ + requiredLabels: ['required label', 'required label 2'] + }) + }), + createPullRequestInfo({ + labels: { + nodes: [{ + name: 'required label' + }, { + name: 'required label 2' + }] + } + }) + ) + expect(result.status).toBe('success') + }) + + it('returns fail with labels in pull request also in configuration', async () => { + const result = requiredLabels( + createHandlerContext({ + config: createConfig({ + requiredLabels: ['required label', 'required label 2'] + }) + }), + createPullRequestInfo({ + labels: { + nodes: [{ + name: 'required label' + }] + } + }) + ) + expect(result.status).toBe('fail') + }) +}) diff --git a/test/conditions/upToDateBranch.test.ts b/test/conditions/upToDateBranch.test.ts new file mode 100644 index 000000000..306d00a7f --- /dev/null +++ b/test/conditions/upToDateBranch.test.ts @@ -0,0 +1,82 @@ +import { defaultPullRequestInfo, createHandlerContext, createPullRequestInfo } from './../mock' +import upToDateBranch from '../../src/conditions/upToDateBranch' + +describe('upToDateBranch', () => { + it('returns fail when pull request is based on strict protected branch and base of PR is not equals to baseRef', async () => { + const status = upToDateBranch( + createHandlerContext(), + createPullRequestInfo({ + baseRef: { + ...defaultPullRequestInfo.baseRef, + name: 'master', + target: { + oid: '1111111111111111111111111111111111111111' + } + }, + baseRefOid: '0000000000000000000000000000000000000000', + repository: { + protectedBranches: { + nodes: [{ + name: 'master', + hasRestrictedPushes: true, + hasStrictRequiredStatusChecks: true + }] + } + } + }) + ) + expect(status.status).toBe('fail') + }) + + it('returns success when pull request is not based on strict protected branch but does have base of PR not equal to baseRef', async () => { + const status = upToDateBranch( + createHandlerContext(), + createPullRequestInfo({ + baseRef: { + ...defaultPullRequestInfo.baseRef, + name: 'master', + target: { + oid: '1111111111111111111111111111111111111111' + } + }, + baseRefOid: '0000000000000000000000000000000000000000', + repository: { + protectedBranches: { + nodes: [{ + name: 'master', + hasRestrictedPushes: true, + hasStrictRequiredStatusChecks: false + }] + } + } + }) + ) + expect(status.status).toBe('success') + }) + + it('returns success when pull request is based on strict protected branch and does have base of PR equal to baseRef', async () => { + const status = upToDateBranch( + createHandlerContext(), + createPullRequestInfo({ + baseRef: { + ...defaultPullRequestInfo.baseRef, + name: 'master', + target: { + oid: '0000000000000000000000000000000000000000' + } + }, + baseRefOid: '0000000000000000000000000000000000000000', + repository: { + protectedBranches: { + nodes: [{ + name: 'master', + hasRestrictedPushes: true, + hasStrictRequiredStatusChecks: true + }] + } + } + }) + ) + expect(status.status).toBe('success') + }) +}) From 84b944fb457e565b1e54b558c7ebc5e4d5a40e54 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Sun, 19 Aug 2018 17:12:52 +0200 Subject: [PATCH 9/9] split condition handling into determining actions and executing actions --- src/conditions/index.ts | 24 ++ src/pull-request-handler.ts | 230 +++++++++++++------ src/pull-request-status.ts | 106 +-------- src/utils.ts | 12 +- test/pull-request-handler.test.ts | 362 +++++++++++++++++------------- test/pull-request-status.test.ts | 351 ----------------------------- 6 files changed, 416 insertions(+), 669 deletions(-) create mode 100644 src/conditions/index.ts delete mode 100644 test/pull-request-status.test.ts diff --git a/src/conditions/index.ts b/src/conditions/index.ts new file mode 100644 index 000000000..3b6c3cd84 --- /dev/null +++ b/src/conditions/index.ts @@ -0,0 +1,24 @@ +import { ConditionResult } from './../condition' +import open from './open' +import mergeable from './mergeable' +import requiredLabels from './requiredLabels' +import blockingLabels from './blockingLabels' +import blockingChecks from './blockingChecks' +import minimumApprovals from './minimumApprovals' +import maximumChangesRequested from './maximumChangesRequested' +import upToDateBranch from './upToDateBranch' +import { keysOf } from '../utils' + +export const conditions = { + open, + mergeable, + requiredLabels, + blockingLabels, + minimumApprovals, + maximumChangesRequested, + blockingChecks, + upToDateBranch +} +export const conditionNames: ConditionName[] = keysOf(conditions) +export type ConditionName = keyof (typeof conditions) +export type ConditionResults = { [key in ConditionName]: ConditionResult } diff --git a/src/pull-request-handler.ts b/src/pull-request-handler.ts index 0d4dfff1b..0f8f5c073 100644 --- a/src/pull-request-handler.ts +++ b/src/pull-request-handler.ts @@ -106,82 +106,184 @@ async function doPullRequestWork ( ) Raven.mergeContext({ - tags: { - pullRequestStatus: pullRequestStatus.code + extra: { + pullRequestStatus } }) - log(`result: ${pullRequestStatus.code}: ${pullRequestStatus.message}`) + log(`result:\n${JSON.stringify(pullRequestStatus, null, 2)}`) await handlePullRequestStatus(context, pullRequestInfo, pullRequestStatus) } -export async function handlePullRequestStatus ( +export type PullRequestAction = 'reschedule' | 'update_branch' | 'merge' | 'delete_branch' + +/** + * Determines which actions to take based on the pull request and the condition results + */ +export function getPullRequestActions ( context: HandlerContext, pullRequestInfo: PullRequestInfo, pullRequestStatus: PullRequestStatus +): PullRequestAction[] { + const { config } = context + const pending = Object.values(pullRequestStatus) + .some(conditionResult => conditionResult.status === 'pending') + const success = Object.values(pullRequestStatus) + .every(conditionResult => conditionResult.status === 'success') + + if (pending) { + return ['reschedule'] + } + + // If upToDateBranch condition has failed, but all other conditions have succeeded + // and we have updateBranch enabled, update the branch of the PR. + if (pullRequestStatus.upToDateBranch.status === 'fail' + && config.updateBranch + && Object.entries(pullRequestStatus) + .filter(([name, _]) => name !== 'upToDateBranch') + .every(([_, value]) => value.status === 'success') + ) { + return isInFork(pullRequestInfo) + ? [] + : ['update_branch'] + } + + if (!success) { + return [] + } + + return [ + 'merge', + ...( + config.deleteBranchAfterMerge && !isInFork(pullRequestInfo) + ? ['delete_branch'] as PullRequestAction[] + : [] + ) + ] +} + +function isInFork (pullRequestInfo: PullRequestInfo): boolean { + return ( + pullRequestInfo.headRef.repository.owner.login !== pullRequestInfo.baseRef.repository.owner.login || + pullRequestInfo.headRef.repository.name !== pullRequestInfo.baseRef.repository.name + ) +} + +/** + * Deletes the branch of the pull request + */ +async function deleteBranch ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo +) { + return result( + await context.github.gitdata.deleteReference({ + owner: pullRequestInfo.headRef.repository.owner.login, + repo: pullRequestInfo.headRef.repository.name, + ref: `heads/${pullRequestInfo.headRef.name}` + }) + ) +} + +export async function executeActions ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo, + actions: PullRequestAction[] +) { + for (let action of actions) { + await executeAction(context, pullRequestInfo, action) + } +} + +export async function executeAction ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo, + action: PullRequestAction +): Promise { + switch (action) { + case 'update_branch': + return updateBranch(context, pullRequestInfo) + case 'reschedule': + return reschedulePullRequest(context, pullRequestInfo) + case 'merge': + return mergePullRequest(context, pullRequestInfo) + case 'delete_branch': + return deleteBranch(context, pullRequestInfo) + default: + throw new Error('Invalid PullRequestAction ' + action) + } +} + +/** + * Merges the base reference of the pull request onto the pull request branch + * This is the equivalent of pushing the 'Update branch' button + */ +async function updateBranch ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo ) { - const { log, github, config } = context - const pullRequestReference: PullRequestReference = { + // This merges the baseRef on top of headRef of the PR. + return result(await context.github.repos.merge({ + owner: pullRequestInfo.headRef.repository.owner.login, + repo: pullRequestInfo.headRef.repository.name, + base: pullRequestInfo.headRef.name, + head: pullRequestInfo.baseRef.name + })) +} + +function getPullRequestReference (pullRequestInfo: PullRequestInfo) { + return { owner: pullRequestInfo.baseRef.repository.owner.login, repo: pullRequestInfo.baseRef.repository.name, number: pullRequestInfo.number } - switch (pullRequestStatus.code) { - case 'ready_for_merge': - // We're ready for merging! - // This presses the merge button. - result( - await github.pullRequests.merge({ - ...pullRequestReference, - merge_method: config.mergeMethod - }) - ) - if (config.deleteBranchAfterMerge) { - // Check whether the pull request's branch was actually part of the same repo, as - // we do not want to (or rather do not have permission to) alter forks of this repo. - if ( - pullRequestInfo.headRef.repository.owner.login === pullRequestInfo.baseRef.repository.owner.login && - pullRequestInfo.headRef.repository.name === pullRequestInfo.baseRef.repository.name - ) { - result( - await github.gitdata.deleteReference({ - owner: pullRequestInfo.headRef.repository.owner.login, - repo: pullRequestInfo.headRef.repository.name, - ref: `heads/${pullRequestInfo.headRef.name}` - }) - ) - } - } - return - case 'out_of_date_branch': - if (config.updateBranch) { - // This merges the baseRef on top of headRef of the PR. - result(await github.repos.merge({ - owner: pullRequestInfo.headRef.repository.owner.login, - repo: pullRequestInfo.headRef.repository.name, - base: pullRequestInfo.headRef.name, - head: pullRequestInfo.baseRef.name - })) - } - return - case 'pending_mergeable': - case 'pending_checks': - // Some checks (like Travis) seem to not always send - // their status updates. Making this process being stalled. - // We work around this issue by scheduling a recheck after - // 1 minutes. The recheck is cancelled once another pull - // request event comes by. - log('Scheduling pull request trigger after 1 minutes') - const pullRequestKey = getPullRequestKey(pullRequestReference) - debug(`Setting timeout for ${pullRequestKey}`) - pullRequestTimeouts[pullRequestKey] = setTimeout(() => { - /* istanbul ignore next */ - debug(`Timeout triggered for ${pullRequestKey}`) - /* istanbul ignore next */ - schedulePullRequestTrigger(context, pullRequestReference) - }, 1 * 60 * 1000) - return - default: - // We will just wait for a next event from GitHub. - } +} + +/** + * Reschedules the pull request for later evaluation + */ +async function reschedulePullRequest ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo +) { + const pullRequestReference = getPullRequestReference(pullRequestInfo) + // Some checks (like Travis) seem to not always send + // their status updates. Making this process being stalled. + // We work around this issue by scheduling a recheck after + // 1 minutes. The recheck is cancelled once another pull + // request event comes by. + context.log('Scheduling pull request trigger after 1 minutes') + const pullRequestKey = getPullRequestKey(pullRequestReference) + debug(`Setting timeout for ${pullRequestKey}`) + pullRequestTimeouts[pullRequestKey] = setTimeout(() => { + debug(`Timeout triggered for ${pullRequestKey}`) + schedulePullRequestTrigger(context, pullRequestReference) + }, 1 * 60 * 1000) +} + +/** + * Presses the merge button on a pull request + */ +async function mergePullRequest ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo +) { + const { config } = context + const pullRequestReference = getPullRequestReference(pullRequestInfo) + // This presses the merge button. + result( + await context.github.pullRequests.merge({ + ...pullRequestReference, + merge_method: config.mergeMethod + }) + ) +} + +export async function handlePullRequestStatus ( + context: HandlerContext, + pullRequestInfo: PullRequestInfo, + pullRequestStatus: PullRequestStatus +) { + const actions = getPullRequestActions(context, pullRequestInfo, pullRequestStatus) + await executeActions(context, pullRequestInfo, actions) } diff --git a/src/pull-request-status.ts b/src/pull-request-status.ts index b7b5a7e8a..8a228e436 100644 --- a/src/pull-request-status.ts +++ b/src/pull-request-status.ts @@ -1,70 +1,10 @@ import { HandlerContext, PullRequestInfo } from './models' import { ConditionResult, Condition } from './condition' -import open from './conditions/open' -import mergeable from './conditions/mergeable' -import requiredLabels from './conditions/requiredLabels' -import blockingLabels from './conditions/blockingLabels' -import blockingChecks from './conditions/blockingChecks' -import minimumApprovals from './conditions/minimumApprovals' -import maximumChangesRequested from './conditions/maximumChangesRequested' -import upToDateBranch from './conditions/upToDateBranch' -export type PullRequestStatus = - | { - code: - | 'merged' - | 'closed' - | 'not_open' - | 'requires_label' - | 'blocking_label' - | 'pending_mergeable' - | 'conflicts' - | 'changes_requested' - | 'need_approvals' - | 'pending_checks' - | 'blocking_check' - | 'out_of_date_branch' - | 'ready_for_merge'; - message: string; - } +import { conditions, ConditionResults, ConditionName } from './conditions/' +import { mapObject } from './utils' -export type PullRequestStatusCode = PullRequestStatus['code'] - -export const PullRequestStatusCodes: PullRequestStatusCode[] = [ - 'merged', - 'closed', - 'not_open', - 'pending_mergeable', - 'conflicts', - 'changes_requested', - 'need_approvals', - 'pending_checks', - 'blocking_check', - 'out_of_date_branch', - 'ready_for_merge' -] - -const conditions = { - open, - mergeable, - requiredLabels, - blockingLabels, - minimumApprovals, - maximumChangesRequested, - blockingChecks, - upToDateBranch -} - -type ConditionName = keyof (typeof conditions) -type ConditionResults = { [key in ConditionName]: ConditionResult } - -function mapObject (obj: { [key in TKey]: TValue }, mapper: (value: TValue) => TMappedValue): { [key in TKey]: TMappedValue } { - const result: any = {} - for (let key in obj) { - result[key] = mapper(obj[key]) - } - return result -} +export type PullRequestStatus = ConditionResults export function getConditionResults ( context: HandlerContext, @@ -76,47 +16,9 @@ export function getConditionResults ( ) } -function isFail (conditionResult: ConditionResult): boolean { - return conditionResult.status === 'fail' -} - -function getPullRequestStatusCode (results: ConditionResults): PullRequestStatusCode { - if (isFail(results.open)) { - return 'not_open' - } else if (isFail(results.mergeable)) { - return 'conflicts' - } else if (results.mergeable.status === 'pending') { - return 'pending_mergeable' - } else if (isFail(results.requiredLabels)) { - return 'requires_label' - } else if (isFail(results.blockingLabels)) { - return 'blocking_label' - } else if (isFail(results.maximumChangesRequested)) { - return 'changes_requested' - } else if (isFail(results.minimumApprovals)) { - return 'need_approvals' - } else if (isFail(results.blockingChecks)) { - return 'blocking_check' - } else if (results.blockingChecks.status === 'pending') { - return 'pending_checks' - } else if (isFail(results.upToDateBranch)) { - return 'out_of_date_branch' - } else { - return 'ready_for_merge' - } -} - export async function getPullRequestStatus ( context: HandlerContext, pullRequestInfo: PullRequestInfo ): Promise { - const results = getConditionResults(context, pullRequestInfo) - return { - code: pullRequestInfo.state === 'CLOSED' - ? 'closed' - : pullRequestInfo.state === 'MERGED' - ? 'merged' - : getPullRequestStatusCode(results), - message: '' - } + return getConditionResults(context, pullRequestInfo) } diff --git a/src/utils.ts b/src/utils.ts index 93d861b49..f90306909 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -5,7 +5,9 @@ export type DeepPartial = { [Key in keyof T]?: DeepPartial; } export type ElementOf = TArray extends Array ? TElement : never export function identity (v: T): T { return v } - +export function keysOf (obj: { [key in TKey]: any }): TKey[] { + return Object.keys(obj) as TKey[] +} export function groupBy ( keyFn: (item: TItem) => string, list: TItem[] @@ -106,3 +108,11 @@ export function getLatestReviews (pullRequestInfo: PullRequestInfo) { const latestReviews = Object.values(latestReviewsByUser) return latestReviews } + +export function mapObject (obj: { [key in TKey]: TValue }, mapper: (value: TValue) => TMappedValue): { [key in TKey]: TMappedValue } { + const result: any = {} + for (let key in obj) { + result[key] = mapper(obj[key]) + } + return result +} diff --git a/test/pull-request-handler.test.ts b/test/pull-request-handler.test.ts index bb756f214..d2df8f058 100644 --- a/test/pull-request-handler.test.ts +++ b/test/pull-request-handler.test.ts @@ -1,7 +1,11 @@ +import { PullRequestStatus } from './../src/pull-request-status' +import { conditionNames } from './../src/conditions/index' +import { conditions, ConditionName } from './../src/conditions/' +import { ConditionResult } from './../src/condition' import { PullRequestInfo } from './../src/models' -import { handlePullRequestStatus } from '../src/pull-request-handler' -import { PullRequestStatusCodes } from '../src/pull-request-status' -import { createHandlerContext, createPullRequestInfo, createGithubApi, createConfig, defaultPullRequestInfo } from './mock' +import { getPullRequestActions, executeAction } from '../src/pull-request-handler' +import { createHandlerContext, createPullRequestInfo, createConfig, defaultPullRequestInfo, createGithubApi } from './mock' +import { mapObject } from '../src/utils' const defaultBaseRef: PullRequestInfo['baseRef'] = { repository: { @@ -16,6 +20,17 @@ const defaultBaseRef: PullRequestInfo['baseRef'] = { } } +const successPullRequestStatus: { [key in ConditionName]: ConditionResult } = mapObject(conditions, (_) => ({ + status: 'success' +} as ConditionResult)) + +function createPullRequestStatus (conditionResults?: Partial): PullRequestStatus { + return { + ...successPullRequestStatus, + ...conditionResults + } +} + const headRefInSameRepository: PullRequestInfo['headRef'] = { ...defaultBaseRef, name: 'pr-some-changes', @@ -24,117 +39,65 @@ const headRefInSameRepository: PullRequestInfo['headRef'] = { } } -describe('handlePullRequestStatus', () => { - beforeEach(() => { - jest.useFakeTimers() - }) - afterEach(() => { - jest.clearAllTimers() - }) +describe('getPullRequestActions', () => { it('merges when status is ready_for_merge', async () => { - const merge = jest.fn(() => ({ status: 200 })) - await handlePullRequestStatus( - createHandlerContext({ - github: createGithubApi({ - pullRequests: { - merge - } - }) - }), - createPullRequestInfo(), { - code: 'ready_for_merge', - message: 'bogus' - } + const actions = getPullRequestActions( + createHandlerContext(), + createPullRequestInfo(), + createPullRequestStatus() ) - expect(merge).toHaveBeenCalledTimes(1) + expect(actions).toEqual(['merge']) }) - const pullRequestStatusCodesOtherThanReadyForMerge = PullRequestStatusCodes - .filter(code => code !== 'ready_for_merge') - test.each(pullRequestStatusCodesOtherThanReadyForMerge)('does not merge on status %s', async (code) => { - const merge = jest.fn(() => ({ status: 200 })) - await handlePullRequestStatus( - createHandlerContext({ - github: createGithubApi({ - pullRequests: { - merge - } - }) - }), - createPullRequestInfo(), { - code, - message: 'bogus' - } as any + test.each(conditionNames)('does not merge when condition %s fails', async (conditionName) => { + const actions = getPullRequestActions( + createHandlerContext(), + createPullRequestInfo(), + createPullRequestStatus({ + [conditionName]: { + status: 'fail', + message: '' + } + }) ) - expect(merge).toHaveBeenCalledTimes(0) + expect(actions).toEqual([]) }) - it('schedules next run when status is pending_checks', async () => { - await handlePullRequestStatus( + it('schedules next run when one of the conditions is pending', async () => { + const actions = getPullRequestActions( createHandlerContext(), - createPullRequestInfo(), { - code: 'pending_checks', - message: 'bogus' + createPullRequestInfo(), + createPullRequestStatus({ + blockingChecks: { + status: 'pending' + } }) - expect(setTimeout).toHaveBeenCalledTimes(1) + ) + expect(actions).toEqual(['reschedule']) }) - const pullRequestStatusCodesOtherThanOutOfDateBranch = PullRequestStatusCodes - .filter(code => code !== 'out_of_date_branch') - test.each(pullRequestStatusCodesOtherThanOutOfDateBranch)('does not update branch on status %s', async (code) => { - const merge = jest.fn(() => ({ status: 200 })) - await handlePullRequestStatus( + it('updates branch when upToDateBranch fails and updateBranch is enabled', async () => { + const actions = getPullRequestActions( createHandlerContext({ - github: createGithubApi({ - pullRequests: { - merge: jest.fn(() => ({ status: 200 })) - }, - repos: { - merge - } - }) - }), - createPullRequestInfo(), { - code, - message: 'bogus' - } as any) - expect(merge).toHaveBeenCalledTimes(0) - }) - it('update branch when status is out_of_date_branch', async () => { - const merge = jest.fn(() => ({ status: 200 })) - await handlePullRequestStatus( - createHandlerContext({ - github: createGithubApi({ - repos: { - merge - } - }), config: createConfig({ updateBranch: true }) }), createPullRequestInfo({ - - }), { - code: 'out_of_date_branch', - message: 'bogus' + baseRef: defaultBaseRef, + headRef: headRefInSameRepository + }), + createPullRequestStatus({ + upToDateBranch: { + status: 'fail', + message: '' + } }) - expect(merge).toHaveBeenCalledTimes(1) - expect(merge.mock.calls[0]).toEqual([{ - base: 'pr-some-change', - head: 'master', - owner: 'bobvanderlinden', - repo: 'probot-auto-merge' - }]) + ) + expect(actions).toEqual(['update_branch']) }) it('update branch when status is out_of_date_branch and update-branch is enabled', async () => { - const merge = jest.fn(() => ({ status: 200 })) - await handlePullRequestStatus( + const actions = getPullRequestActions( createHandlerContext({ - github: createGithubApi({ - repos: { - merge - } - }), config: createConfig({ updateBranch: true }) @@ -142,27 +105,19 @@ describe('handlePullRequestStatus', () => { createPullRequestInfo({ baseRef: defaultBaseRef, headRef: headRefInSameRepository - }), { - code: 'out_of_date_branch', - message: 'bogus' + }), + createPullRequestStatus({ + upToDateBranch: { + status: 'fail', + message: '' + } }) - expect(merge).toHaveBeenCalledTimes(1) - expect(merge.mock.calls[0]).toEqual([{ - base: 'pr-some-changes', - head: 'master', - owner: 'bobvanderlinden', - repo: 'probot-auto-merge' - }]) + ) + expect(actions).toEqual(['update_branch']) }) it('not update branch when status is out_of_date_branch and update-branch is disabled', async () => { - const merge = jest.fn(() => ({ status: 200 })) - await handlePullRequestStatus( + const actions = getPullRequestActions( createHandlerContext({ - github: createGithubApi({ - repos: { - merge - } - }), config: createConfig({ updateBranch: false }) @@ -170,63 +125,150 @@ describe('handlePullRequestStatus', () => { createPullRequestInfo({ baseRef: defaultBaseRef, headRef: headRefInSameRepository - }), { - code: 'out_of_date_branch', - message: 'bogus' + }), + createPullRequestStatus({ }) - expect(merge).toHaveBeenCalledTimes(0) + ) + expect(actions).toEqual(['merge']) }) 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({ + config: createConfig({ + deleteBranchAfterMerge: true + }) + }), + createPullRequestInfo(), + createPullRequestStatus() + ) + + expect(actions).toEqual(['merge', 'delete_branch']) + }) + it('do not delete branch when status is ready_for_merge and delete-branch-after-merge is enabled, but branch resides in another repository', async () => { + const actions = getPullRequestActions( + createHandlerContext({ + config: createConfig({ + deleteBranchAfterMerge: true + }) + }), + createPullRequestInfo({ + baseRef: { + ...defaultPullRequestInfo.baseRef, + name: 'master', + repository: { + owner: { + login: 'bobvanderlinden' + }, + name: 'probot-auto-merge' + } + }, + headRef: { + ...defaultPullRequestInfo.headRef, + name: 'pr', + repository: { + owner: { + login: 'someone-else' + }, + name: 'probot-auto-merge' + } + } + }), + createPullRequestStatus() + ) + expect(actions).toEqual(['merge']) + }) +}) + +describe('executeAction with action', () => { + it('merge', async () => { const merge = jest.fn(() => ({ status: 200 })) - const deleteReference = jest.fn(() => ({ status: 200 })) - await handlePullRequestStatus( + await executeAction( createHandlerContext({ github: createGithubApi({ pullRequests: { merge + } + }) + }), + createPullRequestInfo({ + baseRef: { + name: 'master', + target: { + oid: '0' }, + repository: { + name: 'probot-auto-merge', + owner: { + login: 'bobvanderlinden' + } + } + }, + number: 2 + }), + 'merge' + ) + + expect(merge).toHaveBeenCalledTimes(1) + expect(merge).toBeCalledWith({ + merge_method: 'merge', + owner: 'bobvanderlinden', + repo: 'probot-auto-merge', + number: 2 + }) + }) + + it('delete_branch', async () => { + const deleteReference = jest.fn(() => ({ status: 200 })) + await executeAction( + createHandlerContext({ + github: createGithubApi({ gitdata: { deleteReference } - }), - config: createConfig({ - deleteBranchAfterMerge: true }) }), createPullRequestInfo({ - baseRef: defaultBaseRef, - headRef: headRefInSameRepository - }), { - code: 'ready_for_merge', - message: 'bogus' - } + headRef: { + name: 'the-merged-branch', + target: { + oid: '0' + }, + repository: { + owner: { + login: 'bobvanderlinden' + }, + name: 'probot-auto-merge' + } + } + }), + 'delete_branch' ) + expect(deleteReference).toHaveBeenCalledTimes(1) - expect(deleteReference.mock.calls[0]).toEqual([ - { owner: 'bobvanderlinden', ref: 'heads/pr-some-changes', repo: 'probot-auto-merge' } - ]) + expect(deleteReference).toBeCalledWith({ + owner: 'bobvanderlinden', + repo: 'probot-auto-merge', + ref: 'heads/the-merged-branch' + }) }) - it('do not delete branch when status is ready_for_merge and delete-branch-after-merge is enabled, but branch resides in another repository', async () => { + + it('update_branch', async () => { const merge = jest.fn(() => ({ status: 200 })) - const deleteReference = jest.fn(() => ({ status: 200 })) - await handlePullRequestStatus( + await executeAction( createHandlerContext({ github: createGithubApi({ - pullRequests: { + repos: { merge - }, - gitdata: { - deleteReference } - }), - config: createConfig({ - deleteBranchAfterMerge: true }) }), createPullRequestInfo({ - baseRef: { - ...defaultPullRequestInfo.baseRef, - name: 'master', + headRefOid: '0', + headRef: { + name: 'the-pr-branch', + target: { + oid: '0' + }, repository: { owner: { login: 'bobvanderlinden' @@ -234,21 +276,39 @@ describe('handlePullRequestStatus', () => { name: 'probot-auto-merge' } }, - headRef: { - ...defaultPullRequestInfo.headRef, - name: 'pr', + baseRefOid: '1', + baseRef: { + name: 'master', + target: { + oid: '2' + }, repository: { owner: { - login: 'someone-else' + login: 'bobvanderlinden' }, name: 'probot-auto-merge' } } - }), { - code: 'ready_for_merge', - message: 'bogus' - } + }), + 'update_branch' ) - expect(deleteReference).toHaveBeenCalledTimes(0) + + expect(merge).toHaveBeenCalledTimes(1) + expect(merge).toBeCalledWith({ + owner: 'bobvanderlinden', + repo: 'probot-auto-merge', + base: 'the-pr-branch', + head: 'master' + }) + }) + + it('reschedule', async () => { + jest.useFakeTimers() + await executeAction( + createHandlerContext(), + createPullRequestInfo(), + 'reschedule' + ) + expect(setTimeout).toHaveBeenCalledTimes(1) }) }) diff --git a/test/pull-request-status.test.ts b/test/pull-request-status.test.ts deleted file mode 100644 index 7465faaef..000000000 --- a/test/pull-request-status.test.ts +++ /dev/null @@ -1,351 +0,0 @@ -import { PullRequestState } from './../src/models' -import { - approvedReview, - changesRequestedReview, - successCheckRun, - queuedCheckRun, - failedCheckRun, - createHandlerContext, - createPullRequestInfo, - createConfig, - defaultPullRequestInfo -} from './mock' -import { getPullRequestStatus } from '../src/pull-request-status' - -describe('getPullRequestStatus', () => { - it('returns not_open when pull request state is a value that is undocumented', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { nodes: [approvedReview()] }, - state: 'this_value_is_undocumented' as PullRequestState - }) - ) - expect(status.code).toBe('not_open') - }) - - it('returns blocking_check when one check run failed', async () => { - const status = await getPullRequestStatus( - createHandlerContext({ - config: createConfig() - }), - createPullRequestInfo({ - reviews: { nodes: [approvedReview()] }, - checkRuns: [successCheckRun, failedCheckRun] - }) - ) - expect(status.code).toBe('blocking_check') - }) - - it('returns changes_requested when one reviewer requested changes', async () => { - const status = await getPullRequestStatus( - createHandlerContext({ - config: createConfig({ - minApprovals: { - MEMBER: 1 - }, - maxRequestedChanges: { - MEMBER: 0 - } - }) - }), - createPullRequestInfo({ - reviews: { - nodes: [ - approvedReview({ author: { login: 'henk' }, authorAssociation: 'MEMBER' }), - changesRequestedReview({ author: { login: 'sjaak' }, authorAssociation: 'MEMBER' }) - ] - }, - checkRuns: [successCheckRun] - }) - ) - expect(status.code).toBe('changes_requested') - }) - - it('returns changes_requested when owner approved, but member requested changes', async () => { - const status = await getPullRequestStatus( - createHandlerContext({ - config: createConfig({ - minApprovals: { - OWNER: 1, - MEMBER: 1 - }, - maxRequestedChanges: { - OWNER: 0, - MEMBER: 0 - } - }) - }), - createPullRequestInfo({ - reviews: { - nodes: [ - approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }), - changesRequestedReview({ author: { login: 'sjaak' }, authorAssociation: 'MEMBER' }) - ] - }, - checkRuns: [successCheckRun] - }) - ) - expect(status.code).toBe('changes_requested') - }) - - it('returns ready_for_merge when owner approved, but user requested changes', async () => { - const status = await getPullRequestStatus( - createHandlerContext({ - config: createConfig({ - minApprovals: { - OWNER: 1, - MEMBER: 1 - }, - maxRequestedChanges: { - OWNER: 0, - MEMBER: 0 - } - }) - }), - createPullRequestInfo({ - reviews: { - nodes: [ - approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }), - changesRequestedReview({ author: { login: 'sjaak' }, authorAssociation: 'NONE' }) - ] - }, - checkRuns: [successCheckRun] - }) - ) - expect(status.code).toBe('ready_for_merge') - }) - - it('returns ready_for_merge when two members approved, but user requested changes', async () => { - const status = await getPullRequestStatus( - createHandlerContext({ - config: createConfig({ - minApprovals: { - MEMBER: 2 - }, - maxRequestedChanges: { - MEMBER: 0 - } - }) - }), - createPullRequestInfo({ - reviews: { - nodes: [ - approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }), - approvedReview({ author: { login: 'sjaak' }, authorAssociation: 'MEMBER' }), - changesRequestedReview({ author: { login: 'piet' }, authorAssociation: 'NONE' }) - ] - }, - checkRuns: [successCheckRun] - }) - ) - expect(status.code).toBe('ready_for_merge') - }) - - it('returns ready_for_merge when two members approved, but user requested changes', async () => { - const status = await getPullRequestStatus( - createHandlerContext({ - config: createConfig({ - minApprovals: { - MEMBER: 2 - }, - maxRequestedChanges: { - MEMBER: 0 - } - }) - }), - createPullRequestInfo({ - reviews: { - nodes: [ - approvedReview({ author: { login: 'henk' }, authorAssociation: 'OWNER' }), - approvedReview({ author: { login: 'sjaak' }, authorAssociation: 'MEMBER' }), - changesRequestedReview({ author: { login: 'piet' }, authorAssociation: 'NONE' }) - ] - }, - checkRuns: [successCheckRun] - }) - ) - expect(status.code).toBe('ready_for_merge') - }) - - it('returns changes_requested when same reviewer approved and requested changes', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { nodes: [approvedReview({ author: { login: 'henk' } }), changesRequestedReview({ author: { login: 'henk' } })] }, - checkRuns: [successCheckRun] - }) - ) - expect(status.code).toBe('changes_requested') - }) - - it('returns pending_checks when check run is still queued', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { nodes: [approvedReview()] }, - checkRuns: [queuedCheckRun] - }) - ) - expect(status.code).toBe('pending_checks') - }) - - it('returns ready_for_merge when reviewer requested changes and approved', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { nodes: [changesRequestedReview({ author: { login: 'henk' } }), approvedReview({ author: { login: 'henk' } })] } - }) - ) - expect(status.code).toBe('ready_for_merge') - }) - - it('returns ready_for_merge when pull request is approved and check run succeeded', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { nodes: [approvedReview()] }, - checkRuns: [successCheckRun] - }) - ) - expect(status.code).toBe('ready_for_merge') - }) - - it('returns ready_for_merge when pull request is approved', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { nodes: [approvedReview()] } - }) - ) - expect(status.code).toBe('ready_for_merge') - }) - - it('returns closed when pull request is closed', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { - nodes: [ - approvedReview() - ] - }, - state: 'CLOSED' - }) - ) - expect(status.code).toBe('closed') - }) - - it('returns conflicts when pull request is not mergeable', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { nodes: [approvedReview()] }, - mergeable: 'CONFLICTING' - }) - ) - expect(status.code).toBe('conflicts') - }) - - it('returns pending_mergeable when pull request is not mergeable', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { nodes: [approvedReview()] }, - mergeable: 'UNKNOWN' - }) - ) - expect(status.code).toBe('pending_mergeable') - }) - - it('returns merged when pull request is already merged', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { nodes: [approvedReview()] }, - state: 'MERGED' - }) - ) - expect(status.code).toBe('merged') - }) - - it('returns need_approvals when pull request is not reviewed', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo() - ) - expect(status.code).toBe('need_approvals') - }) - - it('returns out_of_date_branch when pull request is based on strict protected branch', async () => { - const status = await getPullRequestStatus( - createHandlerContext(), - createPullRequestInfo({ - reviews: { - nodes: [ - approvedReview() - ] - }, - baseRef: { - ...defaultPullRequestInfo.baseRef, - name: 'master', - target: { - oid: '1111111111111111111111111111111111111111' - } - }, - baseRefOid: '0000000000000000000000000000000000000000', - repository: { - protectedBranches: { - nodes: [{ - name: 'master', - hasRestrictedPushes: true, - hasStrictRequiredStatusChecks: true - }] - } - } - }) - ) - expect(status.code).toBe('out_of_date_branch') - }) - - it('returns requires_label when a required label is configured, but not set on pull request', async () => { - const status = await getPullRequestStatus( - createHandlerContext({ - config: createConfig({ - requiredLabels: [ - 'mylabel' - ] - }) - }), - createPullRequestInfo({ - reviews: { nodes: [approvedReview()] } - }) - ) - expect(status.code).toBe('requires_label') - }) - - it('returns ready_for_merge when a required label is configured and it is set on pull request', async () => { - const status = await getPullRequestStatus( - createHandlerContext({ - config: createConfig({ - requiredLabels: [ - 'mylabel' - ] - }) - }), - createPullRequestInfo({ - reviews: { - nodes: [ - approvedReview() - ] - }, - labels: { - nodes: [{ - name: 'mylabel' - }] - } - }) - ) - expect(status.code).toBe('ready_for_merge') - }) -})