From b905f9e53a3b8c2e7614eecfbd3db0cdaaab396a Mon Sep 17 00:00:00 2001 From: quality Date: Tue, 6 Oct 2026 02:36:26 -0400 Subject: [PATCH 1/2] test(bundle): drive the missing issue/pull number guards of every command handler and onPrLgtm through dist/index.js Every command handler throws 'github context payload missing issue number' (pull number in cc/uncc, pr number in onPrLgtm) before its first api call when the payload carries no number. The unit suite reaches all 18 guards; the bundle suite never did because every fixture carries a number. Delete issue.number from the issue_comment fixture for each command (and pull_request.number from the synchronize payload for the lgtm job) and assert the guard's message reaches core.setFailed, with no api request for the hand-written commands that read nothing before the guard. Closes #342 Signed-off-by: quality --- __tests__/bundle/missingNumberGuards.test.ts | 98 ++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 __tests__/bundle/missingNumberGuards.test.ts diff --git a/__tests__/bundle/missingNumberGuards.test.ts b/__tests__/bundle/missingNumberGuards.test.ts new file mode 100644 index 0000000..e0ba687 --- /dev/null +++ b/__tests__/bundle/missingNumberGuards.test.ts @@ -0,0 +1,98 @@ +import type { FakeGithub } from './fakeGithub' +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' + +import pullRequestEvent from '../fixtures/pullReq/pullReqOpenedEvent.json' +import { start } from './fakeGithub' +import { comment, token } from './helpers' +import { runBundle } from './runBundle' + +vi.setConfig({ testTimeout: 30_000 }) + +// every command handler guards against an issue_comment payload without `issue.number` before its first +// api call, and onPrLgtm guards against a pull_request payload without `pull_request.number`. The +// fixtures always carry a number, so the guards are driven here, through dist/index.js, by deleting it +describe('dist/index.js missing issue and pull request number guards', () => { + let gh: FakeGithub + + beforeAll(async () => { + gh = await start() + }) + afterEach(() => gh.reset()) + afterAll(() => gh.close()) + + function commentWithoutNumber(body: string) { + const payload = comment(body) as { issue: { number?: number } } + delete payload.issue.number + return payload + } + + const issueGuard = 'github context payload missing issue number: [object Object]' + const pullGuard = 'github context payload missing pull number: [object Object]' + + it.each([ + ['/assign', '/assign @someone', issueGuard], + ['/unassign', '/unassign @someone', issueGuard], + ['/cc', '/cc @someone', pullGuard], + ['/uncc', '/uncc @someone', pullGuard], + ['/approve', '/approve', issueGuard], + ['/retitle', '/retitle a new title', issueGuard], + ['/remove', '/remove bug', issueGuard], + ['/hold', '/hold', issueGuard], + ['/lgtm', '/lgtm', issueGuard], + ['/close', '/close', issueGuard], + ['/lock', '/lock', issueGuard], + ['/reopen', '/reopen', issueGuard], + ['/milestone', '/milestone v1', issueGuard], + ['/meow', '/meow', issueGuard], + ['/retest', '/retest', issueGuard], + ['/help', '/help', issueGuard], + ['/kind', '/kind bug', issueGuard], + ])('%s on a comment without an issue number fails naming the guard', async (command, body, guard) => { + const result = await runBundle({ + eventName: 'issue_comment', + payload: commentWithoutNumber(body), + inputs: { ...token, 'prow-commands': command }, + apiUrl: gh.url, + }) + + expect(result.status, result.stdout).toBe(1) + expect(result.errors.some(e => e.includes(`error handling issue comment: Error: ${guard}`)), result.stdout).toBe(true) + }) + + it.each([ + ['/assign', '/assign @someone'], + ['/cc', '/cc @someone'], + ['/retitle', '/retitle a new title'], + ['/close', '/close'], + ['/lock', '/lock'], + ['/reopen', '/reopen'], + ['/milestone', '/milestone v1'], + ['/meow', '/meow'], + ['/retest', '/retest'], + ])('%s without an issue number makes no api call: the guard runs before the first read', async (command, body) => { + await runBundle({ + eventName: 'issue_comment', + payload: commentWithoutNumber(body), + inputs: { ...token, 'prow-commands': command }, + apiUrl: gh.url, + }) + + expect(gh.requests).toEqual([]) + }) + + it('the lgtm job on a synchronize payload without a pull request number fails naming the guard', async () => { + const payload = structuredClone(pullRequestEvent) as { action: string, pull_request: { number?: number } } + payload.action = 'synchronize' + delete payload.pull_request.number + + const result = await runBundle({ + eventName: 'pull_request', + payload, + inputs: { ...token, jobs: 'lgtm' }, + apiUrl: gh.url, + }) + + expect(result.status, result.stdout).toBe(1) + expect(result.errors.some(e => e.includes('error handling pull request: Error: github context payload missing pr number: [object Object]')), result.stdout).toBe(true) + }) +}) From e93ff2241b0c1f4fb6d8bd7b54837d3a989355ff Mon Sep 17 00:00:00 2001 From: quality Date: Wed, 7 Oct 2026 22:11:08 -0400 Subject: [PATCH 2/2] test(bundle): drop the no-api-call table and stop pinning the [object Object] payload rendering Signed-off-by: quality --- __tests__/bundle/missingNumberGuards.test.ts | 29 +++----------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/__tests__/bundle/missingNumberGuards.test.ts b/__tests__/bundle/missingNumberGuards.test.ts index e0ba687..e6d9458 100644 --- a/__tests__/bundle/missingNumberGuards.test.ts +++ b/__tests__/bundle/missingNumberGuards.test.ts @@ -26,8 +26,8 @@ describe('dist/index.js missing issue and pull request number guards', () => { return payload } - const issueGuard = 'github context payload missing issue number: [object Object]' - const pullGuard = 'github context payload missing pull number: [object Object]' + const issueGuard = 'github context payload missing issue number' + const pullGuard = 'github context payload missing pull number' it.each([ ['/assign', '/assign @someone', issueGuard], @@ -56,28 +56,7 @@ describe('dist/index.js missing issue and pull request number guards', () => { }) expect(result.status, result.stdout).toBe(1) - expect(result.errors.some(e => e.includes(`error handling issue comment: Error: ${guard}`)), result.stdout).toBe(true) - }) - - it.each([ - ['/assign', '/assign @someone'], - ['/cc', '/cc @someone'], - ['/retitle', '/retitle a new title'], - ['/close', '/close'], - ['/lock', '/lock'], - ['/reopen', '/reopen'], - ['/milestone', '/milestone v1'], - ['/meow', '/meow'], - ['/retest', '/retest'], - ])('%s without an issue number makes no api call: the guard runs before the first read', async (command, body) => { - await runBundle({ - eventName: 'issue_comment', - payload: commentWithoutNumber(body), - inputs: { ...token, 'prow-commands': command }, - apiUrl: gh.url, - }) - - expect(gh.requests).toEqual([]) + expect(result.errors.some(e => e.includes(guard)), result.stdout).toBe(true) }) it('the lgtm job on a synchronize payload without a pull request number fails naming the guard', async () => { @@ -93,6 +72,6 @@ describe('dist/index.js missing issue and pull request number guards', () => { }) expect(result.status, result.stdout).toBe(1) - expect(result.errors.some(e => e.includes('error handling pull request: Error: github context payload missing pr number: [object Object]')), result.stdout).toBe(true) + expect(result.errors.some(e => e.includes('github context payload missing pr number')), result.stdout).toBe(true) }) })