From ad78a97903f2077b47ae842d25af9fcded009efc Mon Sep 17 00:00:00 2001 From: Zhe Li Date: Wed, 19 Aug 2026 18:24:46 +0200 Subject: [PATCH] fix(ui): stop handler tests from booting real Canton stacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/ui/handlers/cancel_test.go | 10 ++++++++++ internal/ui/handlers/create_test.go | 9 +++++++++ internal/ui/handlers/instances.go | 22 ++++++++++----------- internal/ui/handlers/main_test.go | 8 ++++++++ internal/ui/handlers/resume_test.go | 30 +++++++++++++++++++++++++---- 5 files changed, 64 insertions(+), 15 deletions(-) diff --git a/internal/ui/handlers/cancel_test.go b/internal/ui/handlers/cancel_test.go index cb29ae20..b83cd692 100644 --- a/internal/ui/handlers/cancel_test.go +++ b/internal/ui/handlers/cancel_test.go @@ -2,6 +2,7 @@ package handlers import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" @@ -9,6 +10,7 @@ import ( "testing" "time" + "github.com/bitdynamics-ab/canton-devkit/internal/localnet" "github.com/bitdynamics-ab/canton-devkit/internal/ui/progress" "github.com/bitdynamics-ab/canton-devkit/internal/ui/stream" ) @@ -91,6 +93,14 @@ func TestCancelUp_HappyPath_204AndCancelledEventEmitted(t *testing.T) { mux := http.NewServeMux() MountInstances(mux, hub) + // Must block so the job is still in-flight when the DELETE lands. + origUp := runUp + t.Cleanup(func() { runUp = origUp }) + runUp = func(ctx context.Context, _ localnet.Progress, _ *localnet.UpOptions) int { + <-ctx.Done() + return localnet.ExitUserError + } + // Subscribe BEFORE the POST so we don't depend on the // replay buffer for this test (cleaner ordering check). hub.EnableBuffering(progress.TopicFor("cancelme"), 32) diff --git a/internal/ui/handlers/create_test.go b/internal/ui/handlers/create_test.go index 4b031457..80244ead 100644 --- a/internal/ui/handlers/create_test.go +++ b/internal/ui/handlers/create_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/bitdynamics-ab/canton-devkit/internal/api/types" + "github.com/bitdynamics-ab/canton-devkit/internal/localnet" "github.com/bitdynamics-ab/canton-devkit/internal/splice" "github.com/bitdynamics-ab/canton-devkit/internal/ui/progress" "github.com/bitdynamics-ab/canton-devkit/internal/ui/stream" @@ -179,6 +180,14 @@ func TestCreate_DuplicateNameReturns409(t *testing.T) { defer hub.Close() handler := handleCreate(hub) + // Must block so the first job is still in-flight for the second POST. + origUp := runUp + t.Cleanup(func() { runUp = origUp }) + runUp = func(ctx context.Context, _ localnet.Progress, _ *localnet.UpOptions) int { + <-ctx.Done() + return localnet.ExitUserError + } + req := httptest.NewRequest(http.MethodPost, "/api/instances", strings.NewReader(`{"name":"dupcreate"}`)) rec1 := httptest.NewRecorder() diff --git a/internal/ui/handlers/instances.go b/internal/ui/handlers/instances.go index 3e80ee11..2a267e41 100644 --- a/internal/ui/handlers/instances.go +++ b/internal/ui/handlers/instances.go @@ -157,6 +157,14 @@ func handleStopInstance() http.HandlerFunc { } } +// Indirected so tests can drive the bring-up paths without booting a +// real Canton stack: RunUp outlives the request on a detached context. +var ( + listContainers = containers.List + runStart = localnet.RunStart + runUp = localnet.RunUp +) + // handleStartInstance: POST /api/instances/{name}/start. // // Mirrors the CLI's intelligent `localnet start`: @@ -173,14 +181,6 @@ func handleStopInstance() http.HandlerFunc { // // The 204-vs-202 split lets the frontend branch: 204 → just refetch; // 202 → open the existing create-progress modal. -// Indirected so tests can drive the start path without booting a real -// Canton stack, which outlives the request and skews every later run. -var ( - listContainers = containers.List - runStart = localnet.RunStart - runUp = localnet.RunUp -) - func handleStartInstance(hub *stream.Hub) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := r.PathValue("name") @@ -765,7 +765,7 @@ func handleCreate(hub *stream.Hub) http.HandlerFunc { defer jobs.Unregister(req.Name) prog := progress.New(hub, req.Name) - exitCode := localnet.RunUp(jobCtx, prog, opts) + exitCode := runUp(jobCtx, prog, opts) log.Printf("create instance %q: exit_code=%d", req.Name, exitCode) }() @@ -873,7 +873,7 @@ func handleResumeInstance(hub *stream.Hub) http.HandlerFunc { defer hub.ClearBuffer(topic) defer jobs.Unregister(name) prog := progress.New(hub, name) - exitCode := localnet.RunUp(jobCtx, prog, opts) + exitCode := runUp(jobCtx, prog, opts) log.Printf("resume instance %q: exit_code=%d", name, exitCode) }() @@ -1076,7 +1076,7 @@ func realRecreateWork(ctx context.Context, hub *stream.Hub, name, version string Version: version, Profiles: profiles, } - exitCode := localnet.RunUp(ctx, prog, upOpts) + exitCode := runUp(ctx, prog, upOpts) log.Printf("restart instance %q: down_exit=%d up_exit=%d", name, downExit, exitCode) } diff --git a/internal/ui/handlers/main_test.go b/internal/ui/handlers/main_test.go index 042d2a68..47dcaa7a 100644 --- a/internal/ui/handlers/main_test.go +++ b/internal/ui/handlers/main_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/bitdynamics-ab/canton-devkit/internal/api/types" + "github.com/bitdynamics-ab/canton-devkit/internal/localnet" "github.com/bitdynamics-ab/canton-devkit/internal/splice" ) @@ -43,6 +44,13 @@ func TestMain(m *testing.M) { return types.PreflightReport{SchemaVersion: types.SchemaVersion, OK: true} } + // Bring-up runs on a detached context and outlives the test, so a + // default no-op keeps any test from stranding a real Canton stack. + // Tests asserting on bring-up override this with a recording stub. + runUp = func(context.Context, localnet.Progress, *localnet.UpOptions) int { + return localnet.ExitSuccess + } + code := m.Run() _ = os.RemoveAll(root) os.Exit(code) diff --git a/internal/ui/handlers/resume_test.go b/internal/ui/handlers/resume_test.go index 19c255db..dd797dd5 100644 --- a/internal/ui/handlers/resume_test.go +++ b/internal/ui/handlers/resume_test.go @@ -1,10 +1,13 @@ package handlers import ( + "context" "net/http" "net/http/httptest" "testing" + "time" + "github.com/bitdynamics-ab/canton-devkit/internal/localnet" "github.com/bitdynamics-ab/canton-devkit/internal/registry" "github.com/bitdynamics-ab/canton-devkit/internal/ui/stream" ) @@ -26,14 +29,21 @@ func resumeMux(t *testing.T) (*httptest.Server, *stream.Hub) { // TestResume_StoppedInstance202 — the happy path the user-reported // bug needs: instance is stopped (registry says so, containers are -// gone), and POST /up brings it back up. We can't easily assert the -// goroutine reaches RunUp (it talks to docker), but we CAN assert -// the handler accepts the request, returns 202, and points the -// caller at the events stream. +// gone), and POST /up brings it back up. The recording stub also pins +// the no-silent-upgrade rule: resume reuses the recorded version. func TestResume_StoppedInstance202(t *testing.T) { t.Setenv("CANTON_DEVKIT_REGISTRY", t.TempDir()) seedInstance(t, "pebble", "0.6.4", map[string]int{"app_user_ui": 44440}, registry.StatusStopped) + + upOpts := make(chan *localnet.UpOptions, 1) + origUp := runUp + t.Cleanup(func() { runUp = origUp }) + runUp = func(_ context.Context, _ localnet.Progress, opts *localnet.UpOptions) int { + upOpts <- opts + return localnet.ExitSuccess + } + srv, _ := resumeMux(t) resp, err := http.Post(srv.URL+"/api/instances/pebble/up", @@ -48,6 +58,18 @@ func TestResume_StoppedInstance202(t *testing.T) { if h := resp.Header.Get("Content-Type"); h != "application/json" { t.Errorf("Content-Type = %q, want application/json", h) } + + select { + case opts := <-upOpts: + if opts.Name != "pebble" { + t.Errorf("bring-up name = %q, want pebble", opts.Name) + } + if opts.Version != "0.6.4" { + t.Errorf("bring-up version = %q, want 0.6.4", opts.Version) + } + case <-time.After(5 * time.Second): + t.Fatal("resume never reached bring-up") + } } // TestResume_UnknownInstance404 — a name that's not in the registry