test: wait for provisioning before driving the poller - #40
Merged
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #38.
Diagnosis
The CI failure was:
The condition and its timestamp were unchanged. That narrows it to one path:
pollOnceskipped the entry entirely. Had it run, it would have either flipped toContainerExited(container present, statestopped) or toProvisioningFailed(container absent) — both writee.cond. Nothing else could leaveAtuntouched.pollOnceskips one.deleting || !e.provisionDone(). And provisioning finishes after the condition is set:So
BackendReadyis always observable beforee.donecloses.waitForCondition(reasonBackendReady)therefore does not imply the entry is out of flight. Tests are the only caller drivingpollOncedirectly —StartPolleris called frommain.goonly, 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, apublishSandbox, apublishPlatformEvent), which is why it takes a loaded CI runner under-raceto hit. I could not reproduce it locally: 300 probe runs, plus 60 runs of the real test underGOMAXPROCS=1 -race, all passed. The diagnosis is from the failure signature and the code structure, not from a local repro — worth stating plainly.That
pollOnceskips in-flight entries is not incidental;TestPollerSkipsInFlightProvisioningasserts it deliberately.Fix
waitForProvisioningwaits on the entry's provisioning task, and the two tests that poll after a create now use it.The second one matters independently:
TestPollerKeepsProvisioningFailureasserts the condition staysProvisioningFailedafter 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
donebefore publishing would fix the window but let a racing delete begin cleanup while provisioning is still writing conditions (deleteSandboxwaits 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, andmake lint— all clean.