Check servers with the requests CIDER actually sends

README.md

@@ -73,6 +73,10 @@ server (or that users will lose data). If your server simply does
 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.
+
 ## Checking Clients
 
 If you're working on a client, `proof proxy` can sit between it and a
@@ -123,16 +127,17 @@ 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),
-along with the requests clients send and the server differences clients
-have to deal with, and `proof list` will show you all the checks.
+what CIDER sends 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.
 
 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)
-- replaying what real clients send (e.g. when CIDER or Calva connect to a
-  server) as client profiles
+- 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

@@ -116,6 +116,24 @@ This split keeps the regular checks simple. A check doesn't have to
 verify that every response has the right `id`, for instance, as the wire
 checks take care of that for all of them.
 
+## Client Profiles
+
+The checks above ask each question in the simplest way possible, which
+doesn't tell you much about the requests real clients send. CIDER, for
+instance, sends the file, line and column of the code it evaluates,
+along with a bunch of options for printing the result (one of them a
+nested dict, another an empty list). A server that can't handle any of
+those breaks CIDER, even if it passes every other check.
+
+Client profiles fill that gap. A client profile says what one client
+sends in a few situations (e.g. while connecting) and what the client
+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.
+
 ## Strict About the Wire, Relaxed About the Rest
 
 proof has its own bencode implementation, as the popular Go libraries
@@ -236,7 +254,7 @@ Here's how the codebase is organized:
 ```
 cmd/proof            the command-line interface
 internal/report      text and JSON reports, the compatibility matrix
-internal/checks      the checks, the wire checks, the client rules and a fake server for testing them
+internal/checks      the checks, the wire checks, the client rules, the client profiles and a fake server for testing them
 internal/proxy       forwarding the traffic between a client and a server
 internal/serve       a server for client tests, which acts like other servers on request
 internal/clients     accepting clients and recording what they say, for proxy and serve
@@ -289,10 +307,9 @@ 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 that replay what specific clients send (e.g. when CIDER
-  or Calva connect to a server), so a report can tell you
-  directly whether CIDER will work with your server (`proof proxy`
-  already sees this traffic, it just doesn't save it yet)
+- client profiles for more clients (e.g. Calva, Conjure and
+  vim-fireplace), and a way to record them with `proof proxy`, which
+  already sees the traffic but doesn't save it yet
 - 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

@@ -45,7 +45,7 @@ $ bin/proof run profiles/clojure.toml
 | `internal/profile` | Loading and validating profiles. |
 | `internal/server` | Starting servers and figuring out their ports. |
 | `internal/check` | The checks framework (`Check`, `T`, `Rule`), grading and expected failures. It doesn't know anything about specific ops. `checktest` has helpers for testing checks. |
-| `internal/checks` | The checks (`describe.go`, `op.go`, `session.go` and `eval.go`), the wire checks (`wire.go`), the client rules (`client.go`), the links to client and server code (`refs.go`), the fake server used to test all of them (`fake_test.go`) and a scripted client for testing the client rules (`client_test.go`). |
+| `internal/checks` | The checks (`describe.go`, `op.go`, `session.go` and `eval.go`), the wire checks (`wire.go`), the client rules (`client.go`), the client profiles (`clients/` and `client_profiles.go`), the links to client and server code (`refs.go`), the fake server used to test all of them (`fake_test.go`) and a scripted client for testing the client rules (`client_test.go`). |
 | `internal/clients` | Accepting clients, recording what they say and stopping, for `proof proxy` and `proof serve`. |
 | `internal/proxy` | Forwarding the traffic between a client and a server, for `proof proxy`. |
 | `internal/serve` | The server behind `proof serve`: its piece of Clojure (`lang.go` and `eval.go`), the scenarios (`scenarios.go`), sessions (`session.go`) and the server itself (`serve.go`). |
@@ -225,6 +225,49 @@ to a socket and prints the replies is all you need for that.
 Every client rule needs a quirk in `clientQuirks` and an entry in the
 table in `TestClientRulesCatchMistakes` (both in `client_test.go`).
 
+## Adding a Client Profile
+
+The client profiles live in `internal/checks/clients`, one TOML file per
+client, and proof reads them when it starts. Here's a check from
+`cider.toml`:
+
+```toml
+[[checks]]
+id = "cider.eval"
+title = "CIDER can evaluate code from a source buffer"
+why = "Evaluating code from a source buffer sends ..."
+needs = ["clojure"]
+
+[[checks.steps]]
+send = { op = "clone", client-name = "CIDER", client-version = "2.1.0-snapshot" }
+new-session = "repl"
+why = "CIDER gives up connecting"
+refs = ["nrepl-client.el#L749-L760"]
+```
+
+Each step sends a request and says what the client needs from the reply
+besides `done`:
+
+| Option | What the reply needs |
+|---|---|
+| `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. |
+| `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).
+
+To find out what a client sends, run it through `proof proxy` with `-v`,
+which shows every request. Then read the client's code to see what it
+does with each reply, and keep only what the client really needs.
+`TestClientProfiles` makes sure a profile hangs together, and the fake
+server should get a quirk for anything a profile catches that the other
+checks don't.
+
 ## Adding a Scenario
 
 The scenarios of `proof serve` live in `internal/serve/scenarios.go`.

doc/profiles.md

@@ -109,6 +109,7 @@ 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` |
 
 ## Snippets
 
