Last wall-clock-dependent test: two writes must land inside a real 2s debounce window or correct code reds #190

Open
opened 2026-08-18 03:59:56 +02:00 by clawbot · 0 comments
Collaborator

Found by the suite-wide wall-clock survey that #186 required. NOT milestoned 1.0.0 — it has never been observed failing, and it is test robustness rather than a defect an internet-facing deployment can be attacked through.

internal/delivery/target_database_test.go:203-211 performs two back-to-back writes that must both land inside the target's 2 s reopen-debounce window. If the test goroutine stalls between them for longer than that, correct code reports 2 reopens instead of 1 and the test fails. It is a real-time margin: slowness flips a correct result to a red, rather than merely making the test slower.

After #188 this is the only remaining site in the repo of that class. The survey classified every time.Sleep / time.After / Eventually / WithTimeout / sub-second constant in the suite as (A) correctness depends on the margin, (B) patience budget that only broken code exhausts, or (C) no real dependency. Everything else is (B) or (C); the two other (A)-class sites both lived in loginguard_test.go and were fixed in 188. The round-2 reviewer of 188 independently re-enumerated every wall-clock site and confirmed the survey is complete with nothing unclassified.

2 s is far looser than the 100 ms and 200 ms margins that were fixed there, which is why it was left alone. It is filed rather than fixed for the same reason it was left: no sighting, and the debounce is deliberate design rather than an accident.

Corrected 2026-08-18. As filed, this issue said four fail-green sites and placed retention_lifecycle_test.go in internal/delivery/. Both were wrong — there are five, and that file is in internal/database/. Caught by the round-2 review of #188. The error was mine.

Separately worth naming, and explicitly not part of this issue: five sleep-for-goroutine-ordering sites that fail green rather than red — a slow host makes them under-observe instead of mis-assert, so they weaken coverage without ever reddening next. internal/delivery/engine_lifecycle_test.go:29,117,230, internal/delivery/engine_test.go:777, internal/delivery/engine_test.go:1023, and internal/database/retention_lifecycle_test.go:209. A test that is too weak is a different problem from one that reds next; if you want those addressed, they should be their own issue.

Definition of done

  • internal/delivery/target_database_test.go:203-211 contains no real-time sleep and its result does not depend on how long the host takes between the two writes. Driving the debounce from the injectable clock the guard tests already use is the expected approach; if that is not reachable without a production change, say so on the issue rather than widening the window and calling it fixed.
  • The mutation still bites: removing the reopen-debounce from the production code must still fail this test. Demonstrate that by actually applying the mutation and pasting the failure — widening a window until a test stops flaking, at the cost of no longer detecting its bug, is the failure mode to avoid here. That exact trade was caught and rejected once already in #188.
  • No other test in internal/delivery acquires a new real-time margin as a side effect.
  • Verify the file paths and counts in this issue before relying on them — two were wrong on filing.

Implementation requirements

  • Branch from next, PR based on next, single commit, title ending (closes #N).
  • Do not modify TODO.md (see #112).
  • Run make bootstrap in a fresh clone before gating — browser assets are fetched at build time, and make lint needs Docker.
  • Gate on make check plus the cache-defeated Docker lint path. All linting runs in Docker, never on the host. Clean up every container and image you start; never run any prune.
  • Do not restate a measurement from this issue without re-running it.
Found by the suite-wide wall-clock survey that https://git.eeqj.de/sneak/webhooker/issues/186 required. NOT milestoned 1.0.0 — it has never been observed failing, and it is test robustness rather than a defect an internet-facing deployment can be attacked through. `internal/delivery/target_database_test.go:203-211` performs two back-to-back writes that must **both** land inside the target's 2 s reopen-debounce window. If the test goroutine stalls between them for longer than that, correct code reports 2 reopens instead of 1 and the test fails. It is a real-time margin: slowness flips a correct result to a red, rather than merely making the test slower. After https://git.eeqj.de/sneak/webhooker/pulls/188 this is the **only** remaining site in the repo of that class. The survey classified every `time.Sleep` / `time.After` / `Eventually` / `WithTimeout` / sub-second constant in the suite as (A) correctness depends on the margin, (B) patience budget that only broken code exhausts, or (C) no real dependency. Everything else is (B) or (C); the two other (A)-class sites both lived in `loginguard_test.go` and were fixed in 188. The round-2 reviewer of 188 independently re-enumerated every wall-clock site and confirmed the survey is complete with nothing unclassified. 2 s is far looser than the 100 ms and 200 ms margins that were fixed there, which is why it was left alone. It is filed rather than fixed for the same reason it was left: no sighting, and the debounce is deliberate design rather than an accident. **Corrected 2026-08-18.** As filed, this issue said *four* fail-green sites and placed `retention_lifecycle_test.go` in `internal/delivery/`. Both were wrong — there are **five**, and that file is in `internal/database/`. Caught by the round-2 review of https://git.eeqj.de/sneak/webhooker/pulls/188. The error was mine. Separately worth naming, and explicitly **not** part of this issue: five sleep-for-goroutine-ordering sites that fail *green* rather than red — a slow host makes them under-observe instead of mis-assert, so they weaken coverage without ever reddening `next`. `internal/delivery/engine_lifecycle_test.go:29,117,230`, `internal/delivery/engine_test.go:777`, `internal/delivery/engine_test.go:1023`, and `internal/database/retention_lifecycle_test.go:209`. A test that is too weak is a different problem from one that reds `next`; if you want those addressed, they should be their own issue. ## Definition of done - `internal/delivery/target_database_test.go:203-211` contains no real-time sleep and its result does not depend on how long the host takes between the two writes. Driving the debounce from the injectable clock the guard tests already use is the expected approach; if that is not reachable without a production change, say so on the issue rather than widening the window and calling it fixed. - **The mutation still bites:** removing the reopen-debounce from the production code must still fail this test. Demonstrate that by actually applying the mutation and pasting the failure — widening a window until a test stops flaking, at the cost of no longer detecting its bug, is the failure mode to avoid here. That exact trade was caught and rejected once already in https://git.eeqj.de/sneak/webhooker/pulls/188. - No other test in `internal/delivery` acquires a new real-time margin as a side effect. - **Verify the file paths and counts in this issue before relying on them** — two were wrong on filing. ## Implementation requirements - Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`. - Do not modify `TODO.md` (see https://git.eeqj.de/sneak/webhooker/issues/112). - Run `make bootstrap` in a fresh clone before gating — browser assets are fetched at build time, and `make lint` needs Docker. - Gate on `make check` plus the cache-defeated Docker lint path. All linting runs in Docker, never on the host. Clean up every container and image you start; never run any prune. - Do not restate a measurement from this issue without re-running it.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#190