diff --git a/spec/fixtures/backport_pull_request.closed.json b/spec/fixtures/backport_pull_request.closed.json index fc5ff68..c910da9 100644 --- a/spec/fixtures/backport_pull_request.closed.json +++ b/spec/fixtures/backport_pull_request.closed.json @@ -12,6 +12,12 @@ "head": { "ref": "123456789iuytdxcvbnjhfdriuyfedfgy54escghjnbg" }, + "base": { + "ref": "main", + "repo": { + "default_branch": "main" + } + }, "body": "Backport of #14\nSee that PR for details.\nNotes: ", "created_at": "2018-11-01T17:29:51Z", "merged_at": "2018-11-01T17:30:11Z", diff --git a/spec/fixtures/backport_pull_request.merged.json b/spec/fixtures/backport_pull_request.merged.json index e3fbe42..00f857d 100644 --- a/spec/fixtures/backport_pull_request.merged.json +++ b/spec/fixtures/backport_pull_request.merged.json @@ -12,6 +12,12 @@ "head": { "ref": "123456789iuytdxcvbnjhfdriuyfedfgy54escghjnbg" }, + "base": { + "ref": "main", + "repo": { + "default_branch": "main" + } + }, "body": "Backport of #14\nSee that PR for details.\nNotes: ", "created_at": "2018-11-01T17:29:51Z", "merged_at": "2018-11-01T17:30:11Z", diff --git a/spec/fixtures/backport_pull_request.opened.json b/spec/fixtures/backport_pull_request.opened.json index 430ded5..75d0821 100644 --- a/spec/fixtures/backport_pull_request.opened.json +++ b/spec/fixtures/backport_pull_request.opened.json @@ -4,6 +4,7 @@ "action": "opened", "number": 7, "pull_request": { + "number": 7, "url": "my_cool_url", "title": "CHANGE README", "merged": true, diff --git a/spec/index.spec.ts b/spec/index.spec.ts index 1d856b5..259ca9b 100644 --- a/spec/index.spec.ts +++ b/spec/index.spec.ts @@ -392,6 +392,12 @@ Notes: `, head: { ref: '123456789iuytdxcvbnjhfdriuyfedfgy54escghjnbg', }, + base: { + ref: 'main', + repo: { + default_branch: 'main', + }, + }, labels: [ { color: 'ededed', @@ -429,6 +435,12 @@ Notes: `, head: { ref: '123456789iuytdxcvbnjhfdriuyfedfgy54escghjnbg', }, + base: { + ref: 'main', + repo: { + default_branch: 'main', + }, + }, labels: [ { color: 'ededed', diff --git a/spec/operations.spec.ts b/spec/operations.spec.ts index 0be51f4..1f4c971 100644 --- a/spec/operations.spec.ts +++ b/spec/operations.spec.ts @@ -3,8 +3,12 @@ import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; import simpleGit from 'simple-git'; + +import { PRChange } from '../src/enums'; import { initRepo } from '../src/operations/init-repo'; import { setupRemotes } from '../src/operations/setup-remotes'; +import { updateManualBackport } from '../src/operations/update-manual-backport'; +import { tagBackportReviewers } from '../src/utils'; let dirObject: { dir?: string } | null = null; @@ -13,6 +17,22 @@ const saveDir = (o: { dir: string }) => { return o.dir; }; +const backportPRClosedEvent = require('./fixtures/backport_pull_request.closed.json'); +const backportPRMergedEvent = require('./fixtures/backport_pull_request.merged.json'); +const backportPROpenedEvent = require('./fixtures/backport_pull_request.opened.json'); + +jest.mock('../src/utils', () => ({ + tagBackportReviewers: jest.fn().mockReturnValue(Promise.resolve()), + isSemverMinorPR: jest.fn().mockReturnValue(false), +})); + +jest.mock('../src/utils/label-utils', () => ({ + labelExistsOnPR: jest.fn().mockResolvedValue(true), + getSemverLabel: jest.fn().mockResolvedValue(false), + addLabels: jest.fn(), + removeLabel: jest.fn(), +})); + describe('runner', () => { jest.setTimeout(30000); console.error = jest.fn(); @@ -101,4 +121,50 @@ describe('runner', () => { } }); }); + + describe('updateManualBackport()', () => { + const octokit = { + pulls: { + get: jest.fn().mockReturnValue(Promise.resolve({})), + }, + issues: { + createComment: jest.fn().mockReturnValue(Promise.resolve({})), + listComments: jest.fn().mockReturnValue(Promise.resolve({ data: [] })), + }, + }; + + it('tags reviewers on manual backport creation', async () => { + const context = { + ...backportPROpenedEvent, + octokit, + repo: jest.fn(), + }; + await updateManualBackport(context, PRChange.OPEN, 1234); + expect(tagBackportReviewers).toHaveBeenCalled(); + expect(tagBackportReviewers).toHaveBeenCalledWith({ + context, + targetPrNumber: 7, + }); + }); + + it('does not tag reviewers on merged PRs', async () => { + const context = { + ...backportPRMergedEvent, + octokit, + repo: jest.fn(), + }; + await updateManualBackport(context, PRChange.MERGE, 1234); + expect(tagBackportReviewers).not.toHaveBeenCalled(); + }); + + it('does not tag reviewers on closed PRs', async () => { + const context = { + ...backportPRClosedEvent, + octokit, + repo: jest.fn(), + }; + await updateManualBackport(context, PRChange.CLOSE, 1234); + expect(tagBackportReviewers).not.toHaveBeenCalled(); + }); + }); }); diff --git a/spec/queue.spec.ts b/spec/queue.spec.ts index 5129cee..14566f1 100644 --- a/spec/queue.spec.ts +++ b/spec/queue.spec.ts @@ -3,8 +3,6 @@ import * as sinon from 'sinon'; import { ExecutionQueue } from '../src/Queue'; -const noop = async () => {}; - const waitForEvent = (emitter: EventEmitter, event: string) => { return new Promise((resolve) => { emitter.once(event, resolve); diff --git a/src/operations/update-manual-backport.ts b/src/operations/update-manual-backport.ts index 261cacd..d2b1b67 100644 --- a/src/operations/update-manual-backport.ts +++ b/src/operations/update-manual-backport.ts @@ -1,13 +1,13 @@ -import * as labelUtils from '../utils/label-utils'; -import { log } from '../utils/log-util'; -import { PRChange, PRStatus, LogLevel } from '../enums'; import { BACKPORT_LABEL, BACKPORT_REQUESTED_LABEL, SKIP_CHECK_LABEL, } from '../constants'; -import { isSemverMinorPR } from '../utils'; +import { PRChange, PRStatus, LogLevel } from '../enums'; import { WebHookPRContext } from '../types'; +import { isSemverMinorPR, tagBackportReviewers } from '../utils'; +import * as labelUtils from '../utils/label-utils'; +import { log } from '../utils/log-util'; /** * Updates the labels on a backport's original PR as well as comments with links @@ -135,6 +135,12 @@ please check out #${pr.number}`; }), ); } + + // Tag default reviewers to manual backport + await tagBackportReviewers({ + context, + targetPrNumber: pr.number, + }); } else if (type === PRChange.MERGE) { log( 'updateManualBackport', diff --git a/src/utils.ts b/src/utils.ts index dd6da9d..d3e9756 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -397,6 +397,38 @@ const createBackportComment = async ( return body; }; +export const tagBackportReviewers = async ({ + context, + targetPrNumber, + user, +}: { + context: SimpleWebHookRepoContext; + targetPrNumber: number; + user?: string; +}) => { + const reviewers = []; + + if (DEFAULT_BACKPORT_REVIEW_TEAM) { + reviewers.push(DEFAULT_BACKPORT_REVIEW_TEAM); + } + + if (user) { + const hasWrite = await checkUserHasWriteAccess(context, user); + // Optionally request a default review team for backports. + // If the PR author has write access, also request their review. + if (hasWrite) reviewers.push(user); + } + + if (reviewers.length > 0) { + await context.octokit.pulls.requestReviewers( + context.repo({ + pull_number: targetPrNumber, + reviewers, + }), + ); + } +}; + export const backportImpl = async ( robot: Probot, context: SimpleWebHookRepoContext, @@ -561,24 +593,11 @@ export const backportImpl = async ( }), ); - const reviewers = []; - const hasWrite = await checkUserHasWriteAccess(context, pr.user.login); - - // Optionally request a default review team for backports. - // If the PR author has write access, also request their review. - if (hasWrite) reviewers.push(pr.user.login); - if (DEFAULT_BACKPORT_REVIEW_TEAM) { - reviewers.push(DEFAULT_BACKPORT_REVIEW_TEAM); - } - - if (reviewers.length > 0) { - await context.octokit.pulls.requestReviewers( - context.repo({ - pull_number: newPr.number, - reviewers, - }), - ); - } + await tagBackportReviewers({ + context, + targetPrNumber: newPr.number, + user: pr.user.login, + }); log( 'backportImpl',