diff --git a/CHANGELOG.md b/CHANGELOG.md index 55f787e..c5f46a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Fixed + +- Fixed a flaky `TestPollerTracksExit` (#38). `provision` sets the sandbox's terminal condition + and only then returns, and the entry's `done` channel is closed after that — so a test that + waited for `BackendReady` and immediately drove `pollOnce` could land in the window where the + entry still counted as in flight, which `pollOnce` skips by design. No transition happened and + the test timed out. The affected tests now wait for the provisioning task itself; the same + window also let `TestPollerKeepsProvisioningFailure` pass for the wrong reason. Test-only — + in production the next 2 s poll covers the window. + ## [0.2.9] - 2026-08-08 ### Fixed diff --git a/internal/grpcsvc/create_test.go b/internal/grpcsvc/create_test.go index 568242a..1189877 100644 --- a/internal/grpcsvc/create_test.go +++ b/internal/grpcsvc/create_test.go @@ -83,6 +83,38 @@ func createRequest() *computev1.CreateSandboxRequest { } } +// waitForProvisioning blocks until the test sandbox's provisioning task has +// finished. +// +// Reaching a terminal condition is NOT enough to know that: provision() sets +// the condition and only then returns, and the goroutine closes the entry's +// done channel after that. So there is a window in which BackendReady (or +// ProvisioningFailed) is already observable while the entry still counts as +// in flight — and pollOnce skips in-flight entries by design. A test that +// drives pollOnce inside that window sees no transition at all. +// +// Any test that calls srv.pollOnce after a create must wait here first, unless +// it is deliberately polling mid-provision like +// TestPollerSkipsInFlightProvisioning. +func waitForProvisioning(t *testing.T, srv *Server) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for { + srv.mu.Lock() + e, ok := srv.sandboxes[testSandboxID] + srv.mu.Unlock() + // done is set once at create and never reassigned, so reading it + // off the entry outside the lock is safe. + if ok && e.provisionDone() { + return + } + if time.Now().After(deadline) { + t.Fatal("provisioning task did not finish") + } + time.Sleep(5 * time.Millisecond) + } +} + // waitForCondition polls until the test sandbox's condition reason matches. func waitForCondition(t *testing.T, srv *Server, reason string) condition { t.Helper() diff --git a/internal/grpcsvc/reconcile_test.go b/internal/grpcsvc/reconcile_test.go index d54a544..e670a87 100644 --- a/internal/grpcsvc/reconcile_test.go +++ b/internal/grpcsvc/reconcile_test.go @@ -125,6 +125,9 @@ func TestPollerTracksExit(t *testing.T) { t.Fatal(err) } waitForCondition(t, srv, reasonBackendReady) + // The poll below only does anything once the entry is no longer in + // flight, and BackendReady becomes visible before that. + waitForProvisioning(t, srv) // Subscribe before the transition to capture the Warning event. subID, events := srv.hub.subscribe() @@ -220,6 +223,9 @@ func TestPollerKeepsProvisioningFailure(t *testing.T) { t.Fatal(err) } waitForCondition(t, srv, reasonProvisioningFailed) + // Without this the poll can skip the entry as still in flight, and the + // assertion below would hold for the wrong reason. + waitForProvisioning(t, srv) // The VM never existed; polling must keep the original failure. srv.pollOnce(context.Background())