diff --git a/CHANGELOG.md b/CHANGELOG.md index e08e44d..b1f7c4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## main (unreleased) +### Features + +- [#23](https://github.com/nrepl/proof/pull/23): A clients tab on the compatibility matrix page, with how many of the checks each client depends on every server passes. + ### Changes - [#22](https://github.com/nrepl/proof/pull/22): The bundled profiles have short names, with versions from their `version` snippets, so the servers in the compatibility matrix get columns of the same width. diff --git a/cmd/proof/main.go b/cmd/proof/main.go index ae00dd3..8525e94 100644 --- a/cmd/proof/main.go +++ b/cmd/proof/main.go @@ -217,6 +217,9 @@ func run(args []string) int { env := &check.Env{Profile: p, Addr: addr} started := time.Now() results := check.Run(env, selected, checks.WireRules()) + for i, res := range results { + results[i].Clients = checks.ClientsOf(res.Severity, res.Refs) + } outcome := check.Apply(results, p.ExpectedFailures, opts.only == "") r := report.Run{Proof: proofVersion(), Server: p.Name, ServerVersion: ver, Homepage: p.Homepage, diff --git a/doc/usage.md b/doc/usage.md index d98ef3b..9e1dcc5 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -236,6 +236,12 @@ shows why it matters and hovering over a verdict shows its details: $ proof matrix -html clojure.json my-server.json > matrix.html ``` +The page has a second tab for the clients proof knows about, with one +row per client. There each server gets the number of checks it passes out +of the ones that would break the client if they failed: the client's own +checks (e.g. `cider.connect`), the checks that link to its code, and the +ones every client depends on (e.g. `eval.value`). + > [!TIP] > proof's own CI builds such a page for all the profiles in the > repository on a daily basis. You can find it at diff --git a/internal/check/check.go b/internal/check/check.go index 9034580..58518ae 100644 --- a/internal/check/check.go +++ b/internal/check/check.go @@ -286,6 +286,9 @@ type Result struct { // Expected holds the reason from the profile's expected failures when // the check failed as expected. Expected string + // Clients names the clients that break when the server fails the + // check. + Clients []string } // Run runs checks one at a time, then grades the wire traffic they diff --git a/internal/checks/client_profiles.go b/internal/checks/client_profiles.go index 4ec91ea..4ab3927 100644 --- a/internal/checks/client_profiles.go +++ b/internal/checks/client_profiles.go @@ -231,3 +231,24 @@ func nonDictField(resp nrepl.Response, path []string) any { } return v } + +// ClientsOf names the clients that break when a server fails a check or +// rule with the given severity and refs: the ones with a client profile +// whose code the refs point at, or every one of them when the refs don't +// point at any (e.g. a value that never comes). A warning breaks nobody. +func ClientsOf(severity check.Severity, refs []check.Ref) []string { + if severity != check.Fail { + return nil + } + var cited, all []string + for _, p := range clientProfiles() { + all = append(all, p.Name) + if slices.ContainsFunc(refs, func(r check.Ref) bool { return strings.HasPrefix(r.URL, p.Code) }) { + cited = append(cited, p.Name) + } + } + if len(cited) == 0 { + return all + } + return cited +} diff --git a/internal/checks/client_profiles_test.go b/internal/checks/client_profiles_test.go index 6172a1b..85c2f73 100644 --- a/internal/checks/client_profiles_test.go +++ b/internal/checks/client_profiles_test.go @@ -2,6 +2,7 @@ package checks import ( "regexp" + "slices" "strings" "testing" @@ -20,9 +21,10 @@ func TestClientProfiles(t *testing.T) { ids[c.ID] = true } pinned := regexp.MustCompile(`^https://github\.com/[^/]+/[^/]+/blob/[0-9a-f]{40}/`) - // The links in refs.go to the same clients. + // The links in refs.go to the same clients, which is how a check knows + // the clients it breaks. bases := map[string]string{"CIDER": ciderBase, "Calva": calvaSrc, "Conjure": conjureSrc, "vim-fireplace": fireplaceBase, - "REPLy": strings.TrimSuffix(replyBase, "eval_modes/nrepl.clj")} + "REPLy": strings.TrimSuffix(replyBase, "eval_modes/nrepl.clj"), "neorepl": neoreplBase} for _, p := range clientProfiles() { if p.Name == "" || !pinned.MatchString(p.Code) { t.Errorf("%q needs a name and links pinned to a commit, got %q", p.Name, p.Code) @@ -56,6 +58,56 @@ func TestClientProfiles(t *testing.T) { } } +// A check breaks the clients whose code it links to, or all of them when +// it links to none, unless it's only a warning. Every client has checks +// of its own. +func TestChecksNameTheClientsTheyBreak(t *testing.T) { + var all []string + for _, p := range clientProfiles() { + all = append(all, p.Name) + } + clients := map[string][]string{} + var everyClient []string + note := func(id string, severity check.Severity, refs []check.Ref) { + clients[id] = ClientsOf(severity, refs) + if slices.Equal(clients[id], all) { + everyClient = append(everyClient, id) + } + } + for _, c := range All() { + note(c.ID, c.Severity, c.Refs) + } + for _, r := range WireRules() { + note(r.ID, r.Severity, r.Refs) + } + for id, want := range map[string][]string{ + "cider.connect": {"CIDER"}, + "eval.error-status": {"Calva", "CIDER", "REPLy"}, + // rebel-readline has no client profile. + "wire.error-terminal": {"REPLy"}, + "describe.versions": nil, + } { + if !slices.Equal(clients[id], want) { + t.Errorf("%s breaks %v, not %v", id, clients[id], want) + } + } + // A check gets here by linking to no client's code, which is also what + // happens when it links to a client without a profile, so each one has + // to be listed. + want := []string{"session.persistent", "eval.value", "eval.stdout", "eval.stderr", "eval.survives-error", + "eval.multiple-forms", "robust.bad-frames", "wire.bencode", "wire.dict"} + if !slices.Equal(everyClient, want) { + t.Errorf("the checks that break every client are %v, not %v", everyClient, want) + } + for _, p := range clientProfiles() { + for _, c := range p.Checks { + if !slices.Equal(clients[c.ID], []string{p.Name}) { + t.Errorf("%s breaks %v, not just %s", c.ID, clients[c.ID], p.Name) + } + } + } +} + // Replies the client can live with still get a note when they say // something went wrong. func TestClientChecksNoteErrors(t *testing.T) { diff --git a/internal/report/matrix.go b/internal/report/matrix.go index 754a59f..03b3603 100644 --- a/internal/report/matrix.go +++ b/internal/report/matrix.go @@ -6,6 +6,7 @@ import ( "fmt" "html" "io" + "maps" "os" "slices" "strings" @@ -77,8 +78,14 @@ func (t table) place(id string) int { // passed returns how many checks the server of the i-th report passed, // out of the ones that ran. func (t table) passed(i int) string { + return score(slices.Collect(maps.Values(t.results[i]))) +} + +// score says how many of the results are passes, out of the ones that +// ran. +func score(results []jsonResult) string { passed, ran := 0, 0 - for _, res := range t.results[i] { + for _, res := range results { if res.Verdict == check.Skipped { continue } @@ -90,6 +97,62 @@ func (t table) passed(i int) string { return fmt.Sprintf("%d/%d", passed, ran) } +// clients are the clients that the checks say they break, the ones the +// most checks break first. Every report has a say, as the ones made +// before proof named clients don't name any. +func (t table) clients() []string { + var names []string + counts := map[string]int{} + for _, id := range t.ids { + var named []string + for _, results := range t.results { + for _, name := range results[id].Clients { + if !slices.Contains(named, name) { + named = append(named, name) + } + } + } + for _, name := range named { + if counts[name] == 0 { + names = append(names, name) + } + counts[name]++ + } + } + slices.SortStableFunc(names, func(a, b string) int { return counts[b] - counts[a] }) + return names +} + +// client is how the server of the i-th report fares with a client: the +// score of the checks that break the client, and the ones it didn't +// pass. The verdict is the worst of theirs, or skip when none ran. +func (t table) client(i int, name string) (check.Verdict, string, []string) { + verdict := check.Pass + var ran []jsonResult + var missed []string + for _, id := range t.ids { + res, ok := t.results[i][id] + if !ok || res.Verdict == check.Skipped || !slices.Contains(res.Clients, name) { + continue + } + ran = append(ran, res) + switch res.Verdict { + case check.Failed: + verdict = check.Failed + missed = append(missed, res.ID+": "+res.Title) + case check.Errored: + if verdict != check.Failed { + verdict = check.Errored + } + missed = append(missed, res.ID+": "+res.Title+" (error)") + } + } + if len(ran) == 0 { + return check.Skipped, "skip", nil + } + return verdict, score(ran), missed +} + var cells = map[check.Verdict]string{ check.Pass: "pass", check.Failed: "**fail**", @@ -129,7 +192,7 @@ func escape(s string) string { return strings.ReplaceAll(s, "|", `\|`) } -// The page comes from matrix.html, with the table and the versions of +// The page comes from matrix.html, with the tables and the versions of // proof where its markers are. // //go:embed matrix.html @@ -137,24 +200,25 @@ var matrixPage string // HTML writes the same matrix as Matrix as a web page, with the checks // grouped like in the text report. The why of a check and the details of -// a verdict show up when hovering over them. +// a verdict show up when hovering over them. Another tab of the page has +// how each client fares with the servers. func HTML(w io.Writer, reports []Report) error { t := newTable(reports) esc := html.EscapeString - var b strings.Builder - b.WriteString("\nCheck") + var servers strings.Builder for _, r := range reports { name := esc(r.Server) if r.Homepage != "" { name = fmt.Sprintf(`%s`, esc(r.Homepage), name) } - fmt.Fprintf(&b, "%s", name) + fmt.Fprintf(&servers, "%s", name) if r.ServerVersion != "" { - fmt.Fprintf(&b, `%s`, esc(r.ServerVersion)) + fmt.Fprintf(&servers, `%s`, esc(r.ServerVersion)) } - b.WriteString("") + servers.WriteString("") } - b.WriteString("\n\n\n") + var b strings.Builder + fmt.Fprintf(&b, "\nCheck%s\n\n\n", servers.String()) group := "" for _, id := range t.ids { if g, _, _ := strings.Cut(id, "."); g != group { @@ -180,6 +244,18 @@ func HTML(w io.Writer, reports []Report) error { } b.WriteString("\n\n") + var c strings.Builder + fmt.Fprintf(&c, "\nClient%s\n\n\n", servers.String()) + for _, client := range t.clients() { + fmt.Fprintf(&c, `%s`, esc(client)) + for i := range reports { + verdict, score, missed := t.client(i, client) + fmt.Fprintf(&c, `%s`, verdict, titleAttr(strings.Join(missed, "\n")), score) + } + c.WriteString("\n") + } + c.WriteString("\n") + var proofs []string var latest time.Time for _, r := range reports { @@ -197,7 +273,8 @@ func HTML(w io.Writer, reports []Report) error { if !latest.IsZero() { made += " on " + latest.UTC().Format(time.DateOnly) } - _, err := strings.NewReplacer("\n", b.String(), "", esc(made)).WriteString(w, matrixPage) + _, err := strings.NewReplacer("\n", b.String(), "\n", c.String(), + "", esc(made)).WriteString(w, matrixPage) return err } diff --git a/internal/report/matrix.html b/internal/report/matrix.html index 72f5e3f..d91f784 100644 --- a/internal/report/matrix.html +++ b/internal/report/matrix.html @@ -46,12 +46,28 @@ font: 15px/1.5 system-ui, -apple-system, "Segoe UI", sans-serif; } a { color: var(--link); } -header, footer { max-width: 60rem; } +header, footer, .tab > p, dl { max-width: 60rem; } h1 { margin: 0 0 8px; font-size: 1.75rem; } p { margin: 0 0 12px; } dl { display: grid; grid-template-columns: auto 1fr; gap: 4px 12px; margin: 0 0 20px; } dt { font-weight: 600; } dd { margin: 0; } +nav { display: flex; gap: 4px; margin: 0 0 16px; border-bottom: 1px solid var(--line); } +nav a { + margin-bottom: -1px; + padding: 6px 14px; + border-bottom: 2px solid transparent; + color: var(--muted); + font-weight: 600; + text-decoration: none; +} +nav a:hover { color: var(--fg); } +main:not(:has(#clients:target)) a[href="#checks"], +main:has(#clients:target) a[href="#clients"] { color: var(--fg); border-bottom-color: var(--link); } +/* The checks are the first tab. The margin keeps a link to a tab from + scrolling the page. */ +.tab { display: none; scroll-margin-top: 100vh; } +.tab:target, main:not(:has(.tab:target)) #checks { display: block; } .scroll { overflow: auto; max-height: 85vh; border: 1px solid var(--line); border-radius: 6px; } table { border-collapse: collapse; width: 100%; } th, td { padding: 4px 10px; border-bottom: 1px solid var(--line); text-align: left; } @@ -97,6 +113,13 @@

