From f02f142af128b336c32a7f0059c5e1606bf9b1dc Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 2 Nov 2026 20:10:58 +0100 Subject: [PATCH] Tests put back the package state they set, so a loaded check machine trips no leaked bound The check of release sdk1 failed on TestStatePutWritesAKeyWhole: TestACallIsAskedAgainOnlyWhenItReachedNoOne left busretry's Bound at 50 ms, and on a loaded machine the state test's first pause overran it. Second failure of the kind "tests leave package-level state set". Quick fix: restore busretry's waits in the one test and lengthen the state test's bound. Thorough fix (chosen, the kind met twice): busretry.Waits sets the waits and returns their restore, used as t.Cleanup(busretry.Waits(...)) by every test that sets them, each with a bound of seconds where it waits to succeed; and servesAlone gives every test that serves its own runtime channel and Served, put back after it, instead of tests clearing the channel an earlier one left. A scan of every test assignment to a package-level variable found no other unrestored one. Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- go/busretry/busretry.go | 10 +++++++++ go/busretry/busretry_test.go | 10 +++++++-- go/caller_test.go | 1 + go/emit_message_id_test.go | 1 + go/keeps_test.go | 5 +---- go/log_test.go | 1 + go/process/call_test.go | 2 +- go/process/state_test.go | 2 +- go/provision_test.go | 1 + go/seat_pull_test.go | 1 + go/seat_test.go | 1 + go/state_counter_test.go | 1 + go/state_history_test.go | 1 + go/state_test.go | 1 + go/stdio_test.go | 42 ++++++++++++++++++++++++------------ 15 files changed, 58 insertions(+), 22 deletions(-) diff --git a/go/busretry/busretry.go b/go/busretry/busretry.go index acc523c..75306ee 100644 --- a/go/busretry/busretry.go +++ b/go/busretry/busretry.go @@ -29,6 +29,16 @@ var ( MostWait = 4 * time.Second ) +// Waits sets Bound, FirstWait and MostWait and returns what puts the earlier values back. They are state every caller +// in a process shares, so a test sets them only through it, as t.Cleanup(busretry.Waits(...)): a test that set them +// and left them set shortened the next test's bound to milliseconds, and a loaded check machine overran it (the check +// of release sdk1, 2026-11-02). A bound a test waits on to succeed is seconds: no slow machine reaches it. +func Waits(bound, first, most time.Duration) (restore func()) { + was := [3]time.Duration{Bound, FirstWait, MostWait} + Bound, FirstWait, MostWait = bound, first, most + return func() { Bound, FirstWait, MostWait = was[0], was[1], was[2] } +} + // Undelivered says err means the request reached no one: no responders, JetStream not enabled (for the account), or // no response from the stream after the client's own retries on no responders; also said in words by a runtime that // passed the error on. diff --git a/go/busretry/busretry_test.go b/go/busretry/busretry_test.go index d6af096..fec103b 100644 --- a/go/busretry/busretry_test.go +++ b/go/busretry/busretry_test.go @@ -15,7 +15,6 @@ import ( // Issue 774 (bus blackouts at every user-list reload): only a request that reached no one is asked again, within the // bound; a blackout probe that says the bus answers ends the wait at once. func TestABusThatAnswersNothingIsWaitedOut(t *testing.T) { - Bound, FirstWait, MostWait = 60*time.Millisecond, time.Millisecond, 4*time.Millisecond for _, row := range []struct { name string errs []error // what each try answers; the last repeats @@ -30,8 +29,15 @@ func TestABusThatAnswersNothingIsWaitedOut(t *testing.T) { {"a timeout may have been delivered: never asked again", []error{context.DeadlineExceeded}, nil, 1, 1, false}, {"an answer that refuses is an answer", []error{errors.New("refused")}, nil, 1, 1, false}, {"nobody answers on a bus that answers: the callee is not running", []error{nats.ErrNoResponders}, func() bool { return false }, 1, 1, false}, - {"blacked out past the bound: said", []error{jetstream.ErrJetStreamNotEnabledForAccount}, func() bool { return true }, 3, 1000, false}, + {"blacked out past the bound: said", []error{jetstream.ErrJetStreamNotEnabledForAccount}, func() bool { return true }, 2, 1000, false}, } { + // A row that ends in an answer waits on a bound of seconds, which no slow machine reaches; only the row that + // runs past it has a short one, and asks only that it was asked again at least once. + bound := 10 * time.Second + if strings.HasPrefix(row.name, "blacked out") { + bound = 100 * time.Millisecond + } + t.Cleanup(Waits(bound, time.Millisecond, 4*time.Millisecond)) tries := 0 _, err := Do("the test", func() (int, error) { e := row.errs[min(tries, len(row.errs)-1)] diff --git a/go/caller_test.go b/go/caller_test.go index 8f87cb1..ee0d5b8 100644 --- a/go/caller_test.go +++ b/go/caller_test.go @@ -11,6 +11,7 @@ import ( // callWith serves one tool that answers the caller CallerOf reads from its arguments, sends it one tools/call // with the params given, and returns the caller it answered. func callWith(t *testing.T, params map[string]any) string { + servesAlone(t) t.Helper() inR, inW := io.Pipe() outR, outW := io.Pipe() diff --git a/go/emit_message_id_test.go b/go/emit_message_id_test.go index 45add73..16de309 100644 --- a/go/emit_message_id_test.go +++ b/go/emit_message_id_test.go @@ -34,6 +34,7 @@ func (r *emitRuntime) next() map[string]any { } func serveEmit(t *testing.T, tools []Tool) *emitRuntime { + servesAlone(t) t.Helper() t.Setenv(ServedAs, "chime") inR, inW := io.Pipe() diff --git a/go/keeps_test.go b/go/keeps_test.go index 9233a62..1c3a708 100644 --- a/go/keeps_test.go +++ b/go/keeps_test.go @@ -15,14 +15,11 @@ import ( // announces through the runtime once the bundle serves, as the module the runtime names; and a state of its own that it // did not announce is refused at the call, naming it, so code and announcement cannot drift. func TestABundleAnnouncesWhatItKeepsAndBindsNothingElse(t *testing.T) { + servesAlone(t) t.Setenv(ServedAs, "chime") t.Setenv("MESH_MODULE", "chime") t.Setenv("MESH_NODE", "anchor") defer forgetKeeps() - // Not yet serving: an earlier test's channel is gone with its test. - currentMu.Lock() - current = nil - currentMu.Unlock() if err := Keeps(announce.State{Name: "rings", Readable: true}, announce.State{Name: "own"}); err != nil { t.Fatal(err) } diff --git a/go/log_test.go b/go/log_test.go index 98bba28..814a79d 100644 --- a/go/log_test.go +++ b/go/log_test.go @@ -130,6 +130,7 @@ func (r *logRuntime) answer(m message) map[string]any { // logging serves a bundle whose one tool, "log", runs run against r, answering each mesh/log ask beside // the loop as the runtime does. It returns what the tool answered. func logging(t *testing.T, r *logRuntime, run func() (any, error)) map[string]any { + servesAlone(t) t.Helper() t.Setenv(ServedAs, "issues") inR, inW := io.Pipe() diff --git a/go/process/call_test.go b/go/process/call_test.go index e56ed26..1f4fb02 100644 --- a/go/process/call_test.go +++ b/go/process/call_test.go @@ -103,7 +103,7 @@ func TestCallOnRefusesANodeThatIsNotOneToken(t *testing.T) { // The review of 2026-10-30 (issue 774): a callee's own answer is delivered, whatever its words, and is never asked // again during a blackout; only the transport's no responders is. func TestACallIsAskedAgainOnlyWhenItReachedNoOne(t *testing.T) { - busretry.Bound, busretry.FirstWait, busretry.MostWait = 50*time.Millisecond, time.Millisecond, 2*time.Millisecond + t.Cleanup(busretry.Waits(10*time.Second, time.Millisecond, 2*time.Millisecond)) for _, row := range []struct { name string answer []error // per ask; the last repeats diff --git a/go/process/state_test.go b/go/process/state_test.go index 0809a29..7c9cfb9 100644 --- a/go/process/state_test.go +++ b/go/process/state_test.go @@ -208,7 +208,7 @@ func TestStatePutWritesAKeyWhole(t *testing.T) { t.Fatalf("Get after Put: %+v, %v", e, err) } // Issue 774: a bus that answers nothing for a moment (a user-list reload) is waited out, not a lost write. - busretry.FirstWait = time.Millisecond + t.Cleanup(busretry.Waits(10*time.Second, time.Millisecond, 2*time.Millisecond)) kv, blackedOut := newFakeKV(), 2 dark := State{Module: "mesh-mail", Name: "sends", kv: func(context.Context) (jetstream.KeyValue, error) { if blackedOut > 0 { diff --git a/go/provision_test.go b/go/provision_test.go index fec0a2c..5778547 100644 --- a/go/provision_test.go +++ b/go/provision_test.go @@ -14,6 +14,7 @@ import ( // returns and each change after is named by the watch's id; what the module does not declare and every secret come // back refused, in the runtime's words; a change for no watch of this bundle is refused. func TestABundleReadsAndWatchesItsValuesThroughTheRuntime(t *testing.T) { + servesAlone(t) t.Setenv(ServedAs, "claude-code") inR, inW := io.Pipe() outR, outW := io.Pipe() diff --git a/go/seat_pull_test.go b/go/seat_pull_test.go index da7fbe5..eb1d914 100644 --- a/go/seat_pull_test.go +++ b/go/seat_pull_test.go @@ -11,6 +11,7 @@ import ( // 1887/A11: a bundle pulls one message through its runtime, naming the worker and the wait, and settles it by the id the // runtime gave. func TestABundlePullsAndSettlesThroughTheRuntime(t *testing.T) { + servesAlone(t) t.Setenv(ServedAs, "mesh-build-go") inR, inW := io.Pipe() outR, outW := io.Pipe() diff --git a/go/seat_test.go b/go/seat_test.go index 933b549..5772fc0 100644 --- a/go/seat_test.go +++ b/go/seat_test.go @@ -12,6 +12,7 @@ import ( // novox/hq ADR 0259 §3: a bundle takes a seat's work and answers proofs through the runtime; a piece of // work it fails is answered as an error, so the runtime offers it again, and a proof's error is said. func TestABundleTakesWorkAndAnswersProofsThroughTheRuntime(t *testing.T) { + servesAlone(t) t.Setenv(ServedAs, "router") inR, inW := io.Pipe() outR, outW := io.Pipe() diff --git a/go/state_counter_test.go b/go/state_counter_test.go index fb89617..aed2a33 100644 --- a/go/state_counter_test.go +++ b/go/state_counter_test.go @@ -76,6 +76,7 @@ func (r *casRuntime) answer(m message) map[string]any { // beside the loop, as the runtime does, so two writers' reads and writes interleave. It returns what the // tool answered. func counting(t *testing.T, r *casRuntime, run func() (any, error)) map[string]any { + servesAlone(t) t.Helper() t.Setenv(ServedAs, "issues") inR, inW := io.Pipe() diff --git a/go/state_history_test.go b/go/state_history_test.go index 33dbcd4..8f2f0a5 100644 --- a/go/state_history_test.go +++ b/go/state_history_test.go @@ -13,6 +13,7 @@ import ( // historying serves a bundle whose one tool, "history", runs run, answering each mesh/state ask with // answer beside the loop, as the runtime does; it returns what the bundle asked. func historying(t *testing.T, answer func(message) map[string]any, run func()) []message { + servesAlone(t) t.Helper() t.Setenv(ServedAs, "issues") inR, inW := io.Pipe() diff --git a/go/state_test.go b/go/state_test.go index 29a3b16..f2584c2 100644 --- a/go/state_test.go +++ b/go/state_test.go @@ -15,6 +15,7 @@ import ( // current values are handled before it returns, its changes are named by the watch's id, a change for // no watch of this bundle is refused, and put, get and keys are asked of the runtime as the module. func TestABundleKeepsAndWatchesStateThroughTheRuntime(t *testing.T) { + servesAlone(t) t.Setenv(ServedAs, "keeper") inR, inW := io.Pipe() outR, outW := io.Pipe() diff --git a/go/stdio_test.go b/go/stdio_test.go index 29a64db..78d64a0 100644 --- a/go/stdio_test.go +++ b/go/stdio_test.go @@ -18,16 +18,7 @@ func TestABundleEmitsThroughTheRuntimeAndServesAsItsModule(t *testing.T) { t.Setenv("MESH_MODULE", "chime") t.Setenv("MESH_NODE", "anchor") // 1887/AC-wake: Served closes once Serve has the runtime's channel, not before; the globals are put back after. - oldServed := served - served, servedOnce = make(chan struct{}), sync.Once{} - t.Cleanup(func() { - served, servedOnce = oldServed, sync.Once{} - select { - case <-oldServed: // it was closed: its Once is spent too - servedOnce.Do(func() {}) - default: - } - }) + servesAlone(t) select { case <-Served(): t.Fatal("Served closed before Serve") @@ -94,6 +85,7 @@ func TestABundleEmitsThroughTheRuntimeAndServesAsItsModule(t *testing.T) { // A Go bundle subscribes and asks through the runtime (novox/hq ADR 0198): the event is answered only // after the handler took it, a handler's error is answered as one, and a tool call is asked as the module. func TestABundleSubscribesAndAsksThroughTheRuntime(t *testing.T) { + servesAlone(t) t.Setenv(ServedAs, "watcher") inR, inW := io.Pipe() outR, outW := io.Pipe() @@ -172,6 +164,7 @@ func TestABundleSubscribesAndAsksThroughTheRuntime(t *testing.T) { // A subscribe the runtime refused leaves no handler behind: asked again until the runtime takes it, a // module's handler is handed each event once, not once per attempt (novox/hq issue 276). func TestARefusedSubscribeLeavesNoHandlerBehind(t *testing.T) { + servesAlone(t) t.Setenv(ServedAs, "plex") inR, inW := io.Pipe() outR, outW := io.Pipe() @@ -257,14 +250,11 @@ func TestARefusedSubscribeLeavesNoHandlerBehind(t *testing.T) { // answer blocked on the pipe, a refused subscribe is still delivered to Subscribe, which returns the // refusal — no lock of the channel is held across a write to the pipe (novox/hq issue 1286/AC1, 1286/AC2). func TestARefusedSubscribeIsDeliveredWhileAMessageWaitsOnThePipe(t *testing.T) { + servesAlone(t) t.Setenv(ServedAs, "plex") inR, inW := io.Pipe() outR, outW := io.Pipe() defer func() { _ = inW.Close() }() - // Not the channel an earlier run left behind: Subscribe goes to the one this run serves. - currentMu.Lock() - current = nil - currentMu.Unlock() go func() { _ = ServeOn("", []Tool{{Name: "quick", Run: func(map[string]any) (any, error) { return "ok", nil }}}, inR, outW) }() @@ -303,3 +293,27 @@ func TestARefusedSubscribeIsDeliveredWhileAMessageWaitsOnThePipe(t *testing.T) { // Let the waiting answer go, so nothing is left blocked. go func() { _, _ = io.Copy(io.Discard, outR) }() } + +// servesAlone gives a test that serves its own serving state, put back when it ends: no runtime's channel yet and a +// Served not yet closed. ServeOn sets both for the whole process, so without it a test saw the channel and the closed +// Served an earlier test left behind (the check of release sdk1, 2026-11-02: tests leaking package state). +func servesAlone(t *testing.T) { + t.Helper() + currentMu.Lock() + oldCurrent := current + current = nil + currentMu.Unlock() + oldServed := served + served, servedOnce = make(chan struct{}), sync.Once{} + t.Cleanup(func() { + currentMu.Lock() + current = oldCurrent + currentMu.Unlock() + served, servedOnce = oldServed, sync.Once{} + select { + case <-oldServed: // it was closed: its Once is spent too + servedOnce.Do(func() {}) + default: + } + }) +}