diff --git a/TODO.md b/TODO.md index 614432b..e0c114a 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,8 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104 # 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: `script/install-precommit` asks git for the repository's git directory, so `make hooks` also works where `.git` is a file (closes #129). - 2026-10-01: `TODO.md` brought up to date: open issues listed by URL, every @@ -103,8 +105,6 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104 - `goimports` in `make fmt-check`, Markdown formatting: https://git.eeqj.de/sneak/dnswatcher/issues/119 - 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 sections required by policy: https://git.eeqj.de/sneak/dnswatcher/issues/173 diff --git a/internal/notify/shutdown_test.go b/internal/notify/shutdown_test.go index 224741d..afc1048 100644 --- a/internal/notify/shutdown_test.go +++ b/internal/notify/shutdown_test.go @@ -143,6 +143,12 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) { srv := blockingNtfyServer(entered, release, &served) 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) svc := notify.NewTestService(http.DefaultTransport) @@ -167,9 +173,7 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) { // delay alone. start := time.Now() - timer := time.AfterFunc(inFlightHold, func() { - close(release) - }) + timer := time.AfterFunc(inFlightHold, releaseHandler) defer timer.Stop() ctx, cancel := context.WithTimeout( @@ -268,7 +272,7 @@ func TestDrainBoundedByContextDeadline(t *testing.T) { // all never returns here (the delivery is parked in a backoff // that never fires), so an unbounded drain must fail this // test promptly instead of hanging the package until the test - // binary's 30s timeout. + // binary's -timeout. returned := make(chan struct{}) go func() { @@ -447,6 +451,11 @@ func TestNewRegistersDrainingStopHook(t *testing.T) { srv := blockingNtfyServer(entered, release, &served) 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{} svc := newNotifyService(t, lifecycle, srv.URL) @@ -472,9 +481,7 @@ func TestNewRegistersDrainingStopHook(t *testing.T) { t.Fatal("delivery never reached the endpoint") } - timer := time.AfterFunc(inFlightHold, func() { - close(release) - }) + timer := time.AfterFunc(inFlightHold, releaseHandler) defer timer.Stop() ctx, cancel := context.WithTimeout(