From 771b29dc3fe9fda2f1abce5f7c0cb39e7a5225d4 Mon Sep 17 00:00:00 2001 From: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Date: Thu, 22 Oct 2026 07:51:47 -0400 Subject: [PATCH] test(sweep): pin the ignore_drafts=false arm of delayed reviewers (sweep.ts:218) With blunderbuss.delay set and ignore_drafts: false, requestReviewersIfDue must not skip drafts and must time every pull request from created_at alone, never reading the timeline. The unit suite only ever ran the delayed sweep with the default ignore_drafts: true, so that arm was reached by the bundle suite alone. Pins it in its own file so it does not collide with the pending edits to the delay block of sweep.test.ts. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> --- .../sweepDelayIgnoreDrafts.test.ts | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 __tests__/cronJobTest/sweepDelayIgnoreDrafts.test.ts diff --git a/__tests__/cronJobTest/sweepDelayIgnoreDrafts.test.ts b/__tests__/cronJobTest/sweepDelayIgnoreDrafts.test.ts new file mode 100644 index 00000000..7d0baed6 --- /dev/null +++ b/__tests__/cronJobTest/sweepDelayIgnoreDrafts.test.ts @@ -0,0 +1,126 @@ +import { Buffer } from 'node:buffer' +import { http } from 'msw' +import { setupServer } from 'msw/node' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +import { sweep } from '../../src/cronJobs/sweep' +import { resetOwnersCaches } from '../../src/utils/owners' +import labelFileContents from '../fixtures/labels/labelFileContentsResp.json' +import pullReqListPulls from '../fixtures/pullReq/pullReqListPulls.json' +import pullReqOpenedEvent from '../fixtures/pullReq/pullReqOpenedEvent.json' +import * as utils from '../testUtils' +import { blobSha } from '../utils/ownersFixtures' + +const server = setupServer() +beforeAll(() => server.listen(utils.failOnUnhandledRequest)) +afterEach(() => server.resetHandlers()) +afterAll(() => server.close()) + +const repo = `${utils.api}/repos/Codertocat/Hello-World` +const now = new Date('2026-09-16T12:00:00Z') +const owners = 'approvers:\n- alice\nreviewers:\n- carol\n' + +function ago(minutes: number): string { + return new Date(now.getTime() - minutes * 60_000).toISOString() +} + +function prowYaml(text: string) { + const file = structuredClone(labelFileContents) + file.content = Buffer.from(text).toString('base64') + return http.get(utils.contentsUrl('.github/prow.yaml'), utils.mockResponse(200, file)) +} + +function draft(number: number, openedMinutesAgo: number) { + return { + ...structuredClone(pullReqListPulls[0]), + number, + draft: true, + labels: [], + created_at: ago(openedMinutesAgo), + updated_at: ago(openedMinutesAgo), + requested_reviewers: [], + requested_teams: [], + assignees: [], + user: { login: 'some-author' }, + head: { sha: `sha${number}` }, + base: { ref: 'master', sha: 'basesha' }, + } +} + +function json(body: unknown, status = 200) { + return new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } }) +} + +// Covers the `ignore_drafts` false arm of `requestReviewersIfDue` +// (src/cronJobs/sweep.ts): with `blunderbuss.ignore_drafts: false` a delayed +// sweep does not skip drafts and times every pull request from `created_at` +// alone, so it never reads the timeline for a `ready_for_review` event. The +// e2e suite drives this through the bundle; this pins it per request. +describe('sweep with blunderbuss.delay and ignore_drafts off', () => { + let context: utils.MockContext + let requested: number[] + let timelineReads: number[] + + function servePulls(pulls: ReturnType[]) { + server.use( + http.get(`${repo}/pulls`, utils.mockResponse(200, pulls)), + http.get(`${repo}/pulls/:number`, ({ params }) => { + const item = pulls.find(pr => pr.number === Number(params.number)) + return json(item ?? { message: 'Not Found' }, item ? 200 : 404) + }), + ) + } + + beforeEach(() => { + utils.setupJobsEnv('sweep') + vi.useFakeTimers({ now, toFake: ['Date'] }) + resetOwnersCaches() + context = new utils.MockContext(pullReqOpenedEvent) + requested = [] + timelineReads = [] + server.use( + prowYaml('blunderbuss:\n delay: 1h\n ignore_drafts: false\n'), + ...utils.noOrgOrRepoConfigExcept(), + utils.defaultBranchTree(['OWNERS']), + http.get(`${repo}/branches/master`, utils.mockResponse(200, { commit: { sha: 'basesha' } })), + http.get(`${repo}/git/trees/basesha`, utils.mockResponse(200, { sha: 'basesha', truncated: false, tree: [{ path: 'OWNERS', type: 'blob', sha: blobSha('OWNERS') }] })), + http.get(`${repo}/git/blobs/${blobSha('OWNERS')}`, utils.mockResponse(200, { encoding: 'base64', content: Buffer.from(owners).toString('base64') })), + http.get(`${repo}/pulls/:number/files`, utils.mockResponse(200, [{ filename: 'x.go', status: 'modified' }])), + http.get(`${repo}/pulls/:number/reviews`, utils.mockResponse(200, [])), + http.get(`${repo}/issues/:number/timeline`, ({ params }) => { + timelineReads.push(Number(params.number)) + return json([{ event: 'ready_for_review', created_at: ago(5) }]) + }), + http.get(`${repo}/issues/:number`, utils.mockResponse(200, { labels: [] })), + http.get(`${repo}/issues/:number/comments`, utils.mockResponse(200, [])), + http.post(`${repo}/issues/:number/comments`, utils.mockResponse(201, {})), + http.post(`${repo}/pulls/:number/requested_reviewers`, ({ params }) => { + requested.push(Number(params.number)) + return json({}, 201) + }), + utils.repoHasLabels(['approved', 'lgtm']), + ) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('requests reviewers on a due draft from its creation time without reading the timeline', async () => { + // the timeline would say the draft became ready 5 minutes ago, which is not consulted + servePulls([draft(1, 70)]) + + await expect(sweep(context)).resolves.toEqual({ candidates: [], merged: [], enqueued: [], failures: [] }) + expect(requested).toEqual([1]) + expect(timelineReads).toEqual([]) + }) + + it('still leaves alone a draft opened after the due window', async () => { + servePulls([draft(1, 30), draft(2, 70)]) + + // #1 was updated inside the lookback, so it is a tide candidate, but it is not due for reviewers yet + await expect(sweep(context)).resolves.toEqual({ candidates: [1], merged: [], enqueued: [], failures: [] }) + expect(requested).toEqual([2]) + expect(timelineReads).toEqual([]) + }) +})