fix(ui): keep the start-handler test from booting a real Canton stack - #322
Merged
Conversation
TestStart_StoppedNoContainers202 drove the handler's bring-up branch against real Docker. startBringUp launches RunUp on a detached context.Background() goroutine, so the test asserted 202 and returned while the goroutine went on to pull images and start a full Canton LocalNet under the compose project canton-pebble. Those containers outlived the test. On the next run containers.List found them, the handler took the fast-start branch instead, and the test failed with 204 rather than 202 -- permanently, on any host that had run it once. It only ever passed on a clean machine, which is why CI stayed green and local runs did not. Route the three Docker-backed calls in the start path through package level seams, matching the existing collectDoctor/captureCantonPorts convention in this package, and stub them in the test. The test now also asserts that the bring-up actually ran with the seeded version and profiles, so the 202 is pinned to the right branch rather than to a status code that both branches could produce.
zheli
force-pushed
the
fix/start-handler-hermetic-test
branch
from
August 19, 2026 15:58
17b8b13 to
3cc3cb3
Compare
zheli
added a commit
that referenced
this pull request
Aug 21, 2026
handleCreate, handleResumeInstance and realRecreateWork called localnet.RunUp directly on a detached context, so any test reaching them booted a real Canton stack that outlived the run. The stranded containers then made later runs take the fast-start branch and return 204/500 instead of 202 — the failure that looked "pre-existing" on a developer machine while CI stayed green on clean runners. Route those call sites through the package-level seam #322 already introduced, and default runUp to a no-op in TestMain so the property holds for tests added later instead of one call site at a time. This matches the existing runPreflightForVersion stub, which is process-wide for the same reason. TestCancelUp_HappyPath and TestCreate_DuplicateNameReturns409 silently depended on the real RunUp being slow enough to keep a job in-flight. They now install a blocking stub, which also removes the timing flake that made the latter fail under -race.
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.
Fixes the pre-existing
TestStart_StoppedNoContainers202failure that PR #321 inherited.Root cause
TestStart_StoppedNoContainers202drove the start handler's bring-up branch against real Docker.startBringUplaunchesRunUpon a detachedcontext.Background()goroutine, so the test asserted 202 and returned while the goroutine went on to pull images and start a full Canton LocalNet under the compose projectcanton-pebble. The leaked containers were still running hours later, tagged with the Go test temp dir:Those containers outlive the test. On the next run
containers.Listfinds them, the handler takes the fast-start branch instead, and the test fails withstatus = 204, want 202— permanently, on any host that had run it once.The test only ever passed on a clean machine. That is why CI stayed green (fresh runner per job) while local runs failed, and why it looked like an unrelated pre-existing failure.
Fix
Route the three Docker-backed calls in the start path (
containers.List,RunStart,RunUp) through package-level seams, matching the existingcollectDoctor/captureCantonPorts/coreServicesForconvention already used in this package, and stub them in the test.The test additionally now asserts the bring-up actually ran with the seeded version and profiles, so the 202 is pinned to the correct branch rather than to a status code both branches could plausibly produce. The assertion was strengthened, not weakened.
Verification
go test ./internal/ui/handlers/ -run TestStart_ -count=3— passes repeatedly while the leakedcanton-pebblecontainers are still present on the host, which is the proof it is now hermeticgo test -race ./internal/ui/handlers/— cleango test ./...— full suite green, no remaining failuresmake lint— 0 issues