Fix the rough edges of running proof

PLAN.md

@@ -35,21 +35,19 @@ lookups and completions, plus requests that go wrong.
 
 ### Rough Edges
 
-- `wire.error-terminal` fails Babashka for a message with nothing but
-  `changed-namespaces` that arrives after `eval-error`. The rule fails
-  on any key that's not in `afterErrorKeys` (`internal/checks/wire.go`),
-  but the clients it names (REPLy and rebel-readline) lose only output,
-  values and `need-input` there, so it should look for those instead.
-- `proof run babashka` and `proof proxy babashka` don't work, even
-  though `proof serve -like babashka` does. The profiles should be
-  embedded in the binary, as people who install proof with `go install`
-  don't have the `profiles` folder at all.
-- `proof run profiles/babashka.toml -only 'eval.*'` fails with
-  "expected one profile", as flags after the profile are taken for
-  arguments. proof should either accept them there or say that flags go
-  first.
-- `proof run -h` prints Go's bare "Usage of run:", and the same goes for
-  `proxy` and `serve`.
+- Done: `wire.error-terminal` no longer fails Babashka for a message
+  with nothing but `changed-namespaces` that arrives after `eval-error`.
+  The clients it names (REPLy and rebel-readline) lose only output,
+  values and `need-input` there, so that's what it looks for now.
+- Done: `proof run babashka` and `proof proxy babashka` work, just like
+  `proof serve -like babashka`, as the profiles are embedded in the
+  binary. People who install proof with `go install` don't have the
+  `profiles` folder at all.
+- Done: `proof run profiles/babashka.toml -only 'eval.*'` works, as
+  flags can come after the profile (and after the scenarios of `proof
+  serve`).
+- Done: `proof run -h` says how to use `proof run`, rather than Go's bare
+  "Usage of run:", and the same goes for `proxy` and `serve`.
 
 ### Releases
 

README.md

@@ -46,7 +46,9 @@ value = "3"
 
 The [profiles](profiles) folder has complete profiles for nREPL itself,
 Babashka, Basilisp, ClojureCLR, jank, dialtone (Erlang) and repartee
-(Elixir), which you can use as a starting point.
+(Elixir), which you can use as a starting point. They come with proof as
+well, so `proof run babashka` will check Babashka (provided you have it
+installed).
 
 Now you can run the checks:
 
