Skip to content

test: wait for provisioning before driving the poller - #40

Merged
vyncint merged 1 commit into
mainfrom
fix/flaky-poller-test
Aug 8, 2026
Merged

test: wait for provisioning before driving the poller#40
vyncint merged 1 commit into
mainfrom
fix/flaky-poller-test

Conversation

@vyncint

@vyncint vyncint commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Closes #38.

Diagnosis

The CI failure was:

--- FAIL: TestPollerTracksExit (5.01s)
    reconcile_test.go:139: condition "ContainerExited" never reached;
      last: {Status:True Reason:BackendReady Message:Sandbox VM is running At:…m=+0.229956887}

The condition and its timestamp were unchanged. That narrows it to one path: pollOnce skipped the entry entirely. Had it run, it would have either flipped to ContainerExited (container present, state stopped) or to ProvisioningFailed (container absent) — both write e.cond. Nothing else could leave At untouched.

pollOnce skips on e.deleting || !e.provisionDone(). And provisioning finishes after the condition is set:

go func() {
    defer s.wg.Done()
    defer close(e.done)      // ← after provision returns
    s.provision(ctx, e, sb, token)   // ← sets BackendReady inside
}()

So BackendReady is always observable before e.done closes. waitForCondition(reasonBackendReady) therefore does not imply the entry is out of flight. Tests are the only caller driving pollOnce directly — StartPoller is called from main.go only, never in tests — so a poll landing in that window yielded no transition and the test waited out its 5 s budget.

The window is normally sub-millisecond (a setCondition, a publishSandbox, a publishPlatformEvent), which is why it takes a loaded CI runner under -race to hit. I could not reproduce it locally: 300 probe runs, plus 60 runs of the real test under GOMAXPROCS=1 -race, all passed. The diagnosis is from the failure signature and the code structure, not from a local repro — worth stating plainly.

That pollOnce skips in-flight entries is not incidental; TestPollerSkipsInFlightProvisioning asserts it deliberately.

Fix

waitForProvisioning waits on the entry's provisioning task, and the two tests that poll after a create now use it.

The second one matters independently: TestPollerKeepsProvisioningFailure asserts the condition stays ProvisioningFailed after a poll. A skipped entry leaves it intact too — so that test could pass without ever exercising the branch it claims to cover.

Why not change the product

Closing done before publishing would fix the window but let a racing delete begin cleanup while provisioning is still writing conditions (deleteSandbox waits on the same channel). In production the poller ticks every 2 s, so the next tick covers the window; nothing user-visible depends on a poll landing in it.

Verification

go test -race -run TestPoller -count=50, full suite, and make lint — all clean.

TestPollerTracksExit timed out on CI with the condition still BackendReady and
its timestamp unchanged — the signature of pollOnce having skipped the entry
rather than having computed the wrong state.

provision() sets the terminal condition and only then returns; the goroutine
closes the entry's done channel after that. So BackendReady is always
observable before the entry stops counting as in flight, and pollOnce skips
in-flight entries by design (TestPollerSkipsInFlightProvisioning asserts
exactly that). Tests are the only caller that drives pollOnce directly —
StartPoller is not running here — so a poll landing in that window produced no
transition at all and the test waited out its 5s budget.

Add waitForProvisioning and use it wherever a test polls after a create. The
same window let TestPollerKeepsProvisioningFailure pass for the wrong reason:
a skipped entry also leaves the failure condition intact.

Test-only. In production the poller ticks every 2s, so the next tick covers
the window; closing done earlier would let a racing delete start cleanup while
provisioning is still writing conditions, which is worse.

Closes #38

Signed-off-by: Vyncint Ng <vyncint@users.noreply.github.com>
@vyncint
vyncint merged commit 683cab7 into main Aug 8, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flaky: TestPollerTracksExit times out on ubuntu CI under -race

1 participant