Check servers with what Calva, Conjure and vim-fireplace send

README.md

@@ -74,8 +74,9 @@ something differently from the reference nREPL implementation, you'll
 get a warning instead.
 
 Some of the checks send what a particular client sends, the way it sends
-it, e.g. `cider.connect` sends the requests CIDER sends while connecting. That way
-the report tells you directly whether CIDER will work with your server.
+it, e.g. `cider.connect` sends the requests CIDER sends while connecting.
+That way the report tells you directly whether CIDER, Calva, Conjure and
+vim-fireplace will work with your server.
 
 ## Checking Clients
 
@@ -127,7 +128,8 @@ the replies). `-like jank` turns on all of jank's scenarios at once, and
 
 proof is still in its early days. Right now it covers the core of the
 protocol (`describe`, unknown ops, `eval`, sessions, `stdin` and the wire format),
-what CIDER sends while connecting and evaluating code, the requests
+what CIDER, Calva, Conjure and vim-fireplace send while connecting and
+evaluating code, the requests
 clients send and the server differences clients have to deal with, and
 `proof list` will show you all the checks.
 
@@ -136,8 +138,6 @@ Here's what's coming next:
 - checks for `interrupt`, `load-file`, `completions` and `lookup`
 - robustness checks (malformed messages, fields of the wrong type,
   clients disconnecting in the middle of an evaluation)
-- client profiles for more clients (e.g. Calva, Conjure and
-  vim-fireplace)
 - publishing the compatibility matrix somewhere nicer than a CI job
   summary
 

doc/design.md

@@ -131,8 +131,9 @@ needs from each reply, along with a link to the client code that needs
 it. Each situation becomes a check that sends those requests, the way
 the client sends them, and fails at the first reply the client couldn't
 use. Replies that only bother the client (e.g. an error the client just
-shows to the user) get a note. Right now there's a profile for CIDER,
-built from what CIDER sends with its default settings.
+shows to the user) get a note. Right now there are profiles for CIDER,
+Calva, Conjure and vim-fireplace, built from what they send with their
+default settings.
 
 A new profile starts with a recording, as `proof proxy -record` saves
 what a client sent as a profile with a check for each connection. What
@@ -205,10 +206,11 @@ requests, though, just like `proof proxy` does.
 
 proof is not a Clojure implementation, so `proof serve` understands only
 a small piece of Clojure. That's enough for the snippets of the nREPL
-profile, the code CIDER sends when it connects and what client tests
-need (output, values, errors, input, something to interrupt), and it
-gives the same replies as nREPL 1.7.0 for the same code. Code in other
-languages wouldn't help, as no client's tests send Erlang to a server.
+profile, the code CIDER and vim-fireplace send when they connect and
+what client tests need (output, values, errors, input, something to
+interrupt), and it gives the same replies as nREPL 1.7.0 for the same
+code. Code in other languages wouldn't help, as no client's tests send
+Erlang to a server.
 
 Some rules are about what a client leaves behind - sessions that were
 never closed and `need-input` that was never answered. They apply only
@@ -311,8 +313,6 @@ what's planned next:
   support them)
 - robustness checks (malformed messages, fields of the wrong type, clients
   disconnecting in the middle of an evaluation)
-- client profiles for more clients (e.g. Calva, Conjure and
-  vim-fireplace)
 - more servers in the compatibility matrix and a proper home for the
   matrix itself
 - incorporating [Spec Changes](spec-changes.md) into the spec, so that

doc/hacking.md

@@ -252,14 +252,18 @@ besides `done`:
 |---|---|
 | `new-session` | A `new-session`, which later steps can use as `$` and this name (e.g. `session = "$repl"`). |
 | `snippet` | The value of this snippet of the server's profile, with all the parts it came in joined together. Its code goes in the request, as it stands for the user's code. |
