test(plugins): pin blunderbuss' file-count tier preference, requireMatchingLabel's unlabeled arm and withdrawalReason's event ordering

__tests__/plugins/approveReviewWithdrawalOrder.test.ts

@@ -0,0 +1,33 @@
+import type { ApprovalEvent, ApprovalState, ApproveSettings } from '../../src/plugins/approve'
+import type { PullRequestOwners } from '../../src/utils/pullRequestOwners'
+import { describe, expect, it } from 'vitest'
+
+import { withdrawalReason } from '../../src/plugins/approveReview'
+
+// collectApprovalEvents in approve.ts appends every comment before any review, so a
+// CHANGES_REQUESTED review submitted before a later `/approve` comment reaches
+// withdrawalReason out of chronological order; the reason must still follow time.
+describe('withdrawalReason orders events by time, not by arrival', () => {
+  const settings: ApproveSettings = { require_self_approval: false, ignore_review_state: false, lgtm_acts_as_approve: false, github_review: true }
+  const owners = { number: 1, files: ['x'] } as PullRequestOwners
+  const state = { uncoveredFiles: ['x'] } as ApprovalState
+  const event = (user: string, kind: ApprovalEvent['kind'], at: number): ApprovalEvent => ({ user, kind, at: new Date(at) })
+
+  it('a later /approve comment listed before an earlier changes-requested review retracts it', () => {
+    const events = [event('bob', 'approve', 2), event('bob', 'review-changes', 1)]
+
+    expect(withdrawalReason(owners, state, events, settings)).toBe('no approver covers x')
+  })
+
+  it('an earlier /approve comment listed before a later changes-requested review is withdrawn by it', () => {
+    const events = [event('bob', 'approve', 1), event('bob', 'review-changes', 2)]
+
+    expect(withdrawalReason(owners, state, events, settings)).toBe('no approver covers x; withdrawn by bob (changes requested)')
+  })
+
+  it('a later cancel listed first still wins over the earlier approve', () => {
+    const events = [event('carol', 'cancel', 5), event('carol', 'approve', 4)]
+
+    expect(withdrawalReason(owners, state, events, settings)).toBe('no approver covers x; withdrawn by carol (/approve cancel)')
+  })
+})

__tests__/plugins/blunderbussTierPreference.test.ts

@@ -0,0 +1,101 @@
+import { Buffer } from 'node:buffer'
+import * as core from '@actions/core'
+import { http } from 'msw'
+import { setupServer } from 'msw/node'
+import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { blunderbuss } from '../../src/plugins/blunderbuss'
+import labelFileContents from '../fixtures/labels/labelFileContentsResp.json'
+import pullReqOpenedEvent from '../fixtures/pullReq/pullReqOpenedEvent.json'
+import * as utils from '../testUtils'
+import { baseSha, changedFiles, filesHandler, repo, treeHandlers } from '../utils/ownersFixtures'
+
+const server = setupServer()
+utils.failTestOnUnhandledRequests(server)
+beforeAll(() => server.listen(utils.failOnUnhandledRequest))
+afterEach(() => server.resetHandlers())
+afterAll(() => server.close())
+
+const project = 'Codertocat/.project:prow.yaml'
+
+// alice and rita own the whole tree; bob, carol and dave only sdk/
+const owners = {
+  'OWNERS': 'approvers:\n- alice\nreviewers:\n- rita\n',
+  'sdk/OWNERS': 'approvers:\n- bob\nreviewers:\n- carol\n- dave\n',
+}
+
+// an rng that always returns 0 picks the first remaining element at every step
+const first = () => 0
+
+function prEvent(action: string) {
+  return new utils.MockContext({ ...structuredClone(pullReqOpenedEvent), action })
+}
+
+function serveConfig(yaml: string) {
+  const file = structuredClone(labelFileContents)
+  file.content = Buffer.from(yaml).toString('base64')
+  server.use(
+    http.get(utils.contentsUrl(project), utils.mockResponse(200, file)),
+    ...utils.noOrgOrRepoConfigExcept(project),
+  )
+}
+
+function servePr(files: string[]) {
+  server.use(
+    http.get(`${repo}/pulls/1`, utils.mockResponse(200, {
+      base: { ref: 'master', sha: baseSha },
+      head: { sha: 'headsha' },
+      user: { login: 'some-author' },
+      draft: false,
+      requested_reviewers: [],
+      assignees: [],
+    })),
+    filesHandler(changedFiles(...files)),
+    ...treeHandlers(owners),
+  )
+}
+
+// The per-reviewer file count is accumulated with `(coverage.get(login) ?? 0) + 1`;
+// a wrong seed (undefined + 1 = NaN) collapses every candidate into one tier, so
+// the owner covering more of the changed files loses the preference the plugin
+// documents. The existing "weights candidates" case cannot see this: with
+// request_count 1 and the last-picking rng both tier layouts yield rita.
+describe('blunderbuss prefers the owners covering more of the changed files', () => {
+  let setFailed: ReturnType<typeof vi.spyOn>
+
+  beforeEach(() => {
+    utils.setupActionsEnv()
+    setFailed = vi.spyOn(core, 'setFailed').mockImplementation(() => {})
+    vi.spyOn(core, 'debug').mockImplementation(() => {})
+  })
+
+  it('fills request_count from the two-file tier before any one-file owner is drawn', async () => {
+    serveConfig('blunderbuss:\n  request_count: 2\n')
+    servePr(['sdk/x.go', 'docs/y.md'])
+    const observe = new utils.ObserveRequest()
+    server.use(http.post(`${repo}/pulls/1/requested_reviewers`, utils.mockResponse(201, {}, observe)))
+
+    await blunderbuss(prEvent('opened'), first)
+
+    await observe.called()
+    const body = await observe.body() as { reviewers: string[] }
+    // alice and rita cover both files and fill the count together; were every
+    // candidate in one tier, the first-picking rng would draw alice and bob
+    expect([...body.reviewers].sort()).toEqual(['alice', 'rita'])
+    expect(setFailed).not.toHaveBeenCalled()
+  })
+
+  it('draws from the one-file tier only once the two-file tier is exhausted', async () => {
+    serveConfig('blunderbuss:\n  request_count: 3\n')
+    servePr(['sdk/x.go', 'docs/y.md'])
+    const observe = new utils.ObserveRequest()
+    server.use(http.post(`${repo}/pulls/1/requested_reviewers`, utils.mockResponse(201, {}, observe)))
+
+    await blunderbuss(prEvent('opened'), first)
+
+    await observe.called()
+    const body = await observe.body() as { reviewers: string[] }
+    expect([...body.reviewers].sort()).toEqual(['alice', 'bob', 'rita'])
+    expect(setFailed).not.toHaveBeenCalled()
+  })
+})

