there is a race condition here, which is that above we wait for notifications to be sent to santa, but sending to santa happensbefore sending on thesuccess andfailure channels. So, triggering stop and the final bulk update might miss one or more success/failure updates and cause this test to flake.

Quartz can help here. The manager's bulk update and the notifier's processing loop depend on Tickers. If you convert them to use aquartz.TickerFunc, then you can control the mock clock and trigger the processing loop and bulk update, and wait for them to finish.

This lets us get rid of theEventually that checks for dispatched notifications (Quartz allows you to wait until the tick processing is done, so there is no race in just asserting santa got what we expected).

It also means we can get rid of theEventually below for the same reason --- we can wait for bulk processing to complete before asserting theinterceptor's state.

I've got a follow-up item to implement Quartz in a subsequent PR.

Addressed in613e074

nit: that needs to be published in our Coder docs

Good call, will add as a follow-up.

This is still racy. What you want is

select {    case <-ctx.Done():        return ctx.Err()    case failure <- newFailedDispatch(n.id, msg.ID, err, retryable):}

My understanding is thatdefault will only be called if there are no cases which can proceed.
https://go.dev/ref/spec#Select_statements

Execution of a "select" statement proceeds in several steps:
...
2. If one or more of the communications can proceed, a single one that can proceed is chosen via a uniform pseudo-random selection. Otherwise, if there is a default case, that case is chosen. ...

Am I misunderstanding something?

It's true thatdefault only executes if it is the only case that can succeed, but then writing tofailure is a TOCTOU race.