diff --git a/__tests__/bundle/fakeGithub.ts b/__tests__/bundle/fakeGithub.ts index 7d766a75..c0915fdc 100644 --- a/__tests__/bundle/fakeGithub.ts +++ b/__tests__/bundle/fakeGithub.ts @@ -38,6 +38,23 @@ export interface MergeQueueFixture { export interface FakeGithub { url: string requests: RecordedRequest[] + /** the requests no route or fallback answered: the fake replied 404 to each of them */ + unmatched: RecordedRequest[] + /** + * declares that the scenario relies on the 404 the fake gives `method` requests to `pattern` + * (a membership check meaning "not a member", an OWNERS read meaning "no such file"), so + * `unexpectedRequests` and `assertNoUnexpectedRequests` leave them out + */ + allowUnmatched: (method: string, pattern: string | RegExp) => void + /** the unmatched requests no allowance covers; the configuration loader's probes are always allowed */ + unexpectedRequests: () => RecordedRequest[] + /** + * throws naming every unexpected request, so a scenario fails on traffic the application + * answered itself (a swallowed GraphQL error, a tolerated read failure) instead of passing on it. + * Call it from `afterEach`, with `reset` in a `finally`, so a failing scenario does not leak its + * routes into the next one. + */ + assertNoUnexpectedRequests: () => void route: (method: string, pattern: string | RegExp, response: Handler | FakeResponse) => void /** answers with one response per call in order, repeating the last one */ routeSequence: (method: string, pattern: string | RegExp, responses: FakeResponse[]) => void @@ -54,6 +71,13 @@ export interface FakeGithub { close: () => Promise } +/** + * the configuration loader's reads, `GET /repos///contents/`: every run that routes no prow configuration + * makes them and takes the defaults from their 404s, so they are never unexpected + */ +const configurationProbe = /^\/repos\/[^/]+\/[^/]+\/contents\/(?:\.github%2F)?\.?prow(?:labels)?\.ya?ml$/ + function matches(route: Route, req: RecordedRequest): boolean { if (route.method !== req.method) return false @@ -84,6 +108,8 @@ export async function start(): Promise { const routes: Route[] = [] const fallbacks: Route[] = [] const requests: RecordedRequest[] = [] + const unmatched: RecordedRequest[] = [] + const allowances: Route[] = [{ method: 'GET', pattern: configurationProbe, handler: () => ({ status: 404 }) }] const server = http.createServer(async (req, res) => { const recorded: RecordedRequest = { @@ -95,6 +121,8 @@ export async function start(): Promise { requests.push(recorded) const route = routes.find(r => matches(r, recorded)) ?? fallbacks.find(r => matches(r, recorded)) + if (route === undefined) + unmatched.push(recorded) const response: FakeResponse = route ? route.handler(recorded) : { status: 404, body: { message: 'Not Found (fake)' } } @@ -115,6 +143,20 @@ export async function start(): Promise { return { url: `http://127.0.0.1:${port}`, requests, + unmatched, + allowUnmatched(method, pattern) { + allowances.push({ method: method.toUpperCase(), pattern, handler: () => ({ status: 404 }) }) + }, + unexpectedRequests() { + return unmatched.filter(r => !allowances.some(a => matches(a, r))) + }, + assertNoUnexpectedRequests() { + const unexpected = this.unexpectedRequests() + if (unexpected.length > 0) { + const listing = unexpected.map(r => ` ${r.method} ${r.path}`).join('\n') + throw new Error(`the fake GitHub API answered ${unexpected.length} request(s) no route or allowance covers:\n${listing}`) + } + }, route(method, pattern, response) { const handler = typeof response === 'function' ? response : () => response routes.push({ method: method.toUpperCase(), pattern, handler }) @@ -143,6 +185,8 @@ export async function start(): Promise { routes.length = 0 fallbacks.length = 0 requests.length = 0 + unmatched.length = 0 + allowances.length = 1 }, close() { return new Promise((resolve, reject) => { diff --git a/__tests__/bundle/fakeGithubUnexpected.test.ts b/__tests__/bundle/fakeGithubUnexpected.test.ts new file mode 100644 index 00000000..536244e5 --- /dev/null +++ b/__tests__/bundle/fakeGithubUnexpected.test.ts @@ -0,0 +1,106 @@ +import type { FakeGithub } from './fakeGithub' +import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest' + +import { start } from './fakeGithub' +import { repo } from './helpers' + +// The fake's own contract for traffic no route answers (fakeGithub.ts: `unmatched`, `allowUnmatched`, +// `unexpectedRequests`, `assertNoUnexpectedRequests`), driven with plain fetches rather than dist/index.js: +// a scenario that forgets a route must be told so by the harness, outside whatever fallback the +// application applies to the 404, and only the probes it declares intentional are left out. +describe('fakeGithub unexpected traffic', () => { + let gh: FakeGithub + + beforeAll(async () => { + gh = await start() + }) + afterEach(() => gh.reset()) + afterAll(() => gh.close()) + + function call(method: string, path: string): Promise { + return fetch(`${gh.url}${path}`, { method }) + } + + it('answers an unrouted request 404 and records it as unmatched and unexpected', async () => { + const response = await call('GET', `${repo}/pulls/1`) + + expect(response.status).toBe(404) + expect(gh.unmatched).toEqual([{ method: 'GET', path: `${repo}/pulls/1`, body: undefined }]) + expect(gh.unexpectedRequests()).toEqual(gh.unmatched) + expect(() => gh.assertNoUnexpectedRequests()).toThrow( + `the fake GitHub API answered 1 request(s) no route or allowance covers:\n GET ${repo}/pulls/1`, + ) + }) + + it('a routed request, and one a fallback answers, are neither unmatched nor unexpected', async () => { + gh.route('GET', `${repo}/pulls/1`, { status: 200, body: {} }) + gh.mergeQueueFallback({ pullRequestId: 'PR_none', headOid: 'head', enabled: false }) + + expect((await call('GET', `${repo}/pulls/1`)).status).toBe(200) + expect((await call('POST', '/graphql')).status).toBe(200) + + expect(gh.requests).toHaveLength(2) + expect(gh.unmatched).toEqual([]) + expect(() => gh.assertNoUnexpectedRequests()).not.toThrow() + }) + + it('the configuration loader\'s probes are unmatched but never unexpected', async () => { + const probes = [ + '/repos/Codertocat/.project/contents/prow.yaml', + '/repos/Codertocat/.github/contents/prow.yaml', + `${repo}/contents/.github%2Fprow.yaml`, + `${repo}/contents/.github%2Fprowlabels.yaml`, + `${repo}/contents/prow.yaml`, + `${repo}/contents/.prowlabels.yaml`, + `${repo}/contents/.github%2Fprow.yml`, + `${repo}/contents/.github%2Fprowlabels.yml`, + `${repo}/contents/prow.yml`, + `${repo}/contents/.prowlabels.yml`, + ] + for (const path of probes) + expect((await call('GET', path)).status).toBe(404) + + expect(gh.unmatched).toHaveLength(probes.length) + expect(gh.unexpectedRequests()).toEqual([]) + expect(() => gh.assertNoUnexpectedRequests()).not.toThrow() + }) + + it('an explicitly sourced configuration read is not a loader probe', async () => { + await call('GET', '/repos/cncf/prow-config/contents/configs%2Fprow.yaml?ref=v2') + + expect(gh.unexpectedRequests().map(r => r.path)).toEqual(['/repos/cncf/prow-config/contents/configs%2Fprow.yaml?ref=v2']) + }) + + it('allowUnmatched leaves the declared requests out, by exact path or pattern, and only for the declared method', async () => { + gh.allowUnmatched('GET', `${repo}/collaborators/stranger`) + gh.allowUnmatched('get', /\/orgs\/Codertocat\/members\//) + + await call('GET', `${repo}/collaborators/stranger`) + await call('GET', `${repo}/collaborators/stranger?per_page=1`) + await call('GET', '/orgs/Codertocat/members/stranger') + await call('POST', '/orgs/Codertocat/members/stranger') + await call('POST', '/graphql') + + expect(gh.unmatched).toHaveLength(5) + expect(gh.unexpectedRequests().map(r => `${r.method} ${r.path}`)).toEqual([ + 'POST /orgs/Codertocat/members/stranger', + 'POST /graphql', + ]) + expect(() => gh.assertNoUnexpectedRequests()).toThrow( + 'answered 2 request(s) no route or allowance covers:\n POST /orgs/Codertocat/members/stranger\n POST /graphql', + ) + }) + + it('reset clears the unmatched record and the scenario\'s allowances, keeping the loader probes allowed', async () => { + gh.allowUnmatched('POST', '/graphql') + await call('POST', '/graphql') + expect(gh.unexpectedRequests()).toEqual([]) + + gh.reset() + + expect(gh.unmatched).toEqual([]) + await call('POST', '/graphql') + await call('GET', `${repo}/contents/prow.yaml`) + expect(gh.unexpectedRequests().map(r => r.path)).toEqual(['/graphql']) + }) +}) diff --git a/__tests__/bundle/ownersParseArms.test.ts b/__tests__/bundle/ownersParseArms.test.ts index 8c30393f..9f8853af 100644 --- a/__tests__/bundle/ownersParseArms.test.ts +++ b/__tests__/bundle/ownersParseArms.test.ts @@ -1,5 +1,5 @@ import type { FakeGithub } from './fakeGithub' -import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import { prCommentEvent } from '../utils/ownersData' import { start } from './fakeGithub' @@ -19,7 +19,21 @@ describe('dist/index.js OWNERS content shapes', () => { beforeAll(async () => { gh = await start() }) - afterEach(() => gh.reset()) + // the tide gate's merge queue read after `/approve` is answered "no queue" rather than left to the + // fake's 404, which queueState would swallow into a warning; the gate's OWNERS probe of master is + // expected to miss (nothing routes that tree); any other unrouted request fails the scenario + beforeEach(() => { + gh.mergeQueueFallback({ pullRequestId: 'PR_none', headOid: 'headsha', enabled: false }) + gh.allowUnmatched('GET', `${repo}/git/trees/master`) + }) + afterEach(() => { + try { + gh.assertNoUnexpectedRequests() + } + finally { + gh.reset() + } + }) afterAll(() => gh.close()) function routeApprove(owners: string) { diff --git a/__tests__/bundle/ownersProbeArms.test.ts b/__tests__/bundle/ownersProbeArms.test.ts index d1a3f3c2..9ebf40c4 100644 --- a/__tests__/bundle/ownersProbeArms.test.ts +++ b/__tests__/bundle/ownersProbeArms.test.ts @@ -1,6 +1,6 @@ import type { FakeGithub } from './fakeGithub' import { Buffer } from 'node:buffer' -import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import { blobSha, prCommentEvent, pullBody } from '../utils/ownersData' import { start } from './fakeGithub' @@ -36,7 +36,18 @@ describe('dist/index.js OWNERS probes', () => { beforeAll(async () => { gh = await start() }) - afterEach(() => gh.reset()) + // `queueRead` is answered "no queue" rather than left to the fake's 404, which queueState would swallow + // into a warning; the OWNERS probes a scenario expects to miss are allowed in that scenario, and any + // other unrouted request fails it + beforeEach(() => gh.mergeQueueFallback({ pullRequestId: 'PR_none', headOid: 'headsha', enabled: false })) + afterEach(() => { + try { + gh.assertNoUnexpectedRequests() + } + finally { + gh.reset() + } + }) afterAll(() => gh.close()) function contents(text: string) { @@ -74,6 +85,7 @@ describe('dist/index.js OWNERS probes', () => { it('root OWNERS present, sdk/OWNERS absent: bob approves from the root file; no blob is read', async () => { routeTruncatedBase() gh.route('GET', `${repo}/contents/OWNERS`, contents(rootOwners)) + gh.allowUnmatched('GET', `${repo}/contents/sdk%2FOWNERS`) routeApprove(['lgtm']) gh.route('GET', `${repo}/git/trees/master`, { status: 200, body: { sha: 'master', truncated: false, tree: [{ path: 'OWNERS', type: 'blob', sha: 'o' }] } }) @@ -99,6 +111,8 @@ describe('dist/index.js OWNERS probes', () => { gh.route('GET', `${repo}/contents/OWNERS`, contents(rootOwners)) gh.route('GET', `${repo}/contents/sdk%2FOWNERS`, { status: 500, body: { message: 'boom' } }) routeApprove() + // the tide gate still probes master's tree after the failed evaluation, and nothing routes it + gh.allowUnmatched('GET', `${repo}/git/trees/master`) const result = await runApprove() @@ -153,6 +167,7 @@ describe('dist/index.js OWNERS probes', () => { it('a truncated tree without an OWNERS blob and no root OWNERS file: the gate wants lgtm alone', async () => { routeApprove(['lgtm']) + gh.allowUnmatched('GET', `${repo}/contents/OWNERS`) gh.route('GET', `${repo}/git/trees/master`, { status: 200, body: { sha: 'master', truncated: true, tree: [{ path: 'README.md', type: 'blob', sha: 'r' }] } }) const result = await runApprove()