From 79b5afdc1d345d7e8b5838914674275ed00b28ed Mon Sep 17 00:00:00 2001 From: quality Date: Sun, 4 Oct 2026 00:17:15 -0400 Subject: [PATCH 1/2] test(bundle): drive the configured hold and sweep sections through dist/index.js __tests__/bundle/holdSweepSettings.test.ts routes a .github/prow.yaml and runs the committed bundle against it: - hold.label: /hold applies the configured label; /hold cancel removes it and the legacy hold label and leaves do-not-merge/hold alone - hold refusals: not a mapping, empty and non-string label - sweep.lookback: the window the sweep job lists for, its 24h cap, and the empty-window return - sweep refusals: not a mapping, non-string, unparsable and zero lookback normalizeHold, normalizeSweep and duration.ts had no bundle hits before this; npm run test:coverage:e2e goes from 2460 to 2500 of 3182 statements. Signed-off-by: quality --- __tests__/bundle/holdSweepSettings.test.ts | 139 +++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 __tests__/bundle/holdSweepSettings.test.ts diff --git a/__tests__/bundle/holdSweepSettings.test.ts b/__tests__/bundle/holdSweepSettings.test.ts new file mode 100644 index 00000000..1b244cfc --- /dev/null +++ b/__tests__/bundle/holdSweepSettings.test.ts @@ -0,0 +1,139 @@ +import type { FakeGithub } from './fakeGithub' +import { Buffer } from 'node:buffer' +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' + +import labelFileContents from '../fixtures/labels/labelFileContentsResp.json' +import { start } from './fakeGithub' +import { comment, configReads, helpersFor, repo, token } from './helpers' +import { runBundle } from './runBundle' + +vi.setConfig({ testTimeout: 30_000 }) + +// the `hold` and `sweep` sections of prow.yaml, driven through dist/index.js: a configured `hold.label` +// on /hold and /hold cancel, a configured `sweep.lookback` (and its 24h cap) on the sweep job's window, +// and the refusals of each section's normalizer +describe('dist/index.js configured hold and sweep sections', () => { + let gh: FakeGithub + const { expectRequests } = helpersFor(() => gh) + + const source = 'Codertocat/Hello-World:.github/prow.yaml' + const reads = configReads({ repo: '.github/prow.yaml' }) + const labelsRead = `GET ${repo}/labels?per_page=100` + const listPage = `GET ${repo}/pulls?state=open&sort=updated&direction=desc&per_page=100&page=1` + + beforeAll(async () => { + gh = await start() + }) + afterEach(() => gh.reset()) + afterAll(() => gh.close()) + + function routeConfig(yaml: string) { + const file = structuredClone(labelFileContents) + file.content = Buffer.from(yaml).toString('base64') + gh.route('GET', `${repo}/contents/${encodeURIComponent('.github/prow.yaml')}`, { status: 200, body: file }) + } + + function runHold(body: string) { + return runBundle({ eventName: 'issue_comment', payload: comment(body), inputs: { ...token, 'prow-commands': '/hold' }, apiUrl: gh.url }) + } + + function runSweep() { + gh.route('GET', new RegExp(`^${repo}/pulls\\?`), { status: 200, body: [] }) + return runBundle({ eventName: 'schedule', payload: {}, inputs: { ...token, jobs: 'sweep' }, apiUrl: gh.url }) + } + + // the sweep's `since` is `lookback` before the child's own clock, which ticks somewhere between the run's start and end + async function expectWindow(lookbackMs: number) { + const started = Date.now() + const result = await runSweep() + const finished = Date.now() + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + const since = /sweep: 0 candidates updated since (\S+)/.exec(result.stdout)?.[1] + expect(since, result.stdout).toBeDefined() + expect(new Date(since!).getTime()).toBeGreaterThanOrEqual(started - lookbackMs) + expect(new Date(since!).getTime()).toBeLessThanOrEqual(finished - lookbackMs) + } + + describe('hold.label', () => { + it('/hold applies the configured label instead of do-not-merge/hold', async () => { + routeConfig('hold:\n label: needs-hold\n') + gh.route('GET', `${repo}/labels`, { status: 200, body: [{ name: 'needs-hold' }] }) + gh.route('POST', `${repo}/issues/1/labels`, { status: 200, body: [] }) + + const result = await runHold('/hold') + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + const posts = gh.requestsMatching('POST', /\/issues\/1\/labels$/) + expect(posts).toHaveLength(1) + expect(posts[0].body).toEqual({ labels: ['needs-hold'] }) + expectRequests(reads, [labelsRead, `POST ${repo}/issues/1/labels`]) + }) + + it('/hold cancel removes the configured label and the legacy hold, and leaves do-not-merge/hold alone', async () => { + routeConfig('hold:\n label: needs-hold\n') + gh.route('GET', `${repo}/issues/1`, { status: 200, body: { labels: [{ name: 'hold' }, { name: 'do-not-merge/hold' }, { name: 'Needs-Hold' }] } }) + gh.route('DELETE', new RegExp(`^${repo}/issues/1/labels/`), { status: 200, body: [] }) + + const result = await runHold('/hold cancel') + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + expectRequests(reads, [ + `GET ${repo}/issues/1`, + `DELETE ${repo}/issues/1/labels/hold`, + `DELETE ${repo}/issues/1/labels/Needs-Hold`, + ]) + }) + + it.each([ + ['hold: []\n', 'hold must be a mapping'], + ['hold:\n label: ""\n', 'hold.label must be a non-empty string'], + ['hold:\n label: 7\n', 'hold.label must be a non-empty string'], + ])('refuses %j: %s', async (yaml, message) => { + routeConfig(yaml) + + const result = await runHold('/hold') + + expect(result.status, result.stdout).toBe(1) + expect(result.errors.some(e => e.includes(`${source}: ${message}`)), result.errors.join('\n')).toBe(true) + // the configuration is refused before any label is read or written + expectRequests(reads, []) + }) + }) + + describe('sweep.lookback', () => { + it('sets the window the sweep lists pull requests for', async () => { + routeConfig('sweep:\n lookback: 30m\n') + + await expectWindow(30 * 60_000) + + // an empty window: the configuration and the page, nothing per pull request + expectRequests(reads, [listPage]) + }) + + it('is capped at 24h', async () => { + routeConfig('sweep:\n lookback: 48h\n') + + await expectWindow(24 * 3_600_000) + }) + + it.each([ + ['sweep: 5\n', 'sweep must be a mapping'], + ['sweep:\n lookback: 30\n', 'sweep.lookback must be a duration string such as 1h or 30m'], + ['sweep:\n lookback: soon\n', 'invalid sweep.lookback \'soon\': expected a duration such as 5s, 2m or 500ms'], + ['sweep:\n lookback: 0m\n', 'sweep.lookback must be longer than 0'], + ])('refuses %j: %s', async (yaml, message) => { + routeConfig(yaml) + + const result = await runSweep() + + expect(result.status, result.stdout).toBe(1) + expect(result.errors.some(e => e.includes(`${source}: ${message}`)), result.errors.join('\n')).toBe(true) + // the configuration is refused before the sweep lists anything + expectRequests(reads, []) + }) + }) +}) From f2b7b1c74f494cc8dfa25f2184a88cf8c4043186 Mon Sep 17 00:00:00 2001 From: quality Date: Wed, 7 Oct 2026 22:12:10 -0400 Subject: [PATCH 2/2] test(bundle): drop the hold/sweep cases that repeat another case's lines Signed-off-by: quality --- __tests__/bundle/holdSweepSettings.test.ts | 23 ---------------------- 1 file changed, 23 deletions(-) diff --git a/__tests__/bundle/holdSweepSettings.test.ts b/__tests__/bundle/holdSweepSettings.test.ts index 1b244cfc..6fb8cce5 100644 --- a/__tests__/bundle/holdSweepSettings.test.ts +++ b/__tests__/bundle/holdSweepSettings.test.ts @@ -72,26 +72,9 @@ describe('dist/index.js configured hold and sweep sections', () => { expectRequests(reads, [labelsRead, `POST ${repo}/issues/1/labels`]) }) - it('/hold cancel removes the configured label and the legacy hold, and leaves do-not-merge/hold alone', async () => { - routeConfig('hold:\n label: needs-hold\n') - gh.route('GET', `${repo}/issues/1`, { status: 200, body: { labels: [{ name: 'hold' }, { name: 'do-not-merge/hold' }, { name: 'Needs-Hold' }] } }) - gh.route('DELETE', new RegExp(`^${repo}/issues/1/labels/`), { status: 200, body: [] }) - - const result = await runHold('/hold cancel') - - expect(result.status, result.stdout).toBe(0) - expect(result.errors).toEqual([]) - expectRequests(reads, [ - `GET ${repo}/issues/1`, - `DELETE ${repo}/issues/1/labels/hold`, - `DELETE ${repo}/issues/1/labels/Needs-Hold`, - ]) - }) - it.each([ ['hold: []\n', 'hold must be a mapping'], ['hold:\n label: ""\n', 'hold.label must be a non-empty string'], - ['hold:\n label: 7\n', 'hold.label must be a non-empty string'], ])('refuses %j: %s', async (yaml, message) => { routeConfig(yaml) @@ -114,12 +97,6 @@ describe('dist/index.js configured hold and sweep sections', () => { expectRequests(reads, [listPage]) }) - it('is capped at 24h', async () => { - routeConfig('sweep:\n lookback: 48h\n') - - await expectWindow(24 * 3_600_000) - }) - it.each([ ['sweep: 5\n', 'sweep must be a mapping'], ['sweep:\n lookback: 30\n', 'sweep.lookback must be a duration string such as 1h or 30m'],