The archive writer decided whether to close and reopen its file by reading the real clock, so the debounce test had to sleep past a real 2 second window, and correct code went red if the host stalled that long between the test's two quick writes (#190).
The writer now reads the time for its reopen debounce from a now field, set to time.Now when the writer is built; nothing in production changes it. The test replaces it with a clock it moves by hand: two writes at the same instant open the file once, and a write after the clock has moved by the full debounce reopens it once. No sleep remains, and every other test in internal/delivery keeps the real clock.
What the field does not cover: the archived time stamped on each row and the expiry prune still read the real clock, since neither is part of the debounce.
Removing the debounce check from the writer was tried, and this test fails with it removed.
Deviation: the issue asks for that failure to be pasted; it is not, because the tracker rules forbid pasting test output.
Judgement call: the test moves the clock by exactly the debounce, so it also holds that a write landing on the window's edge reopens.
Model: opus-5-5
The archive writer decided whether to close and reopen its file by reading the real clock, so the debounce test had to sleep past a real 2 second window, and correct code went red if the host stalled that long between the test's two quick writes (https://git.eeqj.de/sneak/webhooker/issues/190).
The writer now reads the time for its reopen debounce from a `now` field, set to `time.Now` when the writer is built; nothing in production changes it. The test replaces it with a clock it moves by hand: two writes at the same instant open the file once, and a write after the clock has moved by the full debounce reopens it once. No sleep remains, and every other test in `internal/delivery` keeps the real clock.
What the field does not cover: the archived time stamped on each row and the expiry prune still read the real clock, since neither is part of the debounce.
Removing the debounce check from the writer was tried, and this test fails with it removed.
- Deviation: the issue asks for that failure to be pasted; it is not, because the tracker rules forbid pasting test output.
- Judgement call: the test moves the clock by exactly the debounce, so it also holds that a write landing on the window's edge reopens.
Model: opus-5-5
internal/delivery/target_database_test.go line 194, the comment in TestArchiveWriter_ReopenDebounce says "The writer reads the time from this clock". That is not true of the tree: on every write the writer still stamps the row's archived time from the real clock, and the expiry prune reads the real clock too (the PR description says so itself). Only the reopen debounce reads the test's clock. Acceptable: limit the sentence to the debounce, for example "The writer measures its reopen debounce on this clock, which only the test moves, so how long the host takes between writes cannot change the result."
Judgement call: moving the clock by exactly the debounce, so that a write on the window's edge must reopen, matches the writer's greater-than-or-equal comparison (unchanged by this PR) and should stay.
Judgement call: not pasting the mutation failure, against the wording of #190, is accepted because the tracker rules forbid pasted test output.
Model: opus-5-5
Review: needs rework.
1. `internal/delivery/target_database_test.go` line 194, the comment in `TestArchiveWriter_ReopenDebounce` says "The writer reads the time from this clock". That is not true of the tree: on every write the writer still stamps the row's archived time from the real clock, and the expiry prune reads the real clock too (the PR description says so itself). Only the reopen debounce reads the test's clock. Acceptable: limit the sentence to the debounce, for example "The writer measures its reopen debounce on this clock, which only the test moves, so how long the host takes between writes cannot change the result."
- Judgement call: moving the clock by exactly the debounce, so that a write on the window's edge must reopen, matches the writer's greater-than-or-equal comparison (unchanged by this PR) and should stay.
- Judgement call: not pasting the mutation failure, against the wording of https://git.eeqj.de/sneak/webhooker/issues/190, is accepted because the tracker rules forbid pasted test output.
Model: opus-5-5
The archive writer now reads the time its reopen debounce is
measured on from a clock field, time.Now outside tests. The
debounce test moves that clock instead of sleeping past a real
2 second window, so a slow host between the two quick writes can
no longer turn a correct result red.
Model: opus-5-5
Narrowed the comment in TestArchiveWriter_ReopenDebounce to the reopen debounce, in the wording the review gave; nothing else changed, and the branch is rebased onto next.
Model: opus-5-5
Narrowed the comment in `TestArchiveWriter_ReopenDebounce` to the reopen debounce, in the wording the review gave; nothing else changed, and the branch is rebased onto `next`.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The archive writer decided whether to close and reopen its file by reading the real clock, so the debounce test had to sleep past a real 2 second window, and correct code went red if the host stalled that long between the test's two quick writes (#190).
The writer now reads the time for its reopen debounce from a
nowfield, set totime.Nowwhen the writer is built; nothing in production changes it. The test replaces it with a clock it moves by hand: two writes at the same instant open the file once, and a write after the clock has moved by the full debounce reopens it once. No sleep remains, and every other test ininternal/deliverykeeps the real clock.What the field does not cover: the archived time stamped on each row and the expiry prune still read the real clock, since neither is part of the debounce.
Removing the debounce check from the writer was tried, and this test fails with it removed.
Model: opus-5-5
Review: needs rework.
internal/delivery/target_database_test.goline 194, the comment inTestArchiveWriter_ReopenDebouncesays "The writer reads the time from this clock". That is not true of the tree: on every write the writer still stamps the row's archived time from the real clock, and the expiry prune reads the real clock too (the PR description says so itself). Only the reopen debounce reads the test's clock. Acceptable: limit the sentence to the debounce, for example "The writer measures its reopen debounce on this clock, which only the test moves, so how long the host takes between writes cannot change the result."Model: opus-5-5
d17a687dbatoddbe8cd8dfNarrowed the comment in
TestArchiveWriter_ReopenDebounceto the reopen debounce, in the wording the review gave; nothing else changed, and the branch is rebased ontonext.Model: opus-5-5
Review passed.
Model: opus-5-5