From 0ee1b8cb766d47125bab631a63ee41343402d56a Mon Sep 17 00:00:00 2001 From: Bozhidar Batsov Date: Fri, 23 Oct 2026 17:22:45 +0300 Subject: [PATCH] Check how servers handle requests that go wrong A server that goes down at a frame it can't read takes every client with it, and a session that stops answering takes a REPL along. The new checks send frames that aren't requests (on connections the wire rules don't grade, as no reply to them can be right), an eval with line as a string, and interrupt an eval whose client went away, and look at whether the server and the session keep answering. One more makes sure an eval in one session doesn't hold up the others. nREPL 1.8.0 never answers the eval with the wrong types (nrepl#477) and, after a client went away in the middle of an eval, never sends the done of the next eval in that session. Babashka hangs on the wrong types too, and a bencode string takes jank down. --- CHANGELOG.md | 3 ++ PLAN.md | 24 +++++---- README.md | 16 +++--- doc/design.md | 5 +- doc/profiles.md | 2 +- doc/spec-changes.md | 7 ++- doc/usage.md | 6 ++- internal/check/check.go | 55 +++++++++++++++++-- internal/checks/all.go | 2 + internal/checks/checks_test.go | 27 +++++++--- internal/checks/fake_test.go | 47 +++++++++++++--- internal/checks/robust.go | 99 ++++++++++++++++++++++++++++++++++ internal/serve/matrix_test.go | 30 ++++++----- 13 files changed, 270 insertions(+), 53 deletions(-) create mode 100644 internal/checks/robust.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a5c3ef..1a95f92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,3 +34,6 @@ Initial release. `load-file` in `proof serve`. - [#13](https://github.com/nrepl/proof/pull/13): Checks for completions and lookups, and nREPL's `completions` and `lookup` in `proof serve`. +- [#14](https://github.com/nrepl/proof/pull/14): Checks for requests that go + wrong: frames that aren't requests, fields of the wrong type and clients + that go away in the middle of an eval. diff --git a/PLAN.md b/PLAN.md index 9834e3c..95aff1d 100644 --- a/PLAN.md +++ b/PLAN.md @@ -117,17 +117,19 @@ lookups and completions, plus requests that go wrong. ### Requests That Go Wrong -- Malformed messages, top-level values that aren't dicts and fields of - the wrong type ([nrepl#477](https://github.com/nrepl/nrepl/issues/477)). -- An eval whose reply bencode can't carry, e.g. a boolean - ([babashka#2201](https://github.com/babashka/babashka/issues/2201)). -- A client that goes away in the middle of an eval - ([babashka#2227](https://github.com/babashka/babashka/issues/2227)). -- Two clients evaluating code at the same time - ([jank#1022](https://github.com/jank-lang/jank/issues/1022)). - -After each of them the session has to keep answering, and the server as -well. +- Done: frames that aren't requests (the server has to keep taking + connections), fields of the wrong type + ([nrepl#477](https://github.com/nrepl/nrepl/issues/477)), a client that + goes away in the middle of an eval + ([babashka#2227](https://github.com/babashka/babashka/issues/2227)) and + an eval in one session holding up the others + ([jank#1022](https://github.com/jank-lang/jank/issues/1022)). A bencode + string takes jank down, and after a client goes away in the middle of + an eval, nREPL (1.8.0 and its main branch) never sends the `done` of + the next eval in that session. +- Not done: an eval whose reply bencode can't carry, e.g. a boolean + ([babashka#2201](https://github.com/babashka/babashka/issues/2201)), as + there's no telling which code makes a server send one. ### Before the Release diff --git a/README.md b/README.md index 19f946b..5392bb5 100644 --- a/README.md +++ b/README.md @@ -131,14 +131,14 @@ the replies). `-like jank` turns on all of jank's scenarios at once, and ## Status proof is still in its early days. Right now it covers the core of the -protocol (`describe`, unknown ops, `eval`, sessions, `stdin`, `interrupt`, `load-file`, completions, lookups and the wire format), -what CIDER, Calva, Conjure, vim-fireplace, REPLy and neorepl send while -connecting, evaluating code and loading files, the requests -clients send and the server differences clients have to deal with, and -`proof list` will show you all the checks. - -Checks for malformed requests are coming next, along with the first -release. +protocol (`describe`, unknown ops, `eval`, sessions, `stdin`, +`interrupt`, `load-file`, completions, lookups and the wire format), +requests that go wrong, what CIDER, Calva, Conjure, vim-fireplace, REPLy +and neorepl send while connecting, evaluating code and loading files, +the requests clients send and the server differences clients have to +deal with, and `proof list` will show you all the checks. + +The first release is coming next. [PLAN.md](PLAN.md) has all the details. ## Why "proof"? diff --git a/doc/design.md b/doc/design.md index 379a8c9..8c6435d 100644 --- a/doc/design.md +++ b/doc/design.md @@ -194,8 +194,9 @@ checks against every scenario, and a scenario has to get the same verdicts as the servers it names. `-like` turns on all the scenarios of a server, and together they have to get that server's column of the matrix (apart from `eval.no-code`, which no client sends, the checks that come -down to the server's language and those of completions and lookups, -which are nREPL's in proof serve). +down to the server's language, those of completions and lookups, which +are nREPL's in proof serve, and the `robust.*` checks, as proof serve +doesn't hang or crash where servers do). The grading works the other way around here. proof can't see what a client shows its users, so the client's own tests have to do that. Most diff --git a/doc/profiles.md b/doc/profiles.md index 6625f39..09d03f5 100644 --- a/doc/profiles.md +++ b/doc/profiles.md @@ -145,7 +145,7 @@ snippet is missing, all the checks that need it are skipped. | `session-state` | `code`, `use`, `value` | leave some state in the session that `use` reads back as `value` | `session.across-connections`, `session.isolated` | | `multiple` | `code`, `values` | consist of several forms that evaluate to `values` (in order) | `eval.multiple-forms` | | `read-line` | `code`, `value`, `eof` | read a line from stdin, evaluating to `value` when the line is `proof` and to `eof` at the end of input | `stdin.need-input`, `stdin.roundtrip`, `stdin.eof` | -| `sleep` | `code`, `out` | print `out` and then keep running for longer than the `timeout` (e.g. a minute), so there's something to interrupt | all the `interrupt.*` checks | +| `sleep` | `code`, `out` | print `out` and then keep running for longer than the `timeout` (e.g. a minute), so there's something to interrupt | all the `interrupt.*` checks, `robust.disconnect`, `robust.sessions-at-once` | | `completion` | `code`, `value` | `code` is the start of a name, which completes to `value` | all the `completion.*` checks | | `lookup` | `code` | `code` is the name of a function | `lookup.lookup`, `lookup.info`, `lookup.eldoc` | diff --git a/doc/spec-changes.md b/doc/spec-changes.md index f4cd9fa..8cf0d59 100644 --- a/doc/spec-changes.md +++ b/doc/spec-changes.md @@ -170,7 +170,12 @@ See the decisions above. - Handling of malformed input - broken messages, top-level values that aren't dictionaries and fields of the wrong type. In [nrepl#477](https://github.com/nrepl/nrepl/issues/477) such a request - kills the session thread and later evals never get a `done`. + kills the session thread and later evals never get a `done`, and a + bencode string on its own takes jank down. +- What happens to a session whose client goes away in the middle of an + eval. It should keep working for the next client + ([babashka#2227](https://github.com/babashka/babashka/issues/2227)), + and evals in other sessions shouldn't have to wait for it. - How the JSON in the examples maps to bencode. There are no nil or boolean values in bencode, so servers shouldn't try to send them ([nrepl#196](https://github.com/nrepl/nrepl/issues/196), diff --git a/doc/usage.md b/doc/usage.md index 9993cce..047be28 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -390,8 +390,10 @@ servers. There are a couple of exceptions. No scenario covers `eval.no-code`, as clients always send some code, and the code is still Clojure, even with `-like dialtone`. So the checks the profiles of dialtone and repartee skip for their languages (e.g. `eval.ns`) pass. -And the completions and lookups of `proof serve` are always nREPL's -(`completions` and `lookup`), whatever server it acts like. +The completions and lookups of `proof serve` are always nREPL's +(`completions` and `lookup`), whatever server it acts like, and it +doesn't hang or crash where other servers do (see the `robust.*` +checks). Most scenarios change only how the replies look on the wire, not what a user should end up seeing. Evaluating `(println "hi") (+ 1 2)` should diff --git a/internal/check/check.go b/internal/check/check.go index 0263ade..9034580 100644 --- a/internal/check/check.go +++ b/internal/check/check.go @@ -12,6 +12,8 @@ package check import ( "errors" "fmt" + "io" + "net" "runtime" "slices" "sort" @@ -125,15 +127,59 @@ func (t *T) Snippet(name string) profile.Snippet { // Connect opens a new connection, stopping the check if it can't. func (t *T) Connect() *nrepl.Conn { - c, err := nrepl.Dial(t.env.Addr, 5*time.Second) + c, err := t.TryConnect() if err != nil { t.harnessErr = fmt.Sprintf("couldn't connect to %s: %v", t.env.Addr, err) t.exit() } - t.conns = append(t.conns, c) return c } +// SendRaw sends bytes that aren't a request (or not a whole one) on a +// connection of their own, and closes it once the server had its say. The +// wire rules don't see that connection, as a server can't get its replies +// to them right (e.g. there's no id to echo). +func (t *T) SendRaw(b []byte) error { + nc, err := net.DialTimeout("tcp", t.env.Addr, 5*time.Second) + if err != nil { + return err + } + defer nc.Close() + if _, err := nc.Write(b); err != nil { + return err + } + // Until the server closes the connection, or goes quiet waiting for + // more. + nc.SetReadDeadline(time.Now().Add(t.settleTime())) + io.Copy(io.Discard, nc) + return nil +} + +// Settle gives the server a moment to notice something it doesn't reply +// to, e.g. a client going away. It's as long as connections settle for +// after a check. +func (t *T) Settle() { + time.Sleep(t.settleTime()) +} + +func (t *T) settleTime() time.Duration { + if t.env.Settle == 0 { + return defaultSettle + } + return t.env.Settle +} + +// TryConnect opens a new connection, for checks that want to know whether +// the server still takes them. +func (t *T) TryConnect() (*nrepl.Conn, error) { + c, err := nrepl.Dial(t.env.Addr, 5*time.Second) + if err != nil { + return nil, err + } + t.conns = append(t.conns, c) + return c, nil +} + // Request sends m and waits for "done". A request that never finishes // fails the check outright, since every client waits for "done". func (t *T) Request(c *nrepl.Conn, m nrepl.Message) nrepl.Response { @@ -244,10 +290,13 @@ type Result struct { // Run runs checks one at a time, then grades the wire traffic they // produced against the wire rules. +// defaultSettle is how long connections settle when Env doesn't say. +const defaultSettle = 100 * time.Millisecond + func Run(env *Env, checks []*Check, rules []*Rule) []Result { settle := env.Settle if settle == 0 { - settle = 100 * time.Millisecond + settle = defaultSettle } results := make([]Result, len(checks)) conns := make([][]*nrepl.Conn, len(checks)) diff --git a/internal/checks/all.go b/internal/checks/all.go index 0a6bc66..5907af0 100644 --- a/internal/checks/all.go +++ b/internal/checks/all.go @@ -15,5 +15,7 @@ func All() []*check.Check { all = append(all, completionChecks()...) all = append(all, lookupChecks()...) all = append(all, clientProfileChecks()...) + // Last, as they may take the server down. + all = append(all, robustChecks()...) return all } diff --git a/internal/checks/checks_test.go b/internal/checks/checks_test.go index 8d0bae5..23c8da9 100644 --- a/internal/checks/checks_test.go +++ b/internal/checks/checks_test.go @@ -77,7 +77,12 @@ func TestChecksCatchMisbehaviour(t *testing.T) { connects := map[string]check.Verdict{"cider.connect": F, "calva.connect": F, "conjure.connect": F, "fireplace.connect": F, "neorepl.connect": F} // The checks that can't tell whether the server has their op. - noInterrupts, noLoadFile := skipped(interruptChecks()), skipped(loadFileChecks()) + noLoadFile := skipped(loadFileChecks()) + // The interrupt checks, and the ones that interrupt the sleep snippet + // they start. + noInterrupts := skipped(slices.DeleteFunc(All(), func(c *check.Check) bool { + return !strings.HasPrefix(c.ID, "interrupt.") && !slices.Contains(c.Requires, "interrupt.running") + })) noLookups := skipped(completionChecks(), lookupChecks()) // What CIDER, Calva and Conjure send to load a file. clientsLoad := map[string]check.Verdict{"cider.load-file": F, "calva.load-file": F, "conjure.load-file": F} @@ -103,12 +108,15 @@ func TestChecksCatchMisbehaviour(t *testing.T) { // Only clients put dicts and lists in their requests. // Conjure's complete has extra-metadata, a list. {"flat fields only", quirks{flatFields: true}, checktest.Merged(clientsLoad, map[string]check.Verdict{"completion.complete-symbol": F, - "cider.connect": F, "cider.eval": F, "cider.repl": F, "calva.connect": F, "calva.eval": F, "conjure.eval": F, + "robust.wrong-types": F, + "cider.connect": F, "cider.eval": F, "cider.repl": F, "calva.connect": F, "calva.eval": F, "conjure.eval": F, "neorepl.eval": F})}, {"any session accepted", quirks{acceptAnySession: true}, map[string]check.Verdict{"session.unknown": F, "session.closed": F}}, - {"shared session state", quirks{sharedState: true}, map[string]check.Verdict{"session.isolated": F}}, + // One session, so one eval at a time. + {"shared session state", quirks{sharedState: true}, map[string]check.Verdict{"session.isolated": F, + "robust.sessions-at-once": F}}, {"sessions tied to sockets", quirks{socketSessions: true}, map[string]check.Verdict{"session.across-connections": W, - "interrupt.other-connection": S}}, + "interrupt.other-connection": S, "robust.disconnect": S}}, {"no ephemeral sessions", quirks{noEphemeral: true}, map[string]check.Verdict{"session.ephemeral": F, "fireplace.connect": F}}, {"last value only", quirks{lastValueOnly: true}, map[string]check.Verdict{"eval.multiple-forms": F, "fireplace.connect": F}}, {"stderr dropped", quirks{dropErr: true}, map[string]check.Verdict{"eval.stderr": F}}, @@ -135,7 +143,8 @@ func TestChecksCatchMisbehaviour(t *testing.T) { {"integer value", quirks{intValue: true}, checktest.Merged(clientsLoad, map[string]check.Verdict{ "wire.field-types": F, "eval.value": F, "eval.survives-error": F, "eval.multiple-forms": F, "session.ephemeral": F, "session.persistent": F, "cider.eval": F, "cider.repl": F, "calva.eval": F, "conjure.eval": F, "fireplace.eval": F, - "neorepl.eval": F, "interrupt.survives": F, "interrupt.queued": F, "load-file.value": F})}, + "neorepl.eval": F, "interrupt.survives": F, "interrupt.queued": F, "load-file.value": F, "robust.wrong-types": W, + "robust.disconnect": F, "robust.sessions-at-once": F, "robust.bad-frames": F})}, {"unsorted keys", quirks{unsortedKeys: true}, map[string]check.Verdict{"wire.canonical": W}}, {"invalid UTF-8", quirks{badUTF8: true}, map[string]check.Verdict{"wire.utf8": W, "eval.stdout": F}}, {"no stdin op", quirks{noStdinOp: true}, map[string]check.Verdict{ @@ -158,7 +167,13 @@ func TestChecksCatchMisbehaviour(t *testing.T) { // The interrupt's connection never sent the eval. {"interrupted on the wrong connection", quirks{interruptHere: true}, map[string]check.Verdict{ "interrupt.other-connection": F, "wire.id": W}}, - {"session dies on interrupt", quirks{diesOnInterrupt: true}, map[string]check.Verdict{"interrupt.survives": F, "interrupt.queued": F}}, + {"session dies on interrupt", quirks{diesOnInterrupt: true}, map[string]check.Verdict{"interrupt.survives": F, "interrupt.queued": F, + "robust.disconnect": F}}, + {"garbage takes the server down", quirks{crashOnGarbage: true}, map[string]check.Verdict{"robust.bad-frames": F}}, + // The eval never gets its done, which fails even a warning. + {"wrong types kill the session", quirks{typesKill: true}, map[string]check.Verdict{"robust.wrong-types": F}}, + {"a session dies with its client", quirks{diesOnDisconnect: true}, map[string]check.Verdict{"robust.disconnect": F}}, + {"one eval at a time", quirks{oneEvalAtATime: true}, map[string]check.Verdict{"robust.sessions-at-once": F}}, // CIDER and Conjure send load-file without asking, and so does // Calva's profile, as it can't load files either way. {"no load-file op", quirks{noLoadFileOp: true}, checktest.Merged(noLoadFile, clientsLoad)}, diff --git a/internal/checks/fake_test.go b/internal/checks/fake_test.go index c42b273..ebe9929 100644 --- a/internal/checks/fake_test.go +++ b/internal/checks/fake_test.go @@ -2,6 +2,7 @@ package checks import ( "bufio" + "errors" "net" "strconv" "strings" @@ -76,6 +77,10 @@ type quirks struct { eldocStrings bool // eldoc's arglists are strings rather than lists pathFile bool // the file of a symbol is a path rather than a URI ciderPrefixed bool // cider-nrepl's ops go by their cider/ names only + crashOnGarbage bool // a frame that isn't a request takes the server down + typesKill bool // an eval with a line that isn't an integer kills its session + diesOnDisconnect bool // a session dies when its eval's connection goes away + oneEvalAtATime bool // an eval in one session holds up all the others } var unknownSession = map[string]any{"status": []any{"error", "unknown-session", "done"}} @@ -138,15 +143,17 @@ func newFakeSession() *fakeSession { func (s *fakeServer) serve(c net.Conn) { defer c.Close() + defer s.disconnected(c) dec := bencode.NewDecoder(bufio.NewReader(c)) local := map[string]bool{} for { v, err := dec.Decode() - if err != nil { - return - } m, ok := v.Data.(map[string]any) - if !ok { + if err != nil || !ok { + var garbage *bencode.SyntaxError + if s.q.crashOnGarbage && (err == nil || errors.As(err, &garbage)) { + s.ln.Close() + } return } if !s.handle(c, nrepl.Message(m), local) { @@ -155,6 +162,21 @@ func (s *fakeServer) serve(c net.Conn) { } } +// disconnected kills the sessions whose running eval came from c, with +// diesOnDisconnect. +func (s *fakeServer) disconnected(c net.Conn) { + if !s.q.diesOnDisconnect { + return + } + s.mu.Lock() + defer s.mu.Unlock() + for _, sess := range s.sessions { + if sess.running != nil && sess.running.c == c { + sess.dead = true + } + } +} + func (s *fakeServer) send(c net.Conn, req nrepl.Message, fields map[string]any) { out := map[string]any{} _, isOut := fields["out"] @@ -320,6 +342,16 @@ func (s *fakeServer) handle(c net.Conn, req nrepl.Message, local map[string]bool return true } +// anyRunning reports whether an eval runs in any session. s.mu is held. +func (s *fakeServer) anyRunning() bool { + for _, sess := range s.sessions { + if sess.running != nil { + return true + } + } + return false +} + // complete offers proof for a prefix of it, like nREPL's completions and // cider-nrepl's complete do. func (s *fakeServer) complete(c net.Conn, req nrepl.Message) { @@ -460,7 +492,7 @@ func (s *fakeServer) interrupt(c net.Conn, req nrepl.Message, local map[string]b sess.running = nil queue := sess.queue sess.queue = nil - sess.dead = q.diesOnInterrupt + sess.dead = sess.dead || q.diesOnInterrupt s.mu.Unlock() to, status := run.c, []any{"done", "interrupted"} if q.interruptHere { @@ -490,7 +522,10 @@ func (s *fakeServer) eval(c net.Conn, req nrepl.Message, local map[string]bool) if busy { sess.queue = append(sess.queue, fakeRun{c, req, local}) } - skip := busy || sess.dead + if _, ok := req["line"].(string); ok && q.typesKill { + sess.dead = true + } + skip := busy || sess.dead || q.oneEvalAtATime && s.anyRunning() s.mu.Unlock() if skip { return diff --git a/internal/checks/robust.go b/internal/checks/robust.go new file mode 100644 index 0000000..93777cd --- /dev/null +++ b/internal/checks/robust.go @@ -0,0 +1,99 @@ +package checks + +import ( + "slices" + + "github.com/nrepl/proof/internal/check" + "github.com/nrepl/proof/nrepl" +) + +// The robust checks run last, as they may take the server down. +func robustChecks() []*check.Check { + return []*check.Check{ + { + ID: "robust.wrong-types", + Title: "A session keeps working after a request with fields of the wrong type", + Severity: check.Warn, + Why: "No client we know of sends line as a string, but one with a bug (see client.field-types) shouldn't take its session down: nREPL never answers that eval or anything after it in the session.", + Refs: []check.Ref{issue("nrepl/nrepl", 477), nreplLineColumn}, + Snippets: []string{"value"}, + Requires: []string{"session.clone"}, + Run: func(t *check.T) { + s := t.Snippet("value") + c := t.Connect() + session := t.Session(c) + t.Eval(c, s.Code, session.With(nrepl.Message{"line": "proof", "column": []any{"proof"}})) + if vs := t.Eval(c, s.Code, session).Values(); !slices.Equal(vs, []string{s.Value}) { + t.Violatef("after an eval with line as a string, evaluating %q gave values %q, want [%q]", s.Code, vs, s.Value) + } + }, + }, + { + ID: "robust.sessions-at-once", + Title: "An eval in one session doesn't hold up the others", + Severity: check.Fail, + Why: "Each client has sessions of its own (e.g. an editor and neorepl, connected to the same server), so a server that runs one eval at a time leaves every other client waiting for whatever the code in one of them does.", + Refs: []check.Ref{issue("jank-lang/jank", 1022), ciderClone}, + Snippets: []string{"sleep", "value"}, + Requires: []string{"interrupt.running"}, + Run: func(t *check.T) { + sleeping := startSleep(t) + defer sleeping.stop() + if s, resp := evalSnippet(t, "value"); !slices.Equal(resp.Values(), []string{s.Value}) { + t.Violatef("evaluating %q in another session while the sleep snippet ran gave values %q, want [%q]", s.Code, resp.Values(), s.Value) + } + sleeping.interrupt(t, sleeping.id) + }, + }, + { + ID: "robust.disconnect", + Title: "A session keeps working after its client went away in the middle of an eval", + Severity: check.Fail, + Why: "neorepl shares a session between its commands, so when one of them is killed in the middle of an eval, the session has to keep working for the next one, which otherwise waits forever.", + Refs: []check.Ref{issue("babashka/babashka", 2227), neoreplInterrupt}, + Snippets: []string{"sleep", "value"}, + Requires: []string{"interrupt.running"}, + Run: func(t *check.T) { + v := t.Snippet("value") + s := startSleep(t) + s.c.Close() + t.Settle() + c := t.Connect() + resp := t.Request(c, s.session.With(nrepl.Message{"op": "interrupt", "interrupt-id": s.id})) + if resp.HasStatus("unknown-session") || resp.HasStatus("session-idle") || resp.HasStatus("error") { + t.Skipf("the session can't be interrupted from another connection, which got status %v (see session.across-connections)", resp.Status()) + } + if vs := t.Eval(c, v.Code, s.session).Values(); !slices.Equal(vs, []string{v.Value}) { + t.Violatef("after its client went away, evaluating %q in the session gave values %q, want [%q]", v.Code, vs, v.Value) + } + }, + }, + { + ID: "robust.bad-frames", + Title: "Frames that aren't requests don't take the server down", + Severity: check.Fail, + Why: "A server that goes down at something it can't read takes every client connected to it along, and users lose their REPLs and everything in them. Closing the connection that sent it is fine, as nREPL does.", + Refs: []check.Ref{nreplConnLoop}, + Snippets: []string{"value"}, + Requires: []string{"session.clone"}, + Run: func(t *check.T) { + s := t.Snippet("value") + // A list, a string, an integer, something that isn't bencode + // and a dict whose string is cut short. + for _, frame := range []string{"le", "5:proof", "i42e", "proof", "d2:op4:ev"} { + if err := t.SendRaw([]byte(frame)); err != nil { + t.Stopf("couldn't send %q: %v", frame, err) + } + t.Settle() + c, err := t.TryConnect() + if err != nil { + t.Stopf("after a connection sent %q, the server took no more connections: %v", frame, err) + } + if vs := t.Eval(c, s.Code, t.Session(c)).Values(); !slices.Equal(vs, []string{s.Value}) { + t.Stopf("after a connection sent %q, evaluating %q on another one gave values %q, want [%q]", frame, s.Code, vs, s.Value) + } + } + }, + }, + } +} diff --git a/internal/serve/matrix_test.go b/internal/serve/matrix_test.go index 7b710ee..c115c5b 100644 --- a/internal/serve/matrix_test.go +++ b/internal/serve/matrix_test.go @@ -2,6 +2,7 @@ package serve import ( "fmt" + "maps" "slices" "strings" "testing" @@ -58,12 +59,13 @@ func runSomeChecksAs(t *testing.T, server string, cs []*check.Check, scenarios . return checktest.ByID(check.Run(env, cs, checks.WireRules())) } -// noInterrupts is what the interrupt checks get from servers without the -// op: skipped. +// noInterrupts is what servers without interrupt get from the interrupt +// checks, and the checks that interrupt the sleep snippet they start: +// skipped. var noInterrupts = func() map[string]check.Verdict { v := map[string]check.Verdict{} for _, c := range checks.All() { - if strings.HasPrefix(c.ID, "interrupt.") { + if strings.HasPrefix(c.ID, "interrupt.") || slices.Contains(c.Requires, "interrupt.running") { v[c.ID] = check.Skipped } } @@ -104,7 +106,7 @@ func TestScenariosGetTheVerdictsOfTheirServers(t *testing.T) { {[]string{"no-session-closed"}, map[string]check.Verdict{"session.close": F}}, {[]string{"shared-state"}, map[string]check.Verdict{"session.isolated": F}}, {[]string{"socket-sessions"}, map[string]check.Verdict{"session.across-connections": W, - "interrupt.other-connection": S}}, + "interrupt.other-connection": S, "robust.disconnect": S}}, {[]string{"any-session"}, map[string]check.Verdict{"session.unknown": F, "session.closed": F}}, {[]string{"ns-fallback"}, map[string]check.Verdict{"eval.unknown-ns": F}}, {[]string{"ns-error"}, map[string]check.Verdict{"eval.unknown-ns": F}}, @@ -127,10 +129,11 @@ func TestScenariosGetTheVerdictsOfTheirServers(t *testing.T) { // A server's scenarios together get its column of the compatibility // matrix. The columns leave out eval.no-code, as no client sends an eval -// without code, and the checks of completions and lookups, which are -// nREPL's in proof serve. That's also why Basilisp's column has no -// wire.canonical warning, and ClojureCLR's no wire.field-types failure, -// which their info replies get in the matrix. +// without code, the checks of completions and lookups, which are nREPL's +// in proof serve, and the robust checks, as proof serve doesn't hang or +// crash where servers do. So they also leave out what the wire rules make +// of the replies to those checks (e.g. Basilisp's duplicate status in +// info, or the replies without an id to frames that aren't requests). func TestPresetsGetTheColumnsOfTheirServers(t *testing.T) { F, W, S := check.Failed, check.Warned, check.Skipped noJava := map[string]check.Verdict{"fireplace.connect": S, "fireplace.eval": S} @@ -163,11 +166,12 @@ func TestPresetsGetTheColumnsOfTheirServers(t *testing.T) { if err != nil { t.Fatal(err) } - // proof serve's completions and lookups are nREPL's whatever - // server it acts like. - cs := slices.DeleteFunc(checks.All(), func(c *check.Check) bool { - return strings.HasPrefix(c.ID, "completion.") || strings.HasPrefix(c.ID, "lookup.") - }) + left := func(id string) bool { + return strings.HasPrefix(id, "completion.") || strings.HasPrefix(id, "lookup.") || strings.HasPrefix(id, "robust.") + } + cs := slices.DeleteFunc(checks.All(), func(c *check.Check) bool { return left(c.ID) }) + want = maps.Clone(want) + maps.DeleteFunc(want, func(id string, _ check.Verdict) bool { return left(id) }) checktest.Verdicts(t, runSomeChecksAs(t, name, cs, scenarios...), want) }) }