From 3862e2b5f107252a12a9c8b984038dfa590032d7 Mon Sep 17 00:00:00 2001 From: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Date: Sun, 8 Nov 2026 18:52:52 -0500 Subject: [PATCH] test(plugins): pin that a non-422 review-request failure is not retried per reviewer and that an authorless marker comment is skipped A Stryker probe of src/utils/pullRequestOwners.ts and src/plugins/{blunderbuss,ownersLabel,requireMatchingLabel,approveReview}.ts (722 mutants, 657 killed, 2 timed out, 63 survived) left two behavioural survivors that no assertion noticed: - blunderbuss.ts:218/:247: with `!isUnprocessable(e)` forced false, or `isUnprocessable` forced true, a 500 from POST requested_reviewers is treated like the 422 "one login spoiled the batch" case: every picked reviewer is requested again on their own, each failure is warned about, and the same error is thrown at the end. The existing test asserted only the thrown message. It now records the POST bodies and expects exactly the batch request and no warning. - requireMatchingLabel.ts:263: `comment.user?.type` and `comment.user?.login` with the optional chaining dropped. GitHub answers `user: null` for a comment whose author deleted their account; under the mutant, botCommentsWithMarker throws a TypeError, which apply() catches and the handler fails. The "other app bot" test now lists such a comment next to the human and app-bot copies of the marker and expects only the app bot's comment deleted. Each mutant applied by hand fails exactly its new assertion and nothing else; with src/ restored both files pass (44 and 54 tests). Tests only. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> --- __tests__/plugins/blunderbuss.test.ts | 14 +++++++++++--- __tests__/plugins/requireMatchingLabel.test.ts | 8 +++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/__tests__/plugins/blunderbuss.test.ts b/__tests__/plugins/blunderbuss.test.ts index 2372ddff..2995f845 100644 --- a/__tests__/plugins/blunderbuss.test.ts +++ b/__tests__/plugins/blunderbuss.test.ts @@ -386,12 +386,20 @@ describe('blunderbuss handler', () => { await expect(observePull.notCalled()).resolves.toBe('not called') }) - it('fails when the review request is refused', async () => { + it('fails when the review request is refused, without retrying each reviewer alone', async () => { serveConfig() servePr(owners, ['sdk/x.go']) - observeRequestReviewers(500) + const calls: string[][] = [] + server.use(http.post(`${repo}/pulls/1/requested_reviewers`, async ({ request }) => { + calls.push((await request.json() as { reviewers: string[] }).reviewers) + return new Response(JSON.stringify({ message: 'boom' }), { status: 500, headers: { 'Content-Type': 'application/json' } }) + })) + const warning = vi.spyOn(core, 'warning').mockImplementation(() => {}) - await expect(blunderbuss(prEvent('opened'))).rejects.toThrow(/could not request reviewers: .*boom/) + await expect(blunderbuss(prEvent('opened'), first)).rejects.toThrow(/could not request reviewers: .*boom/) + // only a 422 means one login spoiled the batch; any other status is reported as is + expect(calls).toEqual([['alice', 'bob']]) + expect(warning).not.toHaveBeenCalled() }) it('when GitHub refuses the batch with a 422, requests each reviewer alone and keeps the ones it accepts', async () => { diff --git a/__tests__/plugins/requireMatchingLabel.test.ts b/__tests__/plugins/requireMatchingLabel.test.ts index a52e2216..8ebf2702 100644 --- a/__tests__/plugins/requireMatchingLabel.test.ts +++ b/__tests__/plugins/requireMatchingLabel.test.ts @@ -73,7 +73,7 @@ interface Writes { deleteComment: utils.ObserveRequest } -function serveIssue(labels: string[], comments: { id: number, body: string, user: { login: string, type: string } }[] = []): Writes { +function serveIssue(labels: string[], comments: { id: number, body: string, user: { login: string, type: string } | null }[] = []): Writes { const writes: Writes = { addLabels: new utils.ObserveRequest(), removeLabel: new utils.ObserveRequest(), @@ -116,6 +116,8 @@ const botComment = { id: 11, body: `Please add a kind label.\n\n${marker}`, user const humanComment = { id: 12, body: `I pasted the marker by hand ${marker}`, user: { login: 'Codertocat', type: 'User' } } // another GitHub App's bot: `type: 'Bot'` without the github-actions login const appBotComment = { id: 13, body: `Please add a kind label.\n\n${marker}`, user: { login: 'other-app[bot]', type: 'Bot' } } +// a comment whose author deleted their account: GitHub answers `user: null` +const ghostComment = { id: 14, body: `Please add a kind label.\n\n${marker}`, user: null } describe('evaluate', () => { it.each([ @@ -264,9 +266,9 @@ describe('requireMatchingLabel handler', () => { expect(setFailed).not.toHaveBeenCalled() }) - it('deletes the marker comment of another app bot but not the human copy of the marker', async () => { + it('deletes the marker comment of another app bot but not the human or authorless copies of the marker', async () => { serveRules([kindRule]) - serveIssue(['kind/bug', 'needs-kind'], [humanComment, appBotComment]) + serveIssue(['kind/bug', 'needs-kind'], [humanComment, appBotComment, ghostComment]) const deleted: string[] = [] server.use(http.delete(`${repo}/issues/comments/:id`, ({ params }) => { deleted.push(String(params.id))