diff --git a/dependent.go b/dependent.go index bcc743f..b379ade 100644 --- a/dependent.go +++ b/dependent.go @@ -2,10 +2,12 @@ package main import ( "fmt" - "github.com/dcadolph/kibble/internal/docblock" - kplan "github.com/dcadolph/kibble/internal/plan" "regexp" "strings" + + "github.com/dcadolph/kibble/internal/docblock" + kplan "github.com/dcadolph/kibble/internal/plan" + "github.com/dcadolph/kibble/internal/shell" ) // Failures that may follow from lines which never ran. None of these @@ -80,11 +82,15 @@ func resolveDependentFailures(run *exampleRun, plan *kplan.Plan) { } } } + // Earlier line results per document, for the shared-operand rule, which + // reads across the document's steps rather than within one. + prior := map[string][]lineResult{} for si := range run.Steps { s := &run.Steps[si] for li := range s.Lines { l := &s.Lines[li] if l.Status != StatusFail { + prior[s.Doc] = append(prior[s.Doc], *l) continue } if cited := citedSkipped(l.output, skippedCmds); cited != "" { @@ -103,9 +109,73 @@ func resolveDependentFailures(run *exampleRun, plan *kplan.Plan) { l.Status = StatusBlocked l.Reason = ReasonDependsOnSkipped l.Detail = fmt.Sprintf("says to run `%s` first, which did not run", need) + } else if op, earlier := sharedOperandNotPassed(prior[s.Doc], l.Cmd); op != "" { + l.Status = StatusBlocked + l.Reason = ReasonDependsOnSkipped + l.Detail = fmt.Sprintf("names %s, which `%s` also named and did not pass", op, earlier) + } + prior[s.Doc] = append(prior[s.Doc], *l) + } + } +} + +// sharedOperandNotPassed returns a file-shaped operand of a failing line and +// the first earlier line of the same document that also named it and was +// skipped, blocked, or stopped by a gap. A query on a database whose load was +// blocked is the same hole surfacing again. An earlier failure does not count, +// since a break repeated on several lines of one file is still several +// reports. Like the other rules here it is not causal: the earlier line still +// reports, and the later ones stop counting as separate breaks. +func sharedOperandNotPassed(prior []lineResult, cmd string) (operand, earlier string) { + for _, op := range fileOperands(cmd) { + for _, p := range prior { + if p.Status == StatusVerified || p.Status == StatusFail { + continue + } + for _, pop := range fileOperands(p.Cmd) { + if pop == op { + return op, docblock.Flatten(p.Cmd) + } + } + } + } + return "", "" +} + +// reScriptFile matches a source file an interpreter runs, which is the program +// of a line such as python3 scripts/bench.py rather than data it reads. +var reScriptFile = regexp.MustCompile(`\.(py|js|mjs|cjs|ts|sh|bash|rb|pl|php|lua)$`) + +// fileOperands returns the operands of a line shaped like files: a dot or a +// slash, no whitespace, not a flag, not a URL and not a script. A flag written +// as --flag=value contributes its value. The program is not an operand, whether +// it is the command name or a script handed to an interpreter, so two runs of +// the same program with different arguments never pair. +func fileOperands(cmd string) []string { + words := shell.Operands(cmd) + if words == nil { + if fields := strings.Fields(cmd); len(fields) > 1 { + words = fields[1:] + } + } + var out []string + for _, w := range words { + if strings.HasPrefix(w, "-") { + _, v, ok := strings.Cut(w, "=") + if !ok { + continue } + w = v + } + switch { + case w == "" || strings.ContainsAny(w, " \t\n"): + case strings.HasPrefix(w, "http://"), strings.HasPrefix(w, "https://"), strings.HasPrefix(w, "ftp://"): + case reScriptFile.MatchString(w): + case strings.ContainsAny(w, "./"): + out = append(out, w) } } + return out } // rePrerequisiteNear matches the phrasings a tool uses to say that something diff --git a/dependent_adversarial_test.go b/dependent_adversarial_test.go index 8d2558c..6ec46a7 100644 --- a/dependent_adversarial_test.go +++ b/dependent_adversarial_test.go @@ -2,8 +2,11 @@ package main import ( "fmt" - kplan "github.com/dcadolph/kibble/internal/plan" "testing" + + "github.com/google/go-cmp/cmp" + + kplan "github.com/dcadolph/kibble/internal/plan" ) // TestSuppressionNeedsMoreThanAMention feeds resolveDependentFailures the @@ -87,3 +90,94 @@ func TestRepeatedSubcommandIsIndependent(t *testing.T) { t.Errorf("status = %s, want FAIL (detail %q)", got, run.Steps[0].Lines[1].Detail) } } + +// TestSharedOperandAfterLineNotPassed checks the shared-operand rule. A query +// on a file an earlier line of the same document named and did not pass is +// blocked behind that line, and nothing weaker than a shared file pairs them. +func TestSharedOperandAfterLineNotPassed(t *testing.T) { + t.Parallel() + const query = "Error: no such table: dogs" + tests := []struct { + Steps []exampleStep + WantStatus []Status + WantDetail string + }{{ // Test 0: Queries after a blocked insert on the same database are blocked. + Steps: []exampleStep{{ID: "b1", Doc: "README.md", Lines: []lineResult{ + {Cmd: "dbtool insert dogs.db dogs dogs.csv --csv", Status: StatusBlocked}, + {Cmd: `dbtool dogs.db "select id, name from dogs"`, Status: StatusFail, output: query}, + {Cmd: `dbtool dogs.db "select * from dogs" --csv`, Status: StatusFail, output: query}, + {Cmd: `dbtool dogs.db "select * from dogs" --table`, Status: StatusFail, output: query}, + }}}, + WantStatus: []Status{StatusBlocked, StatusBlocked, StatusBlocked, StatusBlocked}, + WantDetail: "names dogs.db, which `dbtool insert dogs.db dogs dogs.csv --csv` also named and did not pass", + }, { // Test 1: The same queries after a verified insert stay failures. + Steps: []exampleStep{{ID: "b1", Doc: "README.md", Lines: []lineResult{ + {Cmd: "dbtool insert dogs.db dogs dogs.csv --csv", Status: StatusVerified}, + {Cmd: `dbtool dogs.db "select id, name from dogs"`, Status: StatusFail, output: query}, + {Cmd: `dbtool dogs.db "select * from dogs" --csv`, Status: StatusFail, output: query}, + }}}, + WantStatus: []Status{StatusVerified, StatusFail, StatusFail}, + }, { // Test 2: A shared bare word is not a file, so it does not pair. + Steps: []exampleStep{{ID: "b1", Doc: "README.md", Lines: []lineResult{ + {Cmd: "dbtool build release", Status: StatusBlocked}, + {Cmd: "dbtool test release", Status: StatusFail, output: "error: test failed"}, + }}}, + WantStatus: []Status{StatusBlocked, StatusFail}, + }, { // Test 3: A shared file in a different document does not pair. + Steps: []exampleStep{ + {ID: "b1", Doc: "docs/load.md", Lines: []lineResult{ + {Cmd: "dbtool insert dogs.db dogs dogs.csv --csv", Status: StatusBlocked}, + }}, + {ID: "b2", Doc: "README.md", Lines: []lineResult{ + {Cmd: `dbtool dogs.db "select * from dogs"`, Status: StatusFail, output: query}, + }}, + }, + WantStatus: []Status{StatusBlocked, StatusFail}, + }, { // Test 4: A flag's value counts, and a step earlier in the document pairs. + Steps: []exampleStep{ + {ID: "b1", Doc: "README.md", Lines: []lineResult{ + {Cmd: "dbtool load --file=data/dogs.db", Status: StatusSkipped}, + }}, + {ID: "b2", Doc: "README.md", Lines: []lineResult{ + {Cmd: "dbtool query --db data/dogs.db", Status: StatusFail, output: query}, + }}, + }, + WantStatus: []Status{StatusSkipped, StatusBlocked}, + WantDetail: "names data/dogs.db, which `dbtool load --file=data/dogs.db` also named and did not pass", + }, { // Test 5: A URL and a query string holding a dot are not files. + Steps: []exampleStep{{ID: "b1", Doc: "README.md", Lines: []lineResult{ + {Cmd: `dbtool fetch https://example.com/x "select a.b from t"`, Status: StatusBlocked}, + {Cmd: `dbtool show https://example.com/x "select a.b from t"`, Status: StatusFail, output: "error"}, + }}}, + WantStatus: []Status{StatusBlocked, StatusFail}, + }, { // Test 6: Two runs of one script with different arguments are separate. + Steps: []exampleStep{{ID: "b1", Doc: "README.md", Lines: []lineResult{ + {Cmd: "python3 scripts/bench.py --engine both", Status: StatusBlocked}, + {Cmd: "python3 scripts/bench.py --engine one", Status: StatusFail, output: "error"}, + }}}, + WantStatus: []Status{StatusBlocked, StatusFail}, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + run := &exampleRun{Steps: test.Steps} + resolveDependentFailures(run, &kplan.Plan{Binaries: []string{"dbtool"}}) + var got []Status + last := "" + for _, s := range run.Steps { + for _, l := range s.Lines { + got = append(got, l.Status) + if l.Reason == ReasonDependsOnSkipped { + last = l.Detail + } + } + } + if diff := cmp.Diff(test.WantStatus, got); diff != "" { + t.Errorf("status mismatch (-want +got):\n%s", diff) + } + if test.WantDetail != "" && last != test.WantDetail { + t.Errorf("detail = %q, want %q", last, test.WantDetail) + } + }) + } +} diff --git a/docs/DESIGN.md b/docs/DESIGN.md index d3e0f1b..7897c63 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -211,6 +211,15 @@ directory the document meant. Resetting the working directory at every heading w rejected, because it would break every document that changes directory once in setup and invent failures. +A `cd` the planner skips, because its target is a placeholder or a name the reader fills +in, loses the directory for the rest of its section. A document keeps one shell, so every +later line under the same heading would run in the wrong directory, and each is a `SKIP` +with the reason `depends-on-skipped` until a `cd` the session can make runs: home, an +absolute path, or a directory the checkout holds while no earlier `cd` has moved the +session. A relative `cd` inside the skipped line's own block reads from the lost directory +and clears nothing. The next heading ends the loss, since a new section starts a new +procedure, which is the same boundary `inherited-state` reads. + The same holds for the checkout itself. A would-be plain `FAIL` that ran in the repository's own checkout, and whose output says the target "already exists" or is "already initialized", is `BLOCKED` with the reason `checkout-state`. The collision is @@ -252,6 +261,15 @@ the document was to write that file, through a redirect, an output flag such as verdict is `BLOCKED` with the reason `depends-on-skipped`, naming that line. Only a file no earlier line was to write is a `GAP`. +A failing line that names a file-shaped operand, one with a dot or a slash that is not a +flag or a URL, which an earlier line of the same document also named and which was skipped, +blocked, or a gap, is `BLOCKED` with the reason `depends-on-skipped`, naming that earlier +line. A database three queries read after the insert that fills it was blocked is one +break, not four. An earlier `FAIL` does not pair, so one real break repeated on several +lines stays several reports, and an operand ending in a script extension such as `.py` or +`.sh` is the program an interpreter runs rather than data, so two runs of one script never +pair. Like the other causal downgrades it suppresses the cascade without claiming cause. + A titled fence whose title is a plain relative file path is a file the document writes. The planner turns it into a line that writes the block's content, so a later line reading that file does not need it, and a path the checkout already holds is never overwritten. A caption @@ -292,14 +310,31 @@ the classifier cannot tell which. A program the tool tried to run, or one the sh could not find, is a `SKIP` for a missing dependency. Git's own report that the repository has no history is `BLOCKED`, since the session's repository is one kibble made with none. A preset, plugin, or other name the reader is meant to define first, and -a configuration the tool could not load, are `BLOCKED` as well. Both are resemblances -rather than names, so neither can ever be a `SKIP` or a pass. The list of nouns is fixed -on purpose: an unknown flag, argument, option, or command is the tool's own complaint -and still fails. The refusal counts when the tool names the kind and not the name, as in -`unknown registry` with nothing after it, and that is `BLOCKED` too. The nouns are the -same: preset, profile, alias, plugin, group, context, workspace, project, remote, -registry, template, and theme. A traceback that ends in a missing-file error naming a -thing is read this way, and one that ends any other way still fails. +a configuration the tool could not load, use, parse, import, find, or open, are `BLOCKED` +as well. Both are resemblances rather than names, so neither can ever be a `SKIP` or a +pass. The list of nouns is fixed on purpose: an unknown flag, argument, option, or command +is the tool's own complaint and still fails. The refusal counts when the tool names the +kind and not the name, as in `unknown registry` with nothing after it, and that is +`BLOCKED` too. The wordings are `unknown `, `no such ` with or without a name +after it, `for this ` as in "no dotfile to install for this profile", and `no +s are currently installed`. A name the tool reports that the command never passed +is `BLOCKED` as well, with a detail that says the command never named it, since a name +the command did not pass came from the tool's default, its environment, or the reader's +configuration, none of which is the document's claim. The nouns are the same: preset, +profile, alias, plugin, group, context, workspace, project, remote, registry, template, +and theme. A tool that tells the reader to define, add, or specify something in or under +their configuration, as in "define one in your config" or "add a `client` block under an +`apis:` entry", is pointing at a configuration entry the reader writes first, and is +`BLOCKED` with the reason `missing-fixture`. So is a tool that asked for a valid path when +the command passed it a bare word after its subcommand, with no dot and no slash, since a +name written where a path goes is an alias from the reader's configuration. The same +message with a file-shaped operand, or with none, still fails. An unquoted path followed +by "does not exist", as in "generators/php/index.ts does not exist", names a missing file +the way a quoted one does. A traceback that ends in a missing-file error naming a thing +is read this way, and one that ends any other way still fails, with two readings of the +traceback itself: its header counts when other text precedes it on the same line, as a +progress line without a newline leaves it, and a traceback ending in `EOFError` is a tool +that waited for an answer on a closed stdin, which is a `SKIP` as interactive. An operand written as a stand-in name is a placeholder the reader fills in, and the line is a `SKIP`. The class is a `my`-prefixed name such as `my-project` or `my_package`, and a @@ -312,12 +347,28 @@ line runs and does what the document says. A reserved example host is a placehol is left to the file rule. A name the repository or an earlier line provides is real, and so is a value given to an output flag, since the line writes it. A value written after a space cannot be told from an operand without the flag's arity, so it is read as an operand. +The stand-in class is read on the documented binary's operands and on those of the +file-reading commands `cd`, `ls`, `tree`, and `cat`, whatever reads the name, and not on +every command: `git commit -m my-message` is not reading a path. A numbered slot such as +`{0}` or `{1}` written as its own shell word is a tool's template left for the reader to +fill, and a placeholder. The same slot inside a quoted argument is the tool's own, as in +fzf's field bindings, and the line runs. Check mode is documented behavior. A subcommand such as `check`, `lint`, or `audit` exits nonzero when it finds something, and so does a documented tool given a flag that says report only: `--fix=false`, `--no-fix`, `--check`, `--dry-run`, or `--diff`. The -subcommands are `check`, `lint`, `audit`, `vet`, `diff`, `format`, and `fmt`. A nonzero -exit from either is the tool working. A crash stays a failure. A check-mode flag applies +subcommands are `check`, `lint`, `audit`, `vet`, `diff`, `format`, and `fmt`, and a +subcommand the page itself introduces as linting, checking, or validating for problems, +as in "The `recheck` command lints Markdown files for structure and prose problems", is +check mode for that document. The inference is by subcommand, never by binary, since a +linter run over a bad configuration exits nonzero too and blessing the binary would turn +that into a hollow pass. A nonzero exit from either is the tool working. A crash stays a +failure, and the classifier now enforces it: a check-mode line whose output holds a +traceback or a panic falls through to the crash rules. Output the document shows under a +command as a comment, which the tool then printed, is documented behavior as well: a +nonzero ordinary exit whose output contains a shown line of at least twelve characters, +with or without its leading program prefix, is `VERIFIED`, and the match needs the tool to +have said it, not a resemblance. A check-mode flag applies to its own line, while a documented note such as "exits non-zero if it finds any" describes the command, so the note reaches every line with the same binary and subcommand and the flag does not. diff --git a/internal/docblock/docblock.go b/internal/docblock/docblock.go index 18a2913..a5cb6c5 100644 --- a/internal/docblock/docblock.go +++ b/internal/docblock/docblock.go @@ -477,7 +477,24 @@ var rePlaceholder = regexp.MustCompile( // before running it. The planner asks it of an example line and the extractor // asks it of an install line, so both read placeholders the same way. func HasPlaceholder(s string) bool { - return rePlaceholder.MatchString(s) + return rePlaceholder.MatchString(s) || hasNumberedSlot(s) +} + +// reNumberedSlot matches a whole shell word that is a numbered template slot +// such as {0} or {1}, the shape a tool's own command template leaves for the +// reader to fill. +var reNumberedSlot = regexp.MustCompile(`^\{[0-9]+\}$`) + +// hasNumberedSlot reports whether one of the line's shell words is a numbered +// slot. It reads parsed words rather than flat text so that a slot inside a +// quoted argument, which the tool itself fills, is not a placeholder. +func hasNumberedSlot(s string) bool { + for _, w := range shell.ArgWordsOrFields(s) { + if reNumberedSlot.MatchString(w) { + return true + } + } + return false } // reShownError matches output a doc displays under a command to demonstrate diff --git a/internal/docblock/docblock_test.go b/internal/docblock/docblock_test.go index 4869db4..ffcd635 100644 --- a/internal/docblock/docblock_test.go +++ b/internal/docblock/docblock_test.go @@ -86,6 +86,18 @@ func TestHasPlaceholder(t *testing.T) { Want: false, }, { // Test 15: A your-github-username owner is a stand-in. In: `git clone https://github.com/your-github-username/repo`, Want: true, + }, { // Test 16: A numbered slot as its own word is a stand-in. + In: `tar -cf {1} -C {0} .`, Want: true, + }, { // Test 17: Empty braces are find's own token. + In: `find . -exec rm {} \;`, Want: false, + }, { // Test 18: A brace range is the shell's. + In: `echo {1..5}`, Want: false, + }, { // Test 19: A repetition count inside a quoted pattern is not a slot. + In: `grep -E 'a{2}' f`, Want: false, + }, { // Test 20: A numbered slot inside a quoted argument is the tool's own. + In: `fzf --delimiter : --nth 3.. --bind 'enter:become(vim {1} +{2})'`, Want: false, + }, { // Test 21: A named slot still is. + In: `tool {name}`, Want: true, }} for testNum, test := range tests { diff --git a/internal/plan/plan.go b/internal/plan/plan.go index d426c8a..b4ec0f8 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -6,6 +6,7 @@ import ( "github.com/dcadolph/kibble/internal/docblock" "github.com/dcadolph/kibble/internal/shell" "github.com/dcadolph/kibble/internal/verdict" + "maps" "regexp" "slices" "sort" @@ -106,6 +107,10 @@ type PlanLine struct { Unplanned string `json:"unplanned,omitempty"` // NonzeroOK accepts a nonzero exit as documented behavior. NonzeroOK bool `json:"nonzeroOk,omitempty"` + // Shown is the output the document shows for this line, as the comment + // lines directly under it in its block, without the leading #. The verdict + // accepts a nonzero exit only when the tool printed one of them. + Shown []string `json:"shown,omitempty"` // Synthetic names the files kibble fabricated so this line had something // to read. The document referenced them and never created them, so a pass // here proves the command accepts kibble's invented input, which is a @@ -215,6 +220,10 @@ func BuildPlan(repo, dir, markdown string, binaries []string, installs []PlanIns } } pl.plan.Settings = DocumentedSettingNames(markdown) + pl.findingSubs = maps.Clone(findingSubs) + for _, sub := range describedAsLinter(markdown) { + pl.findingSubs[sub] = true + } for b := range pl.binaries { if describedAsWatcher(markdown, b) { pl.watcher = true @@ -315,6 +324,10 @@ type planner struct { packages map[string]bool // fixed tracks fixture paths already fabricated, to avoid duplicates. fixed map[string]bool + // findingSubs holds the subcommands whose nonzero exit is a finding for + // this document: the package-level ones and those its prose introduces as + // linting. + findingSubs map[string]bool // staged marks a document that has run git add on a line that is not // skipped. It is the only repository state the planner tracks, because a // commit is the only allowed git form that needs any. @@ -322,6 +335,14 @@ type planner struct { // moved marks a document that has run a cd, after which a relative path // is read from a directory the planner does not track. moved bool + // lostDir marks a section whose last cd was skipped. The document keeps + // one shell, so every later line in the section would run in the wrong + // directory until a cd the session can make runs. + lostDir bool + // lostDirHeading is the heading of the block whose skipped cd set + // lostDir. A block under another heading starts a new procedure, so the + // lost directory ends there. + lostDirHeading string // synthetic collects the fixtures fabricated while classifying the line // currently being planned, and is drained onto that line. A line that // reads a file kibble invented was not tested against the document's own @@ -377,7 +398,11 @@ func (pl *planner) addBlock(block docblock.Block) { lineIn := docblock.SourceLineIndex(block) shownErr := docblock.ShownFailures(block) nonzero := false - lostDir := false + lostHere := false + showing := false + if pl.lostDir && block.Heading != pl.lostDirHeading { + pl.lostDir = false + } for _, ln := range lines { ln = pl.substituted(ln) flat := docblock.Flatten(ln) @@ -387,6 +412,11 @@ func (pl *planner) addBlock(block docblock.Block) { } else if trimmed != "" { nonzero = false } + showing = showing && trimmed != "" + if showing { + last := &step.Lines[len(step.Lines)-1] + last.Shown = append(last.Shown, strings.TrimSpace(strings.TrimLeft(trimmed, "#"))) + } continue } line := PlanLine{Cmd: ln, Line: lineIn(ln)} @@ -396,7 +426,7 @@ func (pl *planner) addBlock(block docblock.Block) { line.NonzeroOK = true nonzero = false } - if _, sub := InvokedBinary(flat, pl.binaries); findingSubs[sub] { + if _, sub := InvokedBinary(flat, pl.binaries); pl.findingSubs[sub] { line.NonzeroOK = true } if shownErr[strings.TrimSpace(flat)] { @@ -412,7 +442,7 @@ func (pl *planner) addBlock(block docblock.Block) { line.Skip = fmt.Sprintf("documented for %s rather than this container", scopedTo) line.SkipReason = verdict.ReasonOtherPlatform } - if line.Skip == "" && lostDir { + if line.Skip == "" && (lostHere || pl.lostDir && !pl.restoresDir(flat)) { line.Skip = "follows a skipped cd, so it would run in the wrong directory" line.SkipReason = verdict.ReasonDependsOnSkipped } @@ -423,8 +453,10 @@ func (pl *planner) addBlock(block docblock.Block) { if line.Need != "" { line.Unplanned = pl.unplanned } - if line.Skip != "" && strings.HasPrefix(strings.TrimSpace(flat), "cd ") { - lostDir = true + if reRunsCd.MatchString(flat) { + pl.lostDir = line.Skip != "" + pl.lostDirHeading = block.Heading + lostHere = lostHere || pl.lostDir } if m := reAssignPrefix.FindStringSubmatch(flat); m != nil { if line.Skip == "" { @@ -443,6 +475,7 @@ func (pl *planner) addBlock(block docblock.Block) { } } step.Lines = append(step.Lines, line) + showing = true } if len(step.Lines) == 0 { return @@ -461,6 +494,30 @@ func (pl *planner) addBlock(block docblock.Block) { pl.plan.Steps = append(pl.plan.Steps, step) } +// restoresDir reports whether a line in a later block than a skipped cd opens +// with a cd the session can make without the lost directory: home, an absolute +// path, or a directory the checkout holds while no earlier cd has moved the +// session. Within the skipped cd's own block a relative cd reads from the lost +// directory, so the block stays lost. +func (pl *planner) restoresDir(flat string) bool { + fields := strings.Fields(flat) + if len(fields) == 0 || fields[0] != "cd" { + return false + } + if len(fields) == 1 || fields[1] == "&&" || fields[1] == ";" { + return true + } + target := strings.TrimSuffix(fields[1], ";") + switch { + case target == "~" || target == "$HOME" || strings.HasPrefix(target, "/"): + return true + case pl.moved: + return false + } + target = strings.TrimSuffix(strings.TrimPrefix(target, "./"), "/") + return target != "" && pl.tree[target] +} + // blockLocation names a code block by its line and heading, for a block the // plan leaves out. func blockLocation(block docblock.Block) string { diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index 09752aa..070d6d0 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -1282,3 +1282,148 @@ func TestCommentOnlyBlockStillPlans(t *testing.T) { }) } } + +// TestPlanShown checks that comment lines directly under a command are kept +// as the output the document shows for it. +func TestPlanShown(t *testing.T) { + t.Parallel() + tests := []struct { + Markdown string + WantShown [][]string + }{{ // Test 0: A refusal shown as a comment under the command. + Markdown: "```sh\nSAFE_RM_SCOPE=\"$HOME\" tool -rf /\n" + + "# rm: target '/' skipped, unsafe directory scope\n```\n", + WantShown: [][]string{{"rm: target '/' skipped, unsafe directory scope"}}, + }, { // Test 1: A note comment is collected too, and only by the line above it. + Markdown: "```sh\ntool check .\n# exits 1 when it finds a problem\ntool fmt .\n```\n", + WantShown: [][]string{{"exits 1 when it finds a problem"}, nil}, + }, { // Test 2: Several comment lines in a row all belong to the command. + Markdown: "```sh\ntool list\n# alpha one\n# beta two\n```\n", + WantShown: [][]string{{"alpha one", "beta two"}}, + }, { // Test 3: A comment before the first command belongs to no line. + Markdown: "```sh\n# first install it\ntool list\n```\n", + WantShown: [][]string{nil}, + }, { // Test 4: A blank line ends what the command shows. + Markdown: "```sh\ntool list\n\n# now build it\ntool build\n```\n", + WantShown: [][]string{nil, nil}, + }, { // Test 5: A skipped line keeps what the document shows for it. + Markdown: "```sh\ntool get https://api.example.com/v1\n" + + "# {\"ok\": true}\n```\n", + WantShown: [][]string{{"{\"ok\": true}"}}, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + plan := BuildPlan("tool", "", test.Markdown, []string{"tool"}, nil, nil, testRecognizer()) + if len(plan.Steps) != 1 { + t.Fatalf("steps = %d, want 1", len(plan.Steps)) + } + var got [][]string + for _, l := range plan.Steps[0].Lines { + got = append(got, l.Shown) + } + if diff := cmp.Diff(test.WantShown, got, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("shown mismatch (-want +got):\n%s", diff) + } + }) + } +} + +// TestDescribedAsLinter checks that a subcommand the page introduces as +// linting for problems is check mode for that page, and that a subcommand +// described as checking something else is not. +func TestDescribedAsLinter(t *testing.T) { + t.Parallel() + recheck := "# recheck\n\nThe `recheck` command lints Markdown files for structure and " + + "prose problems.\n\n" + tests := []struct { + Markdown string + WantSubs []string + WantNonzero []bool + }{{ // Test 0: Both a plain run and one whose stdout goes to a file are check mode. + Markdown: recheck + "```sh\ntool recheck docs\n```\n\n" + + "```sh\ntool recheck docs --format=json > out.json\n```\n", + WantSubs: []string{"recheck"}, WantNonzero: []bool{true, true}, + }, { // Test 1: Checking whether something exists is not linting. + Markdown: "# init\n\nThe `init` command checks whether a project exists.\n\n" + + "```sh\ntool init\n```\n", + WantNonzero: []bool{false}, + }, { // Test 2: A linter subcommand of another page does not carry over. + Markdown: "# build\n\nThe `build` command bundles files.\n\n```sh\ntool recheck docs\n```\n", + WantNonzero: []bool{false}, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + if diff := cmp.Diff(test.WantSubs, describedAsLinter(test.Markdown), + cmpopts.EquateEmpty()); diff != "" { + t.Errorf("describedAsLinter mismatch (-want +got):\n%s", diff) + } + plan := BuildPlan("repo", "", test.Markdown, []string{"tool"}, nil, nil, testRecognizer()) + var got []bool + for _, l := range projectPlan(plan) { + for _, pl := range l { + got = append(got, pl.NonzeroOK) + } + } + if diff := cmp.Diff(test.WantNonzero, got); diff != "" { + t.Errorf("NonzeroOK mismatch (-want +got):\n%s", diff) + } + }) + } +} + +// TestLostDirectorySpansBlocks checks that a skipped cd loses the directory for +// the rest of its section, and that a later cd the session can make restores it. +func TestLostDirectorySpansBlocks(t *testing.T) { + t.Parallel() + const lost = "follows a skipped cd" + tests := []struct { + Markdown string + WantLost []bool + }{{ // Test 0: A skipped cd in one block skips the next block's lines. + Markdown: "```sh\ntool init\ncd \n```\n\n```sh\ngit add .gitmodules\ntool sync\n```\n", + WantLost: []bool{false, false, true, true}, + }, { // Test 1: A later cd into a directory the tree holds clears it. + Markdown: "```sh\ntool init\ncd \n```\n\n```sh\ntool sync\n```\n\n" + + "```sh\ncd examples\ntool run\n```\n", + WantLost: []bool{false, false, true, false, false}, + }, { // Test 2: A skipped cd still skips the rest of its own block. + Markdown: "```sh\ncd \ntool sync\n```\n", + WantLost: []bool{false, true}, + }, { // Test 3: A cd after a skipped cd in the same block reads from the lost + // directory, so it does not clear it. + Markdown: "```sh\ncd \ncd examples\ntool sync\n```\n", + WantLost: []bool{false, true, true}, + }, { // Test 4: A document without a skipped cd loses nothing. + Markdown: "```sh\ncd examples\ntool sync\n```\n\n```sh\ntool run\n```\n", + WantLost: []bool{false, false, false}, + }, { // Test 5: A new section ends the lost directory. + Markdown: "## Build\n\n```sh\ncd \n```\n\n```sh\ntool sync\n```\n\n" + + "## Install\n\n```sh\ntool run\n```\n", + WantLost: []bool{false, true, false}, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, "examples"), 0o755); err != nil { + t.Fatal(err) + } + plan := BuildPlan("repo", dir, test.Markdown, []string{"tool"}, nil, nil, testRecognizer()) + var got []bool + for _, s := range plan.Steps { + for _, l := range s.Lines { + isLost := strings.Contains(l.Skip, lost) + if isLost && l.SkipReason != verdict.ReasonDependsOnSkipped { + t.Errorf("%q: reason %q, want %q", l.Cmd, l.SkipReason, verdict.ReasonDependsOnSkipped) + } + got = append(got, isLost) + } + } + if diff := cmp.Diff(test.WantLost, got); diff != "" { + t.Errorf("lost mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/internal/plan/planskip.go b/internal/plan/planskip.go index 37b7d9c..d62691d 100644 --- a/internal/plan/planskip.go +++ b/internal/plan/planskip.go @@ -3,6 +3,7 @@ package plan import ( "fmt" "regexp" + "slices" "strings" "github.com/dcadolph/kibble/internal/docblock" @@ -136,7 +137,7 @@ func (pl *planner) skipReason(cmd, flat string) (string, verdict.Reason, bool) { return fmt.Sprintf("docs use %q as a placeholder the reader must fill in", bareWordPlaceholder(flat)), verdict.ReasonPlaceholder, false } - if bin != "" { + if bin != "" || readsPathCommands[firstWord(flat)] { if op := pl.readerNamedOperand(flat); op != "" { return fmt.Sprintf("docs use %q as a placeholder the reader must fill in", op), verdict.ReasonPlaceholder, false @@ -359,6 +360,19 @@ func bareWordPlaceholder(flat string) string { return "" } +// readsPathCommands are the commands whose operands name paths they read, so +// a reader-named operand of one is a path the reader must supply. +var readsPathCommands = map[string]bool{"cd": true, "ls": true, "tree": true, "cat": true} + +// firstWord returns the first whitespace-separated word of a line. +func firstWord(flat string) string { + fields := strings.Fields(flat) + if len(fields) == 0 { + return "" + } + return fields[0] +} + // reReaderNamed matches an operand docs use as a stand-in name the reader // replaces: a my-prefixed name such as my-project, or a noun with a number // such as repo1. Both are a naming convention that says "yours goes here". @@ -474,6 +488,30 @@ func watcherInvocation(sub, flat string) bool { return reWatcherFlag.MatchString(flat) } +// reLinterSub matches prose that introduces a subcommand as one that lints or +// checks for problems, capturing the subcommand's name. +var reLinterSub = regexp.MustCompile("(?i)\\b`?([a-z][a-z0-9-]+)`? (?:sub)?command " + + "(?:lints?|checks?|validates?|audits?)\\b[^.\\n]{0,60}\\b(?:for|finds?|reports?|flags?)\\b" + + "[^.\\n]{0,40}\\b(?:problems|errors|issues|violations|warnings|findings)\\b") + +// describedAsLinter returns the subcommands a document introduces as linting +// for problems, which exit nonzero when they find some. Only the subcommand is +// the claim: a linter binary exits nonzero on a configuration error too, so +// blessing the binary would turn that into a pass. +func describedAsLinter(markdown string) []string { + head := docblock.ProseOnly(markdown) + if len(head) > 1500 { + head = head[:1500] + } + var subs []string + for _, m := range reLinterSub.FindAllStringSubmatch(head, -1) { + if sub := strings.ToLower(m[1]); !slices.Contains(subs, sub) { + subs = append(subs, sub) + } + } + return subs +} + // describedAsWatcher reports whether a document introduces a binary as a tool // that keeps running. The words have to sit near the tool's name, since a // document can mention watching for reasons that say nothing about the diff --git a/internal/plan/rules_test.go b/internal/plan/rules_test.go index 5eac30a..19e7d02 100644 --- a/internal/plan/rules_test.go +++ b/internal/plan/rules_test.go @@ -3,6 +3,8 @@ package plan import ( "fmt" "github.com/dcadolph/kibble/internal/config" + "github.com/dcadolph/kibble/internal/docblock" + "github.com/dcadolph/kibble/internal/verdict" "testing" ) @@ -50,3 +52,51 @@ func TestRuleSelects(t *testing.T) { }) } } + +// TestSkipReasonReadsPathCommands checks that a reader-named operand of a +// command that reads paths skips as a placeholder when nothing provides it, +// and that provided names and unrelated commands run. +func TestSkipReasonReadsPathCommands(t *testing.T) { + t.Parallel() + + tests := []struct { + Tree map[string]bool + Created map[string]bool + Line string + WantSkip string + WantReason verdict.Reason + }{{ // Test 0: tree of a stand-in directory is a placeholder. + Line: `tree my-dotfiles`, + WantSkip: `docs use "my-dotfiles" as a placeholder the reader must fill in`, + WantReason: verdict.ReasonPlaceholder, + }, { // Test 1: cd into a directory an earlier line made runs. + Created: map[string]bool{"my-app": true}, Line: `cd my-app`, + }, { // Test 2: cat of a name the tree provides runs. + Tree: map[string]bool{"my-notes": true}, Line: `cat my-notes`, + }, { // Test 3: a commit message is not a path, so only the git rule fires. + Line: `git commit -m my-message`, + WantSkip: "reads the git repository, which in the session is one kibble made rather than the reader's", + WantReason: verdict.ReasonMissingFixture, + }, { // Test 4: tree of an ordinary directory runs. + Line: `tree docs`, + }} + + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + tree := test.Tree + if tree == nil { + tree = map[string]bool{} + } + pl := newReaderNameTestPlanner(tree) + for k := range test.Created { + pl.created[k] = true + } + skip, reason, _ := pl.skipReason(test.Line, docblock.Flatten(test.Line)) + if skip != test.WantSkip || reason != test.WantReason { + t.Errorf("skipReason = (%q, %q), want (%q, %q)", + skip, reason, test.WantSkip, test.WantReason) + } + }) + } +} diff --git a/lineverdict.go b/lineverdict.go index 478cbba..cf7e171 100644 --- a/lineverdict.go +++ b/lineverdict.go @@ -73,9 +73,20 @@ func classifyLineResult(lr lineResult, l kplan.PlanLine, o lineOutcome, wrapped lr.Detail = "ran against " + strings.Join(lr.Synthetic, ", ") + ", which kibble fabricated because no documented step creates it" } - case l.NonzeroOK && documentedNonzeroCode(o.code): + case l.NonzeroOK && documentedNonzeroCode(o.code) && !reCrash.MatchString(o.output): + // A crash stays a failure even where the document blesses a nonzero exit. lr.Status = StatusVerified lr.Detail = fmt.Sprintf("exit %d is documented behavior", o.code) + case documentedNonzeroCode(o.code) && shownInOutput(l.Shown, o.output): + // The document shows the output under the command and the tool printed + // it, so the nonzero exit is the outcome the document describes. + lr.Status = StatusVerified + lr.Detail = fmt.Sprintf("exit %d is documented behavior: the document shows this output", + o.code) + case reCrash.MatchString(o.output) && crashWantsInput(o.output): + lr.Status = StatusSkipped + lr.Reason = ReasonInteractive + lr.Detail = "waited for input, and the session's stdin is closed: " + tail case reCrash.MatchString(o.output) && crashMissingName(o.output) != "": // The one crash that is not unexplained. A traceback whose last line // names the file or program that was missing is the tool saying what @@ -187,18 +198,14 @@ func classifyLineResult(lr lineResult, l kplan.PlanLine, o lineOutcome, wrapped lr.Reason = ReasonMissingFixture lr.Detail = "names a git ref this repository does not have, which a document writes " + "both as a placeholder the reader replaces and as a ref that moved: " + tail - case registeredNoun(lr.Cmd, o.output) != "" || namelessRegisteredNoun(o.output) != "": - // The tool named a preset, plugin, or similar that the command passed, - // or said it has none without naming one. The reader defines those - // first, and the tool says only that it has none. + case registeredDetail(lr.Cmd, o.output) != "": + // The tool said it has no preset, plugin, or similar by that name, or + // none at all. The reader defines those first. A name the command never + // passed came from the tool's defaults, its environment, or the reader's + // configuration, none of which is the document's claim. lr.Status = StatusBlocked lr.Reason = ReasonMissingFixture - if noun := registeredNoun(lr.Cmd, o.output); noun != "" { - lr.Detail = fmt.Sprintf("names a %s the reader defines first: %s", noun, tail) - } else { - lr.Detail = fmt.Sprintf("reached for a %s the reader defines first: %s", - namelessRegisteredNoun(o.output), tail) - } + lr.Detail = registeredDetail(lr.Cmd, o.output) + ": " + tail case reRemoteConfig.MatchString(o.output): lr.Status = StatusBlocked lr.Reason = ReasonMissingFixture @@ -207,6 +214,15 @@ func classifyLineResult(lr lineResult, l kplan.PlanLine, o lineOutcome, wrapped lr.Status = StatusBlocked lr.Reason = ReasonMissingFixture lr.Detail = "could not load a configuration, which the reader's project supplies: " + tail + case reReaderConfig.MatchString(o.output): + lr.Status = StatusBlocked + lr.Reason = ReasonMissingFixture + lr.Detail = "points to a configuration entry the reader writes first: " + tail + case reWantsPath.MatchString(o.output) && bareOperandAfterSub(lr.Cmd) != "": + lr.Status = StatusBlocked + lr.Reason = ReasonMissingFixture + lr.Detail = "passed " + bareOperandAfterSub(lr.Cmd) + + " where the tool wanted a path, a name the reader defines in configuration: " + tail case reNothingSearched.MatchString(o.output): // Not a skip: the line ran. Not a failure either, since a search that // was handed nothing to read has not tested what the document claims. @@ -293,6 +309,13 @@ func classifyLineResult(lr lineResult, l kplan.PlanLine, o lineOutcome, wrapped return lr } +// registeredNouns is the closed list of names a reader defines before a tool +// can use them. It is closed on purpose: argument, flag, option, command, and +// subcommand name a tool's own vocabulary, so a document using one wrongly is +// broken. +const registeredNouns = "preset|profile|alias|plugin|group|context|workspace|project|remote" + + "|registry|template|theme" + var ( // reUsageError matches a tool rejecting the words of the command itself. reUsageError = regexp.MustCompile(`(?i)\b(unexpected|unrecognized|unknown|invalid) ` + @@ -358,25 +381,32 @@ var ( `|could not find remote (branch|ref) '?([^\s']+)'?` + `|(?:remote )?ref '?([^\s']+)'? not found`) // reUnknownRegistered matches a tool saying it has no preset, profile, - // plugin, or similar of the name given. The noun list is closed on purpose: - // argument, flag, option, command, and subcommand name a tool's own - // vocabulary, so a document using one wrongly is broken. + // plugin, or similar of the name given. reUnknownRegistered = regexp.MustCompile( - `(?i)unknown (preset|profile|alias|plugin|group|context|workspace|project|remote` + - `|registry|template|theme) '?([^\s'"]+)'?`) + `(?i)unknown (` + registeredNouns + `) '?([^\s'"]+)'?`) // reUnknownNameless matches the same refusal over the same closed noun list, // with an optional group that captures a name written after the noun and a // space or a colon. The nameless form applies only when that group is empty, // i.e. the noun is at the end of the text or is followed by a closing quote - // or punctuation. When a name follows, only the named form decides, because - // it deliberately returns FAIL for a name the command never passed. + // or punctuation. When a name follows, only the named forms decide, and a + // name after a colon is not one they read. reUnknownNameless = regexp.MustCompile( - `(?i)\bunknown (preset|profile|alias|plugin|group|context|workspace|project|remote` + - `|registry|template|theme)\b( +'?[^\s'"]|\s*:\s*'?[^\s'"])?`) + `(?i)\bunknown (` + registeredNouns + `)\b( +'?[^\s'"]|\s*:\s*'?[^\s'"])?`) // reRegisteredNotFound matches the same refusal worded as a name not found. reRegisteredNotFound = regexp.MustCompile( - `(?i)\b(preset|profile|alias|plugin|group|context|workspace|project|remote|registry` + - `|template|theme) '([^']+)' not found`) + `(?i)\b(` + registeredNouns + `) '([^']+)' not found`) + // reNoSuchRegistered matches the same refusal worded as no such noun, with + // the name after it optional and optionally quoted. + reNoSuchRegistered = regexp.MustCompile( + `(?i)\bno such (` + registeredNouns + `)\b(?: +["']?([^\s"']+)["']?)?`) + // reForThisRegistered matches a tool reporting nothing to do for the + // current noun, which names no name. + reForThisRegistered = regexp.MustCompile( + `(?i)\bfor (?:this|the current) (` + registeredNouns + `)\b`) + // reNoneRegistered matches a tool saying it has none of the noun at all. + reNoneRegistered = regexp.MustCompile( + `(?i)\bno (` + registeredNouns + `)s? (?:are|is) (?:currently )?` + + `(?:installed|configured|registered|defined|loaded|enabled)\b`) // reRemoteConfig matches a tool naming a remote or configuration section // that the reader configures first, in rclone's family of wordings. reRemoteConfig = regexp.MustCompile( @@ -385,7 +415,20 @@ var ( // reConfigLoad matches a tool that could not load a configuration, which // the reader's project supplies and a fresh session does not have. reConfigLoad = regexp.MustCompile( - `(?i)(failed|unable|could not) to (load|resolve|read) (the )?config(uration)?( file)?`) + `(?i)(failed|unable|could not) to (load|resolve|read|use|parse|import|find|open) ` + + `(the )?config(uration)?( file)?`) + // reReaderConfig matches output telling the reader to define, add, or set + // something in or under their configuration. + reReaderConfig = regexp.MustCompile( + `(?i)\b(define|configure|declare|add|set|specify|list)\b[^.\n]{0,80}` + + `\b(in|under|to) (your|the|an?) (` + + "`" + `?[\w.-]*config(uration)?[\w.-]*` + "`" + `?( file)?` + + `|` + "`" + `?[\w.-]+\.(ya?ml|toml|json|ini)` + "`" + `?` + + `|` + "`" + `[\w-]+:?` + "`" + ` (entry|section|block|key))`) + // reWantsPath matches a tool asking for a valid path, file, or directory. + reWantsPath = regexp.MustCompile( + `(?i)\b(provide|specify|expected|requires?|needs?) (a )?valid ` + + `(path|file ?name|file|directory)\b`) // reNothingSearched matches a search reporting that its filters left it // nothing to look at. The command ran and worked; what it says is that // this repository holds no file of the kind the example targets, as @@ -443,7 +486,7 @@ var ( // rule that excuses a line: a tool that panicked after printing "no // results" did not find no results, it broke. reCrash = regexp.MustCompile( - `(?im)^(panic|fatal error|traceback \(most recent call last\)):` + + `(?im)(^(panic|fatal error):|(^|\s)traceback \(most recent call last\):)` + `|\bsegmentation fault\b|\bassertion failed\b|\bstack overflow\b` + `|\bunhandled exception\b|\bthread '.*' panicked at\b`) // reGitNoHistory matches git reporting that the repository it was handed @@ -525,6 +568,9 @@ var reNodeEnoent = regexp.MustCompile(`ENOENT: no such file or directory, \w+ '( // tach says "The path 'my_package' does not exist." var reNotExist = regexp.MustCompile(`(?i)\b(?:path|file|directory) '([^']+)' does not exist`) +// rePathNotExist matches a slash-bearing path a tool says does not exist. +var rePathNotExist = regexp.MustCompile(`(?:^|\s)([\w./-]*/[\w.-]+) does not exist`) + // reFileNotFound matches uv's sentence for a missing input, as in // "error: File not found: requirements.in". var reFileNotFound = regexp.MustCompile(`(?i)\bfile not found: '?"?([^\s'"]+)`) @@ -544,7 +590,8 @@ var reErrnoName = regexp.MustCompile(`^E[A-Z]+$`) // generic shape, which would otherwise capture ENOENT itself. func reportedNames(output string) []string { var out []string - for _, re := range []*regexp.Regexp{reNodeEnoent, reNotExist, reFileNotFound, reEnoent} { + shapes := []*regexp.Regexp{reNodeEnoent, reNotExist, rePathNotExist, reFileNotFound, reEnoent} + for _, re := range shapes { m := re.FindStringSubmatch(output) if m == nil { continue @@ -581,6 +628,13 @@ func crashMissingName(output string) string { return "" } +// crashWantsInput reports whether a crash's last line is an EOFError, which is +// a program reading a prompt from a stdin that is closed. +func crashWantsInput(output string) bool { + lines := strings.Split(strings.TrimSpace(output), "\n") + return strings.HasPrefix(strings.TrimSpace(lines[len(lines)-1]), "EOFError") +} + // namedInCommand reports whether the command passed name. Words are compared // whole against the words the shell would pass, so a quoted path containing a // space is one argument here rather than two. The value of a --flag=name @@ -772,33 +826,113 @@ func undocumentedSetting(lines []string, documented map[string]bool) string { return "" } -// registeredNoun returns the lowercase noun of a registered name the tool -// reported missing when the command passed that name as a word, or empty. A -// name the command never passed is the tool's own business, so it convicts -// nothing here. -func registeredNoun(cmd, output string) string { +// bareOperandAfterSub returns the first operand after the command word that is +// not a flag, is not the subcommand, and holds no dot and no slash, or empty. +func bareOperandAfterSub(cmd string) string { + words := shell.ArgWordsOrFields(cmd) + if len(words) < 2 { + return "" + } + for i, w := range words[1:] { + if strings.HasPrefix(w, "-") || (i == 0 && shell.IsSubcommandName(w)) { + continue + } + if !strings.ContainsAny(w, "./") { + return w + } + return "" + } + return "" +} + +// reProgramPrefix matches the program name a tool writes before its message, +// which a document showing the output may spell differently. +var reProgramPrefix = regexp.MustCompile(`^[\w.-]+: `) + +// minShownLen is the shortest shown line that counts as evidence, so a word +// such as "error" that any failure prints proves nothing. +const minShownLen = 12 + +// shownInOutput reports whether the output contains a line the document +// shows under the command, whole or without its leading program name. Without +// the name, the output must print a program name of its own right before the +// rest, since a label such as "Or: " in front of a command the tool echoes is +// not a program name. +func shownInOutput(shown []string, output string) bool { + for _, s := range shown { + s = strings.TrimSpace(s) + if len(s) >= minShownLen && strings.Contains(output, s) { + return true + } + rest := reProgramPrefix.ReplaceAllString(s, "") + if rest == s || len(rest) < minShownLen { + continue + } + if regexp.MustCompile(`(?m)(^|\s)[\w.-]+: ` + regexp.QuoteMeta(rest)).MatchString(output) { + return true + } + } + return false +} + +// registeredDetail returns the verdict detail, without the output tail, for a +// tool reporting a registered name it does not have, or empty when the output +// reports none. A name the command passed is preferred over one it did not. +func registeredDetail(cmd, output string) string { + noun, passed := registeredNoun(cmd, output) + switch { + case passed: + return fmt.Sprintf("names a %s the reader defines first", noun) + case noun != "": + return fmt.Sprintf("reached for a %s the reader defines first, which the command never named", + noun) + } + if noun := namelessRegisteredNoun(output); noun != "" { + return fmt.Sprintf("reached for a %s the reader defines first", noun) + } + return "" +} + +// registeredNoun returns the lowercase noun of the first registered name the +// tool reported missing and whether the command passed that name as a word. A +// name the command passed is returned before one it did not. +func registeredNoun(cmd, output string) (noun string, passed bool) { words := shell.ArgWordsOrFields(cmd) - for _, re := range []*regexp.Regexp{reUnknownRegistered, reRegisteredNotFound} { + named := []*regexp.Regexp{reUnknownRegistered, reRegisteredNotFound, reNoSuchRegistered} + for _, re := range named { for _, m := range re.FindAllStringSubmatch(output, -1) { + if m[2] == "" { + continue + } + if noun == "" { + noun = strings.ToLower(m[1]) + } for _, tok := range words { if _, val, ok := strings.Cut(tok, "="); ok && val == m[2] { - return strings.ToLower(m[1]) + return strings.ToLower(m[1]), true } if tok == m[2] { - return strings.ToLower(m[1]) + return strings.ToLower(m[1]), true } } } } - return "" + return noun, false } // namelessRegisteredNoun returns the lowercase noun of a registered name the -// tool said it does not know without naming one, or empty. A noun followed by +// tool said it does not have without naming one, or empty. A noun followed by // a space and a name is left to registeredNoun. func namelessRegisteredNoun(output string) string { - for _, m := range reUnknownNameless.FindAllStringSubmatch(output, -1) { - if m[2] == "" { + for _, re := range []*regexp.Regexp{reUnknownNameless, reNoSuchRegistered} { + for _, m := range re.FindAllStringSubmatch(output, -1) { + if m[2] == "" { + return strings.ToLower(m[1]) + } + } + } + for _, re := range []*regexp.Regexp{reForThisRegistered, reNoneRegistered} { + if m := re.FindStringSubmatch(output); m != nil { return strings.ToLower(m[1]) } } diff --git a/lineverdict_evidence_test.go b/lineverdict_evidence_test.go index 121c286..8951b31 100644 --- a/lineverdict_evidence_test.go +++ b/lineverdict_evidence_test.go @@ -33,10 +33,11 @@ func TestClassifyRegisteredAndEnvironment(t *testing.T) { Output: "\n✗ Plugin Error\n\n Plugin 'my_plugin' not found\n\n", Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, WantDetail: "names a plugin the reader defines first", - }, { // Test 2: The same preset text for a command that never names it is a failure. + }, { // Test 2: A preset the command never named came from defaults or configuration. Cmd: "lla --preset other", Output: "Error: Filter(\"Unknown preset 'source-files'\")\n", - Code: 1, WantStatus: StatusFail, + Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "reached for a preset the reader defines first, which the command never named", }, { // Test 3: A configuration that would not resolve is the reader's project to supply. Cmd: "cspell .", Output: `Failed to process "/work/repo/packages/cspell-trie-lib/api/README.md" Configuration Loader Error: Failed to resolve configuration file: "@cspell/dict-es-es/cspell-ext.json" referenced from "./packages/cspell-trie-lib/cspell.json" @@ -102,10 +103,80 @@ Require stack: Cmd: "tool frob", Output: "unknown command \"frob\"\n", Code: 1, WantStatus: StatusFail, - }, { // Test 12: A name after a colon is a name, so only the named form decides. + }, { // Test 12: No named form reads a name after a colon, so the line falls through to FAIL. Cmd: "tool run", Output: "Error: unknown profile: dev\n", Code: 1, WantStatus: StatusFail, + }, { // Test 13: Told to define an API in the config, the reader writes that first. + Cmd: "redocly bundle", + Output: "No APIs were provided. Specify an API via the command argument " + + "or define one in your config.\n", + Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "points to a configuration entry the reader writes first", + }, { // Test 14: An entry told to be added under a key is configuration the reader writes. + Cmd: "redocly generate-client", + Output: "No API to generate. Add a `client` block or `clientOutput` under an " + + "`apis:` entry, or pass \n", + Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "points to a configuration entry the reader writes first", + }, { // Test 15: A bare word where a path was wanted is a name defined in configuration. + Cmd: "redocly stats core@v1", + Output: "Please provide a valid path.\n", + Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "passed core@v1 where the tool wanted a path", + }, { // Test 16: A file-shaped operand that is refused stays a failure. + Cmd: "redocly stats spec.yaml", + Output: "Please provide a valid path.\n", + Code: 1, WantStatus: StatusFail, + }, { // Test 17: No operand at all stays a failure. + Cmd: "redocly stats", + Output: "Please provide a valid path.\n", + Code: 1, WantStatus: StatusFail, + }, { // Test 18: A configuration file that will not import is the reader's project to supply. + Cmd: `mdlint "**/*.md"`, + Output: "Error: Unable to use configuration file '/work/repo/test/x/.lintrc.cjs'; " + + "Unable to import module\n", + Code: 2, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "could not load a configuration", + }, { // Test 19: A relative path the tool says does not exist is a file the reader lacks. + Cmd: "redocly eject-generator php --update", + Output: "Nothing to update: generators/php/index.ts does not exist. " + + "Eject first.\n", + Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "generators/php/index.ts", + }, { // Test 20: An unknown flag named config is the document's defect. + Cmd: "redocly bundle --config", + Output: "Unknown argument: --config\n", + Code: 1, WantStatus: StatusFail, + }, { // Test 21: Nothing to do for this profile is a profile the reader defines first. + Cmd: "dotdrop install", + Output: "[WARN] no dotfile to install for this profile (\"87271e6263ea\")\n", + Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "reached for a profile the reader defines first", + }, { // Test 22: A profile only the tool names is one the reader defines first. + Cmd: "dotdrop update --key f_vimrc", + Output: "[ERR] no such profile \"87271e6263ea\"\n", + Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "reached for a profile the reader defines first, which the command never named", + }, { // Test 23: No such profile the command passed names one the reader defines first. + Cmd: "dotdrop install -p work", + Output: "[ERR] no such profile \"work\"\n", + Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "names a profile the reader defines first", + }, { // Test 24: No plugins installed is a plugin the reader installs first. + Cmd: "lla update", + Output: "\n No plugins are currently installed. Run 'lla install' to add one.\n", + Code: 1, WantStatus: StatusBlocked, WantReason: ReasonMissingFixture, + WantDetail: "reached for a plugin the reader defines first", + }, { // Test 25: A missing file is not a registered name. + Cmd: "tool load config.yaml", + Output: "Error: open config.yaml: no such file or directory\n", + Code: 1, WantStatus: StatusGap, + WantDetail: "references config.yaml, which no documented step creates", + }, { // Test 26: An unknown flag named profile is the document's defect. + Cmd: "tool run --profile dev", + Output: "Error: unknown flag: --profile\n", + Code: 1, WantStatus: StatusFail, }} for testNum, test := range tests { @@ -123,3 +194,85 @@ Require stack: }) } } + +// TestClassifyShownOutput checks that a nonzero exit is documented behavior +// only when the tool printed the output the document shows under the command. +func TestClassifyShownOutput(t *testing.T) { + t.Parallel() + tests := []struct { + WantStatus Status + WantDetail string + Shown []string + Output string + Code int + }{{ // Test 0: The refusal the document shows, printed under the tool's own name. + Shown: []string{"rm: target '/' skipped, unsafe directory scope"}, + Output: "safe-rm: target '/' skipped, unsafe directory scope\n", + Code: 1, + WantStatus: StatusVerified, + WantDetail: "exit 1 is documented behavior: the document shows this output", + }, { // Test 1: A shown line that the tool did not print leaves the failure. + Shown: []string{"all done"}, + Output: "safe-rm: target '/' skipped, unsafe directory scope\n", + Code: 1, + WantStatus: StatusFail, + }, { // Test 2: A matching line on exit 127 is not an ordinary exit. + Shown: []string{"rm: target '/' skipped, unsafe directory scope"}, + Output: "safe-rm: target '/' skipped, unsafe directory scope\n", + Code: 127, + WantStatus: StatusSkipped, + }, { // Test 3: A shown line under 12 characters is too short to be evidence. + Shown: []string{"error"}, + Output: "error\n", + Code: 1, + WantStatus: StatusFail, + }, { // Test 4: A prose label is not a program name, so an echoed command proves nothing. + Shown: []string{"Or: tool schema generate"}, + Output: "tool schema generate\nmake: *** [schema] Error 1\n", + Code: 2, + WantStatus: StatusFail, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + lr := classifyLineResult(lineResult{Cmd: `SAFE_RM_SCOPE="$HOME" safe-rm -rf /`}, + kplan.PlanLine{Shown: test.Shown}, + lineOutcome{code: test.Code, output: test.Output}, false, nil, lineTimeout) + if lr.Status != test.WantStatus { + t.Errorf("verdict = %s %q, want %s (detail %q)", + lr.Status, lr.Reason, test.WantStatus, lr.Detail) + } + if !strings.Contains(lr.Detail, test.WantDetail) { + t.Errorf("detail = %q, want it to contain %q", lr.Detail, test.WantDetail) + } + }) + } +} + +// TestClassifyCheckModeCrash checks that a check-mode line which crashed is +// not a documented finding, while one that reported findings is. +func TestClassifyCheckModeCrash(t *testing.T) { + t.Parallel() + tests := []struct { + WantStatus Status + Output string + }{{ // Test 0: A traceback on a check-mode line stays a failure. + Output: "Traceback (most recent call last):\n File \"/usr/bin/tool\", line 8, in \n" + + " sys.exit(main())\nKeyError: 'rules'\n", + WantStatus: StatusFail, + }, { // Test 1: A plain finding report is the documented exit. + Output: "docs/a.md:3: MD001 heading levels should only increment by one\n1 problem\n", + WantStatus: StatusVerified, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + lr := classifyLineResult(lineResult{Cmd: "tool lint ."}, kplan.PlanLine{NonzeroOK: true}, + lineOutcome{code: 1, output: test.Output}, false, nil, lineTimeout) + if lr.Status != test.WantStatus { + t.Errorf("verdict = %s %q, want %s (detail %q)", + lr.Status, lr.Reason, test.WantStatus, lr.Detail) + } + }) + } +} diff --git a/lineverdict_missing_test.go b/lineverdict_missing_test.go index 5dd7a48..2872182 100644 --- a/lineverdict_missing_test.go +++ b/lineverdict_missing_test.go @@ -232,6 +232,35 @@ FileNotFoundError: [Errno 2] No such file or directory: 'patfile' WantStatus: StatusBlocked, WantReason: ReasonDependsOnSkipped, WantDetail: "references patfile, which an earlier line that did not pass may create: " + "make-patterns > patfile", + }, { // Test 22: A traceback header after a progress line still reads as the crash it is. + Cmd: "MMD_CLI_JOBS=1 python3 scripts/bench_compare.py", + Output: " flowchart_small... Traceback (most recent call last):\n" + + " File \"/work/repo/scripts/bench_compare.py\", line 10, in \n" + + " main()\n" + + " File \"/usr/lib/python3.13/subprocess.py\", line 1972, in _execute_child\n" + + "FileNotFoundError: [Errno 2] No such file or directory: 'npx'\n", + Code: 1, WantStatus: StatusSkipped, WantReason: ReasonMissingDependency, + WantDetail: "reported by the tool that invoked it", + }, { // Test 23: A traceback ending at a closed stdin is interactive. + Cmd: "dotdrop import ~/.zshrc", + Output: "Apply these changes? [y/N] ? Traceback (most recent call last):\n" + + " File \"/usr/bin/dotdrop\", line 8, in \n" + + " sys.exit(main())\n" + + " File \"/usr/lib/python3/dotdrop/dotdrop.py\", line 90, in ask\n" + + " resp = input(qmsg)\n" + + "EOFError: EOF when reading a line\n", + Code: 1, WantStatus: StatusSkipped, WantReason: ReasonInteractive, + WantDetail: "waited for input", + }, { // Test 24: A line-start traceback ending in another error stays a failure. + Cmd: "python3 tool.py", + Output: "Traceback (most recent call last):\n" + + " File \"tool.py\", line 3, in \n" + + "KeyError: 'x'\n", + Code: 1, WantStatus: StatusFail, WantDetail: "crashed with exit 1", + }, { // Test 25: A Node module error with no traceback stays a failure. + Cmd: "nve 18 node app.js", + Output: "Error: Cannot find module 'node:path'\n", + Code: 1, WantStatus: StatusFail, }} for testNum, test := range tests {