notify: release held deliveries so shutdown tests fail, not hang (closes #176)
check / check (push) Successful in 1m9s

Two shutdown tests hold a delivery inside the test server's handler and
release it from a timer. The deferred timer stop ran before the server
was closed, so a drain that returned early left the handler blocked and
the server's close waited on it until the package timed out. Each test
now defers a release, guarded so the timer and the defer can both call
it, ahead of closing the server. The watchdog comment no longer names a
30-second timeout the test script does not use.

Model: opus-5-5
This commit is contained in:
2026-10-01 19:33:14 +00:00
parent 6070356676
commit 3095b252d1
2 changed files with 16 additions and 9 deletions
+2 -2
View File
@@ -20,6 +20,8 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
# Completed Steps # Completed Steps
- 2026-10-01: two notify shutdown tests always release the delivery they hold,
so a drain that returns early fails them instead of hanging (closes #176).
- 2026-10-01: `TODO.md` brought up to date: open issues listed by URL, every - 2026-10-01: `TODO.md` brought up to date: open issues listed by URL, every
Completed Steps entry cut to at most two lines (closes #146). Completed Steps entry cut to at most two lines (closes #146).
- 2026-10-01: wildcard CORS now applies only to the public routes, not to - 2026-10-01: wildcard CORS now applies only to the public routes, not to
@@ -101,8 +103,6 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
- `goimports` in `make fmt-check`, Markdown formatting: - `goimports` in `make fmt-check`, Markdown formatting:
https://git.eeqj.de/sneak/dnswatcher/issues/119 https://git.eeqj.de/sneak/dnswatcher/issues/119
- final state save at shutdown: https://git.eeqj.de/sneak/dnswatcher/issues/114 - final state save at shutdown: https://git.eeqj.de/sneak/dnswatcher/issues/114
- `internal/notify` shutdown tests hang when a drain returns early:
https://git.eeqj.de/sneak/dnswatcher/issues/176
- README accuracy sweep: https://git.eeqj.de/sneak/dnswatcher/issues/108 - README accuracy sweep: https://git.eeqj.de/sneak/dnswatcher/issues/108
- README sections required by policy: - README sections required by policy:
https://git.eeqj.de/sneak/dnswatcher/issues/173 https://git.eeqj.de/sneak/dnswatcher/issues/173
+14 -7
View File
@@ -143,6 +143,12 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) {
srv := blockingNtfyServer(entered, release, &served) srv := blockingNtfyServer(entered, release, &served)
defer srv.Close() defer srv.Close()
// srv.Close waits for the handler, so release it however the
// test ends; otherwise a drain that returns early hangs the
// package instead of failing this test.
releaseHandler := sync.OnceFunc(func() { close(release) })
defer releaseHandler()
topicURL, _ := url.Parse(srv.URL) topicURL, _ := url.Parse(srv.URL)
svc := notify.NewTestService(http.DefaultTransport) svc := notify.NewTestService(http.DefaultTransport)
@@ -167,9 +173,7 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) {
// delay alone. // delay alone.
start := time.Now() start := time.Now()
timer := time.AfterFunc(inFlightHold, func() { timer := time.AfterFunc(inFlightHold, releaseHandler)
close(release)
})
defer timer.Stop() defer timer.Stop()
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(
@@ -268,7 +272,7 @@ func TestDrainBoundedByContextDeadline(t *testing.T) {
// all never returns here (the delivery is parked in a backoff // all never returns here (the delivery is parked in a backoff
// that never fires), so an unbounded drain must fail this // that never fires), so an unbounded drain must fail this
// test promptly instead of hanging the package until the test // test promptly instead of hanging the package until the test
// binary's 30s timeout. // binary's -timeout.
returned := make(chan struct{}) returned := make(chan struct{})
go func() { go func() {
@@ -447,6 +451,11 @@ func TestNewRegistersDrainingStopHook(t *testing.T) {
srv := blockingNtfyServer(entered, release, &served) srv := blockingNtfyServer(entered, release, &served)
defer srv.Close() defer srv.Close()
// As in TestDrainWaitsForInFlightDelivery: release the handler
// however the test ends, before srv.Close waits for it.
releaseHandler := sync.OnceFunc(func() { close(release) })
defer releaseHandler()
lifecycle := &recordingLifecycle{} lifecycle := &recordingLifecycle{}
svc := newNotifyService(t, lifecycle, srv.URL) svc := newNotifyService(t, lifecycle, srv.URL)
@@ -472,9 +481,7 @@ func TestNewRegistersDrainingStopHook(t *testing.T) {
t.Fatal("delivery never reached the endpoint") t.Fatal("delivery never reached the endpoint")
} }
timer := time.AfterFunc(inFlightHold, func() { timer := time.AfterFunc(inFlightHold, releaseHandler)
close(release)
})
defer timer.Stop() defer timer.Stop()
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(