The drain tests measured elapsed time from an instant captured after
the clock they compared it against had already started, so the lower
bounds were structurally unreachable and passed only when the gap
between the two statements rounded to zero. TestDrainBoundedByContext-
Deadline failed the Docker gate outright (49.9ms against its own 50ms
deadline) and roughly 1 run in 12 locally.
- TestDrainBoundedByContextDeadline: capture start before
context.WithTimeout, so the measured interval is a superset of the
deadline interval and only an early return can fail the lower bound.
The upper bound moves to a watchdog around the drain, which turns an
unbounded drain into a prompt failure instead of a package-timeout
hang.
- TestDrainWaitsForInFlightDelivery: same ordering fix, ahead of the
timer that releases the held delivery.
- TestDrainWithoutDeliveriesReturnsImmediately: its 50ms ceiling was
under the observed cost of the goroutine hop through inFlight.Wait()
on a loaded box (57ms), and failed once in 20 runs. It now bounds the
idle drain at 500ms, still well under the 2s deadline a stalled drain
would hit.
- drain: an OnStop context already expired on entry with nothing
outstanding logged a WARN about abandoning deliveries with
abandoned=0 and closed the abandon channel for no reason. The timeout
branch now reports at debug level when the outstanding count is zero,
and warns only when deliveries genuinely are abandoned.
TestDrainWithCancelledContextDoesNotWarn covers it.
Verified: script/cibuild passes; 25 consecutive cache-bypassed
make test runs under -race, all clean; make check green at 5.2s.
Both corrected assertions were confirmed non-vacuous by temporarily
breaking drain and watching them fail.
notify.New accepted an fx.Lifecycle and never used it, so the three
dispatch goroutines were untracked. context.WithoutCancel kept a
delivery alive past its caller's cancellation but made nothing wait
for it: the process could exit while a delivery was still in its
retry backoff (up to five attempts, 60s max delay), silently losing
exactly the alert most worth keeping.
Deliveries are now tracked in a sync.WaitGroup whose counter is
incremented on the dispatching goroutine before the worker starts,
and notify.New registers an OnStop hook that drains them. The drain
is bounded by the context fx passes to OnStop; when it expires with
work outstanding, the count is logged at warn level and parked retry
backoffs are released via an abandon channel so they stop retrying
rather than outliving the drain. Deliveries submitted after the
drain has begun are refused and logged, so a stream of new
notifications cannot extend shutdown indefinitely.
The three near-identical dispatchers now share one tracked dispatch
helper. Tests use httptest servers and the existing retry knobs
(SetRetryConfig/SetSleepFunc) so nothing waits on a real backoff.
README's shutdown claim is reworded to match the bounded semantics.
