diff --git a/__tests__/plugins/approveReviewWithdrawalOrder.test.ts b/__tests__/plugins/approveReviewWithdrawalOrder.test.ts new file mode 100644 index 00000000..f785329c --- /dev/null +++ b/__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)') + }) +}) diff --git a/__tests__/plugins/blunderbussTierPreference.test.ts b/__tests__/plugins/blunderbussTierPreference.test.ts new file mode 100644 index 00000000..50e6c95d --- /dev/null +++ b/__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 + + 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() + }) +}) diff --git a/__tests__/plugins/requireMatchingLabelUnlabeled.test.ts b/__tests__/plugins/requireMatchingLabelUnlabeled.test.ts new file mode 100644 index 00000000..dc81c678 --- /dev/null +++ b/__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 + + 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() + }) +})