From 0a75a7cd43869079194f87142cd5cebc73278b23 Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Mon, 17 Aug 2026 22:52:05 +0000 Subject: [PATCH 1/2] test(corpus): pin what completes where the cursor is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completion was the one area with three implementations and no shared fixture. `argv/src/complete.rs` has its own unit tests, `cli/src/cli/complete_word.rs` has its own, and the Go implementation landed in #984 with a third set. Three sets of tests written against three readings of the same rules is the arrangement behind every drift this project has chased — the help renderers agreed on mise and differed on five of the other six jdx CLIs until #972 held them to one fixture. 17 vectors in `corpus/complete/`, plain JSON like the others so a Go, JS or Python implementation can run them without reimplementing a test format: positions (command, prefix, dash, hidden, aliases, globals in and out of scope) and values (a flag's choices, a positional's, and restart tokens). Restart tokens are why this file exists. mise declares `:::` on `run`, nothing covered it, and it is the one rule that makes the cursor's position depend on a word rather than a count — past the token the answer is the command's *first* argument again, whatever the words before it filled. Closes two thirds of PLAN.md's "not covered by the corpus yet". Mounts stay uncovered on purpose: resolving one *runs a command*, which a corpus cannot do hermetically, as the differential fuzzer found the expensive way. **Every expectation was measured before it was written.** Two of the first vectors asserted behaviour neither implementation has — a restart token offering itself, and `--format=j` completing its value. Both were plausible and both were mine rather than the grammar's. The corpus is the definition of correct, so one author's opinion is not enough to put a rule in it: the restart-token vector now pins what both implementations do (an argument with no choices defers to paths), and the attached-value case is left out rather than pinned, because a vector asserting "nothing happens" would block the fix. Both are noted where a reader will find them. Mutation-checked from both directions: unhiding a hidden subcommand in `argv/src/complete.rs` fails `hidden-is-never-offered`, and a vector claiming one word too many fails on its own id. --- conformance/src/complete.rs | 181 ++++++++++++++++++++ conformance/src/lib.rs | 1 + conformance/tests/complete.rs | 98 +++++++++++ corpus/complete/01-positions.json | 76 ++++++++ corpus/complete/02-values-and-restarts.json | 70 ++++++++ corpus/complete/README.md | 69 ++++++++ 6 files changed, 495 insertions(+) create mode 100644 conformance/src/complete.rs create mode 100644 conformance/tests/complete.rs create mode 100644 corpus/complete/01-positions.json create mode 100644 corpus/complete/02-values-and-restarts.json create mode 100644 corpus/complete/README.md diff --git a/conformance/src/complete.rs b/conformance/src/complete.rs new file mode 100644 index 000000000..ebfe4dd7c --- /dev/null +++ b/conformance/src/complete.rs @@ -0,0 +1,181 @@ +//! The completion corpus: its format, a loader, and the runner. +//! +//! The argv corpus pins what a command line *binds*; `render` pins what a spec *reads as*. This +//! one pins what could go where the cursor is. +//! +//! # Why it exists +//! +//! Completion was the one area with three implementations and no shared fixture. +//! `argv/src/complete.rs` has its own unit tests, `cli/src/cli/complete_word.rs` has its own, and +//! a Go implementation landed with a third set. Three sets of tests written against three +//! readings of the same rules is the arrangement that produced every drift this project has had +//! to chase: the help renderers agreed on mise and differed on five of the other six jdx CLIs +//! until one fixture held them together. +//! +//! It also closes two thirds of PLAN.md's "not covered by the corpus yet" — completion parsing, +//! which is `parse_partial` over deliberately incomplete input, and restart tokens, which only +//! matter at a cursor. Mounts stay uncovered on purpose: resolving one *runs a command*, which a +//! corpus cannot do hermetically. +//! +//! # Every expectation here was measured first +//! +//! Two of the first vectors written asserted behaviour neither implementation has — a restart +//! token offering itself, and an attached `--format=j` completing its value. Both were plausible +//! and both were invented. The corpus is the definition of correct, so a vector may not assert a +//! rule on one author's opinion: what goes in is measured on both implementations, and where they +//! agree on something that looks wrong, the vector says so rather than pinning it silently. + +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; +use usage::Spec; +use usage_argv::complete::{complete, split, Shell}; + +use crate::tables; + +/// One `corpus/complete/*.json` file: a themed group of vectors. +#[derive(Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct VectorFile { + /// What this file covers, e.g. `"positions"`. + pub section: String, + /// What the group establishes, and anything a reader needs in order to judge whether these + /// expectations are the right ones. + pub about: String, + pub vectors: Vec, +} + +/// A single case: ask `spec` what completes at the cursor in `line` and you must get `expect`. +#[derive(Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct Vector { + /// Stable identifier, unique across the corpus. Failures quote it, so renaming one breaks + /// anybody tracking known failures. + pub id: String, + /// What this vector pins down, in one sentence. + pub doc: String, + /// A complete spec, as KDL. + pub spec: String, + /// The command line as typed, program name included. + pub line: String, + /// Where the cursor is, as a byte offset into `line`. Defaults to its end, which is where a + /// shell asks from; a vector completing mid-line says so. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cursor: Option, + pub expect: Expect, + /// Whether `usage-cli`, the reference implementation, agrees with `expect`. + #[serde(default)] + pub reference: Reference, +} + +impl Vector { + fn cursor(&self) -> usize { + self.cursor.unwrap_or(self.line.len()) + } +} + +/// What completion must produce. +#[derive(Debug, Default, Deserialize, Serialize, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct Expect { + /// The words offered. Order-insensitive unless `ordered`: which order a shell shows them in + /// is the shell's business, and two implementations sorting differently is not a + /// disagreement about what completes. + #[serde(default)] + pub candidates: Vec, + /// Whether the answer defers to the shell's own path completion. A vector cannot state what + /// the filesystem holds without becoming a test of the machine it runs on, so it states that + /// paths belong here and stops. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub files: bool, + /// Whether the order of `candidates` is itself the claim. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub ordered: bool, +} + +/// Whether the reference implementation matches a vector's expectation. +#[derive(Debug, Default, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum Reference { + /// `usage-cli` produces exactly `expect`. + #[default] + Agrees, + /// It produces something else. The note says what, and why the corpus keeps its own + /// expectation regardless. + Diverges(String), +} + +/// What one implementation offered. +#[derive(Debug, PartialEq, Eq)] +pub struct Offered { + pub candidates: Vec, + pub files: bool, +} + +impl Offered { + /// Whether this satisfies a vector. Sorted unless the vector claims an order, so a failure + /// is about the set rather than about two implementations' sort stability. + pub fn matches(&self, expect: &Expect) -> bool { + if self.files != expect.files { + return false; + } + if expect.ordered { + return self.candidates == expect.candidates; + } + let mine: BTreeSet<&str> = self.candidates.iter().map(String::as_str).collect(); + let theirs: BTreeSet<&str> = expect.candidates.iter().map(String::as_str).collect(); + mine == theirs + } +} + +/// What `usage-argv` offers for a vector. +/// +/// Its tables are built from the vector's KDL by `tables::build_spec`, which is the same bridge +/// the render corpus uses: the alternative is a Rust type per vector, and a corpus a reader +/// cannot extend is not one. +pub fn run(vector: &Vector) -> Result { + let spec: Spec = vector + .spec + .parse() + .map_err(|e| format!("the spec would not load: {e}"))?; + let tables = tables::build_spec(&spec); + // Bash because the split has to be *some* shell and this one has no quoting rules of its + // own to fold in; which shell renders the answer is `complete::render`'s business, tested + // separately. + let at = split(&vector.line, vector.cursor(), Shell::Bash); + let answer = complete(tables, &at); + Ok(Offered { + candidates: answer.candidates.iter().map(|c| c.value.clone()).collect(), + files: answer.files.is_some(), + }) +} + +/// Every vector in the corpus, in file order. +pub fn load() -> Vec<(String, Vector)> { + let mut out = Vec::new(); + let mut files: Vec = std::fs::read_dir(dir()) + .expect("the completion corpus should be readable") + .filter_map(|e| e.ok().map(|e| e.path())) + .filter(|p| p.extension().is_some_and(|e| e == "json")) + .collect(); + files.sort(); + for path in files { + let text = std::fs::read_to_string(&path).expect("a corpus file should be readable"); + let file: VectorFile = serde_json::from_str(&text) + .unwrap_or_else(|e| panic!("{} is not a valid corpus file: {e}", path.display())); + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default() + .to_string(); + for vector in file.vectors { + out.push((name.clone(), vector)); + } + } + out +} + +fn dir() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("../corpus/complete") +} diff --git a/conformance/src/lib.rs b/conformance/src/lib.rs index e98e27646..d5f041c02 100644 --- a/conformance/src/lib.rs +++ b/conformance/src/lib.rs @@ -14,6 +14,7 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; pub mod argv; +pub mod complete; pub mod config; pub mod reference; pub mod render; diff --git a/conformance/tests/complete.rs b/conformance/tests/complete.rs new file mode 100644 index 000000000..5fd170752 --- /dev/null +++ b/conformance/tests/complete.rs @@ -0,0 +1,98 @@ +//! The completion corpus, run against `usage-argv`. +//! +//! `corpus/complete/README.md` says what a vector means and why the corpus exists. This is the +//! half that makes it a gate rather than a document. + +use std::collections::BTreeSet; + +use usage_conformance::complete::{load, run, Reference}; + +#[test] +fn every_vector_is_offered_what_it_expects() { + let mut failures = Vec::new(); + let vectors = load(); + assert!( + !vectors.is_empty(), + "the corpus loaded nothing, so this would pass by measuring nothing" + ); + + for (file, vector) in &vectors { + match run(vector) { + Err(why) => failures.push(format!(" {} [{file}]: {why}", vector.id)), + Ok(offered) => { + if !offered.matches(&vector.expect) { + failures.push(format!( + " {} [{file}]\n wanted: {:?}{}\n got: {:?}{}", + vector.id, + vector.expect.candidates, + if vector.expect.files { " + files" } else { "" }, + offered.candidates, + if offered.files { " + files" } else { "" }, + )); + } + } + } + } + + assert!( + failures.is_empty(), + "{} of {} vector(s) failed:\n{}", + failures.len(), + vectors.len(), + failures.join("\n") + ); +} + +#[test] +fn every_id_is_unique() { + // Failures quote the id, and two vectors sharing one makes a report ambiguous about which + // case moved. The render corpus checks the same thing for the same reason. + let mut seen = BTreeSet::new(); + let mut duplicates = Vec::new(); + for (_, vector) in load() { + if !seen.insert(vector.id.clone()) { + duplicates.push(vector.id); + } + } + assert!(duplicates.is_empty(), "duplicate ids: {duplicates:?}"); +} + +#[test] +fn every_vector_says_something() { + // A vector with an empty `doc` is a case nobody can review: the expectation may be right and + // there is no way to tell. And one expecting nothing at all — no candidates, no files — is + // almost always a vector whose spec did not say what its author thought. + for (file, vector) in load() { + assert!( + !vector.doc.trim().is_empty(), + "{} [{file}] has no doc", + vector.id + ); + assert!( + !vector.expect.candidates.is_empty() || vector.expect.files, + "{} [{file}] expects nothing at all — if that is really the claim, say so in `doc` \ + and relax this check", + vector.id + ); + } +} + +#[test] +fn a_divergence_from_the_reference_is_labelled() { + // The corpus's rule, kept the same way the argv corpus keeps it: a vector the reference + // disagrees with must say so, so a divergence is a recorded decision rather than a mystery. + // + // Only the label is checked here, not the reference's answer: `usage-cli` completes through + // a `Command` with its own shell splitting, and re-deriving that inside a test would be a + // second implementation of the thing under test. `benches/gate` compares the two over mise's + // real spec, which is where the reference is actually held to account. + for (file, vector) in load() { + if let Reference::Diverges(note) = &vector.reference { + assert!( + !note.trim().is_empty(), + "{} [{file}] is labelled as diverging with no note saying how", + vector.id + ); + } + } +} diff --git a/corpus/complete/01-positions.json b/corpus/complete/01-positions.json new file mode 100644 index 000000000..b47e8e1fc --- /dev/null +++ b/corpus/complete/01-positions.json @@ -0,0 +1,76 @@ +{ + "section": "positions", + "about": "Where the cursor is decides what could go there. A word beginning with a dash is a flag or nothing; a bare word at command position is a subcommand or the next positional's choices; a word after a flag that takes a value is that value. These are the branches every implementation has to agree on before any of the harder cases matter.", + "vectors": [ + { + "id": "empty-word-offers-commands", + "doc": "At command position with nothing typed, the subcommands are the answer.", + "spec": "name \"ex\"\nbin \"ex\"\ncmd \"go\" {}\ncmd \"stop\" {}\n", + "line": "ex ", + "expect": { "candidates": ["go", "stop"] } + }, + { + "id": "a-prefix-narrows-commands", + "doc": "A partial word offers only what it could still become. Filtering is the implementation's job, not the shell's: two of the five shells do not filter for you.", + "spec": "name \"ex\"\nbin \"ex\"\ncmd \"go\" {}\ncmd \"stop\" {}\n", + "line": "ex g", + "expect": { "candidates": ["go"] } + }, + { + "id": "a-dash-offers-flags-only", + "doc": "A word beginning with `-` is a flag or nothing: no path and no subcommand starts with one, so offering them would put a word there that cannot be completed.", + "spec": "name \"ex\"\nbin \"ex\"\nflag \"--force\"\nflag \"--quiet\"\ncmd \"go\" {}\n", + "line": "ex -", + "expect": { "candidates": ["--force", "--quiet"] } + }, + { + "id": "hidden-is-never-offered", + "doc": "`hide` keeps a thing off the page and out of the answer alike. A completion offering a hidden command teaches it to people the help deliberately did not.", + "spec": "name \"ex\"\nbin \"ex\"\nflag \"--force\"\nflag \"--secret\" hide=#true\ncmd \"go\" {}\ncmd \"internal\" hide=#true {}\n", + "line": "ex ", + "expect": { "candidates": ["go"] } + }, + { + "id": "hidden-flags-are-not-offered-either", + "doc": "The same for flags, asked at the position where flags are the answer.", + "spec": "name \"ex\"\nbin \"ex\"\nflag \"--force\"\nflag \"--secret\" hide=#true\n", + "line": "ex -", + "expect": { "candidates": ["--force"] } + }, + { + "id": "an-alias-completes-like-a-name", + "doc": "An alias is a way to invoke a command, so it is a way to complete one. Both appear: which a person prefers to type is theirs to choose.", + "spec": "name \"ex\"\nbin \"ex\"\ncmd \"install\" {\n alias \"i\"\n}\n", + "line": "ex ", + "expect": { "candidates": ["install", "i"] } + }, + { + "id": "a-hidden-alias-is-not-offered", + "doc": "`alias \"add\" hide=#true` exists so a spelling can keep working without being advertised, which is exactly a promise about completion.", + "spec": "name \"ex\"\nbin \"ex\"\ncmd \"install\" {\n alias \"i\"\n alias \"add\" hide=#true\n}\n", + "line": "ex ", + "expect": { "candidates": ["install", "i"] } + }, + { + "id": "inside-a-command-offers-its-own", + "doc": "Past a command word the answer is that command's, not the root's.", + "spec": "name \"ex\"\nbin \"ex\"\ncmd \"config\" {\n cmd \"get\" {}\n cmd \"set\" {}\n}\ncmd \"go\" {}\n", + "line": "ex config ", + "expect": { "candidates": ["get", "set"] } + }, + { + "id": "a-global-is-offered-below-where-it-was-declared", + "doc": "A global is in scope for every descendant, so it is offerable there. The parser accepts it; a completion that hid it would be describing a different CLI.", + "spec": "name \"ex\"\nbin \"ex\"\nflag \"--verbose\" global=#true\ncmd \"go\" {\n flag \"--fast\"\n}\n", + "line": "ex go -", + "expect": { "candidates": ["--fast", "--verbose"] } + }, + { + "id": "a-non-global-does-not-leak-downward", + "doc": "The other half: a flag declared on the root without `global` is out of scope inside a subcommand, and offering it would complete a word the parser then refuses.", + "spec": "name \"ex\"\nbin \"ex\"\nflag \"--local\"\ncmd \"go\" {\n flag \"--fast\"\n}\n", + "line": "ex go -", + "expect": { "candidates": ["--fast"] } + } + ] +} diff --git a/corpus/complete/02-values-and-restarts.json b/corpus/complete/02-values-and-restarts.json new file mode 100644 index 000000000..f9dd98973 --- /dev/null +++ b/corpus/complete/02-values-and-restarts.json @@ -0,0 +1,70 @@ +{ + "section": "values-and-restarts", + "about": "The positions where the answer is a value rather than a name: a flag's declared choices, a positional's, and the position a restart token puts the cursor back to. `restart_token` is the reason this file exists \u2014 mise declares `:::` on `run`, no other corpus covers it, and it is the one rule that makes the cursor's position depend on a word rather than a count.", + "vectors": [ + { + "id": "a-flags-choices-are-its-values", + "doc": "After a flag that takes a value, the answer is what that value may be.", + "spec": "name \"ex\"\nbin \"ex\"\nflag \"--format \" {\n arg \"\" {\n choices \"json\" \"yaml\" \"toml\"\n }\n}\n", + "line": "ex --format ", + "expect": { + "candidates": ["json", "yaml", "toml"] + } + }, + { + "id": "a-choice-prefix-narrows", + "doc": "The same filtering a name gets.", + "spec": "name \"ex\"\nbin \"ex\"\nflag \"--format \" {\n arg \"\" {\n choices \"json\" \"yaml\" \"toml\"\n }\n}\n", + "line": "ex --format t", + "expect": { + "candidates": ["toml"] + } + }, + { + "id": "a-positionals-choices-are-offered", + "doc": "A positional carrying choices answers at command position, alongside the subcommands rather than instead of them \u2014 a word there could be either.", + "spec": "name \"ex\"\nbin \"ex\"\narg \"\" {\n choices \"fast\" \"slow\"\n}\ncmd \"go\" {}\n", + "line": "ex ", + "expect": { + "candidates": ["fast", "slow", "go"] + } + }, + { + "id": "an-unconstrained-argument-defers-to-paths", + "doc": "An argument declaring no choices has nothing to offer of its own, so the answer is the shell's own file completion rather than an empty list. Measured on both implementations before being written down: `ex run build ` offers paths, and the restart token is *not* among them \u2014 neither implementation offers `:::` itself, which is a gap rather than a rule, so this vector pins what they agree on.", + "spec": "name \"ex\"\nbin \"ex\"\ncmd \"run\" restart_token=\":::\" {\n arg \"\" {\n choices \"build\" \"test\"\n }\n arg \"[ARGS]...\"\n}\n", + "line": "ex run build ", + "expect": { + "candidates": [], + "files": true + } + }, + { + "id": "past-a-restart-token-the-first-argument-answers-again", + "doc": "The point of a restart token: everything after it starts a fresh invocation, so the cursor is back at the *first* argument however many words came before. Without this the answer would be `[ARGS]...` \u2014 the position the count reached \u2014 and completing a second task name would be impossible.", + "spec": "name \"ex\"\nbin \"ex\"\ncmd \"run\" restart_token=\":::\" {\n arg \"\" {\n choices \"build\" \"test\"\n }\n arg \"[ARGS]...\"\n}\n", + "line": "ex run build ::: ", + "expect": { + "candidates": ["build", "test"] + } + }, + { + "id": "a-restart-token-narrows-like-any-other-position", + "doc": "And filtering still applies past it, which is what says the position really was reset rather than special-cased into offering everything.", + "spec": "name \"ex\"\nbin \"ex\"\ncmd \"run\" restart_token=\":::\" {\n arg \"\" {\n choices \"build\" \"test\"\n }\n arg \"[ARGS]...\"\n}\n", + "line": "ex run build ::: t", + "expect": { + "candidates": ["test"] + } + }, + { + "id": "a-flag-still-completes-past-a-restart-token", + "doc": "A restart resets which argument is next, not whether flags are accepted.", + "spec": "name \"ex\"\nbin \"ex\"\ncmd \"run\" restart_token=\":::\" {\n flag \"--dry-run\"\n arg \"\" {\n choices \"build\"\n }\n arg \"[ARGS]...\"\n}\n", + "line": "ex run build ::: -", + "expect": { + "candidates": ["--dry-run"] + } + } + ] +} diff --git a/corpus/complete/README.md b/corpus/complete/README.md new file mode 100644 index 000000000..0ddffdb18 --- /dev/null +++ b/corpus/complete/README.md @@ -0,0 +1,69 @@ +# The completion corpus + +> The corpus two directories up is about what a command line _binds_; +> [`render/`](../render/README.md) is about what a spec _reads as_; [`config/`](../config/README.md) +> is about resolving a CLI's settings. This one is about what could go where the cursor is. + +Test vectors for completion. Each pairs a spec with a partially typed command line and the +candidates an implementation must offer. + +Plain JSON, for the same reason the others are: an implementation in any language can run these +without reimplementing a test format. If you are answering `complete` from a usage spec — in Go, +in JavaScript, or as a second Rust implementation — this directory is the definition of correct. + +## Why this exists + +Completion is the one area with three implementations and no shared fixture. Parsing has the +corpus above; rendering got one in `render/`; completion has `argv/src/complete.rs` tested by its +own unit tests, `cli/src/cli/complete_word.rs` tested by its own, and a Go implementation landed +in #984 tested by a third set. Three sets of tests written against three readings of the same +rules is the arrangement that produced every drift this project has had to chase — the help +renderers agreed on mise and differed on five of the other six CLIs until #972 held them to one +fixture. + +It also closes the two easier thirds of PLAN.md's "not covered by the corpus yet": completion +parsing, which is `parse_partial` over deliberately incomplete input, and restart tokens, which +only matter at a cursor. Mounts remain uncovered, and deliberately — resolving one _runs a +command_, which a corpus cannot do hermetically. The differential fuzzer learned that the +expensive way: its first draft spawned real `mise` processes that fetched vfox metadata and +shelled out to `apt-cache`. + +## What a vector says + +```jsonc +{ + "id": "flags-after-a-dash", + "doc": "A word beginning with `-` offers flags rather than subcommands or values.", + "spec": "name \"ex\"\nbin \"ex\"\nflag \"--force\"\ncmd \"go\" {}\n", + "line": "ex -", + "expect": { "candidates": ["--force"] }, +} +``` + +`line` is the command line as typed, and the cursor sits at its end — which is where a shell asks +from. A vector needing the cursor elsewhere says so with `cursor`, a byte offset into `line`. + +`candidates` is the set an implementation must offer, order-insensitive: the order a shell shows +them in is the shell's business, and two implementations sorting differently is not a +disagreement about what completes. A vector that _does_ mean to pin order says +`"ordered": true`. + +## Candidates that cannot be a fixed list + +Some answers are not a list of words. `run=` shells out; a `complete` callback asks the binary +itself; `files` and `dirs` ask the filesystem. A vector cannot state those without becoming a +test of the machine it runs on, so it states the _kind_ instead: + +```jsonc +"expect": { "files": true } +``` + +which asserts that the implementation defers to the shell's own file completion rather than +offering words of its own. What the filesystem then contains is not the corpus's business. + +## Keeping it honest + +Every vector carries a `reference` label, defaulting to `agrees`, saying whether `usage-cli` — +the reference implementation — produces exactly this. A vector the reference disagrees with must +say so and why, so a divergence is a recorded decision rather than a mystery. That is the same +rule the argv corpus runs on, and `conformance/tests/complete.rs` is what enforces it. From 01c90b9b812b169f114b6a546306bfd88219f205 Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Tue, 18 Aug 2026 00:46:57 +0000 Subject: [PATCH 2/2] test(corpus): validate completion corpus inputs --- Cargo.lock | 1 + cli/src/cli/complete_word.rs | 37 +++++++++++++++++++++++-- cli/src/lib.rs | 4 ++- conformance/Cargo.toml | 1 + conformance/src/complete.rs | 29 ++++++++++++++++++- conformance/tests/complete.rs | 52 +++++++++++++++++++++++++---------- 6 files changed, 105 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9e386508a..4b4c518f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2382,6 +2382,7 @@ dependencies = [ "serde", "serde_json", "usage-argv", + "usage-cli", "usage-config", "usage-derive", "usage-lib", diff --git a/cli/src/cli/complete_word.rs b/cli/src/cli/complete_word.rs index 17db39aea..f0c74250f 100644 --- a/cli/src/cli/complete_word.rs +++ b/cli/src/cli/complete_word.rs @@ -59,6 +59,29 @@ pub fn candidates( cword: usize, shell: &str, ) -> miette::Result> { + Ok(answer(spec, words, cword, shell)?.candidates) +} + +/// The reference implementation's candidates and whether they came from its path fallback. +/// +/// `candidates` keeps returning the concrete paths the CLI has always printed. Conformance +/// needs the extra bit because a portable corpus can say "files belong here" but cannot pin +/// whichever files happen to be in the checkout running it. +#[derive(Debug, PartialEq, Eq)] +pub struct CandidateAnswer { + /// The concrete values the CLI would print for the shell. + pub candidates: Vec<(String, String)>, + /// Whether the CLI generated those values by scanning the filesystem. + pub files: bool, +} + +/// Complete a partial command line while preserving path-fallback metadata. +pub fn answer( + spec: &Spec, + words: &[String], + cword: usize, + shell: &str, +) -> miette::Result { CompleteWord { words: words.to_vec(), file: None, @@ -66,7 +89,7 @@ pub fn candidates( cword: Some(cword), shell: shell.to_string(), } - .complete_word(spec) + .complete_word_answer(spec) } impl CompleteWord { @@ -111,6 +134,10 @@ impl CompleteWord { } pub fn complete_word(&self, spec: &Spec) -> miette::Result> { + Ok(self.complete_word_answer(spec)?.candidates) + } + + fn complete_word_answer(&self, spec: &Spec) -> miette::Result { let cword = self.cword.unwrap_or(self.words.len().max(1) - 1); let ctoken = self.words.get(cword).cloned().unwrap_or_default(); let words: Vec<_> = self.words.iter().take(cword).cloned().collect(); @@ -233,13 +260,17 @@ impl CompleteWord { // flag. Past a `--` a dash-prefixed word is not a flag but a value, so a path like // `-input` still gets completed there. let looks_like_a_flag = flags_possible && ctoken.starts_with('-'); - if choices.is_empty() && !looks_like_a_flag && !has_explicit_choices { + let files = choices.is_empty() && !looks_like_a_flag && !has_explicit_choices; + if files { let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let files = self.complete_path(&cwd, &ctoken, |_| true); choices = files.into_iter().map(|n| (n, String::new())).collect(); } trace!("choices: {}", choices.iter().map(|(c, _)| c).join(", ")); - Ok(choices) + Ok(CandidateAnswer { + candidates: choices, + files, + }) } fn complete_subcommands(&self, cmd: &SpecCommand, ctoken: &str) -> Vec<(String, String)> { diff --git a/cli/src/lib.rs b/cli/src/lib.rs index e687ce41b..fb8fb10e0 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -9,7 +9,9 @@ use miette::Result; /// /// Re-exported rather than the whole `cli` module: the conformance comparison needs this one /// answer, and nothing else in here is a promise to anybody. -pub use cli::complete_word::candidates as complete_candidates; +pub use cli::complete_word::{ + answer as complete_answer, candidates as complete_candidates, CandidateAnswer, +}; pub use cli::Cli; mod cli; diff --git a/conformance/Cargo.toml b/conformance/Cargo.toml index c7df827e9..65d48e99a 100644 --- a/conformance/Cargo.toml +++ b/conformance/Cargo.toml @@ -19,6 +19,7 @@ serde = { version = "1", features = ["derive"] } serde_json = "1" usage-argv = { workspace = true, features = ["spec", "complete", "diagnostics"] } usage-config = { workspace = true } +usage-cli = { workspace = true } usage-lib = { workspace = true } [dev-dependencies] diff --git a/conformance/src/complete.rs b/conformance/src/complete.rs index ebfe4dd7c..e7561043d 100644 --- a/conformance/src/complete.rs +++ b/conformance/src/complete.rs @@ -151,12 +151,39 @@ pub fn run(vector: &Vector) -> Result { }) } +/// What the reference `usage-cli` implementation offers for a vector. +pub fn reference(vector: &Vector) -> Result { + let spec: Spec = vector + .spec + .parse() + .map_err(|e| format!("the spec would not load: {e}"))?; + let at = split(&vector.line, vector.cursor(), Shell::Bash); + let answer = usage_cli::complete_answer(&spec, &at.words, at.cword, "bash") + .map_err(|e| format!("the reference would not complete it: {e}"))?; + Ok(Offered { + candidates: if answer.files { + Vec::new() + } else { + answer + .candidates + .into_iter() + .map(|(value, _)| value) + .collect() + }, + files: answer.files, + }) +} + /// Every vector in the corpus, in file order. pub fn load() -> Vec<(String, Vector)> { let mut out = Vec::new(); let mut files: Vec = std::fs::read_dir(dir()) .expect("the completion corpus should be readable") - .filter_map(|e| e.ok().map(|e| e.path())) + .map(|entry| { + entry + .expect("a completion corpus directory entry should be readable") + .path() + }) .filter(|p| p.extension().is_some_and(|e| e == "json")) .collect(); files.sort(); diff --git a/conformance/tests/complete.rs b/conformance/tests/complete.rs index 5fd170752..5cc3b4f57 100644 --- a/conformance/tests/complete.rs +++ b/conformance/tests/complete.rs @@ -5,7 +5,7 @@ use std::collections::BTreeSet; -use usage_conformance::complete::{load, run, Reference}; +use usage_conformance::complete::{load, reference, run, Reference}; #[test] fn every_vector_is_offered_what_it_expects() { @@ -78,21 +78,45 @@ fn every_vector_says_something() { } #[test] -fn a_divergence_from_the_reference_is_labelled() { - // The corpus's rule, kept the same way the argv corpus keeps it: a vector the reference - // disagrees with must say so, so a divergence is a recorded decision rather than a mystery. - // - // Only the label is checked here, not the reference's answer: `usage-cli` completes through - // a `Command` with its own shell splitting, and re-deriving that inside a test would be a - // second implementation of the thing under test. `benches/gate` compares the two over mise's - // real spec, which is where the reference is actually held to account. +fn the_reference_label_is_true_in_both_directions() { + // A vector claiming agreement must agree, and a vector claiming divergence must still + // diverge. A fixed divergence therefore fails with an instruction to delete the label + // instead of quietly rotting into folklore. + let mut wrong = Vec::new(); for (file, vector) in load() { - if let Reference::Diverges(note) = &vector.reference { - assert!( - !note.trim().is_empty(), - "{} [{file}] is labelled as diverging with no note saying how", + let observed = match reference(&vector) { + Ok(observed) => observed, + Err(why) => { + wrong.push(format!("{} [{file}]: {why}", vector.id)); + continue; + } + }; + let agrees = observed.matches(&vector.expect); + match (&vector.reference, agrees) { + (Reference::Agrees, true) => {} + (Reference::Diverges(note), false) if !note.trim().is_empty() => {} + (Reference::Agrees, false) => wrong.push(format!( + "{} [{file}]: labelled as agreeing\n wanted: {:?}{}\n reference: {:?}{}", + vector.id, + vector.expect.candidates, + if vector.expect.files { " + files" } else { "" }, + observed.candidates, + if observed.files { " + files" } else { "" }, + )), + (Reference::Diverges(note), true) => wrong.push(format!( + "{} [{file}]: labelled as diverging ({note}), but the reference now agrees — delete the label", + vector.id + )), + (Reference::Diverges(_), false) => wrong.push(format!( + "{} [{file}]: labelled as diverging with no note saying how", vector.id - ); + )), } } + assert!( + wrong.is_empty(), + "{} reference label(s) are wrong:\n {}", + wrong.len(), + wrong.join("\n ") + ); }