diff --git a/__tests__/bundle/bundle.test.ts b/__tests__/bundle/bundle.test.ts index 3eb2e635..3145a4ef 100644 --- a/__tests__/bundle/bundle.test.ts +++ b/__tests__/bundle/bundle.test.ts @@ -5,7 +5,6 @@ import fs from 'node:fs' import process from 'node:process' import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' -import issueCommentEvent from '../fixtures/issues/issueCommentEvent.json' import issuesLabeledEvent from '../fixtures/issues/issuesLabeledEvent.json' import labelFileContents from '../fixtures/labels/labelFileContentsResp.json' import checkSuiteCompletedEvent from '../fixtures/pullReq/checkSuiteCompletedEvent.json' @@ -14,22 +13,14 @@ import pullReqOpenedEvent from '../fixtures/pullReq/pullReqOpenedEvent.json' import pullReqReviewSubmittedEvent from '../fixtures/pullReq/pullReqReviewSubmittedEvent.json' import { blobSha, prCommentEvent, pullBody } from '../utils/ownersFixtures' import { start } from './fakeGithub' +import { comment, configReads, helpersFor, membershipReads, ownersProbe, ownersReads, queueRead, repo, token } from './helpers' import { bundlePath, runBundle } from './runBundle' vi.setConfig({ testTimeout: 30_000 }) -const repo = '/repos/Codertocat/Hello-World' -const token = { 'github-token': 'some-token' } // the read every label command makes before it applies a label const labelsRead = `GET ${repo}/labels?per_page=100` -function comment(body: string, author = issueCommentEvent.issue.user.login) { - const payload = structuredClone(issueCommentEvent) - payload.comment.body = body - payload.issue.user.login = author - return payload -} - function openPr(labels: string[], overrides: Record = {}) { const pr = structuredClone(pullReqListPulls[0]) return { ...pr, labels: labels.map(name => ({ name })), ...overrides } @@ -45,31 +36,9 @@ function yamlFile(text: string) { return file } -const orgConfigRepos = ['.project', '.github'] -const repoConfigFiles = [ - '.github/prow.yaml', - '.github/prowlabels.yaml', - 'prow.yaml', - '.prowlabels.yaml', - '.github/prow.yml', - '.github/prowlabels.yml', - 'prow.yml', - '.prowlabels.yml', -] - -// the configuration reads the loader makes before it finds `org` and `repo` (or gives up on a tier) -function configReads({ org, repo: file }: { org?: string, repo?: string } = {}): string[] { - const orgReads = orgConfigRepos - .slice(0, org ? orgConfigRepos.indexOf(org) + 1 : orgConfigRepos.length) - .map(name => `GET /repos/Codertocat/${name}/contents/prow.yaml`) - const repoReads = repoConfigFiles - .slice(0, file ? repoConfigFiles.indexOf(file) + 1 : repoConfigFiles.length) - .map(path => `GET ${repo}/contents/${encodeURIComponent(path)}`) - return [...orgReads, ...repoReads] -} - describe('dist/index.js', () => { let gh: FakeGithub + const { expectCommandThenConfig, expectRequests, routeOwners } = helpersFor(() => gh) beforeAll(async () => { gh = await start() @@ -79,62 +48,6 @@ describe('dist/index.js', () => { afterEach(() => gh.reset()) afterAll(() => gh.close()) - // the org and repo tiers are probed concurrently, so the reads have no fixed order among themselves - function expectRequests(reads: string[], rest: string[]) { - const calls = gh.requests.map(r => `${r.method} ${r.path}`) - expect(calls.slice(0, reads.length).sort()).toEqual([...reads].sort()) - expect(calls.slice(reads.length)).toEqual(rest) - } - - // a label command that never reads the configuration, then the sweep that follows it: the configuration - // reads (unordered) for the needs-* re-check and, on a pull request, tide's calls - function expectCommandThenSweep(command: string[], sweep: string[] = []) { - const calls = gh.requests.map(r => `${r.method} ${r.path}`) - expect(calls.slice(0, command.length)).toEqual(command) - const reads = configReads() - expect(calls.slice(command.length, command.length + reads.length).sort()).toEqual([...reads].sort()) - expect(calls.slice(command.length + reads.length)).toEqual(sweep) - } - - // the pull request, its changed files, the tip of its base branch and the OWNERS files there, as the OWNERS plugins read them - function routeOwners(ownersFiles: Record, files: string[], pull: Record = {}) { - gh.route('GET', `${repo}/pulls/1`, { status: 200, body: { ...pullBody, user: { login: 'Codertocat' }, requested_reviewers: [], assignees: [], ...pull } }) - gh.route('GET', `${repo}/pulls/1/files`, { - status: 200, - body: files.map(filename => ({ filename, status: 'modified' })), - }) - gh.route('GET', `${repo}/branches/master`, { status: 200, body: { name: 'master', commit: { sha: 'basesha' } } }) - gh.route('GET', `${repo}/git/trees/basesha`, { - status: 200, - body: { - sha: 'basesha', - truncated: false, - tree: Object.keys(ownersFiles).map(path => ({ path, type: 'blob', sha: blobSha(path) })), - }, - }) - for (const [path, contents] of Object.entries(ownersFiles)) { - gh.route('GET', `${repo}/git/blobs/${blobSha(path)}`, { - status: 200, - body: { encoding: 'base64', content: Buffer.from(contents).toString('base64') }, - }) - } - } - - const ownersReads = [ - `GET ${repo}/pulls/1`, - `GET ${repo}/pulls/1/files?per_page=100`, - `GET ${repo}/branches/master`, - `GET ${repo}/git/trees/basesha?recursive=true`, - ] - - // the tide gate learns whether the pull request's base branch (master in every fixture) has OWNERS files from - // its tree, once per branch per run, after the pull request read that names the branch; the fake answers 404 - // (an empty repository) unless a test routes it - const ownersProbe = `GET ${repo}/git/trees/master?recursive=true` - // tide asks GraphQL once whether the base branch requires a merge queue: once the gate passes, or when it - // fails on an event (to dequeue the bot's own entry); the fake answers "no queue" unless a test routes it - const queueRead = 'POST /graphql' - it('is a syntactically valid bundle with no unresolved modules', () => { expect(fs.existsSync(bundlePath)).toBe(true) @@ -459,7 +372,7 @@ describe('dist/index.js', () => { const posts = gh.requestsMatching('POST', /\/issues\/1\/labels$/) expect(posts).toHaveLength(1) expect(posts[0].body).toEqual({ labels: ['help wanted'] }) - expectCommandThenSweep([labelsRead, `POST ${repo}/issues/1/labels`]) + expectCommandThenConfig([labelsRead, `POST ${repo}/issues/1/labels`]) }) it('issue_comment /assign self-assigns an org member', async () => { @@ -723,7 +636,7 @@ describe('dist/index.js', () => { expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/)).toEqual([]) expect(gh.requestsMatching('POST', /\/issues\/1\/comments$/)).toEqual([]) // the membership fallback checks org membership and collaborator status; the sweep then reads the configuration and the pr - expectCommandThenSweep([ + expectCommandThenConfig([ ...ownersReads, `GET /orgs/Codertocat/members/bob`, `GET ${repo}/collaborators/bob`, @@ -753,7 +666,7 @@ describe('dist/index.js', () => { gh.route('POST', `${repo}/issues/comments/492700400/reactions`, { status: 201, body: { content: 'rocket' } }) } - const authReads = [...ownersReads, `GET /orgs/Codertocat/members/Codertocat`, `GET ${repo}/collaborators/Codertocat`] + const authReads = [...ownersReads, ...membershipReads('Codertocat')] const runsRead = `GET ${repo}/actions/runs?head_sha=headsha&per_page=100` const rocket = `POST ${repo}/issues/comments/492700400/reactions` @@ -818,7 +731,7 @@ describe('dist/index.js', () => { expect(result.status, result.stdout).toBe(1) expect(result.errors).toHaveLength(1) expect(result.errors[0]).toMatch(/could not remove label foo/) - expectCommandThenSweep([ + expectCommandThenConfig([ `GET ${repo}/collaborators/Codertocat`, `GET ${repo}/issues/1`, `DELETE ${repo}/issues/1/labels/foo`, diff --git a/__tests__/bundle/collaborationCommands.test.ts b/__tests__/bundle/collaborationCommands.test.ts index c98595f1..0a326009 100644 --- a/__tests__/bundle/collaborationCommands.test.ts +++ b/__tests__/bundle/collaborationCommands.test.ts @@ -1,8 +1,8 @@ import type { FakeGithub } from './fakeGithub' import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' -import issueCommentEvent from '../fixtures/issues/issueCommentEvent.json' import { start } from './fakeGithub' +import { comment, helpersFor, membershipReads, repo, token } from './helpers' import { runBundle } from './runBundle' vi.setConfig({ testTimeout: 30_000 }) @@ -10,10 +10,9 @@ vi.setConfig({ testTimeout: 30_000 }) // the issue_comment commands that touch assignees, reviewers, title and lock state, // driven through dist/index.js like the label commands in bundle.test.ts describe('dist/index.js collaboration commands', () => { - const repo = '/repos/Codertocat/Hello-World' - const token = { 'github-token': 'some-token' } const collaboratorRead = `GET ${repo}/collaborators/Codertocat` let gh: FakeGithub + const { calls } = helpersFor(() => gh) beforeAll(async () => { gh = await start() @@ -21,17 +20,6 @@ describe('dist/index.js collaboration commands', () => { afterEach(() => gh.reset()) afterAll(() => gh.close()) - function comment(body: string, author = issueCommentEvent.issue.user.login) { - const payload = structuredClone(issueCommentEvent) - payload.comment.body = body - payload.issue.user.login = author - return payload - } - - function calls() { - return gh.requests.map(r => `${r.method} ${r.path}`) - } - async function run(body: string, command: string, author?: string) { const result = await runBundle({ eventName: 'issue_comment', @@ -43,12 +31,8 @@ describe('dist/index.js collaboration commands', () => { } // the three reads checkCommenterAuth / getOrgCollabCommentUsers make for one user, in order - function membershipReads(user: string) { - return [ - `GET /orgs/Codertocat/members/${user}`, - `GET ${repo}/collaborators/${user}`, - `GET ${repo}/issues/1/comments`, - ] + function membershipOrCommentReads(user: string) { + return [...membershipReads(user), `GET ${repo}/issues/1/comments`] } describe('/cc', () => { @@ -87,7 +71,7 @@ describe('dist/index.js collaboration commands', () => { expect(result.status, result.stdout).toBe(0) expect(result.errors).toEqual([]) // the two users are checked concurrently, so their reads interleave - expect(calls().slice(0, 6).sort()).toEqual([...membershipReads('octocat'), ...membershipReads('stranger')].sort()) + expect(calls().slice(0, 6).sort()).toEqual([...membershipOrCommentReads('octocat'), ...membershipOrCommentReads('stranger')].sort()) expect(calls().slice(6)).toEqual([`POST ${repo}/pulls/1/requested_reviewers`]) expect(gh.requestsMatching('POST', /requested_reviewers$/)[0].body).toEqual({ reviewers: ['octocat'] }) }) @@ -128,7 +112,7 @@ describe('dist/index.js collaboration commands', () => { expect(result.status, result.stdout).toBe(0) expect(result.errors).toEqual([]) - expect(calls()).toEqual([...membershipReads('Codertocat'), `DELETE ${repo}/pulls/1/requested_reviewers`]) + expect(calls()).toEqual([...membershipOrCommentReads('Codertocat'), `DELETE ${repo}/pulls/1/requested_reviewers`]) expect(gh.requestsMatching('DELETE', /requested_reviewers$/)[0].body).toEqual({ reviewers: ['octocat', 'hubot'] }) }) @@ -141,7 +125,7 @@ describe('dist/index.js collaboration commands', () => { expect(result.status, result.stdout).toBe(0) expect(result.errors).toEqual([]) - expect(calls()).toEqual(membershipReads('Codertocat')) + expect(calls()).toEqual(membershipOrCommentReads('Codertocat')) }) }) @@ -167,7 +151,7 @@ describe('dist/index.js collaboration commands', () => { expect(result.status, result.stdout).toBe(0) expect(result.errors).toEqual([]) - expect(calls()).toEqual([...membershipReads('Codertocat'), `DELETE ${repo}/issues/1/assignees`]) + expect(calls()).toEqual([...membershipOrCommentReads('Codertocat'), `DELETE ${repo}/issues/1/assignees`]) expect(gh.requestsMatching('DELETE', /assignees$/)[0].body).toEqual({ assignees: ['octocat'] }) }) diff --git a/__tests__/bundle/helpers.ts b/__tests__/bundle/helpers.ts new file mode 100644 index 00000000..78d61e38 --- /dev/null +++ b/__tests__/bundle/helpers.ts @@ -0,0 +1,116 @@ +import type { FakeGithub } from './fakeGithub' +import { Buffer } from 'node:buffer' +import { expect } from 'vitest' + +import issueCommentEvent from '../fixtures/issues/issueCommentEvent.json' +import { blobSha, pullBody } from '../utils/ownersFixtures' + +export const repo = '/repos/Codertocat/Hello-World' +export const token = { 'github-token': 'some-token' } + +// Codertocat's comment `body` on issue #1 (not a pull request), opened by `author` +export function comment(body: string, author = issueCommentEvent.issue.user.login) { + const payload = structuredClone(issueCommentEvent) + payload.comment.body = body + payload.issue.user.login = author + return payload +} + +const orgConfigRepos = ['.project', '.github'] +const repoConfigFiles = [ + '.github/prow.yaml', + '.github/prowlabels.yaml', + 'prow.yaml', + '.prowlabels.yaml', + '.github/prow.yml', + '.github/prowlabels.yml', + 'prow.yml', + '.prowlabels.yml', +] + +// the configuration reads the loader makes before it finds `org` and `repo` (or gives up on a tier) +export function configReads({ org, repo: file }: { org?: string, repo?: string } = {}): string[] { + const orgReads = orgConfigRepos + .slice(0, org ? orgConfigRepos.indexOf(org) + 1 : orgConfigRepos.length) + .map(name => `GET /repos/Codertocat/${name}/contents/prow.yaml`) + const repoReads = repoConfigFiles + .slice(0, file ? repoConfigFiles.indexOf(file) + 1 : repoConfigFiles.length) + .map(path => `GET ${repo}/contents/${encodeURIComponent(path)}`) + return [...orgReads, ...repoReads] +} + +// the OWNERS plugins' reads of what routeOwners serves, before any OWNERS blob +export const ownersReads = [ + `GET ${repo}/pulls/1`, + `GET ${repo}/pulls/1/files?per_page=100`, + `GET ${repo}/branches/master`, + `GET ${repo}/git/trees/basesha?recursive=true`, +] + +// the tide gate learns whether the pull request's base branch (master in every fixture) has OWNERS files from +// its tree, once per branch per run, after the pull request read that names the branch; the fake answers 404 +// (an empty repository) unless a test routes it +export const ownersProbe = `GET ${repo}/git/trees/master?recursive=true` +// tide asks GraphQL once whether the base branch requires a merge queue: once the gate passes, or when it +// fails on an event (to dequeue the bot's own entry); the fake answers "no queue" unless a test routes it +export const queueRead = 'POST /graphql' + +// the reads that authorize `login` when no OWNERS file does: its org membership and its collaborator status +export function membershipReads(login: string) { + return [`GET /orgs/Codertocat/members/${login}`, `GET ${repo}/collaborators/${login}`] +} + +// the helpers that route or inspect a suite's fake GitHub; the suite starts it in beforeAll, after binding these, +// so `fake` is called on each use +export function helpersFor(fake: () => FakeGithub) { + function calls() { + return fake().requests.map(r => `${r.method} ${r.path}`) + } + + // `reads` in any order, then exactly `rest`: the org and repo tiers are probed concurrently, so the reads have no + // fixed order among themselves + function expectRequests(reads: string[], rest: string[]) { + const recorded = calls() + expect(recorded.slice(0, reads.length).sort()).toEqual([...reads].sort()) + expect(recorded.slice(reads.length)).toEqual(rest) + } + + // `commandReads` in any order, then `command`, then the configuration reads in any order (for the post-command + // sweep's needs-* re-check, or for lgtm.bind_to_commit), then `tail`: on a pull request, tide's calls + function expectCommandThenConfig(command: string[], tail: string[] = [], commandReads: string[] = []) { + const recorded = calls() + expect(recorded.slice(0, commandReads.length).sort()).toEqual([...commandReads].sort()) + const afterAuth = recorded.slice(commandReads.length) + expect(afterAuth.slice(0, command.length)).toEqual(command) + const reads = configReads() + expect(afterAuth.slice(command.length, command.length + reads.length).sort()).toEqual([...reads].sort()) + expect(afterAuth.slice(command.length + reads.length)).toEqual(tail) + } + + // the pull request, its changed files, the tip of its base branch and the OWNERS files there, as the OWNERS plugins read them + function routeOwners(ownersFiles: Record, files: string[], pull: Record = {}) { + const gh = fake() + gh.route('GET', `${repo}/pulls/1`, { status: 200, body: { ...pullBody, user: { login: 'Codertocat' }, requested_reviewers: [], assignees: [], ...pull } }) + gh.route('GET', `${repo}/pulls/1/files`, { + status: 200, + body: files.map(filename => ({ filename, status: 'modified' })), + }) + gh.route('GET', `${repo}/branches/master`, { status: 200, body: { name: 'master', commit: { sha: 'basesha' } } }) + gh.route('GET', `${repo}/git/trees/basesha`, { + status: 200, + body: { + sha: 'basesha', + truncated: false, + tree: Object.keys(ownersFiles).map(path => ({ path, type: 'blob', sha: blobSha(path) })), + }, + }) + for (const [path, contents] of Object.entries(ownersFiles)) { + gh.route('GET', `${repo}/git/blobs/${blobSha(path)}`, { + status: 200, + body: { encoding: 'base64', content: Buffer.from(contents).toString('base64') }, + }) + } + } + + return { calls, expectCommandThenConfig, expectRequests, routeOwners } +} diff --git a/__tests__/bundle/triggerTestAndLgtmCancel.test.ts b/__tests__/bundle/triggerTestAndLgtmCancel.test.ts index d72918d1..513daafc 100644 --- a/__tests__/bundle/triggerTestAndLgtmCancel.test.ts +++ b/__tests__/bundle/triggerTestAndLgtmCancel.test.ts @@ -3,8 +3,9 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } import issueCommentEvent from '../fixtures/issues/issueCommentEvent.json' import pullReqOpenedEvent from '../fixtures/pullReq/pullReqOpenedEvent.json' -import { prCommentEvent, pullBody } from '../utils/ownersFixtures' +import { prCommentEvent } from '../utils/ownersFixtures' import { start } from './fakeGithub' +import { helpersFor, membershipReads, ownersProbe, ownersReads, queueRead, repo, token } from './helpers' import { runBundle } from './runBundle' vi.setConfig({ testTimeout: 30_000 }) @@ -12,12 +13,11 @@ vi.setConfig({ testTimeout: 30_000 }) // the /test trigger command and the lgtm cancel spellings (/lgtm cancel, /remove-lgtm), // driven through dist/index.js like /retest and /lgtm in bundle.test.ts describe('dist/index.js /test and lgtm cancel', () => { - const repo = '/repos/Codertocat/Hello-World' - const token = { 'github-token': 'some-token' } const commentPost = `POST ${repo}/issues/1/comments` const rocket = `POST ${repo}/issues/comments/492700400/reactions` const runsRead = `GET ${repo}/actions/runs?head_sha=headsha&per_page=100` let gh: FakeGithub + const { calls, expectCommandThenConfig, expectRequests, routeOwners } = helpersFor(() => gh) beforeAll(async () => { gh = await start() @@ -26,44 +26,23 @@ describe('dist/index.js /test and lgtm cancel', () => { afterEach(() => gh.reset()) afterAll(() => gh.close()) - function calls() { - return gh.requests.map(r => `${r.method} ${r.path}`) - } - function comments() { return gh.requestsMatching('POST', /\/issues\/1\/comments$/).map(r => (r.body as { body: string }).body) } // the pull request, its files and an OWNERS-less base tree: the commenter is then authorized by membership function routePullRequest(labels: string[] = []) { - gh.route('GET', `${repo}/pulls/1`, { status: 200, body: { ...pullBody, user: { login: 'some-author' }, requested_reviewers: [], assignees: [], labels: labels.map(name => ({ name })) } }) - gh.route('GET', `${repo}/pulls/1/files`, { status: 200, body: [{ filename: 'src/file1.txt', status: 'modified' }] }) - gh.route('GET', `${repo}/branches/master`, { status: 200, body: { name: 'master', commit: { sha: 'basesha' } } }) - gh.route('GET', `${repo}/git/trees/basesha`, { status: 200, body: { sha: 'basesha', truncated: false, tree: [] } }) + routeOwners({}, ['src/file1.txt'], { user: { login: 'some-author' }, labels: labels.map(name => ({ name })) }) } - const ownersReads = [ - `GET ${repo}/pulls/1`, - `GET ${repo}/pulls/1/files?per_page=100`, - `GET ${repo}/branches/master`, - `GET ${repo}/git/trees/basesha?recursive=true`, - ] - function routeMember(login: string, member: boolean) { gh.route('GET', `/orgs/Codertocat/members/${login}`, member ? { status: 204 } : { status: 404, body: { message: 'Not Found' } }) gh.route('GET', `${repo}/collaborators/${login}`, { status: 404, body: { message: 'Not Found' } }) } - function membershipReads(login: string) { - return [`GET /orgs/Codertocat/members/${login}`, `GET ${repo}/collaborators/${login}`] - } - // the OWNERS-less authorization of a reviewer: owners reads, then org membership and collaborator (unordered among themselves) function expectAuthorized(rest: string[], login = 'Codertocat') { - const reads = [...ownersReads, ...membershipReads(login)] - const recorded = calls() - expect(recorded.slice(0, reads.length).sort()).toEqual([...reads].sort()) - expect(recorded.slice(reads.length)).toEqual(rest) + expectRequests([...ownersReads, ...membershipReads(login)], rest) } describe('/test', () => { @@ -248,13 +227,7 @@ describe('dist/index.js /test and lgtm cancel', () => { const unbind = `POST ${repo}/statuses/headsha` const pullRead = `GET ${repo}/pulls/1` // tide's sweep: the pr (now without lgtm), whether its base has OWNERS files, and the merge queue state - const sweep = [pullRead, `GET ${repo}/git/trees/master?recursive=true`, 'POST /graphql'] - const configReads = [ - 'GET /repos/Codertocat/.project/contents/prow.yaml', - 'GET /repos/Codertocat/.github/contents/prow.yaml', - ...['.github/prow.yaml', '.github/prowlabels.yaml', 'prow.yaml', '.prowlabels.yaml', '.github/prow.yml', '.github/prowlabels.yml', 'prow.yml', '.prowlabels.yml'] - .map(path => `GET ${repo}/contents/${encodeURIComponent(path)}`), - ] + const sweep = [pullRead, ownersProbe, queueRead] // the issue read answers with `labels`; the pull request read (before and after the removal) never carries lgtm, // as tide's sweep would see it once the label is gone @@ -274,16 +247,6 @@ describe('dist/index.js /test and lgtm cancel', () => { }) } - // after the command: the configuration reads (unordered) for lgtm.bind_to_commit, then a fixed tail - function expectCommandThenConfig(command: string[], tail: string[], commandReads: string[]) { - const recorded = calls() - expect(recorded.slice(0, commandReads.length).sort()).toEqual([...commandReads].sort()) - const afterAuth = recorded.slice(commandReads.length) - expect(afterAuth.slice(0, command.length)).toEqual(command) - expect(afterAuth.slice(command.length, command.length + configReads.length).sort()).toEqual([...configReads].sort()) - expect(afterAuth.slice(command.length + configReads.length)).toEqual(tail) - } - it('/lgtm cancel by a reviewer on an lgtm\'d pr: removes the label, voids the binding with a pending status, and the sweep skips the pr', async () => { routeCancel(['lgtm']) routeMember('Codertocat', true)