From b04bdb43f4e72f49e47a5b612ccfd2e1322d1154 Mon Sep 17 00:00:00 2001 From: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Date: Mon, 19 Oct 2026 08:04:59 -0400 Subject: [PATCH] test(approve): pin any-Bot comment and user-token review skips, the require_self_approval continue, approver ordering, label casing and the isBot notifier match Ten single-operator mutants of src/plugins/approve.ts survived the full 'vitest related' set although the file is 100% line-covered: the humanLogin and approvalEvents guards (:118, :144), the require_self_approval continue (:172), both approver sorts (:191, :213), syncLabel's case fold (:396) and all three operators of isBot (:430). Each of the ten new assertions fails under exactly those mutants and passes on main. Fixes #521 Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> --- __tests__/plugins/approve.test.ts | 40 +++++++++++++++++++++++++ __tests__/plugins/approveEvents.test.ts | 31 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/__tests__/plugins/approve.test.ts b/__tests__/plugins/approve.test.ts index 0a5cc6d2..a891fdb1 100644 --- a/__tests__/plugins/approve.test.ts +++ b/__tests__/plugins/approve.test.ts @@ -98,6 +98,27 @@ describe('computeApproval', () => { expect(computeApproval(owners, [at('zed', 'approve')], settings).approved).toBe(true) }) + it('require_self_approval: skipping the author\'s own event does not stop the later events from counting', () => { + const owners = pullOwners({ OWNERS: 'approvers:\n- alice\n- zed\n' }, ['src/a.go'], 'alice') + const settings = { ...defaults, require_self_approval: true } + + const state = computeApproval(owners, [at('alice', 'approve'), at('zed', 'approve')], settings) + expect(state.approved).toBe(true) + expect([...state.approvers]).toEqual(['zed']) + }) + + it('lists the approvers of the pull request and of each file alphabetically, whatever order they approved in', () => { + const owners = pullOwners({ OWNERS: 'approvers:\n- alice\n- bob\n' }, ['src/a.go']) + + const state = computeApproval(owners, [at('bob', 'approve'), at('alice', 'approve')], defaults) + expect([...state.approvers]).toEqual(['alice', 'bob']) + expect([...state.coveredFiles]).toEqual([['src/a.go', ['alice', 'bob']]]) + + // the first changed file is covered by bob alone, the second by alice alone + const split = pullOwners({ 'a/OWNERS': 'approvers:\n- bob\n', 'b/OWNERS': 'approvers:\n- alice\n' }, ['a/x.go', 'b/y.go']) + expect([...computeApproval(split, [at('bob', 'approve'), at('alice', 'approve')], defaults).approvers]).toEqual(['alice', 'bob']) + }) + it('the latest of /approve and /approve cancel per user wins', () => { const owners = pullOwners({ OWNERS: rootOwners }, ['src/a.go']) @@ -241,6 +262,13 @@ describe('approvalEvents', () => { ], [])).toEqual([]) }) + it('skips a comment by any Bot user, not only github-actions[bot], and one whose login is empty', () => { + expect(approvalEvents([ + { id: 1, body: '/approve', user: user('renovate[bot]', 'Bot'), created_at: '2024-01-01T00:00:01Z' }, + { id: 2, body: '/approve', user: user('', 'User'), created_at: '2024-01-01T00:00:02Z' }, + ], [])).toEqual([]) + }) + it('turns APPROVED and CHANGES_REQUESTED reviews by humans into events and ignores the rest', () => { const events = approvalEvents([], [ { id: 1, state: 'APPROVED', user: user('Alice'), submitted_at: '2024-01-01T00:00:01Z' }, @@ -257,6 +285,18 @@ describe('approvalEvents', () => { ]) }) + it('with a user token, a bot\'s APPROVED review and a pending review are still skipped, not mistaken for the token\'s own', () => { + const events = approvalEvents([], [ + { id: 1, state: 'APPROVED', user: user('github-actions[bot]', 'Bot'), submitted_at: '2024-01-01T00:00:01Z' }, + { id: 2, state: 'APPROVED', user: user('alice'), submitted_at: null }, + { id: 3, state: 'APPROVED', user: user('bob'), submitted_at: '2024-01-01T00:00:03Z' }, + ], 'token-user') + + expect(events).toEqual([ + { user: 'bob', kind: 'review-approved', at: new Date('2024-01-01T00:00:03Z') }, + ]) + }) + it('a dismissed approval no longer counts while a later review still does', () => { const owners = pullOwners({ OWNERS: rootOwners }, ['src/a.go']) const events = approvalEvents([], [ diff --git a/__tests__/plugins/approveEvents.test.ts b/__tests__/plugins/approveEvents.test.ts index b4518cb2..d909fe72 100644 --- a/__tests__/plugins/approveEvents.test.ts +++ b/__tests__/plugins/approveEvents.test.ts @@ -322,6 +322,37 @@ describe('approveOnPullRequest', () => { await expect(writes.patchComment.notCalled()).resolves.toBe('not called') }) + it('a notifier posted by another App\'s bot is edited in place, not duplicated', async () => { + const stale: Comment = { id: 900, body: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**\n\nstale\n${notifierMarker}`, user: { login: 'prow-bot[bot]', type: 'Bot' } } + const writes = serve({ labels: ['approved'], comments: [stale, { body: '/approve', user: { login: 'bob' } }, { body: '/approve', user: { login: 'carol' } }] }) + + await approveOnPullRequest(prEvent('synchronize')) + + await expect(writes.patchComment.called()).resolves.toBe('called') + await expect(writes.postComment.notCalled()).resolves.toBe('not called') + }) + + it('a human quoting the notifier marker is not mistaken for the notifier', async () => { + const quote: Comment = { id: 900, body: `> ${notifierMarker}\n\nwhat is this marker?`, user: { login: 'dave', type: 'User' } } + const writes = serve({ comments: [quote, { body: '/approve', user: { login: 'bob' } }] }) + + await approveOnPullRequest(prEvent('opened')) + + await expect(writes.postComment.called()).resolves.toBe('called') + await expect(writes.patchComment.notCalled()).resolves.toBe('not called') + }) + + it('a capitalised Approved label already on the pull request is not added again', async () => { + const writes = serve({ labels: ['Approved'], comments: [{ body: '/approve', user: { login: 'bob' } }, { body: '/approve', user: { login: 'carol' } }] }) + + await approveOnPullRequest(prEvent('synchronize')) + + await expect(writes.postComment.called()).resolves.toBe('called') + expect(await commentBody(writes.postComment)).toContain('This PR is **APPROVED**') + await expect(writes.addLabels.notCalled()).resolves.toBe('not called') + expect(debug).toHaveBeenCalledWith('approve: the approved label on #1 is already correct') + }) + it('throws when the payload has no pull request', async () => { await expect(approveOnPullRequest(new utils.MockContext({ action: 'opened' }))).rejects.toThrow('missing pull request') })