test(bundle): record the fake GitHub API's unmatched requests and assert on them outside the application

__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<void>
 }
 
+/**
+ * the configuration loader's reads, `GET /repos/<owner>/<repo>/contents/<prow.yaml | prow.yml |
+ * .prowlabels.yaml | ... | .github%2Fprowlabels.yml>`: 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<FakeGithub> {
   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<FakeGithub> {
     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<FakeGithub> {
   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<FakeGithub> {
       routes.length = 0
       fallbacks.length = 0
       requests.length = 0
+      unmatched.length = 0
+      allowances.length = 1
     },
     close() {
       return new Promise((resolve, reject) => {

__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<Response> {
+    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'])
+  })
+})

__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) {

__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()