__tests__/plugins/requireMatchingLabelUnlabeled.test.ts

@@ -0,0 +1,96 @@
+import type { RequireMatchingLabel } from '../../src/utils/config'
+import { Buffer } from 'node:buffer'
+import * as core from '@actions/core'
+import { http, HttpResponse } from 'msw'
+import { setupServer } from 'msw/node'
+import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { requireMatchingLabel } from '../../src/plugins/requireMatchingLabel'
+import issuesLabeledEvent from '../fixtures/issues/issuesLabeledEvent.json'
+import labelFileContents from '../fixtures/labels/labelFileContentsResp.json'
+import * as utils from '../testUtils'
+
+const server = setupServer(utils.noMergeQueue)
+utils.failTestOnUnhandledRequests(server)
+beforeAll(() => server.listen(utils.failOnUnhandledRequest))
+afterEach(() => server.resetHandlers())
+afterAll(() => server.close())
+
+const repo = `${utils.api}/repos/Codertocat/Hello-World`
+const project = 'Codertocat/.project:prow.yaml'
+
+const kindRule: RequireMatchingLabel = { regexp: '^kind/', missing_label: 'needs-kind', missing_comment: 'Please add a kind label.' }
+const areaRule: RequireMatchingLabel = { regexp: '^area/', missing_label: 'needs-area', missing_comment: 'Please add an area label.' }
+
+function serveRules(rules: RequireMatchingLabel[]) {
+  const file = structuredClone(labelFileContents)
+  file.content = Buffer.from(`require_matching_label:\n${rules.map(rule => `  - ${JSON.stringify(rule)}`).join('\n')}\n`).toString('base64')
+  server.use(
+    http.get(utils.contentsUrl(project), utils.mockResponse(200, file)),
+    ...utils.noOrgOrRepoConfigExcept(project),
+  )
+}
+
+function issueEvent(action: string, labels: string[], changed: string) {
+  const payload = structuredClone(issuesLabeledEvent)
+  payload.action = action
+  payload.issue.labels = labels.map(name => ({ ...payload.issue.labels[0], name }))
+  payload.label = { ...payload.label, name: changed }
+  return payload
+}
+
+// serves the issue's labels and records every label and comment write
+function serveIssue(labels: string[]) {
+  const added: string[][] = []
+  const commented: string[] = []
+  server.use(
+    http.get(`${repo}/issues/1`, utils.mockResponse(200, { labels: labels.map(name => ({ name })) })),
+    utils.repoHasLabels(['needs-kind', 'needs-area']),
+    http.post(`${repo}/issues/1/labels`, async ({ request }) => {
+      added.push(((await request.json()) as { labels: string[] }).labels)
+      return HttpResponse.json([])
+    }),
+    http.get(`${repo}/issues/1/comments`, utils.mockResponse(200, [])),
+    http.post(`${repo}/issues/1/comments`, async ({ request }) => {
+      commented.push(((await request.json()) as { body: string }).body)
+      return HttpResponse.json({}, { status: 201 })
+    }),
+  )
+  return { added, commented }
+}
+
+// `labeled` and `unlabeled` both narrow the rules to the changed label; the arm
+// for `unlabeled` has no test of its own, so an issue that misses every rule
+// pins that removing a kind/* label re-evaluates the kind rule and nothing else.
+describe('requireMatchingLabel on an unlabeled event', () => {
+  let setFailed: ReturnType<typeof vi.spyOn>
+
+  beforeEach(() => {
+    utils.setupActionsEnv()
+    setFailed = vi.spyOn(core, 'setFailed').mockImplementation(() => {})
+    vi.spyOn(core, 'debug').mockImplementation(() => {})
+  })
+
+  it('applies only the rule the removed label matched, leaving the other rule untouched', async () => {
+    serveRules([kindRule, areaRule])
+    const writes = serveIssue([])
+
+    await requireMatchingLabel(new utils.MockContext(issueEvent('unlabeled', [], 'kind/bug')))
+
+    expect(writes.added).toEqual([['needs-kind']])
+    expect(writes.commented).toHaveLength(1)
+    expect(writes.commented[0]).toContain('Please add a kind label.')
+    expect(setFailed).not.toHaveBeenCalled()
+  })
+
+  it('labeled narrows the same way: adding area/docs satisfies the area rule and never evaluates the kind rule', async () => {
+    serveRules([kindRule, areaRule])
+    const writes = serveIssue(['area/docs'])
+
+    await requireMatchingLabel(new utils.MockContext(issueEvent('labeled', ['area/docs'], 'area/docs')))
+
+    expect(writes.added).toEqual([])
+    expect(writes.commented).toEqual([])
+    expect(setFailed).not.toHaveBeenCalled()
+  })
+})