Skip to content

fix(ui): keep the start-handler test from booting a real Canton stack - #322

Merged
zheli merged 1 commit into
mainfrom
fix/start-handler-hermetic-test
Aug 19, 2026
Merged

fix(ui): keep the start-handler test from booting a real Canton stack#322
zheli merged 1 commit into
mainfrom
fix/start-handler-hermetic-test

Conversation

@zheli

@zheli zheli commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Fixes the pre-existing TestStart_StoppedNoContainers202 failure that PR #321 inherited.

Root cause

TestStart_StoppedNoContainers202 drove the start 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. The leaked containers were still running hours later, tagged with the Go test temp dir:

com.docker.compose.project=canton-pebble
config_files=.../T/TestStart_StoppedNoContainers202.../pebble/containers.yaml
Name: pebble-canton, pebble-postgres, pebble-scan-web-ui, ...

Those containers outlive the test. On the next run containers.List finds them, the handler takes the fast-start branch instead, and the test fails with status = 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 existing collectDoctor / captureCantonPorts / coreServicesFor convention 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 leaked canton-pebble containers are still present on the host, which is the proof it is now hermetic
  • go test -race ./internal/ui/handlers/ — clean
  • go test ./... — full suite green, no remaining failures
  • make lint — 0 issues

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
zheli force-pushed the fix/start-handler-hermetic-test branch from 17b8b13 to 3cc3cb3 Compare August 19, 2026 15:58
@zheli
zheli merged commit caee168 into main Aug 19, 2026
25 checks passed
@zheli
zheli deleted the fix/start-handler-hermetic-test branch August 19, 2026 16:06
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.
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.

1 participant