amadan
test(guards): a menu command with no implementation fails silently
data-cmd="bold" in sheet.html means nothing on its own. commands.js has to carry a matching entry or the control renders perfectly, takes the click, and does nothing — the one failure a person has no reason to look twice at. Three guards, tied in both directions: every data-cmd in the markup has an implementation, every implementation is reachable from the markup or a shortcut, and the parser that reads the table still finds names it must find. That last one exists because the first two read JavaScript with a regex, which is exactly the check that stops matching after a reformat and then passes forever. All three mutation-checked: a renamed data-cmd, an unreachable command, and a reformat that blinds the parser each fail on their own line. Prompted by the Docs session, which shipped this exact shape and lost four menu items for a review cycle — its command table was merged for two readers and only one reader was changed to use it, so the items were permanently disabled and no test saw it because that module had none. Their other two warnings were checked and do not apply here: Sheets uses no position:sticky at all (the chrome is a flex column under height:100dvh, so there is no shared sticky band for two strips to collapse into), and rastrillo already bounds [rst-dropdown-menu] with max-block-size and overflow-y — measured at a 420px-tall viewport, the fill submenu caps at 20rem and scrolls.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182f7QfojuuqdS4Xd2fYFaL
1 file changed,
+151
−0
internal/guardstest/commands_test.go+151 −0
diff --git a/internal/guardstest/commands_test.go b/internal/guardstest/commands_test.go| new file mode 100644 |
| index 0000000..c05ea1d |
| --- /dev/null |
| +++ b/internal/guardstest/commands_test.go |
| @@ -0,0 +1,151 @@ |
| +package guards |
| + |
| +import ( |
| + "os" |
| + "path/filepath" |
| + "regexp" |
| + "sort" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// The menu bar and the toolbar are markup in one file and behaviour in |
| +// another. `data-cmd="bold"` in sheet.html means nothing on its own — |
| +// commands.js has to carry a `bold` in its dispatch table for the |
| +// button to do anything, and if it does not, the control renders |
| +// perfectly, takes the click, and silently does nothing. |
| +// |
| +// That is not hypothetical. The Docs session shipped exactly this shape |
| +// and lost four menu items for a review cycle: their command table was |
| +// merged for two readers and only one reader was changed to use it, so |
| +// the items were permanently disabled and no test saw it because the |
| +// module had none. Every failure on this branch of the same family — |
| +// a CSS rule matching no element, a warning flag invisible on the cells |
| +// it flags, an alignment attribute the browser outranked — was invisible |
| +// for the same reason: nothing tied the two halves together. |
| +// |
| +// This is that tie, in both directions. |
| + |
| +var ( |
| + dataCmd = regexp.MustCompile(`data-cmd="([a-z-]+)"`) |
| + cmdKey = regexp.MustCompile(`(?m)^\s{6}(?:'([a-z-]+)'|([a-z-]+)):\s*function`) |
| + shortcut = regexp.MustCompile(`(?m)^\s*var KEYS = \{([^}]*)\}`) |
| + keyValue = regexp.MustCompile(`'([a-z-]+)'`) |
| +) |
| + |
| +const ( |
| + sheetTemplate = "internal/sheets/templates/sheet.html" |
| + commandsJS = "internal/sheets/static/commands.js" |
| +) |
| + |
| +func readRepoFile(t *testing.T, root, rel string) string { |
| + t.Helper() |
| + body, err := os.ReadFile(filepath.Join(root, rel)) |
| + if err != nil { |
| + t.Fatalf("guards: read %s: %v", rel, err) |
| + } |
| + return string(body) |
| +} |
| + |
| +// commandTable is the set of names commands.js can actually run. |
| +func commandTable(t *testing.T, root string) map[string]bool { |
| + t.Helper() |
| + out := map[string]bool{} |
| + for _, m := range cmdKey.FindAllStringSubmatch(readRepoFile(t, root, commandsJS), -1) { |
| + name := m[1] |
| + if name == "" { |
| + name = m[2] |
| + } |
| + out[name] = true |
| + } |
| + if len(out) == 0 { |
| + t.Fatalf("guards: parsed no commands out of %s — this test is checking nothing, "+ |
| + "which is the failure it exists to prevent", commandsJS) |
| + } |
| + return out |
| +} |
| + |
| +// TestEveryMenuCommandIsImplemented walks the markup for commands |
| +// nothing can run. |
| +func TestEveryMenuCommandIsImplemented(t *testing.T) { |
| + root := repoRoot(t) |
| + table := commandTable(t, root) |
| + |
| + markup := readRepoFile(t, root, sheetTemplate) |
| + found := map[string]bool{} |
| + for _, m := range dataCmd.FindAllStringSubmatch(markup, -1) { |
| + found[m[1]] = true |
| + } |
| + if len(found) == 0 { |
| + t.Fatalf("guards: %s carries no data-cmd at all", sheetTemplate) |
| + } |
| + |
| + for _, name := range sorted(found) { |
| + if !table[name] { |
| + t.Errorf("%s offers data-cmd=%q and %s has no such command — the control "+ |
| + "renders, takes the click and does nothing, which is the one failure "+ |
| + "a person has no reason to look twice at", |
| + sheetTemplate, name, commandsJS) |
| + } |
| + } |
| +} |
| + |
| +// TestEveryImplementedCommandIsReachable is the other direction: a |
| +// command nothing can invoke. |
| +// |
| +// Dead in the ordinary way — it costs nothing at runtime — but it is |
| +// the shape a rename leaves behind, where the markup moves to a new |
| +// name and the old implementation stays looking live. Reachable means |
| +// named by the markup or bound to a keyboard shortcut. |
| +func TestEveryImplementedCommandIsReachable(t *testing.T) { |
| + root := repoRoot(t) |
| + table := commandTable(t, root) |
| + |
| + markup := readRepoFile(t, root, sheetTemplate) |
| + reachable := map[string]bool{} |
| + for _, m := range dataCmd.FindAllStringSubmatch(markup, -1) { |
| + reachable[m[1]] = true |
| + } |
| + if block := shortcut.FindStringSubmatch(readRepoFile(t, root, commandsJS)); block != nil { |
| + for _, m := range keyValue.FindAllStringSubmatch(block[1], -1) { |
| + reachable[m[1]] = true |
| + } |
| + } |
| + |
| + for _, name := range sorted(table) { |
| + if !reachable[name] { |
| + t.Errorf("%s implements %q and nothing invokes it — no data-cmd in %s and "+ |
| + "no keyboard shortcut. Either the markup was renamed and this was left, "+ |
| + "or a control was never added.", commandsJS, name, sheetTemplate) |
| + } |
| + } |
| +} |
| + |
| +func sorted(m map[string]bool) []string { |
| + out := make([]string, 0, len(m)) |
| + for k := range m { |
| + out = append(out, k) |
| + } |
| + sort.Strings(out) |
| + return out |
| +} |
| + |
| +// TestTheCommandTableIsParsedAsWritten guards the regexes above. |
| +// |
| +// Both tests read JavaScript with a pattern, which is exactly the kind |
| +// of check that quietly stops matching after a reformat and then passes |
| +// forever. This asserts a few names it must find, so a parse that has |
| +// silently gone blind fails here rather than fifty lines up. |
| +func TestTheCommandTableIsParsedAsWritten(t *testing.T) { |
| + table := commandTable(t, repoRoot(t)) |
| + for _, want := range []string{"bold", "italic", "align", "fill", "ink", "undo"} { |
| + if !table[want] { |
| + t.Errorf("the command table parser did not find %q — it has stopped "+ |
| + "reading %s and both tests above are now vacuous", want, commandsJS) |
| + } |
| + } |
| + if len(table) < 10 { |
| + t.Errorf("parsed only %d commands (%s); the table is larger than that", |
| + len(table), strings.Join(sorted(table), ", ")) |
| + } |
| +} |