diff --git a/scripts/conformance/rubyspec/README.md b/scripts/conformance/rubyspec/README.md index 14ed22ef..952f2827 100644 --- a/scripts/conformance/rubyspec/README.md +++ b/scripts/conformance/rubyspec/README.md @@ -212,6 +212,47 @@ Three engines is the shape the measurement rules want: reference, control (`main`), treatment (the branch). A difference between the last two that the reference does not share is the defect, or the fix. +## Holding the corpus still — `cmd/specdelta` + +`cmd/ratchet` judges one sweep against `BASELINE`. That is the right gate for +*"may this land"* and the wrong instrument for *"what does this change do"*: a +baseline comparison folds in every difference between the recording machine and +this one. `cmd/specdelta` asks the other question — hold the corpus and the +machine fixed, vary only the interpreter, and name every file that moved. + +```console +$ specdelta -root ~/ruby-spec-corpus -dir core/set -quiet -repeat 3 ./rbgo-main ./rbgo-409 +file control treat delta +core/set/flatten_merge_spec.rb 3 0 -3 +core/set/to_s_spec.rb 2 4 +2 +core/set/divide_spec.rb 9 8 -1 +... +57 file(s): control 175 passing, treatment 167 passing, -8 +9 file(s) moved, 8 regressed +``` + +That table took the `Set` half of the 4.0 version move from "eight files to work +through" to **two defects** — the other six are examples 4.0 stopped asking for, +which the deltas cannot tell you and the `ruby_version_is` guards can. + +**`-repeat N`.** One run per engine cannot tell a FLAKE from a difference. The +tool's first real use reported `core/random/bytes_spec.rb` as `-1` between two +binaries that differ by one file and three constants; three runs of each gave +control 9, 9, 8 and treatment 8, 9, 9. Above 1, each file runs N times per +engine, the **lowest** count is kept, and an engine that disagrees with itself +marks the file `UNSTABLE` and has its delta excluded from the verdict. + +**Two channels.** `RBGO_RESULT pass=…` comes out of the subject's own stdout, so +a subject that miscounts can claim anything. specdelta also counts the +`RBGO_DETAIL` lines and flags a file that printed **fewer** than its own +`fail + error` tally. It is an inequality and not an equality, because the shim +emits a detail with no counter for a `before(:all)` failure and a counter with +no detail for a guard-driven skip. + +Exit **2**, never 1, for a refusal: an engine that would not start, an +unreadable result line, two result lines in one run, or an **empty population** +— a mistyped `-dir` would otherwise print `0 files, no regressions` and exit 0. + ## Caveats The shim is not full MSpec: a few matchers (`complain`, `output`, diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main.go b/scripts/conformance/rubyspec/cmd/specdelta/main.go new file mode 100644 index 00000000..45222648 --- /dev/null +++ b/scripts/conformance/rubyspec/cmd/specdelta/main.go @@ -0,0 +1,368 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +// Command specdelta runs a set of ruby/spec files through TWO interpreters and +// reports, file by file, how many examples each one passed. +// +// It answers a question `cmd/ratchet` cannot. The ratchet judges one sweep +// against `BASELINE`, which is the right gate for "may this land" but the wrong +// instrument for "what does THIS change do" — a baseline comparison folds in +// every difference between the recording machine and this one, and #799's +// version move needed the other reading: hold the corpus and the machine fixed, +// vary only the binary, and name every file that moved. +// +// specdelta -dir core/set -root ~/ruby-spec ./rbgo-main ./rbgo-branch +// +// ⛔ Two channels, not one. The `RBGO_RESULT pass=…` line comes out of the +// SUBJECT's own stdout (#843), so a subject that miscounts can report any total +// it likes. specdelta also counts the `RBGO_DETAIL` lines, which the shim emits +// one per failure and error, and flags any file where the two disagree — +// `fail+error` claimed against detail lines seen. That is not a theoretical +// concern: reading the detail lines rather than the tally is what caught a +// figure misattributed between two branches on 2026-11-01. +// +// Exit status: +// +// 0 every file ran and no file passed fewer examples on the second engine +// 1 at least one file regressed +// 2 refused: no files, an engine that would not start, or output that could +// not be read — never reported as a regression, because "could not measure" +// and "measured worse" need opposite responses +package main + +import ( + "bytes" + "errors" + "flag" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "sort" + "strconv" + "strings" + "time" +) + +// result is one spec file's outcome under one engine. +type result struct { + pass, fail, errs, skip int + details int // RBGO_DETAIL lines seen + ran bool // a RESULT line was found at all + killed bool +} + +// tallyDisagrees reports whether the subject printed FEWER detail lines than +// its own tally of failures and errors. That is a one-sided check, and the +// asymmetry is the shim's, not a hedge: +// +// kind counter detail line +// fail $RB_FAIL yes (spec_helper.rb:983, :987) +// error $RB_ERROR yes (:995, :1029, :1102) +// skip $RB_SKIP yes (:990, :993) +// skip $RB_SKIP NO (:970 — a version/platform guard) +// hookerror NONE yes (:965, :1020 — before/after :all) +// +// So `details == fail+error` is wrong in both directions: a guard-skipped file +// has a detail with no failure, and a `before(:all)` that raises has a detail +// counted in no tally at all. `details >= fail+error` is what holds, because +// those two kinds each push exactly one line — and a shortfall means detail +// lines went missing, which makes the whole report unreadable. +// +// ⛔ The first version of this asserted equality and fired on the first real +// run, against `core/set/set_spec.rb` — which is `pass=0 fail=0 error=0 skip=1` +// with one `skip` detail, entirely well-formed. Reading the shim rather than +// assuming its shape is what produced the table above; the line +// `hookerror` has no counter is a defect in the harness, filed separately. +func (r result) tallyDisagrees() bool { + return r.ran && r.details < r.fail+r.errs +} + +func main() { + os.Exit(main1(os.Args[1:], os.Stdout, os.Stderr)) +} + +func main1(args []string, out, errOut io.Writer) int { + fs := flag.NewFlagSet("specdelta", flag.ContinueOnError) + fs.SetOutput(errOut) + root := fs.String("root", ".", "the ruby/spec checkout") + dir := fs.String("dir", "", "a directory under -root to walk (recursively), e.g. core/set") + timeout := fs.Duration("timeout", 60*time.Second, "per-file limit for one engine") + quiet := fs.Bool("quiet", false, "print only the files that moved") + // ⛔ One run per engine cannot tell a FLAKE from a difference, and the first + // real use of this tool reported `core/random/bytes_spec.rb` as -1 between two + // binaries that differ only in a version string. Three runs of each: control + // 9, 9, 8 and treatment 8, 9, 9 — a coin flip, attributed to the change. + // With -repeat above 1 a file whose own engine disagrees with itself is named + // UNSTABLE and kept out of the regression verdict. + repeat := fs.Int("repeat", 1, "run each file this many times per engine; above 1, detect flaky files") + if err := fs.Parse(args); err != nil { + return 2 + } + engines := fs.Args() + if len(engines) != 2 { + fmt.Fprintln(errOut, "specdelta: name exactly two interpreters — the control first, the treatment second") + return 2 + } + if *dir == "" { + fmt.Fprintln(errOut, "specdelta: -dir is required") + return 2 + } + files, err := specFiles(*root, *dir) + if err != nil { + fmt.Fprintf(errOut, "specdelta: %v\n", err) + return 2 + } + if len(files) == 0 { + // ⛔ An empty population is a refusal, not a clean result: a mistyped + // -dir would otherwise print "0 files, no regressions" and pass. + fmt.Fprintf(errOut, "specdelta: no _spec.rb files under %s\n", filepath.Join(*root, *dir)) + return 2 + } + + if *repeat < 1 { + fmt.Fprintln(errOut, "specdelta: -repeat must be at least 1") + return 2 + } + type row struct { + file string + a, b result // the LOWEST pass count seen, so a verdict is never built on a lucky run + unstable bool // an engine disagreed with itself across repeats + } + rows := make([]row, 0, len(files)) + refused := false + for _, f := range files { + r := row{file: f} + for i, engine := range engines { + lowest, varied, err := runRepeatedly(engine, *root, f, *timeout, *repeat) + if err != nil { + fmt.Fprintf(errOut, "specdelta: %s on %s: %v\n", engines[i], f, err) + refused = true + continue + } + if varied { + r.unstable = true + } + if i == 0 { + r.a = lowest + } else { + r.b = lowest + } + } + rows = append(rows, r) + } + if refused { + return 2 + } + + // Loudest movement first: a reader wants the biggest change, not the + // alphabetically first one. + sort.SliceStable(rows, func(i, j int) bool { + di := rows[i].b.pass - rows[i].a.pass + dj := rows[j].b.pass - rows[j].a.pass + if di != dj { + return abs(di) > abs(dj) + } + return rows[i].file < rows[j].file + }) + + var totalA, totalB, moved, regressed, suspect int + fmt.Fprintf(out, "%-56s %7s %7s %6s\n", "file", "control", "treat", "delta") + for _, r := range rows { + d := r.b.pass - r.a.pass + if d != 0 { + moved++ + } + // ⛔ An unstable file is kept OUT of the verdict: its delta is noise, and + // counting it would make the exit status depend on a coin flip. + if d < 0 && !r.unstable { + regressed++ + } + totalA += r.a.pass + totalB += r.b.pass + note := "" + switch { + case !r.a.ran && !r.b.ran: + note = " DID NOT RUN on either" + case !r.a.ran: + note = " DID NOT RUN on the control" + case !r.b.ran: + note = " DID NOT RUN on the treatment" + case r.a.killed || r.b.killed: + note = " KILLED on timeout" + } + if r.unstable { + note += " ⛔ UNSTABLE: an engine disagreed with itself, delta excluded" + } + if r.a.tallyDisagrees() || r.b.tallyDisagrees() { + note += " ⛔ fewer detail lines than its own fail+error tally" + suspect++ + } + if *quiet && d == 0 && note == "" { + continue + } + fmt.Fprintf(out, "%-56s %7d %7d %+6d%s\n", r.file, r.a.pass, r.b.pass, d, note) + } + fmt.Fprintf(out, "\n%d file(s): control %d passing, treatment %d passing, %+d\n", + len(rows), totalA, totalB, totalB-totalA) + fmt.Fprintf(out, "%d file(s) moved, %d regressed\n", moved, regressed) + if suspect > 0 { + // A total built on a tally the subject contradicts is not a measurement. + fmt.Fprintf(errOut, "specdelta: %d file(s) reported a tally their detail lines contradict\n", suspect) + return 2 + } + if regressed > 0 { + return 1 + } + return 0 +} + +func abs(n int) int { + if n < 0 { + return -n + } + return n +} + +// specFiles lists the *_spec.rb files under root/dir, as paths relative to root +// — which is how the shim expects to be handed them. +func specFiles(root, dir string) ([]string, error) { + base := filepath.Join(root, dir) + info, err := os.Stat(base) + if err != nil { + return nil, err + } + if !info.IsDir() { + return nil, fmt.Errorf("%s is not a directory", base) + } + var out []string + err = filepath.Walk(base, func(p string, fi os.FileInfo, err error) error { + if err != nil { + return err + } + if fi.IsDir() || !strings.HasSuffix(p, "_spec.rb") { + return nil + } + rel, err := filepath.Rel(root, p) + if err != nil { + return err + } + out = append(out, rel) + return nil + }) + if err != nil { + return nil, err + } + sort.Strings(out) + return out, nil +} + +// runRepeatedly runs one file under one engine `times` over, and returns the +// LOWEST-scoring result plus whether the engine disagreed with itself. +// +// The lowest rather than the mean or the last, because the question a reader +// asks of this table is "did anything get worse", and the answer must not rest +// on a lucky run. With times == 1 `varied` is always false, which is why the +// regression verdict is only trustworthy above 1 — said in the flag's help. +func runRepeatedly(engine, root, file string, timeout time.Duration, times int) (result, bool, error) { + var lowest result + varied := false + for i := 0; i < times; i++ { + r, err := run(engine, root, file, timeout) + if err != nil { + return lowest, false, err + } + switch { + case i == 0: + lowest = r + case r.pass != lowest.pass || r.ran != lowest.ran: + varied = true + if r.pass < lowest.pass { + lowest = r + } + } + } + return lowest, varied, nil +} + +// run executes one spec file under one engine, from the corpus root so the +// shim's relative requires resolve. +func run(engine, root, file string, timeout time.Duration) (result, error) { + var r result + var out bytes.Buffer + cmd := exec.Command(engine, file) + cmd.Dir = root + cmd.Stdin = nil + cmd.Stdout = &out + cmd.Stderr = io.Discard // the shim reports on stdout; stderr is warnings + cmd.WaitDelay = time.Second + + done := make(chan error, 1) + if err := cmd.Start(); err != nil { + return r, err + } + go func() { done <- cmd.Wait() }() + select { + case err := <-done: + var ee *exec.ExitError + if err != nil && !errors.As(err, &ee) { + return r, err + } + case <-time.After(timeout): + _ = cmd.Process.Kill() + <-done + r.killed = true + } + return parse(out.String()) +} + +// parse reads the shim's report. It REFUSES a RESULT line it cannot read rather +// than defaulting a field to zero, because a zero that means "unreadable" and a +// zero that means "nothing passed" lead to opposite conclusions. +func parse(s string) (result, error) { + var r result + for _, line := range strings.Split(s, "\n") { + switch { + case strings.HasPrefix(line, "RBGO_DETAIL"): + r.details++ + case strings.HasPrefix(line, "RBGO_RESULT"): + if r.ran { + return r, fmt.Errorf("two RBGO_RESULT lines in one run") + } + fields := strings.Fields(strings.TrimPrefix(line, "RBGO_RESULT")) + seen := map[string]bool{} + for _, f := range fields { + k, v, ok := strings.Cut(f, "=") + if !ok { + return r, fmt.Errorf("unreadable field %q in %q", f, line) + } + n, err := strconv.Atoi(v) + if err != nil { + return r, fmt.Errorf("field %q is not a number in %q", f, line) + } + seen[k] = true + switch k { + case "pass": + r.pass = n + case "fail": + r.fail = n + case "error": + r.errs = n + case "skip": + r.skip = n + default: + return r, fmt.Errorf("unknown field %q in %q", k, line) + } + } + for _, k := range []string{"pass", "fail", "error", "skip"} { + if !seen[k] { + return r, fmt.Errorf("%q missing from %q", k, line) + } + } + r.ran = true + } + } + return r, nil +} diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main_test.go b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go new file mode 100644 index 00000000..5aefe3f7 --- /dev/null +++ b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go @@ -0,0 +1,343 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package main + +import ( + "bytes" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// parse is where a silent wrong answer would live, so it is tested first and +// hardest: every field it cannot read must be an ERROR, never a zero. +func TestParseRefusesWhatItCannotRead(t *testing.T) { + // fail=2 + error=1 is THREE detail lines; the shim emits one per failure and + // one per error, so the consistent case has to add up. + good := "RBGO_RESULT pass=12 fail=2 error=1 skip=0\n" + r, err := parse(good + "RBGO_DETAIL\tfail\tX\ta\tb\nRBGO_DETAIL\tfail\tX\tc\td\nRBGO_DETAIL\terror\tY\te\tf\n") + if err != nil { + t.Fatalf("parse(good): %v", err) + } + if r.pass != 12 || r.fail != 2 || r.errs != 1 || r.skip != 0 || !r.ran { + t.Errorf("parse(good) = %+v", r) + } + if r.details != 3 { + t.Errorf("details = %d, want 3", r.details) + } + if r.tallyDisagrees() { + t.Error("fail=2 error=1 with three detail lines must agree") + } + + // ⛔ Each of these used to be a plausible "default it to zero" case. A zero + // that means "unreadable" and a zero that means "nothing passed" lead to + // opposite conclusions, so neither may be guessed. + for _, bad := range []string{ + "RBGO_RESULT pass=12 fail=3 error=1\n", // skip missing + "RBGO_RESULT pass=12 fail=3 skip=0\n", // error missing + "RBGO_RESULT fail=3 error=1 skip=0\n", // pass missing + "RBGO_RESULT pass=twelve fail=3 error=1 skip=0", // not a number + "RBGO_RESULT pass fail=3 error=1 skip=0", // no '=' + "RBGO_RESULT pass=1 fail=1 error=1 skip=1 oops=1", + good + good, // two runs in one output + } { + if _, err := parse(bad); err == nil { + t.Errorf("parse(%q) returned no error", bad) + } + } + + // No RESULT line at all is not an error — it is a file that did not run, + // which the caller reports differently from one that ran and scored zero. + r, err = parse("some warning on stdout\n") + if err != nil { + t.Fatalf("parse(no result): %v", err) + } + if r.ran { + t.Error("a run with no RESULT line must not be marked as having run") + } +} + +// ⛔ The tally comes out of the SUBJECT's stdout, so a subject that miscounts +// can claim anything. The detail lines are the independent channel — but the +// relationship is an INEQUALITY, not an equality, and getting that wrong is how +// the first version of this check fired on a perfectly well-formed file. +// +// The shim's shape, read from spec_helper.rb rather than assumed: `fail` and +// `error` each push exactly one detail line; a guard-driven `skip` bumps the +// counter with NO detail (:970) while a runtime skip pushes one (:990); and a +// `before/after(:all)` error pushes a detail counted in NO tally (:965, :1020). +func TestDetailShortfallIsDetected(t *testing.T) { + for _, tc := range []struct { + name string + out string + flag bool + why string + }{{ + name: "a missing detail line", + out: "RBGO_RESULT pass=1 fail=2 error=0 skip=0\nRBGO_DETAIL\tfail\tX\ta\tb\n", + flag: true, + why: "fail=2 with one detail line means a line went missing", + }, { + name: "exactly as many as claimed", + out: "RBGO_RESULT pass=1 fail=1 error=0 skip=0\nRBGO_DETAIL\tfail\tX\ta\tb\n", + flag: false, + why: "fail=1 with one detail line is consistent", + }, { + // ⛔ The row the first version got wrong: core/set/set_spec.rb, skipped + // by a guard, is pass=0 fail=0 error=0 skip=1 with one `skip` detail. + name: "a skip with its detail line", + out: "RBGO_RESULT pass=0 fail=0 error=0 skip=1\nRBGO_DETAIL\tskip\tSpecSkip\tx\ty\n", + flag: false, + why: "a skip carries a detail and no failure; that is well-formed", + }, { + name: "a guard skip with no detail line", + out: "RBGO_RESULT pass=0 fail=0 error=0 skip=3\n", + flag: false, + why: "spec_helper.rb:970 bumps $RB_SKIP without pushing a detail", + }, { + name: "a hook error, counted nowhere", + out: "RBGO_RESULT pass=0 fail=0 error=0 skip=0\nRBGO_DETAIL\thookerror\tX\t(before :all)\ty\n", + flag: false, + why: "a hookerror detail belongs to no counter; MORE details is legitimate", + }} { + t.Run(tc.name, func(t *testing.T) { + r, err := parse(tc.out) + if err != nil { + t.Fatal(err) + } + if got := r.tallyDisagrees(); got != tc.flag { + t.Errorf("tallyDisagrees() = %v, want %v — %s", got, tc.flag, tc.why) + } + }) + } +} + +// An empty population is a REFUSAL. A mistyped -dir would otherwise print +// "0 files, no regressions" and exit 0, which is the most convincing way for +// this tool to tell someone their change is safe. +func TestEmptyPopulationIsRefused(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "core", "empty"), 0o755); err != nil { + t.Fatal(err) + } + var out, errOut bytes.Buffer + if code := main1([]string{"-root", root, "-dir", "core/empty", "/bin/echo", "/bin/echo"}, &out, &errOut); code != 2 { + t.Errorf("exit = %d, want 2; err %q", code, errOut.String()) + } + if !strings.Contains(errOut.String(), "no _spec.rb files") { + t.Errorf("the refusal does not say why: %q", errOut.String()) + } +} + +// Usage refusals, each exit 2 — distinct from 1, which means a real regression. +func TestUsageRefusals(t *testing.T) { + root := t.TempDir() + for _, tc := range []struct { + name string + args []string + }{ + {"one engine", []string{"-root", root, "-dir", "x", "/bin/echo"}}, + {"three engines", []string{"-root", root, "-dir", "x", "/bin/echo", "/bin/echo", "/bin/echo"}}, + {"no dir", []string{"-root", root, "/bin/echo", "/bin/echo"}}, + {"absent dir", []string{"-root", root, "-dir", "nope", "/bin/echo", "/bin/echo"}}, + } { + t.Run(tc.name, func(t *testing.T) { + var out, errOut bytes.Buffer + if code := main1(tc.args, &out, &errOut); code != 2 { + t.Errorf("exit = %d, want 2; out %q err %q", code, out.String(), errOut.String()) + } + }) + } +} + +// End to end against fake engines, because the tool must not know anything +// about Ruby. The control passes 2 of 3 in one file; the treatment passes 3 — +// an improvement, exit 0 — and then the reverse, which must be exit 1. +func TestReportsMovementAndRegression(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the fake engines are POSIX shell scripts") + } + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "core", "x"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "core", "x", "a_spec.rb"), []byte("#\n"), 0o644); err != nil { + t.Fatal(err) + } + two := fakeEngine(t, root, "two", "echo 'RBGO_RESULT pass=2 fail=1 error=0 skip=0'\nprintf 'RBGO_DETAIL\\tfail\\tX\\ta\\tb\\n'\n") + three := fakeEngine(t, root, "three", "echo 'RBGO_RESULT pass=3 fail=0 error=0 skip=0'\n") + + var out, errOut bytes.Buffer + if code := main1([]string{"-root", root, "-dir", "core/x", two, three}, &out, &errOut); code != 0 { + t.Errorf("improvement: exit = %d, want 0; err %q", code, errOut.String()) + } + if !strings.Contains(out.String(), "+1") { + t.Errorf("the improvement is not reported:\n%s", out.String()) + } + + out.Reset() + errOut.Reset() + if code := main1([]string{"-root", root, "-dir", "core/x", three, two}, &out, &errOut); code != 1 { + t.Errorf("regression: exit = %d, want 1; err %q", code, errOut.String()) + } + if !strings.Contains(out.String(), "-1") || !strings.Contains(out.String(), "1 regressed") { + t.Errorf("the regression is not reported:\n%s", out.String()) + } +} + +// The loudest movement prints first, whichever direction it points, because a +// reader wants the biggest change and not the alphabetically first one. With a +// single file the comparator is never called, so this needs two — and it is the +// documented ordering, which is reason enough to pin it. +func TestTheBiggestMovementIsReportedFirst(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the fake engines are POSIX shell scripts") + } + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "core", "w"), 0o755); err != nil { + t.Fatal(err) + } + // `zz` moves by one and `aa` by five, so alphabetical order and movement + // order disagree — which is what makes the assertion mean anything. + for name, body := range map[string]string{ + "aa_spec.rb": "#\n", + "zz_spec.rb": "#\n", + } { + if err := os.WriteFile(filepath.Join(root, "core", "w", name), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + // One engine scores by file name; the other adds 5 to aa and 1 to zz. + low := fakeEngine(t, root, "low", `case "$1" in + *aa_spec.rb) echo 'RBGO_RESULT pass=10 fail=0 error=0 skip=0' ;; + *) echo 'RBGO_RESULT pass=10 fail=0 error=0 skip=0' ;; +esac +`) + high := fakeEngine(t, root, "high", `case "$1" in + *aa_spec.rb) echo 'RBGO_RESULT pass=15 fail=0 error=0 skip=0' ;; + *) echo 'RBGO_RESULT pass=11 fail=0 error=0 skip=0' ;; +esac +`) + var out, errOut bytes.Buffer + if code := main1([]string{"-root", root, "-dir", "core/w", low, high}, &out, &errOut); code != 0 { + t.Fatalf("exit = %d, want 0; err %q", code, errOut.String()) + } + iAA := strings.Index(out.String(), "aa_spec.rb") + iZZ := strings.Index(out.String(), "zz_spec.rb") + if iAA < 0 || iZZ < 0 { + t.Fatalf("both files must appear:\n%s", out.String()) + } + if iAA > iZZ { + t.Errorf("+5 must print before +1:\n%s", out.String()) + } +} + +// A file that produces no RESULT line is named as such rather than counted as +// zero — the distinction the ratchet's "FAILED TO LOAD" verdict exists for, and +// the one that made an endless range look like a load failure in #853. +func TestAFileThatDidNotRunIsNamed(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the fake engines are POSIX shell scripts") + } + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "core", "y"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "core", "y", "b_spec.rb"), []byte("#\n"), 0o644); err != nil { + t.Fatal(err) + } + ok := fakeEngine(t, root, "ok", "echo 'RBGO_RESULT pass=1 fail=0 error=0 skip=0'\n") + mute := fakeEngine(t, root, "mute", "exit 1\n") + + var out, errOut bytes.Buffer + main1([]string{"-root", root, "-dir", "core/y", ok, mute}, &out, &errOut) + if !strings.Contains(out.String(), "DID NOT RUN on the treatment") { + t.Errorf("a silent engine is not named:\n%s", out.String()) + } +} + +// ⛔ A single run per engine cannot tell a FLAKE from a difference. The first +// real use of this tool reported `core/random/bytes_spec.rb` as -1 between two +// binaries differing only in a version string; three runs of each gave control +// 9, 9, 8 and treatment 8, 9, 9 — a coin flip, attributed to the change. +// +// So above `-repeat 1` an engine that disagrees with ITSELF marks the file +// UNSTABLE, its delta is excluded from the verdict, and the exit status stops +// depending on which way the coin landed. +func TestAnUnstableFileIsExcludedFromTheVerdict(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the fake engines are POSIX shell scripts") + } + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "core", "z"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "core", "z", "c_spec.rb"), []byte("#\n"), 0o644); err != nil { + t.Fatal(err) + } + stable := fakeEngine(t, root, "stable", "echo 'RBGO_RESULT pass=9 fail=0 error=0 skip=0'\n") + // Scores 8 the first time it is asked and 9 after: a flake, in the direction + // that looks like a regression on the first run. + flaky := fakeEngine(t, root, "flaky", `if [ -f "`+root+`/seen" ]; then + echo 'RBGO_RESULT pass=9 fail=0 error=0 skip=0' +else + : > "`+root+`/seen" + echo 'RBGO_RESULT pass=8 fail=1 error=0 skip=0' + printf 'RBGO_DETAIL\tfail\tX\ta\tb\n' +fi +`) + + // With one run the flake reads as a regression — which is the defect this + // flag exists for, asserted so the difference between the two modes is + // visible rather than claimed. + var out, errOut bytes.Buffer + if code := main1([]string{"-root", root, "-dir", "core/z", stable, flaky}, &out, &errOut); code != 1 { + t.Errorf("-repeat 1: exit = %d, want 1 (the flake reads as a regression); err %q", code, errOut.String()) + } + + // With three, the engine contradicts itself and the file is set aside. + if err := os.Remove(filepath.Join(root, "seen")); err != nil { + t.Fatal(err) + } + out.Reset() + errOut.Reset() + if code := main1([]string{"-root", root, "-dir", "core/z", "-repeat", "3", stable, flaky}, &out, &errOut); code != 0 { + t.Errorf("-repeat 3: exit = %d, want 0 (unstable, excluded); out %q", code, out.String()) + } + if !strings.Contains(out.String(), "UNSTABLE") { + t.Errorf("the unstable file is not named:\n%s", out.String()) + } + if !strings.Contains(out.String(), "0 regressed") { + t.Errorf("an excluded delta still counted as a regression:\n%s", out.String()) + } + + // A stable engine must NOT be called unstable, or every file would be. + out.Reset() + errOut.Reset() + if code := main1([]string{"-root", root, "-dir", "core/z", "-repeat", "3", stable, stable}, &out, &errOut); code != 0 { + t.Errorf("stable pair: exit = %d, want 0; out %q", code, out.String()) + } + if strings.Contains(out.String(), "UNSTABLE") { + t.Errorf("a stable engine was called unstable:\n%s", out.String()) + } + + // And -repeat below 1 is a usage refusal, not a silent no-op. + out.Reset() + errOut.Reset() + if code := main1([]string{"-root", root, "-dir", "core/z", "-repeat", "0", stable, stable}, &out, &errOut); code != 2 { + t.Errorf("-repeat 0: exit = %d, want 2; err %q", code, errOut.String()) + } +} + +func fakeEngine(t *testing.T, dir, name, body string) string { + t.Helper() + p := filepath.Join(dir, name) + if err := os.WriteFile(p, []byte("#!/bin/sh\n"+body), 0o755); err != nil { + t.Fatal(err) + } + return p +}