From e3378bccb709cc663abd5e02411d2c3c477c3996 Mon Sep 17 00:00:00 2001 From: Bob Killen Date: Tue, 13 Oct 2026 19:57:00 -0500 Subject: [PATCH] test(bundle): drive suggestApprovers' second greedy round (approve.ts:260) through dist/index.js Every bundle scenario so far was coverable by one approver, so the notifier never named two: BRDA:260,42,1 was 0 in coverage-e2e/lcov.info. One pull_request_review scenario in approveEvaluationArms.test.ts now asserts the multi-login form end-to-end: the a/b/c no_parent_owners set cover where y beats x on the tie and z finishes, so x is never suggested. Refs #466 Co-authored-by: quality Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Bob Killen --- .../bundle/approveEvaluationArms.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/__tests__/bundle/approveEvaluationArms.test.ts b/__tests__/bundle/approveEvaluationArms.test.ts index aed57184..972b6c80 100644 --- a/__tests__/bundle/approveEvaluationArms.test.ts +++ b/__tests__/bundle/approveEvaluationArms.test.ts @@ -151,6 +151,30 @@ describe('dist/index.js approve plugin arms', () => { expect(body).toContain(`${sdk}\n- **docs/a.md** (no OWNERS file covers this file)\n- **docs/b.md** (no OWNERS file covers this file)\n`) }) + it('the greedy rounds pick the approver covering most remaining files, ties alphabetically, until every file is covered', async () => { + // every other scenario is covered by a single approver, so this is the only one that reaches + // suggestApprovers' second round + routeEvaluation(['a/1', 'b/1', 'c/1'], { + owners: { + 'a/OWNERS': 'options:\n no_parent_owners: true\napprovers: [x, y]\n', + 'b/OWNERS': 'options:\n no_parent_owners: true\napprovers: [y, z]\n', + 'c/OWNERS': 'options:\n no_parent_owners: true\napprovers: [z]\n', + }, + }) + + const result = await runReview() + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + expect(result.stdout).toContain('approve: #1 is not approved; nobody approves a/1, b/1, c/1') + const body = postedNotifier() + // y and z each cover two files; y wins the tie, then only z covers c/1, and x is never needed + expect(body).toContain('please assign **y**, **z** after the PR has been reviewed.') + expect(body).toContain('`/assign @y @z`') + expect(body).not.toContain('**x**') + expect(body).not.toContain('@x') + }) + it('a notifier already saying what the evaluation would say is neither re-posted nor edited', async () => { const comments = [{ id: 1, body: '/approve', user: { login: 'bob', type: 'User' }, created_at: '2024-01-01T00:00:01Z' }] routeEvaluation(['sdk/x.go'], { comments })