fix(ui): stop handler tests from booting real Canton stacks - #325
Merged
Conversation
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.
Follow-up to #322, which sealed only the
startpath.Problem
handleCreate,handleResumeInstanceandrealRecreateWorkcalledlocalnet.RunUpdirectly on a detachedcontext.Background()goroutine. Any test reaching them booted a real Canton stack that outlived the test run.Confirmed empirically: on a clean host,
make testonmainleaks 9 containers under compose projectcanton-pebble, traced via container labels to thedevkit-handlers-tests-*tempdir.Those stranded containers then make later runs take the fast-start branch and return 204/500 instead of 202. That is the failure #322 diagnosed — it looked "pre-existing" on a developer machine while CI stayed green, because CI runners start clean.
Fix
RunUpcall sites through the package-level seam fix(ui): keep the start-handler test from booting a real Canton stack #322 already added.runUpto a no-op inTestMain, so hermeticity is structural — a test added later cannot reintroduce the leak. This mirrors the existingrunPreflightForVersionstub, which is process-wide for the same reason.varblock abovehandleStartInstanceso its godoc is no longer separated from the func (wart from fix(ui): keep the start-handler test from booting a real Canton stack #322).Two latent flakes surfaced
TestCancelUp_HappyPathandTestCreate_DuplicateNameReturns409silently depended on the realRunUpbeing slow (pulling images) to keep a job in-flight. Both now install a blocking stub that returns onctx.Done(), matching their existingjobs.Cancel+waitJobsDrainteardown.TestCreate_DuplicateNameReturns409was failing under-raceonmainfor this reason.TestResume_StoppedInstance202now asserts what its own comment said it could not ("we cannot easily assert the goroutine reaches RunUp"): that bring-up receives the recorded version, pinning the no-silent-upgrade rule.Verification
make testexit 0, container delta 0 (was +9 onmain)go test -race -count=3green; plain-count=3greenmake lint->0 issues.No user-facing behaviour change, so no
docs/changes-from-proposal.mdentry (internal indirection + tests only).