nREPL Compatibility Matrix

proof, which are mostly about what the clients people use (CIDER, Calva, Conjure, vim-fireplace, REPLy and so on) need from a server.

+ +
+ +
pass
The server does what the clients need.
fail
Some client won't work properly with the server, or users lose data.
@@ -105,12 +128,26 @@

nREPL Compatibility Matrix

error
proof couldn't run the check.

Hover over a check to see why it matters, and over a verdict for the details.

-
- + +
+
+
+
+

Each cell says how many of the checks a client depends on the server +passes, out of the ones that apply to it. Those are the client's own checks +(e.g. cider.connect), which send what the client sends, the +checks that link to its code, and the ones every client depends on (e.g. +eval.value).

+

Hover over a number to see the checks the server didn't pass.

+
+ +
+
+
diff --git a/internal/report/report.go b/internal/report/report.go index e24f460..36024dd 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -141,6 +141,7 @@ type jsonResult struct { Severity string `json:"severity"` Why string `json:"why"` Refs []check.Ref `json:"refs"` + Clients []string `json:"clients,omitempty"` Details []string `json:"details,omitempty"` Notes []string `json:"notes,omitempty"` Expected string `json:"expected,omitempty"` @@ -158,7 +159,7 @@ func JSON(w io.Writer, run Run) error { for _, r := range run.Results { out.Results = append(out.Results, jsonResult{ ID: r.ID, Title: r.Title, Verdict: r.Verdict, Severity: r.Severity.String(), - Why: r.Why, Refs: r.Refs, Details: r.Details, Notes: r.Notes, Expected: r.Expected, + Why: r.Why, Refs: r.Refs, Clients: r.Clients, Details: r.Details, Notes: r.Notes, Expected: r.Expected, Millis: r.Duration.Milliseconds(), }) } diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 7454024..9c08e04 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -4,6 +4,7 @@ import ( "bytes" "os" "path/filepath" + "slices" "strings" "testing" "time" @@ -14,9 +15,11 @@ import ( func sampleRun(server string, verdicts ...check.Verdict) Run { ids := []string{"describe.reply", "eval.value", "eval.ns"} + clients := [][]string{{"Calva", "CIDER"}, {"CIDER"}, {"CIDER", "Conjure"}} r := Run{Proof: "test", Server: server, Address: "localhost:1", Started: time.Unix(0, 0)} for i, v := range verdicts { - r.Results = append(r.Results, check.Result{ID: ids[i], Title: "title " + ids[i], Why: "why " + ids[i], Verdict: v}) + r.Results = append(r.Results, check.Result{ID: ids[i], Title: "title " + ids[i], Why: "why " + ids[i], + Clients: clients[i], Verdict: v}) } return r } @@ -82,6 +85,11 @@ func TestMatrixAsAPage(t *testing.T) { `passfail`, `skip`, `3/31/2`, + // The clients the most checks break come first. + `ClientAlpha`, + `CIDER3/31/2 +Calva1/11/1 +Conjure1/1skip`, "Made with proof test on 2026-11-03.", } { if !strings.Contains(got, want) { @@ -167,3 +175,37 @@ func TestTranscriptShowsWhoHungUp(t *testing.T) { t.Errorf("got:\n%s\nwant:\n%s", buf.String(), want) } } + +// A client fares as badly as the worst of the checks that break it. +func TestClientsGetTheWorstVerdict(t *testing.T) { + reports := []Report{{Run: Run{Server: "Alpha"}, Results: []jsonResult{ + {ID: "a.x", Title: "x", Verdict: check.Errored, Clients: []string{"CIDER"}}, + {ID: "a.y", Title: "y", Verdict: check.Pass, Clients: []string{"CIDER"}}, + {ID: "a.z", Title: "z", Verdict: check.Failed, Clients: []string{"Calva"}}, + }}} + verdict, score, missed := newTable(reports).client(0, "CIDER") + if verdict != check.Errored || score != "1/2" || !slices.Equal(missed, []string{"a.x: x (error)"}) { + t.Errorf("got %s %s %v", verdict, score, missed) + } + reports[0].Results[2].Clients = []string{"CIDER"} + verdict, score, missed = newTable(reports).client(0, "CIDER") + if verdict != check.Failed || score != "1/3" || !slices.Equal(missed, []string{"a.x: x (error)", "a.z: z"}) { + t.Errorf("got %s %s %v", verdict, score, missed) + } +} + +// Reports made before proof named clients don't hide the clients that +// later reports name. +func TestClientsFromAnyReport(t *testing.T) { + reports := []Report{ + {Run: Run{Server: "Old"}, Results: []jsonResult{{ID: "eval.value", Verdict: check.Pass}}}, + {Run: Run{Server: "New"}, Results: []jsonResult{{ID: "eval.value", Verdict: check.Pass, Clients: []string{"CIDER"}}}}, + } + tbl := newTable(reports) + if got := tbl.clients(); !slices.Equal(got, []string{"CIDER"}) { + t.Errorf("got clients %v", got) + } + if verdict, _, _ := tbl.client(0, "CIDER"); verdict != check.Skipped { + t.Errorf("a report without clients gives %s", verdict) + } +}