amadan

carlos / platform Public

Make the non-blocking dispatch tests fail fast instead of hanging

Paul Campbell pushed by paul@keymail.dev 90e89eed4c2d3ae85cde118707466223c72a3d66 parent a562ec3
2 files changed, +86 −32
  • internal/host/restart_test.go +39 −13
  • internal/host/sidecarwake_test.go +47 −19
diff --git a/internal/host/restart_test.go b/internal/host/restart_test.go
index 2991818ea..627691b6a 100644
--- a/internal/host/restart_test.go
+++ b/internal/host/restart_test.go
@@ -813,17 +813,32 @@ func TestCycleWorkersReturnsBeforeASlowWakeCompletes(t *testing.T) {
bounce := func(h string) bool { return true }
wake := func(rt registry.Route) error { <-slow; return nil }
- start := time.Now()
- n := cycleWorkers(routes, moved, bounce, wake)
- elapsed := time.Since(start)
- close(slow) // release the parked wake goroutine so the test can exit cleanly
+ // Run under a deadline rather than measuring elapsed time on the test's
+ // own goroutine (fix round 3): under a REGRESSION cycleWorkers itself
+ // never returns, so a synchronous call plus an elapsed-time check never
+ // fails at all — it hangs until Go's 10-minute default test timeout,
+ // which is exactly what forced fix round 2's mutation check to run
+ // under an external `timeout` to see anything fail. Wrapping the call
+ // in its own goroutine and racing it against time.After (the same
+ // shape TestDispatchSidecarWakesBoundsConcurrency already uses) makes a
+ // regression here fail cleanly and promptly, in this test alone.
+ var n int
+ done := make(chan struct{})
+ go func() {
+ n = cycleWorkers(routes, moved, bounce, wake)
+ close(done)
+ }()
+ select {
+ case <-done:
+ close(slow) // release the parked wake goroutine so the test can exit cleanly
+ case <-time.After(time.Second):
+ close(slow)
+ t.Fatal("cycleWorkers did not return within 1s with a wake blocked indefinitely — the wake must be dispatched asynchronously, not awaited")
+ }
if n != 1 {
t.Fatalf("cycleWorkers = %d, want 1 (the bounce itself is synchronous)", n)
}
- if elapsed > time.Second {
- t.Fatalf("cycleWorkers took %s to return with a wake blocked indefinitely — the wake must be dispatched asynchronously, not awaited", elapsed)
- }
}
// TestCycleWorkersWakesEveryBouncedSidecarEventually: dispatching wakes
@@ -968,15 +983,26 @@ func TestCycleWorkersReturnsPromptlyWithManySlowWakesQueued(t *testing.T) {
return nil
}
- start := time.Now()
- got := cycleWorkers(routes, moved, bounce, wake)
- elapsed := time.Since(start)
+ // Same deadline-vs-elapsed-time correction as
+ // TestCycleWorkersReturnsBeforeASlowWakeCompletes (fix round 3): a
+ // synchronous call on the test's own goroutine never fails under a
+ // regression, it hangs — this is the test fix round 2's own mutation
+ // check needed an external `timeout` to see fail at all.
+ var got int
+ done := make(chan struct{})
+ go func() {
+ got = cycleWorkers(routes, moved, bounce, wake)
+ close(done)
+ }()
+ select {
+ case <-done:
+ case <-time.After(time.Second):
+ close(slow)
+ t.Fatalf("cycleWorkers did not return within 1s with %d wakes blocked (bound %d) — the whole bounded fan-out must run off the caller's goroutine, not just each individual wake", n, workerBootConcurrency)
+ }
if got != n {
t.Fatalf("cycleWorkers = %d, want %d (every route bounced)", got, n)
}
- if elapsed > time.Second {
- t.Fatalf("cycleWorkers took %s to return with %d wakes blocked (bound %d) — the whole bounded fan-out must run off the caller's goroutine, not just each individual wake", elapsed, n, workerBootConcurrency)
- }
close(slow)
waitForCond(t, "every queued sidecar to be woken eventually", func() bool {
diff --git a/internal/host/sidecarwake_test.go b/internal/host/sidecarwake_test.go
index bbedae5d5..3c082d2cc 100644
--- a/internal/host/sidecarwake_test.go
+++ b/internal/host/sidecarwake_test.go
@@ -205,13 +205,24 @@ func TestDispatchSidecarWakesReturnsBeforeASlowWakeCompletes(t *testing.T) {
slow := make(chan struct{})
wake := func(rt registry.Route) error { <-slow; return nil }
- start := time.Now()
- dispatchSidecarWakes(moved, wake)
- elapsed := time.Since(start)
- close(slow) // release the parked wake goroutine so the test can exit cleanly
-
- if elapsed > time.Second {
- t.Fatalf("dispatchSidecarWakes took %s to return with a wake blocked indefinitely — wakes must be dispatched asynchronously, not awaited", elapsed)
+ // Run under a deadline rather than measuring elapsed time on the test's
+ // own goroutine (fix round 3): under a REGRESSION dispatchSidecarWakes
+ // itself never returns, so a synchronous call plus an elapsed-time
+ // check never fails — it hangs until Go's 10-minute default test
+ // timeout. Racing a goroutine against time.After (the same shape
+ // TestDispatchSidecarWakesBoundsConcurrency already uses) makes a
+ // regression here fail cleanly and promptly, in this test alone.
+ done := make(chan struct{})
+ go func() {
+ dispatchSidecarWakes(moved, wake)
+ close(done)
+ }()
+ select {
+ case <-done:
+ close(slow) // release the parked wake goroutine so the test can exit cleanly
+ case <-time.After(time.Second):
+ close(slow)
+ t.Fatal("dispatchSidecarWakes did not return within 1s with a wake blocked indefinitely — wakes must be dispatched asynchronously, not awaited")
}
}
@@ -370,17 +381,27 @@ func TestSidecarWakeLegReturnsBeforeASlowWakeCompletes(t *testing.T) {
slow := make(chan struct{})
wake := func(rt registry.Route) error { <-slow; return nil }
- start := time.Now()
- moved, _ := sidecarWakeLeg(ctx, routes, s, cache, wake)
- elapsed := time.Since(start)
- close(slow)
+ // Deadline-vs-elapsed-time correction, same as
+ // TestDispatchSidecarWakesReturnsBeforeASlowWakeCompletes (fix round 3):
+ // a synchronous call on the test's own goroutine never fails under a
+ // regression, it hangs.
+ var moved []registry.Route
+ done := make(chan struct{})
+ go func() {
+ moved, _ = sidecarWakeLeg(ctx, routes, s, cache, wake)
+ close(done)
+ }()
+ select {
+ case <-done:
+ close(slow)
+ case <-time.After(time.Second):
+ close(slow)
+ t.Fatal("sidecarWakeLeg did not return within 1s with a wake blocked indefinitely — the wake must be dispatched asynchronously")
+ }
if len(moved) != 1 {
t.Fatalf("moved = %v, want exactly one (detection is synchronous)", moved)
}
- if elapsed > time.Second {
- t.Fatalf("sidecarWakeLeg took %s to return with a wake blocked indefinitely — the wake must be dispatched asynchronously", elapsed)
- }
}
// ---- wired into fastTick: rides the same serial move as the restart leg --
@@ -457,11 +478,18 @@ func TestDispatchSidecarWakesReturnsPromptlyWithManySlowWakesQueued(t *testing.T
return nil
}
- start := time.Now()
- dispatchSidecarWakes(moved, wake)
- elapsed := time.Since(start)
- if elapsed > time.Second {
- t.Fatalf("dispatchSidecarWakes took %s to return with %d wakes blocked (bound %d) — the whole bounded fan-out must run off the caller's goroutine", elapsed, n, workerBootConcurrency)
+ // Same deadline-vs-elapsed-time correction as
+ // TestDispatchSidecarWakesReturnsBeforeASlowWakeCompletes (fix round 3).
+ done := make(chan struct{})
+ go func() {
+ dispatchSidecarWakes(moved, wake)
+ close(done)
+ }()
+ select {
+ case <-done:
+ case <-time.After(time.Second):
+ close(slow)
+ t.Fatalf("dispatchSidecarWakes did not return within 1s with %d wakes blocked (bound %d) — the whole bounded fan-out must run off the caller's goroutine", n, workerBootConcurrency)
}
close(slow)