@@ -119,7 +120,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` |
+| `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` |
 | `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,6 +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
+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.
+
 Each check gets one of the following verdicts:
 
 | Verdict | Meaning |

internal/checks/all.go

@@ -10,5 +10,6 @@ func All() []*check.Check {
 	all = append(all, sessionChecks()...)
 	all = append(all, evalChecks()...)
 	all = append(all, stdinChecks()...)
+	all = append(all, clientProfileChecks()...)
 	return all
 }

internal/checks/checks_test.go

@@ -14,7 +14,7 @@ import (
 var fakeProfile = &profile.Profile{
 	Name:         "fake",
 	Timeout:      time.Second,
-	Capabilities: map[string]bool{"namespaces": true},
+	Capabilities: map[string]bool{"namespaces": true, "clojure": true},
 	Snippets: map[string]profile.Snippet{
 		"value":         {Code: "value", Value: "3"},
 		"stdout":        {Code: "stdout", Out: "proof"},
@@ -28,9 +28,14 @@ var fakeProfile = &profile.Profile{
 }
 
 func runFake(t *testing.T, q quirks) map[string]check.Result {
+	t.Helper()
+	return runFakeChecks(t, q, All(), WireRules()...)
+}
+
+func runFakeChecks(t *testing.T, q quirks, checks []*check.Check, rules ...*check.Rule) map[string]check.Result {
 	t.Helper()
 	env := &check.Env{Profile: fakeProfile, Addr: startFake(t, q), Settle: 20 * time.Millisecond}
-	return checktest.ByID(check.Run(env, All(), WireRules()))
+	return checktest.ByID(check.Run(env, checks, rules))
 }
 
 func TestWellBehavedServerPassesEverything(t *testing.T) {
@@ -47,17 +52,20 @@ func TestChecksCatchMisbehaviour(t *testing.T) {
 		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}},
+			"stdin.need-input": S, "stdin.roundtrip": S, "stdin.eof": S, "cider.connect": F}},
 		{"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.reply": F, "describe.ops-dict": S, "describe.required-ops": S, "describe.versions": S,
-			"stdin.need-input": S, "stdin.roundtrip": S, "stdin.eof": S}},
+			"stdin.need-input": S, "stdin.roundtrip": S, "stdin.eof": S, "cider.connect": F}},
 		{"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}},
 		{"status is a string", quirks{statusString: true}, map[string]check.Verdict{
 			"op.unknown": F, "op.unknown-echo": F, "wire.status-type": 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}},
 		{"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}},
@@ -79,7 +87,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}},
+			"session.ephemeral": F, "session.persistent": F, "cider.eval": F, "cider.repl": 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{