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).
2026-10-19 22:53:18 +02:00
parent 523d7846cd
commit 5b57422523
1 changed files with 82 additions and 28 deletions
@@ -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.<verb> (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.<verb>) — 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, &params)
 	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, &params)
 	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)
 	}
 }