+| `values` | At least this many values, told apart by the `ns` that comes with each one (or after it), which is how some clients tell them apart. |
 | `dicts` | These fields have to be dicts (or empty lists), if the reply has them. Each one is a list of keys, e.g. `["versions", "clojure"]`. |
 
 `why` says what happens in the client when the reply doesn't have what
 the step needs, and `refs` point at the client code in question,
 starting from the profile's `code` (which is pinned to a commit, just
 like the links in `refs.go`). A check that sends code in some language
 should list the capability for it in `needs`, unless the client sends
-that code to any server (like CIDER's startup code).
+that code to any server and carries on when it fails (like CIDER's
+startup code). vim-fireplace sends its classpath code to any server too,
+but it can't connect without it, so its checks need `java` and servers
+it was never meant for don't fail them.
 
 To find out what a client sends, run it through `proof proxy -record
 client.toml`. That gives you a profile with a check for each connection,

doc/profiles.md

@@ -109,7 +109,8 @@ capability your profile doesn't declare are skipped.
 | Capability | When to set it | Checks that need it |
 |---|---|---|
 | `namespaces` | Your language has a notion of a current namespace that can be set with the `ns` field of a request (like Clojure). | `eval.ns`, `eval.unknown-ns` |
-| `clojure` | Your server evaluates Clojure, or a dialect close enough that the code Clojure clients send (e.g. `ns` forms) runs. | `cider.eval` |
+| `clojure` | Your server evaluates Clojure, or a dialect close enough that the code Clojure clients send (e.g. `ns` forms) runs. | `cider.eval`, `calva.eval`, `conjure.eval` |
+| `java` | Your server evaluates Clojure with Java interop (e.g. `System/getProperty`), like Clojure on the JVM and Babashka. | `fireplace.connect`, `fireplace.eval` |
 
 ## Snippets
 
@@ -120,7 +121,7 @@ snippet is missing, all the checks that need it are skipped.
 
 | Snippet | Options | What it should do | Checks that use it |
 |---|---|---|---|
-| `value` | `code`, `value` | evaluate to `value` | `eval.value`, `eval.survives-error`, `eval.ns`, `eval.unknown-ns`, `session.ephemeral`, `session.unknown`, `session.closed`, `cider.eval`, `cider.repl` |
+| `value` | `code`, `value` | evaluate to `value` | `eval.value`, `eval.survives-error`, `eval.ns`, `eval.unknown-ns`, `session.ephemeral`, `session.unknown`, `session.closed`, `cider.eval`, `cider.repl`, `calva.eval`, `conjure.eval`, `fireplace.eval` |
 | `stdout` | `code`, `out` | print `out` to the standard output | `eval.stdout`, `eval.stdout-order` |
 | `stderr` | `code`, `err` | print `err` to the standard error | `eval.stderr` |
 | `throw` | `code` | raise an error | `eval.error-status`, `eval.error-report`, `eval.survives-error` |

doc/usage.md

@@ -89,15 +89,16 @@ eval
          see: nrepl/nrepl#147 https://github.com/nrepl/nrepl/issues/147
 ```
 
-The checks in the `cider` group send what CIDER sends while connecting,
-evaluating code from a source buffer and evaluating code in its REPL,
-with all the extra fields CIDER puts in its requests. If one of them
-fails, CIDER won't work properly with your server, and the report says
-what CIDER does with the reply it got (e.g. "CIDER gives up
+The checks in the `cider`, `calva`, `conjure` and `fireplace` groups send
+what those clients send while connecting and evaluating code, with all
+the extra fields they put in their requests. If one of them fails, the
+client won't work properly with your server, and the report says what
+the client does with the reply it got (e.g. "CIDER gives up
 connecting"). The code they evaluate for the user is the `value`
-snippet of your profile. Before evaluating code from a source buffer,
-CIDER evaluates the buffer's `ns` form, so `cider.eval` also needs the
-`clojure` capability.
+snippet of your profile. Before evaluating code from a source file,
+CIDER, Calva and Conjure evaluate an `ns` form for the file's namespace,
+so those checks also need the `clojure` capability. vim-fireplace finds the classpath
+with Java interop while connecting, so its checks need `java`.
 
 Each check gets one of the following verdicts:
 
@@ -388,13 +389,15 @@ for the same code:
 | `(def x 1)`, `x`, `#'x`, `(resolve 'x)`, `@#'x` | Definitions, which all sessions share |
 | `(ns foo)`, `(in-ns 'foo)`, `*ns*` | Namespaces |
 | `*1`, `*2`, `*3`, `*e` | The last results and the last exception in the session |
-| `do`, `if`, `when`, `let`, `when-let` | The usual |
+| `do`, `if`, `when`, `or`, `let`, `when-let` | The usual |
+| `(System/getProperty "user.dir")` | The path separator, the working directory and `src` as the classpath, which is what vim-fireplace asks for |
 | `(require ...)` | Nothing, as there's nothing to load |
 
 Other functions get the error Clojure gives for a symbol it can't
 resolve, and syntax proof doesn't read (e.g. sets or anonymous functions)
-gets a read error. That's enough for CIDER to connect and work, and it's
-all you need for checking output, values, errors, input and interrupts.
+gets a read error. That's enough for CIDER, Calva, Conjure and
+vim-fireplace to connect and work, and it's all you need for checking
+output, values, errors, input and interrupts.
 
 When you stop it, `proof serve` checks the requests your client sent,
 just like `proof proxy` does, with the same report, exit codes and

internal/check/checktest/checktest.go

@@ -3,6 +3,7 @@
 package checktest
 
 import (
+	"maps"
 	"sort"
 	"testing"
 
@@ -43,3 +44,12 @@ func Verdicts(t testing.TB, results map[string]check.Result, want map[string]che
 		}
 	}
 }