@@ -84,7 +86,7 @@ If you're working on a client, `proof proxy` can sit between it and a
 server and record everything the two of them say to each other:
 
 ```shell
-$ proof proxy -listen 127.0.0.1:7888 profiles/babashka.toml
+$ proof proxy -listen 127.0.0.1:7888 babashka
 ```
 
 Connect your client to port 7888, use it for a while (or run its test

cmd/proof/main.go

@@ -4,6 +4,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -34,6 +35,9 @@ Usage:
   proof list                          list every check, rule and scenario
   proof version
 
+PROFILE is a profile file or the name of a profile that comes with proof
+(e.g. babashka), and proof list shows their names.
+
 The exit status of run is 0 when everything passed (or failed as the
 profile expects), 1 when the server failed checks, 2 when proof couldn't
 start (bad flags or profile, or the server didn't come up), and 3 when some
@@ -87,9 +91,51 @@ type options struct {
 
 var opts options
 
-func runFlags(out io.Writer) *flag.FlagSet {
-	fs := flag.NewFlagSet("run", flag.ContinueOnError)
+// newFlags makes the flag set of a command, whose -h says how to use it
+// with the given arguments.
+func newFlags(name, args string, out io.Writer) *flag.FlagSet {
+	fs := flag.NewFlagSet(name, flag.ContinueOnError)
 	fs.SetOutput(out)
+	fs.Usage = func() {
+		fmt.Fprintf(fs.Output(), "Usage: proof %s [flags] %s\n\nFlags:\n", name, args)
+		fs.PrintDefaults()
+	}
+	return fs
+}
+
+// parseArgs parses the flags in args wherever they are, as the flag
+// package stops at the first argument that isn't one, and returns the
+// other arguments. Everything after "--" is an argument.
+func parseArgs(fs *flag.FlagSet, args []string) ([]string, error) {
+	var rest []string
+	for {
+		if err := fs.Parse(args); err != nil {
+			return nil, err
+		}
+		parsed := len(args) - fs.NArg()
+		if parsed > 0 && args[parsed-1] == "--" {
+			return append(rest, fs.Args()...), nil
+		}
+		args = fs.Args()
+		if len(args) == 0 {
+			return rest, nil
+		}
+		rest = append(rest, args[0])
+		args = args[1:]
+	}
+}
+
+// flagStatus is the exit status for an error from parseArgs, which has
+// shown how to use the command already. Asking for that with -h is fine.
+func flagStatus(err error) int {
+	if errors.Is(err, flag.ErrHelp) {
+		return 0
+	}
+	return 2
+}
+
+func runFlags(out io.Writer) *flag.FlagSet {
+	fs := newFlags("run", "PROFILE", out)
 	fs.StringVar(&opts.address, "address", "", "connect to a server already running at `host:port` instead of launching one")
 	fs.StringVar(&opts.json, "json", "", "also write a JSON report to `file`")
 	fs.StringVar(&opts.only, "only", "", "run only checks whose id matches the glob `pattern` (e.g. 'eval.*')")
@@ -99,14 +145,15 @@ func runFlags(out io.Writer) *flag.FlagSet {
 
 func run(args []string) int {
 	fs := runFlags(os.Stderr)
-	if err := fs.Parse(args); err != nil {
-		return 2
+	args, err := parseArgs(fs, args)
+	if err != nil {
+		return flagStatus(err)
 	}
-	if fs.NArg() != 1 {
+	if len(args) != 1 {
 		fmt.Fprintln(os.Stderr, "proof run: expected one profile")
 		return 2
 	}
-	p, err := profile.Load(fs.Arg(0))
+	p, err := profile.Find(args[0])
 	if err != nil {
 		fmt.Fprintln(os.Stderr, "proof:", err)
 		return 2
@@ -115,7 +162,7 @@ func run(args []string) int {
 	// Catch mistakes in the profile or flags before paying for a server
 	// start, which can take minutes.
 	if err := check.UnknownIDs(p.ExpectedFailures, allIDs()); err != nil {
-		fmt.Fprintf(os.Stderr, "proof: %s: %v\n", fs.Arg(0), err)
+		fmt.Fprintf(os.Stderr, "proof: %s: %v\n", args[0], err)
 		return 2
 	}
 	selected, err := filter(checks.All(), opts.only)
@@ -293,7 +340,7 @@ func list(w io.Writer) {
 	for _, s := range serve.Scenarios() {
 		fmt.Fprintf(w, "%-*s %s (%s)\n", nameWidth, s.Name, s.Title, s.Who)
 	}
-	fmt.Fprint(w, "\nServers for proof serve -like, with their scenarios:\n")
+	fmt.Fprint(w, "\nProfiles that come with proof, and their scenarios for proof serve -like:\n")
 	for _, p := range serve.Presets() {
 		line := fmt.Sprintf("%-*s", nameWidth, p.Name)
 		for i, s := range p.Scenarios {

cmd/proof/proxy.go

@@ -41,8 +41,7 @@ type proxyOptions struct {
 }
 
 func proxyFlags(out io.Writer, o *proxyOptions) *flag.FlagSet {
-	fs := flag.NewFlagSet("proxy", flag.ContinueOnError)
-	fs.SetOutput(out)
+	fs := newFlags("proxy", "[PROFILE]", out)
 	fs.StringVar(&o.address, "address", "", "forward clients to a server already running at `host:port` instead of launching the one the profile describes")
 	o.register(fs)
 	return fs
@@ -74,18 +73,18 @@ func proxyUntil(ctx context.Context, args []string, stdout, stderr io.Writer, li
 	stderr = &syncWriter{w: stderr}
 	var o proxyOptions
 	fs := proxyFlags(stderr, &o)
-	if err := fs.Parse(args); err != nil {
-		return 2
+	args, err := parseArgs(fs, args)
+	if err != nil {
+		return flagStatus(err)
 	}
-	if fs.NArg() > 1 || (fs.NArg() == 0 && o.address == "") {
+	if len(args) > 1 || (len(args) == 0 && o.address == "") {
 		fmt.Fprintln(stderr, "proof proxy: expected a profile or -address")
 		return 2
 	}
 	upstream, name := o.address, "the server"
 	var p *profile.Profile
-	if fs.NArg() == 1 {
-		var err error
-		if p, err = profile.Load(fs.Arg(0)); err != nil {
+	if len(args) == 1 {
+		if p, err = profile.Find(args[0]); err != nil {
 			fmt.Fprintln(stderr, "proof:", err)
 			return 2
 		}
@@ -98,7 +97,6 @@ func proxyUntil(ctx context.Context, args []string, stdout, stderr io.Writer, li
 	var srv *server.Server
 	if upstream == "" {
 		fmt.Fprintf(stderr, "Starting %s...\n", p.Name)
-		var err error
 		if srv, err = server.Start(ctx, p); err != nil {
 			if ctx.Err() != nil {
 				return 130

cmd/proof/proxy_test.go

@@ -157,6 +157,35 @@ func TestProxyNeedsAServer(t *testing.T) {
 	}
 }
 
+func TestProxyHelp(t *testing.T) {
+	var stderr bytes.Buffer
+	if code := proxyUntil(context.Background(), []string{"-h"}, io.Discard, &stderr, nil); code != 0 {
+		t.Errorf("exit status %d, want 0", code)
+	}
+	if !strings.Contains(stderr.String(), "Usage: proof proxy [flags] [PROFILE]") {
+		t.Errorf("-h doesn't say how to use proof proxy:\n%s", stderr.String())
+	}
+}
+
+func TestFlagsCanFollowArguments(t *testing.T) {
+	var o proxyOptions
+	fs := proxyFlags(io.Discard, &o)
+	args, err := parseArgs(fs, []string{"-v", "babashka", "-listen", "127.0.0.1:7888", "extra", "-json", "r.json"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Join(args, " ") != "babashka extra" || !o.verbose || o.listen != "127.0.0.1:7888" || o.json != "r.json" {
+		t.Errorf("got arguments %q and options %+v", args, o)
+	}
+	if _, err := parseArgs(fs, []string{"babashka", "-no-such-flag"}); err == nil {
+		t.Error("an unknown flag after an argument should be an error")
+	}
+	args, err = parseArgs(fs, []string{"a", "--", "-b", "c"})
+	if err != nil || strings.Join(args, " ") != "a -b c" {
+		t.Errorf("everything after -- should be an argument, got %q (%v)", args, err)
+	}
+}
+
 func TestListShowsEverything(t *testing.T) {
 	var buf bytes.Buffer
 	list(&buf)

cmd/proof/serve.go

@@ -8,7 +8,6 @@ import (
 	"os"
 	"os/signal"
 	"path/filepath"
-	"slices"
 	"strings"
 	"syscall"
 	"time"
@@ -23,8 +22,7 @@ type serveOptions struct {
 }
 
 func serveFlags(out io.Writer, o *serveOptions) *flag.FlagSet {
-	fs := flag.NewFlagSet("serve", flag.ContinueOnError)
-	fs.SetOutput(out)
+	fs := newFlags("serve", "[SCENARIO...]", out)
 	fs.StringVar(&o.like, "like", "", "behave like the server whose `profile` has this name (e.g. jank or profiles/jank.toml), along with any scenarios given")
 	o.register(fs)
 	return fs
@@ -45,18 +43,13 @@ func serveUntil(ctx context.Context, args []string, stdout, stderr io.Writer, li
 	stderr = &syncWriter{w: stderr}
 	var o serveOptions
 	fs := serveFlags(stderr, &o)
-	if err := fs.Parse(args); err != nil {
-		return 2
-	}
-	scenarios, shown := fs.Args(), fs.Args()
-	// The flag package stops at the first scenario.
-	if i := slices.IndexFunc(scenarios, func(s string) bool { return strings.HasPrefix(s, "-") }); i >= 0 {
-		fmt.Fprintf(stderr, "proof: %s comes after a scenario, but flags have to go first\n", scenarios[i])
-		return 2
+	scenarios, err := parseArgs(fs, args)
+	if err != nil {
+		return flagStatus(err)
 	}
+	shown := scenarios
 	if o.like != "" {
 		name := strings.TrimSuffix(filepath.Base(o.like), ".toml")
-		var err error
 		if scenarios, err = serve.Like(name, scenarios); err != nil {
 			fmt.Fprintln(stderr, "proof:", err)
 			return 2

cmd/proof/serve_test.go

@@ -26,7 +26,9 @@ func TestServeExitCodes(t *testing.T) {
 			"client traffic to proof serve (like jank)"},
 		{"like a server, but not quite", []string{"-like", "jank", "ns-error"}, nil, 2,
 			"scenarios ns-fallback and ns-error don't go together"},
-		{"flag after the scenarios", []string{"byte-writes", "-like", "jank"}, nil, 2, "-like comes after a scenario, but flags have to go first"},
+		{"flag after the scenarios", []string{"byte-writes", "-like", "jank"}, []string{"d2:id1:12:op8:describee"}, 0,
+			"client traffic to proof serve (like jank, byte-writes)"},
+		{"help", []string{"-h"}, nil, 0, "Usage: proof serve [flags] [SCENARIO...]"},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {

doc/design.md

@@ -269,6 +269,7 @@ internal/server      starting servers
 internal/profile     loading profiles
 nrepl                a client that records all messages
 bencode              a strict bencode implementation
+profiles             the profiles of the servers in the matrix, embedded in the binary
 ```
 
 Each package depends only on the ones below it. `internal/check` knows

doc/hacking.md

@@ -35,6 +35,10 @@ $ bin/proof run profiles/babashka.toml
 $ bin/proof run profiles/clojure.toml
 ```
 
+Note that `bin/proof run clojure` would use the copy of the profile
+that's embedded in the binary, so while you're working on a profile it's
+best to pass its path.
+
 ## Codebase Overview
 
 | Path | Contents |
@@ -50,7 +54,7 @@ $ bin/proof run profiles/clojure.toml
 | `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`). |
 | `internal/report` | Text and JSON reports and the compatibility matrix. |
-| `profiles` | The profiles for the servers in the compatibility matrix. |
+| `profiles` | The profiles for the servers in the compatibility matrix. `profiles/profiles.go` embeds them in the binary, so they can be used by name. |
 | `doc/spec-changes.md` | All the gaps and disagreements found in the draft spec. |
 
 ## Testing

doc/usage.md

@@ -40,6 +40,11 @@ closest to your server. If you're working on a Clojure dialect you can
 probably use `clojure.toml` almost as is, otherwise `dialtone.toml` is a
 better starting point.
 
+The profiles in this folder come with proof, so you can also use them by
+name instead of passing a file (e.g. `proof run babashka`). If you've
+made a copy of one of them, pass its path instead (e.g. `proof run
+babashka.toml`).
+
 Here's a minimal profile:
 
 ```toml
@@ -202,7 +207,7 @@ proof:
 with one row per check and one column per server:
 
 ```shell
-$ proof run -json clojure.json profiles/clojure.toml
+$ proof run -json clojure.json clojure
 $ proof run -json my-server.json my-server.toml
 $ proof matrix clojure.json my-server.json > matrix.md
 ```
@@ -224,7 +229,7 @@ running) and forwards everything between your client and the server,
 recording every message on the way:
 
 ```shell
-$ proof proxy -listen 127.0.0.1:7888 profiles/clojure.toml
+$ proof proxy -listen 127.0.0.1:7888 clojure
 Starting Clojure (nrepl/nrepl 1.8.0)...
 Forwarding 127.0.0.1:7888 to localhost:53613. Connect your client to 127.0.0.1:7888 and press Ctrl-C when it's done.
 ```
@@ -265,8 +270,8 @@ the chance.
 
 It's a good idea to try your client with a few servers, as they don't
 all support the same ops. Any profile from the [profiles](../profiles)
-folder will do, and with `-address` proof will forward your client to a
-server that's already running:
+folder will do (e.g. `proof proxy babashka`), and with `-address` proof
+will forward your client to a server that's already running:
 
 ```shell
 $ proof proxy -address localhost:1667
@@ -290,7 +295,7 @@ the proxy in the background, wait for it to accept connections, run the
 tests against it and stop it with `SIGINT`:
 
 ```shell
-proof proxy -listen 127.0.0.1:7888 profiles/babashka.toml &
+proof proxy -listen 127.0.0.1:7888 babashka &
 proxy=$!
 until nc -z 127.0.0.1 7888; do
   kill -0 $proxy || exit 2

internal/checks/checks_test.go

@@ -90,8 +90,12 @@ func TestChecksCatchMisbehaviour(t *testing.T) {
 		{"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}},
+		// REPLy and rebel-readline don't read changed-namespaces, so it
+		// doesn't matter that they miss it after an error.
+		{"state after done", quirks{stateAfterDone: true}, map[string]check.Verdict{"wire.after-done": W}},
 		{"out and value together", quirks{payloadTogether: true}, map[string]check.Verdict{"wire.one-payload": F}},
 		{"output after eval-error", quirks{errorThenOut: true}, map[string]check.Verdict{"wire.error-terminal": F}},
+		{"state after eval-error", quirks{errorThenState: true}, map[string]check.Verdict{}},
 		{"session not echoed", quirks{noSessionEcho: true}, map[string]check.Verdict{"wire.session-echo": F}},
 		{"output without id", quirks{noID: true}, map[string]check.Verdict{
 			"wire.id": W, "eval.stdout": F, "eval.stderr": F, "eval.error-report": W}},