From f6a5a5658dec9775e9944253dd273baa6143dae1 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Mon, 7 Oct 2024 14:21:42 -0700 Subject: [PATCH] chore: improve typings and test mocks --- spec/index.spec.ts | 235 +++++++++++------------ spec/operations.spec.ts | 8 +- spec/utils.spec.ts | 12 +- src/Queue.ts | 4 +- src/operations/update-manual-backport.ts | 4 +- src/utils.ts | 5 +- src/utils/label-utils.ts | 2 +- src/utils/log-util.ts | 2 +- 8 files changed, 127 insertions(+), 145 deletions(-) diff --git a/spec/index.spec.ts b/spec/index.spec.ts index 9daabab..fbe2d34 100644 --- a/spec/index.spec.ts +++ b/spec/index.spec.ts @@ -55,7 +55,7 @@ const targetLabel = { jest.mock('../src/utils', () => ({ labelClosedPR: jest.fn(), - isAuthorizedUser: jest.fn().mockReturnValue(Promise.resolve([true])), + isAuthorizedUser: jest.fn().mockResolvedValue([true]), getPRNumbersFromPRBody: jest.fn().mockReturnValue([12345]), })); @@ -74,11 +74,9 @@ jest.mock('../src/operations/backport-to-location', () => ({ jest.mock('../src/utils/checks-util', () => ({ updateBackportValidityCheck: jest.fn(), - getBackportInformationCheck: jest - .fn() - .mockReturnValue(Promise.resolve({ status: 'thing' })), - updateBackportInformationCheck: jest.fn().mockReturnValue(Promise.resolve()), - queueBackportInformationCheck: jest.fn().mockReturnValue(Promise.resolve()), + getBackportInformationCheck: jest.fn().mockResolvedValue({ status: 'thing' }), + updateBackportInformationCheck: jest.fn().mockResolvedValue(undefined), + queueBackportInformationCheck: jest.fn().mockResolvedValue(undefined), })); describe('trop', () => { @@ -90,59 +88,51 @@ describe('trop', () => { nock.disableNetConnect(); octokit = { repos: { - getBranch: jest.fn().mockReturnValue(Promise.resolve()), - listBranches: jest.fn().mockReturnValue( - Promise.resolve({ - data: [{ name: '8-x-y' }, { name: '7-1-x' }], - }), - ), + getBranch: jest.fn().mockResolvedValue(undefined), + listBranches: jest.fn().mockResolvedValue({ + data: [{ name: '8-x-y' }, { name: '7-1-x' }], + }), }, git: { - deleteRef: jest.fn().mockReturnValue(Promise.resolve()), + deleteRef: jest.fn().mockResolvedValue(undefined), }, pulls: { - get: jest.fn().mockReturnValue( - Promise.resolve({ - data: { - merged: true, - head: { - sha: '6dcb09b5b57875f334f61aebed695e2e4193db5e', - }, - labels: [ - { - url: 'my_cool_url', - name: 'target/X-X-X', - color: 'fc2929', - }, - ], + get: jest.fn().mockResolvedValue({ + data: { + merged: true, + head: { + sha: '6dcb09b5b57875f334f61aebed695e2e4193db5e', }, - }), - ), - }, - issues: { - addLabels: jest.fn().mockReturnValue(Promise.resolve({})), - removeLabel: jest.fn().mockReturnValue(Promise.resolve({})), - createLabel: jest.fn().mockReturnValue(Promise.resolve({})), - createComment: jest.fn().mockReturnValue(Promise.resolve({})), - listLabelsOnIssue: jest.fn().mockReturnValue( - Promise.resolve({ - data: [ + labels: [ { - id: 208045946, - url: 'https://api.github.com/repos/octocat/Hello-World/labels/bug', - name: 'bug', - description: "Something isn't working", - color: 'f29513', + url: 'my_cool_url', + name: 'target/X-X-X', + color: 'fc2929', }, ], - }), - ), + }, + }), + }, + issues: { + addLabels: jest.fn().mockResolvedValue({}), + removeLabel: jest.fn().mockResolvedValue({}), + createLabel: jest.fn().mockResolvedValue({}), + createComment: jest.fn().mockResolvedValue({}), + listLabelsOnIssue: jest.fn().mockResolvedValue({ + data: [ + { + id: 208045946, + url: 'https://api.github.com/repos/octocat/Hello-World/labels/bug', + name: 'bug', + description: "Something isn't working", + color: 'f29513', + }, + ], + }), }, checks: { - listForRef: jest - .fn() - .mockReturnValue(Promise.resolve({ data: { check_runs: [] } })), - create: jest.fn().mockReturnValue(Promise.resolve({ data: jest.fn() })), + listForRef: jest.fn().mockResolvedValue({ data: { check_runs: [] } }), + create: jest.fn().mockResolvedValue({ data: jest.fn() }), }, }; @@ -153,7 +143,7 @@ describe('trop', () => { throttle: { enabled: false }, }), }); - (robot as any).state.octokit.auth = () => Promise.resolve(octokit); + robot['state'].octokit.auth = () => Promise.resolve(octokit); robot.auth = () => Promise.resolve(octokit); robot.load(trop); }); @@ -175,7 +165,7 @@ describe('trop', () => { it('does not trigger the backport on comment if the PR is not merged', async () => { octokit.pulls.get = jest .fn() - .mockReturnValue(Promise.resolve({ data: { merged: false } })); + .mockResolvedValue({ data: { merged: false } }); await robot.receive(issueCommentBackportCreatedEvent); @@ -229,33 +219,29 @@ describe('trop', () => { it('queues the check if there is no backport information for a new PR', async () => { const event = JSON.parse( - (await fs.readFile(newPROpenedEventPath, 'utf-8')) as string, + await fs.readFile(newPROpenedEventPath, 'utf-8'), ); event.payload.pull_request.labels = []; await robot.receive(event); - expect( - checkUtils.queueBackportInformationCheck as jest.Mock, - ).toHaveBeenCalledTimes(1); + expect(checkUtils.queueBackportInformationCheck).toHaveBeenCalledTimes(1); }); it('fails the check if there is conflicting backport information in a new PR', async () => { - (getPRNumbersFromPRBody as jest.Mock).mockReturnValueOnce( - Promise.resolve([]), - ); + jest.mocked(getPRNumbersFromPRBody).mockReturnValueOnce([]); const event = JSON.parse( - (await fs.readFile(newPROpenedEventPath, 'utf-8')) as string, + await fs.readFile(newPROpenedEventPath, 'utf-8'), ); event.payload.pull_request.labels = [noBackportLabel, targetLabel]; await robot.receive(event); - const updatePayload = ( - checkUtils.updateBackportInformationCheck as jest.Mock + const updatePayload = jest.mocked( + checkUtils.updateBackportInformationCheck, ).mock.calls[0][2]; expect(updatePayload).toMatchObject({ @@ -267,20 +253,18 @@ describe('trop', () => { }); it('passes the check if there is a "no-backport" label and no "target/" label in a new PR', async () => { - (getPRNumbersFromPRBody as jest.Mock).mockReturnValueOnce( - Promise.resolve([]), - ); + jest.mocked(getPRNumbersFromPRBody).mockReturnValueOnce([]); const event = JSON.parse( - (await fs.readFile(newPROpenedEventPath, 'utf-8')) as string, + await fs.readFile(newPROpenedEventPath, 'utf-8'), ); event.payload.pull_request.labels = [noBackportLabel]; await robot.receive(event); - const updatePayload = ( - checkUtils.updateBackportInformationCheck as jest.Mock + const updatePayload = jest.mocked( + checkUtils.updateBackportInformationCheck, ).mock.calls[0][2]; expect(updatePayload).toMatchObject({ @@ -291,20 +275,18 @@ describe('trop', () => { }); it('passes the check if there is no "no-backport" label and a "target/" label in a new PR', async () => { - (getPRNumbersFromPRBody as jest.Mock).mockReturnValueOnce( - Promise.resolve([]), - ); + jest.mocked(getPRNumbersFromPRBody).mockReturnValueOnce([]); const event = JSON.parse( - (await fs.readFile(newPROpenedEventPath, 'utf-8')) as string, + await fs.readFile(newPROpenedEventPath, 'utf-8'), ); event.payload.pull_request.labels = [targetLabel]; await robot.receive(event); - const updatePayload = ( - checkUtils.updateBackportInformationCheck as jest.Mock + const updatePayload = jest.mocked( + checkUtils.updateBackportInformationCheck, ).mock.calls[0][2]; expect(updatePayload).toMatchObject({ @@ -352,9 +334,12 @@ Notes: `, }, }; - expect((labelClosedPR as any).mock.calls[0][1]).toEqual(pr); - expect((labelClosedPR as any).mock.calls[0][2]).toBe('5-0-x'); - expect((labelClosedPR as any).mock.calls[0][3]).toBe(PRChange.CLOSE); + expect(jest.mocked(labelClosedPR)).toHaveBeenCalledWith( + expect.anything(), + pr, + '5-0-x', + PRChange.CLOSE, + ); }); it('updates labels on the original PR when a bot backport PR has been merged', async () => { @@ -387,9 +372,12 @@ Notes: `, }, }; - expect((labelClosedPR as any).mock.calls[0][1]).toEqual(pr); - expect((labelClosedPR as any).mock.calls[0][2]).toBe('4-0-x'); - expect((labelClosedPR as any).mock.calls[0][3]).toBe(PRChange.MERGE); + expect(jest.mocked(labelClosedPR)).toHaveBeenCalledWith( + expect.anything(), + pr, + '4-0-x', + PRChange.MERGE, + ); }); it('updates labels on the original PR when a manual backport PR has been closed with unmerged commits', async () => { @@ -430,9 +418,12 @@ Notes: `, expect(updateManualBackport).toHaveBeenCalled(); - expect((labelClosedPR as any).mock.calls[0][1]).toEqual(pr); - expect((labelClosedPR as any).mock.calls[0][2]).toBe('4-0-x'); - expect((labelClosedPR as any).mock.calls[0][3]).toBe(PRChange.CLOSE); + expect(jest.mocked(labelClosedPR)).toHaveBeenCalledWith( + expect.anything(), + pr, + '4-0-x', + PRChange.CLOSE, + ); }); it('updates labels on the original PR when a manual backport PR has been merged', async () => { @@ -473,36 +464,34 @@ Notes: `, expect(updateManualBackport).toHaveBeenCalled(); - expect((labelClosedPR as any).mock.calls[0][1]).toEqual(pr); - expect((labelClosedPR as any).mock.calls[0][2]).toBe('4-0-x'); - expect((labelClosedPR as any).mock.calls[0][3]).toBe(PRChange.MERGE); + expect(jest.mocked(labelClosedPR)).toHaveBeenCalledWith( + expect.anything(), + pr, + '4-0-x', + PRChange.MERGE, + ); }); }); describe('updateBackportValidityCheck from pull_request events', () => { it('skips the backport validity check if there is skip check label in a new PR', async () => { - (getPRNumbersFromPRBody as jest.Mock).mockReturnValueOnce( - Promise.resolve([]), - ); - (octokit.issues.listLabelsOnIssue as jest.Mock).mockReturnValueOnce( - Promise.resolve({ - data: [ - { - name: SKIP_CHECK_LABEL, - }, - ], - }), - ); + jest.mocked(getPRNumbersFromPRBody).mockReturnValueOnce([]); + octokit.issues.listLabelsOnIssue.mockResolvedValue({ + data: [ + { + name: SKIP_CHECK_LABEL, + }, + ], + }); const event = JSON.parse( - (await fs.readFile(newPRBackportOpenedEventPath, 'utf-8')) as string, + await fs.readFile(newPRBackportOpenedEventPath, 'utf-8'), ); event.payload.action = 'synchronize'; event.payload.pull_request.base.ref = '30-x-y'; await robot.receive(event); - const updatePayload = ( - checkUtils.updateBackportValidityCheck as jest.Mock - ).mock.calls[0][2]; + const updatePayload = jest.mocked(checkUtils.updateBackportValidityCheck) + .mock.calls[0][2]; expect(updatePayload).toMatchObject({ title: 'Backport Check Skipped', @@ -512,19 +501,16 @@ Notes: `, }); it('cancels the backport validity check if branch is targeting main', async () => { - (getPRNumbersFromPRBody as jest.Mock).mockReturnValueOnce( - Promise.resolve([]), - ); + jest.mocked(getPRNumbersFromPRBody).mockReturnValueOnce([]); const event = JSON.parse( - (await fs.readFile(newPRBackportOpenedEventPath, 'utf-8')) as string, + await fs.readFile(newPRBackportOpenedEventPath, 'utf-8'), ); await robot.receive(event); - const updatePayload = ( - checkUtils.updateBackportValidityCheck as jest.Mock - ).mock.calls[0][2]; + const updatePayload = jest.mocked(checkUtils.updateBackportValidityCheck) + .mock.calls[0][2]; expect(updatePayload).toMatchObject({ title: 'Cancelled', @@ -534,8 +520,8 @@ Notes: `, }); it('fails the backport validity check if old PR was not merged to a supported release branch', async () => { - (getPRNumbersFromPRBody as jest.Mock).mockReturnValueOnce([1234]); - (octokit.pulls.get as jest.Mock).mockResolvedValueOnce({ + jest.mocked(getPRNumbersFromPRBody).mockReturnValueOnce([1234]); + octokit.pulls.get.mockResolvedValueOnce({ data: { merged: true, base: { @@ -544,15 +530,14 @@ Notes: `, }, }); const event = JSON.parse( - (await fs.readFile(newPRBackportOpenedEventPath, 'utf-8')) as string, + await fs.readFile(newPRBackportOpenedEventPath, 'utf-8'), ); event.payload.pull_request.base.ref = '30-x-y'; event.payload.action = 'synchronize'; await robot.receive(event); - const updatePayload = ( - checkUtils.updateBackportValidityCheck as jest.Mock - ).mock.calls[0][2]; + const updatePayload = jest.mocked(checkUtils.updateBackportValidityCheck) + .mock.calls[0][2]; expect(updatePayload).toMatchObject({ title: 'Invalid Backport', @@ -563,8 +548,8 @@ Notes: `, }); it('fails the backport validity check if old PR has not been merged yet', async () => { - (getPRNumbersFromPRBody as jest.Mock).mockReturnValueOnce([1234]); - (octokit.pulls.get as jest.Mock).mockResolvedValueOnce({ + jest.mocked(getPRNumbersFromPRBody).mockReturnValueOnce([1234]); + octokit.pulls.get.mockResolvedValueOnce({ data: { merged: false, base: { @@ -573,15 +558,14 @@ Notes: `, }, }); const event = JSON.parse( - (await fs.readFile(newPRBackportOpenedEventPath, 'utf-8')) as string, + await fs.readFile(newPRBackportOpenedEventPath, 'utf-8'), ); event.payload.pull_request.base.ref = '30-x-y'; event.payload.action = 'synchronize'; await robot.receive(event); - const updatePayload = ( - checkUtils.updateBackportValidityCheck as jest.Mock - ).mock.calls[0][2]; + const updatePayload = jest.mocked(checkUtils.updateBackportValidityCheck) + .mock.calls[0][2]; expect(updatePayload).toMatchObject({ title: 'Invalid Backport', @@ -592,8 +576,8 @@ Notes: `, }); it('succeeds the backport validity check if all checks pass', async () => { - (getPRNumbersFromPRBody as jest.Mock).mockReturnValueOnce([1234]); - (octokit.pulls.get as jest.Mock).mockResolvedValueOnce({ + jest.mocked(getPRNumbersFromPRBody).mockReturnValueOnce([1234]); + octokit.pulls.get.mockResolvedValueOnce({ data: { merged: true, base: { @@ -602,15 +586,14 @@ Notes: `, }, }); const event = JSON.parse( - (await fs.readFile(newPRBackportOpenedEventPath, 'utf-8')) as string, + await fs.readFile(newPRBackportOpenedEventPath, 'utf-8'), ); event.payload.pull_request.base.ref = '30-x-y'; event.payload.action = 'synchronize'; await robot.receive(event); - const updatePayload = ( - checkUtils.updateBackportValidityCheck as jest.Mock - ).mock.calls[0][2]; + const updatePayload = jest.mocked(checkUtils.updateBackportValidityCheck) + .mock.calls[0][2]; expect(updatePayload).toMatchObject({ title: 'Valid Backport', diff --git a/spec/operations.spec.ts b/spec/operations.spec.ts index 1f4c971..4c9ea7c 100644 --- a/spec/operations.spec.ts +++ b/spec/operations.spec.ts @@ -22,7 +22,7 @@ const backportPRMergedEvent = require('./fixtures/backport_pull_request.merged.j const backportPROpenedEvent = require('./fixtures/backport_pull_request.opened.json'); jest.mock('../src/utils', () => ({ - tagBackportReviewers: jest.fn().mockReturnValue(Promise.resolve()), + tagBackportReviewers: jest.fn().mockResolvedValue(undefined), isSemverMinorPR: jest.fn().mockReturnValue(false), })); @@ -125,11 +125,11 @@ describe('runner', () => { describe('updateManualBackport()', () => { const octokit = { pulls: { - get: jest.fn().mockReturnValue(Promise.resolve({})), + get: jest.fn().mockResolvedValue({}), }, issues: { - createComment: jest.fn().mockReturnValue(Promise.resolve({})), - listComments: jest.fn().mockReturnValue(Promise.resolve({ data: [] })), + createComment: jest.fn().mockResolvedValue({}), + listComments: jest.fn().mockResolvedValue({ data: [] }), }, }; diff --git a/spec/utils.spec.ts b/spec/utils.spec.ts index 2fb7b03..f84caf2 100644 --- a/spec/utils.spec.ts +++ b/spec/utils.spec.ts @@ -16,13 +16,11 @@ describe('utils', () => { requestReviewers: jest.fn(), }, repos: { - getCollaboratorPermissionLevel: jest.fn().mockReturnValue( - Promise.resolve({ - data: { - permission: 'admin', - }, - }), - ), + getCollaboratorPermissionLevel: jest.fn().mockResolvedValue({ + data: { + permission: 'admin', + }, + }), }, }; diff --git a/src/Queue.ts b/src/Queue.ts index f154e83..2f11a9b 100644 --- a/src/Queue.ts +++ b/src/Queue.ts @@ -3,7 +3,7 @@ import { log } from './utils/log-util'; import { LogLevel } from './enums'; export type Executor = () => Promise; -export type ErrorExecutor = (err: any) => Promise; +export type ErrorExecutor = (err: unknown) => Promise; const DEFAULT_MAX_ACTIVE = 5; @@ -36,7 +36,7 @@ export class ExecutionQueue extends EventEmitter { this.active += 1; fns[1]() .then(() => this.runNext(fns[0])) - .catch((err: any) => { + .catch((err: unknown) => { if (!process.env.SPEC_RUNNING) { console.error(err); } diff --git a/src/operations/update-manual-backport.ts b/src/operations/update-manual-backport.ts index a28b14b..55ad9af 100644 --- a/src/operations/update-manual-backport.ts +++ b/src/operations/update-manual-backport.ts @@ -27,8 +27,8 @@ export const updateManualBackport = async ( const newPRLabelsToAdd = [pr.base.ref]; // Changed labels on the original PR. - let labelToAdd; - let labelToRemove; + let labelToAdd: string | undefined; + let labelToRemove: string; log( 'updateManualBackport', diff --git a/src/utils.ts b/src/utils.ts index b100d25..e50ed43 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -721,7 +721,7 @@ export const backportImpl = async ( await fs.remove(createdDir); }, async () => { - let annotations: any[] | null = null; + let annotations: unknown[] | null = null; let diff; let rawDiff; if (createdDir) { @@ -751,7 +751,8 @@ export const backportImpl = async ( message: 'Patch Conflict', raw_details: hunk.lines .filter( - (_: any, i: number) => i >= startOffset && i <= finalOffset, + (_: unknown, i: number) => + i >= startOffset && i <= finalOffset, ) .join('\n'), }); diff --git a/src/utils/label-utils.ts b/src/utils/label-utils.ts index 642ce06..641461a 100644 --- a/src/utils/label-utils.ts +++ b/src/utils/label-utils.ts @@ -23,7 +23,7 @@ export const addLabels = async ( }; export const getSemverLabel = (pr: Pick) => { - return pr.labels.find((l: any) => l.name.startsWith(SEMVER_PREFIX)); + return pr.labels.find((l) => l.name.startsWith(SEMVER_PREFIX)); }; export const getHighestSemverLabel = (...labels: string[]) => { diff --git a/src/utils/log-util.ts b/src/utils/log-util.ts index ce32ae0..19d43b0 100644 --- a/src/utils/log-util.ts +++ b/src/utils/log-util.ts @@ -10,7 +10,7 @@ import { LogLevel } from '../enums'; export const log = ( functionName: string, logLevel: LogLevel, - ...message: any[] + ...message: unknown[] ) => { const output = `${functionName}: ${message}`;