+
+// Merged merges sets of verdicts, where the later ones win.
+func Merged(verdicts ...map[string]check.Verdict) map[string]check.Verdict {
+	all := map[string]check.Verdict{}
+	for _, v := range verdicts {
+		maps.Copy(all, v)
+	}
+	return all
+}

internal/checks/checks_test.go

@@ -1,20 +1,22 @@
 package checks
 
 import (
+	"slices"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/nrepl/proof/internal/check"
 	"github.com/nrepl/proof/internal/check/checktest"
 	"github.com/nrepl/proof/internal/profile"
+	"github.com/nrepl/proof/nrepl"
 )
 
 // The fake server understands these made-up forms; see fakeServer.eval.
 var fakeProfile = &profile.Profile{
 	Name:         "fake",
 	Timeout:      time.Second,
-	Capabilities: map[string]bool{"namespaces": true, "clojure": true},
+	Capabilities: map[string]bool{"namespaces": true, "clojure": true, "java": true},
 	Snippets: map[string]profile.Snippet{
 		"value":         {Code: "value", Value: "3"},
 		"stdout":        {Code: "stdout", Out: "proof"},
@@ -46,37 +48,41 @@ func TestWellBehavedServerPassesEverything(t *testing.T) {
 // other check and rule must still pass.
 func TestChecksCatchMisbehaviour(t *testing.T) {
 	F, W, S := check.Failed, check.Warned, check.Skipped
+	// What every client goes through first.
+	connects := map[string]check.Verdict{"cider.connect": F, "calva.connect": F, "conjure.connect": F, "fireplace.connect": F}
 	cases := []struct {
 		name string
 		q    quirks
 		want map[string]check.Verdict
 	}{
-		{"ops as a list", quirks{opsList: true}, map[string]check.Verdict{"describe.ops-dict": F, "describe.required-ops": S,
-			"stdin.need-input": S, "stdin.roundtrip": S, "stdin.eof": S, "cider.connect": F}},
+		// Conjure does without the features that need ops.
+		{"ops as a list", quirks{opsList: true}, checktest.Merged(connects, map[string]check.Verdict{"describe.ops-dict": F,
+			"describe.required-ops": S, "stdin.need-input": S, "stdin.roundtrip": S, "stdin.eof": S, "conjure.connect": check.Pass})},
 		{"clone not advertised", quirks{noClone: true}, map[string]check.Verdict{"describe.required-ops": F}},
 		{"no versions", quirks{noVersions: true}, map[string]check.Verdict{"describe.versions": W}},
-		{"describe kills the connection", quirks{crashOnDescribe: true}, map[string]check.Verdict{
+		{"describe kills the connection", quirks{crashOnDescribe: true}, checktest.Merged(connects, map[string]check.Verdict{
 			"describe.reply": F, "describe.ops-dict": S, "describe.required-ops": S, "describe.versions": S,
-			"stdin.need-input": S, "stdin.roundtrip": S, "stdin.eof": S, "cider.connect": F}},
+			"stdin.need-input": S, "stdin.roundtrip": S, "stdin.eof": S})},
 		{"no unknown-op", quirks{noUnknownOp: true}, map[string]check.Verdict{"op.unknown": F, "op.unknown-echo": W}},
 		{"no op echo", quirks{noOpEcho: true}, map[string]check.Verdict{"op.unknown-echo": W}},
+		// Conjure asks for ls-sessions, which the fake doesn't know.
 		{"status is a string", quirks{statusString: true}, map[string]check.Verdict{
-			"op.unknown": F, "op.unknown-echo": F, "wire.status-type": F}},
+			"op.unknown": F, "op.unknown-echo": F, "wire.status-type": F, "conjure.connect": F}},
 		{"no session-closed", quirks{noSessionClosed: true}, map[string]check.Verdict{"session.close": F}},
 		// Only clients put dicts and lists in their requests.
 		{"flat fields only", quirks{flatFields: true}, map[string]check.Verdict{
-			"cider.connect": F, "cider.eval": F, "cider.repl": F}},
+			"cider.connect": F, "cider.eval": F, "cider.repl": F, "calva.connect": F, "calva.eval": F, "conjure.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}},
 		{"sessions tied to sockets", quirks{socketSessions: true}, map[string]check.Verdict{"session.across-connections": W}},
-		{"no ephemeral sessions", quirks{noEphemeral: true}, map[string]check.Verdict{"session.ephemeral": F}},
-		{"last value only", quirks{lastValueOnly: true}, map[string]check.Verdict{"eval.multiple-forms": F}},
+		{"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}},
 		{"output after value", quirks{outAfterValue: true}, map[string]check.Verdict{"eval.stdout-order": W}},
 		{"no eval-error", quirks{noEvalError: true}, map[string]check.Verdict{"eval.error-status": F}},
 		{"no ex", quirks{noEx: true}, map[string]check.Verdict{"eval.error-report": W}},
 		{"no no-code", quirks{noNoCode: true}, map[string]check.Verdict{"eval.no-code": W}},
-		{"no ns", quirks{noNs: true}, map[string]check.Verdict{"eval.ns": W}},
+		{"no ns", quirks{noNs: true}, map[string]check.Verdict{"eval.ns": W, "fireplace.connect": F}},
 		{"ns fallback", quirks{nsFallback: true}, map[string]check.Verdict{"eval.unknown-ns": F}},
 		{"two dones", quirks{twoDones: true}, map[string]check.Verdict{"wire.one-done": W}},
 		{"value after done", quirks{valueAfterDone: true}, map[string]check.Verdict{"wire.after-done": W, "wire.error-terminal": F}},
@@ -87,7 +93,7 @@ func TestChecksCatchMisbehaviour(t *testing.T) {
 			"wire.id": W, "eval.stdout": F, "eval.stderr": F, "eval.error-report": W}},
 		{"integer value", quirks{intValue: true}, 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}},
+			"session.ephemeral": F, "session.persistent": F, "cider.eval": F, "cider.repl": F, "calva.eval": F, "conjure.eval": F, "fireplace.eval": 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{
@@ -132,3 +138,39 @@ func TestBrokenCloneSkipsSessionChecksQuickly(t *testing.T) {
 		}
 	}
 }
+
+// nREPL goes on with the next form after one throws, so what the next form
+// sends isn't late.
+func TestOnlyTheFormThatThrewIsOver(t *testing.T) {
+	// Only Clojure code can be told apart into forms.
+	for code, want := range map[string]check.Verdict{`(/ 1 0) (println "proof")`: check.Pass, "(/ 1 0)": check.Failed,
+		`raise "proof"`: check.Failed} {
+		events := []nrepl.Event{
+			{Dir: nrepl.Sent, Msg: nrepl.Message{"id": "1", "op": "eval", "code": code}},
+			{Dir: nrepl.Received, Msg: nrepl.Message{"id": "1", "status": []any{"eval-error"}}},
+			{Dir: nrepl.Received, Msg: nrepl.Message{"id": "1", "out": "proof"}},
+			{Dir: nrepl.Received, Msg: nrepl.Message{"id": "1", "status": []any{"done"}}},
+		}
+		results := checktest.ByID(check.Grade(WireRules(), []check.Traffic{{Label: "test", Events: events}}))
+		if got := results["wire.error-terminal"].Verdict; got != want {
+			t.Errorf("%q: got %s, want %s", code, got, want)
+		}
+	}
+}
+
+func TestForms(t *testing.T) {
+	cases := map[string][]string{
+		"value":                       {"value"},
+		"1 2\n":                       {"1", "2"},
+		`(f "a ) b" [1 2]) {:a 1}`:    {`(f "a ) b" [1 2])`, "{:a 1}"},
+		`(str "\"" ")") x`:            {`(str "\"" ")")`, "x"},
+		"1, 2 ; (3 \"\n4":             {"1", "2", "4"},
+		`(= c \() (a)(b) x;c` + "\ny": {`(= c \()`, "(a)", "(b)", "x", "y"},
+		"":                            nil,
+	}
+	for code, want := range cases {
+		if got := forms(code); !slices.Equal(got, want) {
+			t.Errorf("forms(%q) = %q, want %q", code, got, want)
+		}
+	}
+}

internal/checks/client_profiles.go

@@ -51,6 +51,10 @@ type clientStep struct {
 	Snippet string `toml:"snippet"`
 	// NewSession names the session the reply has to hand back.
 	NewSession string `toml:"new-session"`
+	// Values is how many values the reply has to have at least, told apart
+	// the way some clients do it: by the ns that comes with each one, or
+	// after it.
+	Values int `toml:"values"`
 	// Dicts are fields that have to be dicts if the reply has them, each
 	// a path of keys (e.g. ["versions", "clojure"]). An empty list will do
 	// too, as clients can't tell the two apart.
@@ -167,6 +171,9 @@ func (c clientCheck) replay(t *check.T) {
 				t.Stopf("%s gave the value %q instead of %q, so %s", step, got, want, s.Why)
 			}
 		}
+		if n := valuesApart(resp); n < s.Values {
+			t.Stopf("%s gave %d values that can be told apart by their ns, not %d, so %s", step, n, s.Values, s.Why)
+		}
 		for _, path := range s.Dicts {
 			if v := nonDictField(resp, path); v != nil {
 				t.Stopf("%s has %s that is %s, not a dict, so %s", step, strings.Join(path, "."), typeName(v), s.Why)
@@ -175,6 +182,25 @@ func (c clientCheck) replay(t *check.T) {
 	}
 }
 
+// valuesApart counts the values of a reply the way clients that tell
+// them apart by ns do, where the parts of a value up to the next ns are
+// one value.
+func valuesApart(resp nrepl.Response) int {
+	n, open := 0, false
+	for _, m := range resp.Messages {
+		if v, _ := m["value"].(string); v != "" {
+			open = true
+		}
+		if m.Has("ns") && open {
+			n, open = n+1, false
+		}
+	}
+	if open {
+		n++
+	}
+	return n
+}
+
 // nonDictField returns the field at path in a reply (e.g. versions, then
 // clojure) unless it's a dict, an empty list or missing.
 func nonDictField(resp nrepl.Response, path []string) any {