From 0d5f427ec18a6342d0021ffd3227d025e7af3d2a Mon Sep 17 00:00:00 2001 From: quality Date: Wed, 7 Oct 2026 11:44:41 -0400 Subject: [PATCH 1/2] test(bundle): drive the OWNERS filters-ignored arm (owners.ts parseOwners) through dist/index.js __tests__/bundle/ownersFilters.test.ts serves one OWNERS blob with a top-level approver and a filters section naming another, and drives /approve through the bundle: the top-level approver still gets approved and the debug log notes the ignored filters; the filter-only login is refused with no label write. Closes #364 Signed-off-by: quality --- __tests__/bundle/ownersFilters.test.ts | 75 ++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 __tests__/bundle/ownersFilters.test.ts diff --git a/__tests__/bundle/ownersFilters.test.ts b/__tests__/bundle/ownersFilters.test.ts new file mode 100644 index 00000000..dd1394f5 --- /dev/null +++ b/__tests__/bundle/ownersFilters.test.ts @@ -0,0 +1,75 @@ +import type { FakeGithub } from './fakeGithub' +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' + +import { prCommentEvent } from '../utils/ownersFixtures' +import { start } from './fakeGithub' +import { helpersFor, repo, token } from './helpers' +import { runBundle } from './runBundle' + +vi.setConfig({ testTimeout: 30_000 }) + +// An OWNERS `filters` section (owners.ts parseOwners) is accepted but ignored, as +// docs/prow-for-your-org.md and docs/commands.md state: the top-level roles alone +// authorize, and the debug log notes the ignored key. Driven through dist/index.js +// with `/approve` on a pull request, like the OWNERS suites in bundle.test.ts. +describe('dist/index.js OWNERS filters are ignored', () => { + // alice approves at the top level; bob only inside a filter, which Prow-style + // filters would honour but this action does not + const ownersFiles: Record = { + OWNERS: 'approvers:\n- alice\nfilters:\n ".*":\n approvers:\n - bob\n', + } + const ignored = 'OWNERS at OWNERS: filters are not supported; ignoring' + let gh: FakeGithub + const { routeOwners } = helpersFor(() => gh) + + beforeAll(async () => { + gh = await start() + }) + afterEach(() => gh.reset()) + afterAll(() => gh.close()) + + function routeApprove(comments: unknown[] = []) { + routeOwners(ownersFiles, ['src/file1.txt']) + gh.route('GET', `${repo}/issues/1/comments`, { status: 200, body: comments }) + gh.route('GET', `${repo}/pulls/1/reviews`, { status: 200, body: [] }) + gh.route('GET', `${repo}/labels`, { status: 200, body: [{ name: 'approved' }, { name: 'lgtm' }] }) + gh.route('POST', `${repo}/issues/1/labels`, { status: 200, body: [] }) + gh.route('POST', `${repo}/issues/1/comments`, { status: 201, body: {} }) + } + + function runApprove(commenter: string) { + return runBundle({ + eventName: 'issue_comment', + payload: prCommentEvent('/approve', commenter), + inputs: { ...token, 'prow-commands': '/approve' }, + apiUrl: gh.url, + }) + } + + it('a top-level approver still approves, and the ignored filters are debug-logged', async () => { + routeApprove([{ id: 1, body: '/approve', user: { login: 'alice', type: 'User' }, created_at: '2024-01-01T00:00:01Z' }]) + + const result = await runApprove('alice') + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + expect(result.stdout).toContain(ignored) + expect(result.stdout).toContain('approve: #1 is approved by alice') + expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/).map(r => r.body)).toEqual([{ labels: ['approved'] }]) + }) + + it('an approver listed only inside a filter is refused: the filter grants nothing', async () => { + routeApprove() + + const result = await runApprove('bob') + + const wantErr = 'bob is not an approver for any changed file' + expect(result.status, result.stdout).toBe(1) + expect(result.stdout).toContain(ignored) + expect(result.errors.some(e => e.includes(wantErr))).toBe(true) + expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/)).toEqual([]) + const comments = gh.requestsMatching('POST', /\/issues\/1\/comments$/) + expect(comments).toHaveLength(1) + expect(comments[0].body).toEqual({ body: `Cannot approve the pull request: Error: ${wantErr}` }) + }) +}) From 845a7a3e65a13f8f9ba0b4fa51ba3ebbee929dd4 Mon Sep 17 00:00:00 2001 From: quality Date: Wed, 7 Oct 2026 22:14:22 -0400 Subject: [PATCH 2/2] test(bundle): drop the filter-only approver refusal case Signed-off-by: quality --- __tests__/bundle/ownersFilters.test.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/__tests__/bundle/ownersFilters.test.ts b/__tests__/bundle/ownersFilters.test.ts index dd1394f5..f42d9cbd 100644 --- a/__tests__/bundle/ownersFilters.test.ts +++ b/__tests__/bundle/ownersFilters.test.ts @@ -57,19 +57,4 @@ describe('dist/index.js OWNERS filters are ignored', () => { expect(result.stdout).toContain('approve: #1 is approved by alice') expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/).map(r => r.body)).toEqual([{ labels: ['approved'] }]) }) - - it('an approver listed only inside a filter is refused: the filter grants nothing', async () => { - routeApprove() - - const result = await runApprove('bob') - - const wantErr = 'bob is not an approver for any changed file' - expect(result.status, result.stdout).toBe(1) - expect(result.stdout).toContain(ignored) - expect(result.errors.some(e => e.includes(wantErr))).toBe(true) - expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/)).toEqual([]) - const comments = gh.requestsMatching('POST', /\/issues\/1\/comments$/) - expect(comments).toHaveLength(1) - expect(comments[0].body).toEqual({ body: `Cannot approve the pull request: Error: ${wantErr}` }) - }) })