notify shutdown tests hang until the 90-second timeout instead of failing when a drain returns early #176

New Issue

2026-10-01T20:26:07+02:00

clawbot commented

2026-10-01 20:26:07 +02:00

Found while working on #116.

In internal/notify/shutdown_test.go, TestDrainWaitsForInFlightDelivery and TestNewRegistersDrainingStopHook start a test webhook server whose handler blocks until the test closes release, and close it from a timer. The deferred timer.Stop() runs before the deferred srv.Close(), so if the drain returns before the in-flight delivery finishes (the defect these tests exist to catch), the timer is stopped, release is never closed, srv.Close() waits for the blocked handler, and the whole package hangs until the 90-second test timeout instead of failing.

Separately, the watchdog comment in TestDrainBoundedByContextDeadline names a 30-second timeout; script/test sets 90 seconds.

Definition of done

In both tests, the handler is released however the test ends, before the server is closed. A drain that returns early makes the test fail within a few seconds instead of hanging; shown by breaking the drain by hand, then reverting.
The watchdog comment names the real timeout, or no number.
No timing bound is loosened. Test-only change. DNS is never mocked.

Model: opus-5-5

Found while working on https://git.eeqj.de/sneak/dnswatcher/issues/116. In `internal/notify/shutdown_test.go`, `TestDrainWaitsForInFlightDelivery` and `TestNewRegistersDrainingStopHook` start a test webhook server whose handler blocks until the test closes `release`, and close it from a timer. The deferred `timer.Stop()` runs before the deferred `srv.Close()`, so if the drain returns before the in-flight delivery finishes (the defect these tests exist to catch), the timer is stopped, `release` is never closed, `srv.Close()` waits for the blocked handler, and the whole package hangs until the 90-second test timeout instead of failing. Separately, the watchdog comment in `TestDrainBoundedByContextDeadline` names a 30-second timeout; `script/test` sets 90 seconds. ## Definition of done - In both tests, the handler is released however the test ends, before the server is closed. A drain that returns early makes the test fail within a few seconds instead of hanging; shown by breaking the drain by hand, then reverting. - The watchdog comment names the real timeout, or no number. - No timing bound is loosened. Test-only change. DNS is never mocked. Model: opus-5-5

clawbot added this to the 1.0 milestone 2026-10-01 20:26:07 +02:00

~~clawbot referenced this issue 2026-10-01 20:30:33 +02:00~~

docs: bring TODO.md up to date (closes #146) #171

clawbot referenced this issue

2026-10-01 20:30:33 +02:00

docs: bring TODO.md up to date (closes #146) #171

clawbot referenced this issue

2026-10-01 20:50:05 +02:00

docs: bring TODO.md up to date (closes #146) #171

clawbot referenced this issue from a commit

2026-10-01 21:36:20 +02:00

notify: release held deliveries so shutdown tests fail, not hang (closes #176)

clawbot referenced a pull request that will close this issue

2026-10-01 21:36:37 +02:00

notify: release held deliveries so shutdown tests fail, not hang (closes #176) #179

clawbot commented

2026-10-01 21:36:42 +02:00

Implemented in #179: both tests now release the held delivery however they end, before the test server is closed, and the watchdog comment names no timeout number.

Model: opus-5-5

Implemented in https://git.eeqj.de/sneak/dnswatcher/pulls/179: both tests now release the held delivery however they end, before the test server is closed, and the watchdog comment names no timeout number. Model: opus-5-5

clawbot referenced this issue from a commit

2026-10-01 21:58:35 +02:00

notify: release held deliveries so shutdown tests fail, not hang (closes #176)

clawbot closed this issue

2026-10-01 21:58:59 +02:00

clawbot referenced this issue from a commit

2026-10-01 21:59:00 +02:00

notify: release held deliveries so shutdown tests fail, not hang (closes #176)

Sign in to join this conversation.