From 3586e3918ae0cde6d095029ee3dcce628712c6f7 Mon Sep 17 00:00:00 2001 From: quality Date: Mon, 5 Oct 2026 07:31:30 -0400 Subject: [PATCH 1/2] test(bundle): drive /hold cancel's no-label, failed-read, failed-removal and already-gone arms through dist/index.js Adds __tests__/bundle/holdCancelArms.test.ts: /hold cancel, /unhold and /remove-hold on an issue carrying no hold label, a 500 on the labels read, a 500 on the label DELETE, and a 404 on one DELETE (a label gone between read and removal), all run through the committed bundle against fakeGithub. Measured with npm run test:coverage:e2e on main @ 3fc21f2: src/labels/hold.ts 80 % -> 96 % lines, 81.81 % -> 90.9 % branches; src/utils/labeling.ts 73.07 % -> 76.92 % lines (removeLabels' benign 404 arm). The one remaining hold.ts line is the missing-issue-number throw, unreachable through the API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality --- __tests__/bundle/holdCancelArms.test.ts | 76 +++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 __tests__/bundle/holdCancelArms.test.ts diff --git a/__tests__/bundle/holdCancelArms.test.ts b/__tests__/bundle/holdCancelArms.test.ts new file mode 100644 index 00000000..b9365adc --- /dev/null +++ b/__tests__/bundle/holdCancelArms.test.ts @@ -0,0 +1,76 @@ +import type { FakeGithub } from './fakeGithub' +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' + +import { start } from './fakeGithub' +import { comment, configReads, helpersFor, repo, token } from './helpers' +import { runBundle } from './runBundle' + +vi.setConfig({ testTimeout: 30_000 }) + +// the arms of cancelHold in src/labels/hold.ts that bundle.test.ts's /unhold case (both labels present, every +// write accepted) does not reach: nothing to remove, a failed labels read, a failed removal, and a label that +// was already gone when the DELETE arrived +describe('dist/index.js /hold cancel arms', () => { + let gh: FakeGithub + const { expectRequests } = helpersFor(() => gh) + + beforeAll(async () => { + gh = await start() + }) + afterEach(() => gh.reset()) + afterAll(() => gh.close()) + + function run(body: string) { + return runBundle({ eventName: 'issue_comment', payload: comment(body), inputs: { ...token, 'prow-commands': '/hold' }, apiUrl: gh.url }) + } + + it.each(['/hold cancel', '/unhold', '/remove-hold'])('%s on an issue without a hold label succeeds and deletes nothing', async (body) => { + gh.route('GET', `${repo}/issues/1`, { status: 200, body: { labels: [{ name: 'lgtm' }] } }) + + const result = await run(body) + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + expect(gh.requestsMatching('DELETE', /./)).toEqual([]) + expectRequests(configReads(), [`GET ${repo}/issues/1`]) + }) + + it('/hold cancel fails naming the labels read when GET /issues/1 answers 500, and deletes nothing', async () => { + gh.route('GET', `${repo}/issues/1`, { status: 500, body: { message: 'boom' } }) + + const result = await run('/hold cancel') + + expect(result.status, result.stdout).toBe(1) + expect(result.errors.some(e => e.includes('could not get labels from issue'))).toBe(true) + expect(gh.requestsMatching('DELETE', /./)).toEqual([]) + expectRequests(configReads(), [`GET ${repo}/issues/1`]) + }) + + it('/hold cancel fails naming the hold label when the DELETE answers 500', async () => { + gh.route('GET', `${repo}/issues/1`, { status: 200, body: { labels: [{ name: 'do-not-merge/hold' }] } }) + gh.route('DELETE', `${repo}/issues/1/labels/do-not-merge%2Fhold`, { status: 500, body: { message: 'boom' } }) + + const result = await run('/hold cancel') + + expect(result.status, result.stdout).toBe(1) + expect(result.errors.some(e => e.includes('could not remove the hold label'))).toBe(true) + expect(result.errors.some(e => e.includes('could not remove label do-not-merge/hold'))).toBe(true) + expectRequests(configReads(), [`GET ${repo}/issues/1`, `DELETE ${repo}/issues/1/labels/do-not-merge%2Fhold`]) + }) + + it('/hold cancel treats a label that is already gone (DELETE 404) as removed', async () => { + gh.route('GET', `${repo}/issues/1`, { status: 200, body: { labels: [{ name: 'hold' }, { name: 'do-not-merge/hold' }] } }) + gh.route('DELETE', `${repo}/issues/1/labels/hold`, { status: 404, body: { message: 'Label does not exist' } }) + gh.route('DELETE', `${repo}/issues/1/labels/do-not-merge%2Fhold`, { status: 200, body: [] }) + + const result = await run('/hold cancel') + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + expectRequests(configReads(), [ + `GET ${repo}/issues/1`, + `DELETE ${repo}/issues/1/labels/hold`, + `DELETE ${repo}/issues/1/labels/do-not-merge%2Fhold`, + ]) + }) +}) From 8a99d52aeabccbb755fdfa419ca0c749aa432c62 Mon Sep 17 00:00:00 2001 From: quality Date: Wed, 7 Oct 2026 22:10:57 -0400 Subject: [PATCH 2/2] test(bundle): keep a single /hold cancel no-label case Signed-off-by: quality --- __tests__/bundle/holdCancelArms.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/__tests__/bundle/holdCancelArms.test.ts b/__tests__/bundle/holdCancelArms.test.ts index b9365adc..e52b30ac 100644 --- a/__tests__/bundle/holdCancelArms.test.ts +++ b/__tests__/bundle/holdCancelArms.test.ts @@ -24,10 +24,10 @@ describe('dist/index.js /hold cancel arms', () => { return runBundle({ eventName: 'issue_comment', payload: comment(body), inputs: { ...token, 'prow-commands': '/hold' }, apiUrl: gh.url }) } - it.each(['/hold cancel', '/unhold', '/remove-hold'])('%s on an issue without a hold label succeeds and deletes nothing', async (body) => { + it('/hold cancel on an issue without a hold label succeeds and deletes nothing', async () => { gh.route('GET', `${repo}/issues/1`, { status: 200, body: { labels: [{ name: 'lgtm' }] } }) - const result = await run(body) + const result = await run('/hold cancel') expect(result.status, result.stdout).toBe(0) expect(result.errors).toEqual([])