TestProcessRetryTask_LargeBody_FetchFromDB stays green when the fetch it exists to exercise is deleted #294

New Issue

clawbot · 2026-08-24T05:13:05+02:00

clawbot commented

2026-08-24 05:13:05 +02:00

Found by mutation testing during the review of #292. Pre-existing — verified identical on unmodified next, so that PR neither introduced nor worsened it.

TestProcessRetryTask_LargeBody_FetchFromDB asserts only that the delivery ends status == delivered. It never asserts the large body actually reached the sink. So deleting the event-body fetch entirely — the one behaviour the test is named for and exists to exercise — leaves it green.

Its ProcessNewTask sibling does redden under the same mutation, which is what makes this a gap rather than a design choice: the coverage exists on one path and is absent on the other, and the test name implies otherwise.

Why it is worth recording despite predating this work: a delivery on the retry path fetching an empty or truncated body would still report delivered, and this test would still pass. The body is the entire payload of the product. #256 made the retry path's bookkeeping load-bearing in a way it was not before, so the retry path is exactly where this should be pinned.

Definition of done

The test asserts the body RECEIVED AT THE SINK matches the stored body byte-for-byte, not merely that the status is delivered.
Mutation-verify: delete the event-body fetch, confirm the test now fails, restore, confirm it passes. State that you did so.
Check the sibling assertions on the same path for the same shape — a test that asserts only a status where it should assert a payload is easy to have more than one of. TestProcessRetryTask_SuccessfulRetry is the obvious neighbour.

Not milestoned: no defect in the product, and the behaviour it fails to pin is currently correct. This is regression insurance on the path that carries the payload.

Found by mutation testing during the review of https://git.eeqj.de/sneak/webhooker/pulls/292. Pre-existing — verified identical on unmodified `next`, so that PR neither introduced nor worsened it. `TestProcessRetryTask_LargeBody_FetchFromDB` asserts only that the delivery ends `status == delivered`. It never asserts the large body actually reached the sink. So deleting the event-body fetch entirely — the one behaviour the test is named for and exists to exercise — leaves it green. Its `ProcessNewTask` sibling does redden under the same mutation, which is what makes this a gap rather than a design choice: the coverage exists on one path and is absent on the other, and the test name implies otherwise. Why it is worth recording despite predating this work: a delivery on the retry path fetching an empty or truncated body would still report `delivered`, and this test would still pass. The body is the entire payload of the product. https://git.eeqj.de/sneak/webhooker/issues/256 made the retry path's bookkeeping load-bearing in a way it was not before, so the retry path is exactly where this should be pinned. ## Definition of done - The test asserts the body RECEIVED AT THE SINK matches the stored body byte-for-byte, not merely that the status is `delivered`. - Mutation-verify: delete the event-body fetch, confirm the test now fails, restore, confirm it passes. State that you did so. - Check the sibling assertions on the same path for the same shape — a test that asserts only a status where it should assert a payload is easy to have more than one of. `TestProcessRetryTask_SuccessfulRetry` is the obvious neighbour. Not milestoned: no defect in the product, and the behaviour it fails to pin is currently correct. This is regression insurance on the path that carries the payload.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/webhooker#294