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

New Issue

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.

clawbot referenced this issue

2026-09-21 09:19:47 +02:00

Review toward 1.0: identify and file all next steps #311

clawbot commented

2026-10-02 17:22:27 +02:00

Plan. Check the test against next first. TestProcessRetryTask_LargeBody_FetchFromDB asserts the body received at the sink equals the stored body byte for byte, not only that the delivery ended delivered. Deleting the event-body fetch on the retry path must fail it. Do the same for any sibling on the retry path that asserts only a status where it should assert the payload (TestProcessRetryTask_SuccessfulRetry first), and say which were changed. Test change only.

Model: opus-5-5

Plan. Check the test against `next` first. `TestProcessRetryTask_LargeBody_FetchFromDB` asserts the body received at the sink equals the stored body byte for byte, not only that the delivery ended `delivered`. Deleting the event-body fetch on the retry path must fail it. Do the same for any sibling on the retry path that asserts only a status where it should assert the payload (`TestProcessRetryTask_SuccessfulRetry` first), and say which were changed. Test change only. Model: opus-5-5

clawbot referenced this issue from a commit

2026-10-02 17:24:33 +02:00

Assert the body a retry delivers, not only its status (closes #294)

clawbot referenced a pull request that will close this issue

2026-10-02 17:33:18 +02:00

Assert the body a retry delivers, not only its status (closes #294) #450

clawbot commented

2026-10-02 17:33:28 +02:00

#450 makes TestProcessRetryTask_LargeBody_FetchFromDB and TestProcessRetryTask_SuccessfulRetry compare the body the target received with the stored event body byte for byte. Before, they checked only that the delivery ended delivered. With the event-body fetch in processRetryTask deleted, both tests fail. With the fetch restored, both pass.

Judgement call: the other retry-path tests (the two deleted-target tests and the retry-queue lifecycle test) are not about the body, so I left them alone.

Model: opus-5-5

https://git.eeqj.de/sneak/webhooker/pulls/450 makes `TestProcessRetryTask_LargeBody_FetchFromDB` and `TestProcessRetryTask_SuccessfulRetry` compare the body the target received with the stored event body byte for byte. Before, they checked only that the delivery ended `delivered`. With the event-body fetch in `processRetryTask` deleted, both tests fail. With the fetch restored, both pass. Judgement call: the other retry-path tests (the two deleted-target tests and the retry-queue lifecycle test) are not about the body, so I left them alone. Model: opus-5-5

clawbot closed this issue

2026-10-02 18:03:20 +02:00

clawbot referenced this issue from a commit

2026-10-02 18:03:22 +02:00

Assert the body a retry delivers, not only its status (closes #294)

clawbot referenced this issue

2026-10-02 18:03:32 +02:00

Milestone: next into main #380

Sign in to join this conversation.