From 5b57422523550d17f0f28d4f037380c92ca97c41 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 22:53:18 +0200 Subject: [PATCH] Report an unknown ask through a small interface, not *testing.T directly (issue 1285) A failed subtest fails its parent, so TestHarnessFailsTheTestWhenTheBundleAsksSomethingUnknown could never pass the way it was written (try 2's finding). The player now reports through askFailer (Helper, Errorf); every other test keeps passing its own *testing.T, which satisfies it already. That one test gives a recorder instead, and asserts it holds exactly one message naming seat:mail-store.accounts, and that mail-webclient.status's own answer shows the ask got an error back. Checked: no other integration test uses the same deliberate-subtest-failure pattern (grepped t.Run). --- .../mesh-mail-web/runtime_integration_test.go | 110 +++++++++++++----- 1 file changed, 82 insertions(+), 28 deletions(-) diff --git a/modules/mesh-mail-web/cmd/mesh-mail-web/runtime_integration_test.go b/modules/mesh-mail-web/cmd/mesh-mail-web/runtime_integration_test.go index b8311f07..a1c2417e 100644 --- a/modules/mesh-mail-web/cmd/mesh-mail-web/runtime_integration_test.go +++ b/modules/mesh-mail-web/cmd/mesh-mail-web/runtime_integration_test.go @@ -239,14 +239,24 @@ func freePort(t *testing.T) int { // ---- the runtime player itself (issue 1285) ------------------------------------------------------------ +// askFailer is the small part of *testing.T the runtime player reports an unknown ask through (Helper, +// Errorf) — never the whole of it. Every ordinary test gives its own *testing.T (which has both). Issue +// 1285/AC4's own test gives a recorder instead: a failed subtest fails its parent (t.Run answers false, but +// the parent is marked failed regardless, try 2's own finding), which would make "the harness fails the +// test" itself untestable the direct way. +type askFailer interface { + Helper() + Errorf(format string, args ...any) +} + // runtimePlayer plays the node's runtime against one started bundle, over its real stdin and stdout: // initialize, tools/list, tools/call (this harness's own outgoing requests, stdio.go's ServeOn switch), and // answers to what the bundle itself asks — mesh/ask, mesh/state. (the bundle's own outgoing requests, // stdio.go's channel.ask / state.go's KeptState.ask). type runtimePlayer struct { - t *testing.T - cmd *exec.Cmd - in io.WriteCloser + fail askFailer + cmd *exec.Cmd + in io.WriteCloser mu sync.Mutex nextID int @@ -259,8 +269,13 @@ type runtimePlayer struct { // startRuntimePlayer starts binary with env, and begins playing the runtime on its stdin/stdout at once: the // handshake (initialize, notifications/initialized) is done here, so a caller gets back a player already // past it, exactly as the SDK's own live() expects before any Ask (stdio.go:391-398, live_before_serve_test.go). -func startRuntimePlayer(t *testing.T, binary string, env []string, mailStore mailStoreFunc, js jetstream.JetStream) *runtimePlayer { +// t is used for the process's own setup and cleanup (Fatal, Cleanup) always; failer is where an unknown ask +// is reported (Errorf) — t itself when nil, a recorder for issue 1285/AC4's own test. +func startRuntimePlayer(t *testing.T, binary string, env []string, mailStore mailStoreFunc, js jetstream.JetStream, failer askFailer) *runtimePlayer { t.Helper() + if failer == nil { + failer = t + } cmd := exec.Command(binary) cmd.Env = env stderr := &prefixedLog{t: t, prefix: "mesh-mail-web stderr: "} @@ -276,7 +291,7 @@ func startRuntimePlayer(t *testing.T, binary string, env []string, mailStore mai if err := cmd.Start(); err != nil { t.Fatal(err) } - p := &runtimePlayer{t: t, cmd: cmd, in: stdin, waiting: map[string]chan wireMessage{}, mailStore: mailStore, js: js} + p := &runtimePlayer{fail: failer, cmd: cmd, in: stdin, waiting: map[string]chan wireMessage{}, mailStore: mailStore, js: js} go p.readLoop(stdout) t.Cleanup(func() { _ = cmd.Process.Kill() @@ -435,6 +450,7 @@ func (p *runtimePlayer) ToolsCall(name string, args map[string]any) (text string // handleBundleRequest answers one request the bundle itself made (mesh/ask, mesh/state.) — never one // this player sent (those are answers, read.Loop already told apart and never reach here). func (p *runtimePlayer) handleBundleRequest(m wireMessage) { + p.fail.Helper() reply := func(result any, errMsg string) { out := map[string]any{"jsonrpc": "2.0", "id": json.RawMessage(m.ID)} if errMsg != "" { @@ -451,7 +467,7 @@ func (p *runtimePlayer) handleBundleRequest(m wireMessage) { case strings.HasPrefix(m.Method, "mesh/state."): p.handleState(m, reply) default: - p.t.Errorf("the bundle asked %s, which the helper does not know (issue 1285/AC4)", m.Method) + p.fail.Errorf("the bundle asked %s, which the helper does not know (issue 1285/AC4)", m.Method) reply(nil, "the helper does not know "+m.Method) } } @@ -459,19 +475,20 @@ func (p *runtimePlayer) handleBundleRequest(m wireMessage) { // handleAsk is mesh/ask (stdio.go's Ask, params {"key":...,"body":...}): the stand-in mail-store's own // answer, or — a key the stand-in was given nothing for — the test failing, naming the ask (issue 1285/AC4). func (p *runtimePlayer) handleAsk(m wireMessage, reply func(result any, errMsg string)) { + p.fail.Helper() var params struct { Key string `json:"key"` Body map[string]any `json:"body"` } _ = json.Unmarshal(m.Params, ¶ms) if p.mailStore == nil { - p.t.Errorf("the bundle asked %s, which the helper does not know (issue 1285/AC4)", params.Key) + p.fail.Errorf("the bundle asked %s, which the helper does not know (issue 1285/AC4)", params.Key) reply(nil, "the helper does not know "+params.Key) return } result, err, known := p.mailStore(params.Key, params.Body) if !known { - p.t.Errorf("the bundle asked %s, which the helper does not know (issue 1285/AC4)", params.Key) + p.fail.Errorf("the bundle asked %s, which the helper does not know (issue 1285/AC4)", params.Key) reply(nil, "the helper does not know "+params.Key) return } @@ -486,6 +503,7 @@ func (p *runtimePlayer) handleAsk(m wireMessage, reply func(result any, errMsg s // get and keys answered from the real JetStream bucket this player was given (issue 1285/AC3); any other // verb, or a `state` this player holds no bucket for, fails the test naming the ask (AC4). func (p *runtimePlayer) handleState(m wireMessage, reply func(result any, errMsg string)) { + p.fail.Helper() verb := strings.TrimPrefix(m.Method, "mesh/state.") var params struct { State string `json:"state"` @@ -493,7 +511,7 @@ func (p *runtimePlayer) handleState(m wireMessage, reply func(result any, errMsg } _ = json.Unmarshal(m.Params, ¶ms) fail := func() { - p.t.Errorf("the bundle asked %s for state %s, which the helper does not know (issue 1285/AC4)", m.Method, params.State) + p.fail.Errorf("the bundle asked %s for state %s, which the helper does not know (issue 1285/AC4)", m.Method, params.State) reply(nil, "the helper does not know mesh/state."+verb+" for "+params.State) } // Only mesh-mail.accounts is wired to a real bucket (issue 1285's own scope); any other `state` is @@ -556,11 +574,19 @@ type startedMailWeb struct { js jetstream.JetStream } -// startMailWeb builds mesh-mail-web's own binary, starts it with a real account bucket (one key per entry -// in accounts) and the trusted setting operators, and plays the runtime on it until initialize and -// notifications/initialized are done. The stand-in mail-store (startedMailWeb.store) answers nothing until a -// test sets its own fields. +// startMailWeb is startMailWebWithFailer with the test's own *testing.T as the ask failer (every ordinary +// test). func startMailWeb(t *testing.T, accounts map[string]accountBucketValue, operators []string) *startedMailWeb { + t.Helper() + return startMailWebWithFailer(t, accounts, operators, nil) +} + +// startMailWebWithFailer builds mesh-mail-web's own binary, starts it with a real account bucket (one key +// per entry in accounts) and the trusted setting operators, and plays the runtime on it until initialize and +// notifications/initialized are done. The stand-in mail-store (startedMailWeb.store) answers nothing until a +// test sets its own fields. failer is where an unknown ask is reported (issue 1285/AC4): nil for the test's +// own *testing.T, or a recorder for a test that must assert on that report without failing itself. +func startMailWebWithFailer(t *testing.T, accounts map[string]accountBucketValue, operators []string, failer askFailer) *startedMailWeb { t.Helper() binary := buildMailWebBinary(t) port := freePort(t) @@ -595,7 +621,7 @@ func startMailWeb(t *testing.T, accounts map[string]accountBucketValue, operator "MESH_MAIL_WEB_OIDC="+oidcPath, "MESH_SERVED_MODULE=mesh-mail-web", ) - player := startRuntimePlayer(t, binary, env, store.answer, js) + player := startRuntimePlayer(t, binary, env, store.answer, js, failer) waitForHTTP(t, port) return &startedMailWeb{ @@ -815,20 +841,48 @@ func TestHarnessStateReadAnswersTheRealBucketsValue(t *testing.T) { } } -// 1285/AC4: the bundle asking something the helper does not know fails the test, naming the ask — checked in -// a subtest, so this test can read whether it failed (t.Run answers false on a failure) without failing -// itself. +// askFailRecorder is a fake askFailer: it keeps every message instead of failing a test (try 2's own +// finding — a failed subtest fails its parent through t.Run, which would make "the harness fails the test" +// itself untestable the direct way), so issue 1285/AC4's own test can assert on what was reported without +// failing itself. +type askFailRecorder struct { + mu sync.Mutex + messages []string +} + +func (r *askFailRecorder) Helper() {} + +func (r *askFailRecorder) Errorf(format string, args ...any) { + r.mu.Lock() + defer r.mu.Unlock() + r.messages = append(r.messages, fmt.Sprintf(format, args...)) +} + +func (r *askFailRecorder) Messages() []string { + r.mu.Lock() + defer r.mu.Unlock() + return append([]string(nil), r.messages...) +} + +// 1285/AC4: the bundle asking something the helper does not know is reported through the ask failer, naming +// the ask, and the bundle's own ask gets an error answer back. func TestHarnessFailsTheTestWhenTheBundleAsksSomethingUnknown(t *testing.T) { - passed := t.Run("deliberately unanswered ask", func(t *testing.T) { - mw := startMailWeb(t, nil, nil) - // mw.store.Accounts is left nil on purpose: mail-webclient.status's own ask of - // seat:mail-store.accounts is then unknown to the stand-in, and the player must fail this subtest, - // naming the ask, rather than only shaping a protocol error (issue 1285/AC4). - if _, _, err := mw.player.ToolsCall(StatusTool, map[string]any{}); err != nil { - t.Logf("tools/call itself answered an error, which is fine here: %v", err) - } - }) - if passed { - t.Fatal("the subtest did not fail despite an ask the stand-in did not know (issue 1285/AC4)") + recorder := &askFailRecorder{} + mw := startMailWebWithFailer(t, nil, nil, recorder) + // mw.store.Accounts is left nil on purpose: mail-webclient.status's own ask of seat:mail-store.accounts + // is then unknown to the stand-in. + text, isError, err := mw.player.ToolsCall(StatusTool, map[string]any{}) + if err != nil { + t.Fatalf("tools/call: %v", err) + } + if isError { + t.Fatalf("mail-webclient.status answered isError; it catches the ask's own error itself: %s", text) + } + if !strings.Contains(text, "does not answer") { + t.Fatalf("mail-webclient.status's own text does not show the ask got an error answer: %s", text) + } + messages := recorder.Messages() + if len(messages) != 1 || !strings.Contains(messages[0], "seat:mail-store.accounts") { + t.Fatalf("the recorder's messages: %v, want exactly one naming seat:mail-store.accounts", messages) } }