diff --git a/scripts/conformance/rubyspec/README.md b/scripts/conformance/rubyspec/README.md index 952f2827..e271e206 100644 --- a/scripts/conformance/rubyspec/README.md +++ b/scripts/conformance/rubyspec/README.md @@ -162,8 +162,37 @@ bound**, since a static reader cannot expand `it_behaves_like`. Read the guard columns as the measurement and the example columns as a floor — and if the control ever stops reproducing, nothing below it is worth reading. -It is an analysis tool, not a gate: nothing fails when it is not run. What it -exists to prevent is a `UPDATE_BASELINE=1` that absorbs a loss silently. +### ⛔ It used to be an analysis tool, and that was the defect + +This section read *"it is an analysis tool, not a gate: nothing fails when it +is not run"*. On 2026-11-01 it was not run, and the 4.0 version move was +measured by comparing **passing** counts between two engines and published as +**ten examples of work**. The real figure was about **seventy-five**. + +Three `core/binding/implicit_parameter*_spec.rb` files are +`pass=0 fail=0 error=0` below 4.0 — every example behind a `ruby_version_is +"4.0"` guard — and **37 failures** above it, because Ruby 4.0 adds three +`Binding` methods rbgo does not have. A pass delta over those files is zero in +both directions, and `-dir` had never been pointed at `core/binding` at all. + +The lesson *"ask what the target version **adds** before measuring what moves"* +was already written down and was not consulted. So the reading moved into +`internal/guardcensus`, and `cmd/specdelta` now **cannot produce a table** until +the caller declares which comparison it is making. The note was not the missing +part; a refusal was. + +```bash +# what the move does to the CORPUS, before any engine runs +go run ./scripts/conformance/rubyspec/cmd/guards \ + -root ~/ruby-spec-corpus -move 3.4.1:4.0.9 +``` + +Against the pinned corpus that move **opens 67 block guards in 50 files** +(≥145 examples), **closes 45** in 37 files, and **flips 5 inline sites** in 3. +Those 50 files are the surface any claim about the move has to cover. + +It remains the right instrument for the other half too: a `UPDATE_BASELINE=1` +that absorbs a loss silently. ## Asking the reference interpreter — `cmd/oracle` @@ -221,7 +250,8 @@ 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 +$ specdelta -root ~/ruby-spec-corpus -move none -dir core/set -quiet -repeat 3 ./rbgo-main ./rbgo-409 +MOVE none declared: both engines read the same corpus, so every delta below is the ENGINE file control treat delta core/set/flatten_merge_spec.rb 3 0 -3 core/set/to_s_spec.rb 2 4 +2 @@ -235,6 +265,64 @@ 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. +**⛔ `-move` is required, and has no default.** The two runs above read the same +corpus; a run across a version move reads two *different* corpora, and the +tables look identical. Only one of them is a reading of the engines. + +| declaration | what it asserts | +|---|---| +| `-move none` | both engines read the corpus at the same `RUBY_VERSION` | +| `-move 3.4.1:4.0.9` | they do not: the guards move too | + +With a move declared, specdelta censuses the corpus through +`internal/guardcensus` — the same reading `cmd/guards` prints — and **refuses +before starting either engine** if the move touches a file `-dir` does not +measure: + +```console +$ specdelta -root ~/ruby-spec-corpus -move 3.4.1:4.0.9 -dir core/set ./a ./b +specdelta: the move 3.4.1 → 4.0.9 touches 49 file(s) that -dir core/set does not measure: + core/array/detect_spec.rb + core/binding/implicit_parameter_defined_spec.rb + core/binding/implicit_parameter_get_spec.rb + core/binding/implicit_parameters_spec.rb + … + A number from this run would not be a number about the move. Widen -dir, or + pass -partial to say so on purpose — the report is then stamped PARTIAL. +``` + +The refusal comes **first** because a `core/` sweep is an hour, and an hour +spent on a figure already known to be incomplete is the cost the gate exists to +avoid. A measured file the move opens is marked `⛔ THE MOVE OPENS ≥N examples +here` and is **never dropped by `-quiet`** — a delta of zero on such a file is +the signature of the defect, not of calm — and the summary reports the move's +share apart from the engines'. + +**⛔ Provenance, and it is a repeat offence.** On 2026-11-01 `pass=118 fail=1` +was published for [#858](https://github.com/go-embedded-ruby/ruby/pull/858) from +a binary built **before that branch existed**; it carried the previous branch's +code, and the true figure was 117/2. The notes already held the same shape from +an earlier arc — a control built from the *working tree* rather than from the +commit, attributing `+3` to the wrong fix. Written down twice, and it happened +again. + +`go build` stamps `vcs.revision` and `vcs.modified` into every binary it +produces from a repository, so a binary can be **asked what it is**. specdelta +reads both engines before running either, prints what it found above the table, +and refuses the three ways a comparison is void before it starts: + +| what it finds | why the comparison is void | +|---|---| +| the same revision twice | one of them was not rebuilt | +| `vcs.modified=true` | the revision does not name the code in the binary | +| the work tree has moved past the binary's revision | the binary is not the code being measured — **#858** | + +An engine with no Go build information (MRI, a shell fake) is **not** refused — +it is legitimately not a Go binary — but the report is stamped +`⛔ UNPROVENANCED`, because a table nobody can attribute must not look like one +that can. `-unprovenanced` is the named derogation and it stamps rather than +silences. + **`-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 @@ -242,6 +330,36 @@ 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. +**⛔ An example that went red inside a file that improved.** On 2026-11-02 a +control and a treatment built from the same commit minus one change read: + +```text +core/thread/raise_spec.rb 51 → 68 +17 +53 file(s): control 328 passing, treatment 345 passing, +17 +1 file(s) moved, 0 regressed +``` + +True, and incomplete. **Nineteen examples turned green and two turned red** — +and no count of passes, of failures, or of both can show a swap. The two were +found by hand, by comparing the two runs' failure *descriptions*; the example +that broke had been passing **because of** a different defect, so nothing about +it looked wrong. + +specdelta now compares those descriptions as a multiset (the same shared block +included twice reports twice), names every one the treatment reports more often, +and **exits 1** for it — a per-file verdict of "0 regressed" must not carry an +example out: + +```console +core/thread/raise_spec.rb 51 68 +17 + ⛔ WENT RED: fail shared kernel_raise | raises a TypeError when passed a message and an extra argument (×2) + +1 file(s) moved, 0 regressed, 2 example(s) WENT RED +``` + +It keys on the **description** and not the message, because a message carries +object addresses that differ between two runs of the *same* binary. + **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 @@ -250,8 +368,10 @@ 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. +unreadable result line, two result lines in one run, a missing or malformed +`-move`, a declared move whose surface this run does not cover, or an **empty +population** — a mistyped `-dir` would otherwise print `0 files, no regressions` +and exit 0. ## Caveats diff --git a/scripts/conformance/rubyspec/cmd/guards/main.go b/scripts/conformance/rubyspec/cmd/guards/main.go index c1966fbe..81289a94 100644 --- a/scripts/conformance/rubyspec/cmd/guards/main.go +++ b/scripts/conformance/rubyspec/cmd/guards/main.go @@ -1,6 +1,7 @@ // guards censuses ruby/spec's `ruby_version_is` by SHAPE, because the two // shapes move the population in different directions and only one of them has -// ever been counted. +// ever been counted. The reading itself lives in internal/guardcensus, which +// cmd/specdelta's version-move gate uses too, so the two cannot drift. // // BLOCK ruby_version_is "4.0" do … end // The examples inside are SKIPPED while CUR_VERSION is below the @@ -18,26 +19,10 @@ // silently. This is the static half of that measurement — the half that needs // no sweep, and so can be run on a loaded machine, where a sweep cannot. // -// ⛔ THE SHAPE IS DECIDED BY WHAT OPENS A BLOCK, NOT BY THE PARENTHESES. A -// first draft keyed on "no parentheses, and the line ends in do". It was wrong -// twice, and both were found by PRINTING THE SITES instead of only the totals: -// -// ruby_version_is "4.0" do # https://bugs.ruby-lang.org/issues/21844 -// a block guard whose trailing comment defeats a `do\s*$` anchor -// ruby_version_is("4.0") do -// a block guard that IS parenthesised -// -// Five guards at "4.0" hide in those two shapes, so a census anchored that way -// reports 56 where there are 61. -// -// ⛔ THE EXAMPLE COUNT IS A LOWER BOUND AND SAYS SO. Counting `it` blocks -// inside a guard needs Ruby `end`-matching, which is a heuristic without a -// parser — and a static count cannot see shared examples (`it_behaves_like`) -// at all. The control below restricts the count to exactly the guards a -// `do\s*$` anchor finds, where #799 measured 56 guards and 167 examples: the -// guard number reproduces and the example number does not, which places the -// difference in the counting rather than in the census. Tuning the heuristic -// until it reached 167 would be fitting the instrument to the expected answer. +// With -move FROM:TO it answers the question that half of #799 actually +// needed and nobody asked: what does the target version ADD? That reading is +// now also a precondition of cmd/specdelta's verdict, rather than a report +// someone has to remember to run. // // Exit status: 0 the census ran; 2 the corpus could not be read — a refusal, // not an empty census. @@ -46,179 +31,22 @@ package main import ( "flag" "fmt" - "io/fs" "os" - "path/filepath" - "regexp" "sort" "strings" -) - -var ( - callRe = regexp.MustCompile(`ruby_version_is`) - argRe = regexp.MustCompile(`ruby_version_is\s*\(?\s*["']([^"']*)["']\s*(?:(\.\.\.?)\s*["']([^"']*)["']\s*)?\)?`) - opensRe = regexp.MustCompile(`(?:\bdo\b|\{)\s*(?:\|[^|]*\|)?\s*$`) - bracesRe = regexp.MustCompile(`ruby_version_is\s*\(?[^)]*\)?\s*\{`) - commentRe = regexp.MustCompile(`\s+#(?:[^{].*)?$`) - // oldAnchorRe is the earlier census's shape, kept ONLY so its number can - // be reproduced as a control. Nothing else should use it. - oldAnchorRe = regexp.MustCompile(`ruby_version_is\s+["'][^"']*["']\s*(\.\.\.?\s*["'][^"']*["']\s*)?do\s*$`) - itRe = regexp.MustCompile(`^\s*it[\s(]`) - openKw = regexp.MustCompile(`(^|\s)(def|class|module|case|begin|while|until|unless|if)\s`) - modifIf = regexp.MustCompile(`\S\s+(if|unless|while|until)\s`) - endKw = regexp.MustCompile(`^\s*end\b`) - doKw = regexp.MustCompile(`\bdo\b\s*(\|[^|]*\|)?\s*(#.*)?$`) - braceEnd = regexp.MustCompile(`^\s*\}`) -) - -// shape is what a mention of ruby_version_is does. -type shape int -const ( - block shape = iota - inline - unknown + gc "github.com/go-embedded-ruby/ruby/scripts/conformance/rubyspec/internal/guardcensus" ) -func (s shape) String() string { - switch s { - case block: - return "block" - case inline: - return "inline" - } - return "unclassified" -} - -// Site is one mention. -type Site struct { - File, Text string - Line int - From, To string - Shape shape - InIt bool // an inline inside an `it` moves exactly one example - Examples int // `it` blocks inside a block guard: a LOWER BOUND - OldAnchor bool // a `do$` anchor would have found it -} - -// Key is the version range a site guards, as it reads in the source. -func (s Site) Key() string { - if s.To != "" { - return s.From + "..." + s.To - } - if s.From == "" { - return `"" (open lower bound)` - } - return s.From -} - -// Classify decides what a single line of Ruby does with ruby_version_is. -// Split out because it is the part that was wrong twice, so it is the part a -// test drives directly. -func Classify(line string) (Site, bool) { - if !callRe.MatchString(line) { - return Site{}, false - } - s := Site{Text: strings.TrimSpace(line)} - m := argRe.FindStringSubmatch(line) - if m == nil { - s.Shape = unknown - return s, true - } - s.From = m[1] - if m[2] != "" { - s.To = m[3] - } - s.OldAnchor = oldAnchorRe.MatchString(line) - bare := commentRe.ReplaceAllString(line, "") - if opensRe.MatchString(bare) || bracesRe.MatchString(bare) { - s.Shape = block - } else { - s.Shape = inline - } - return s, true -} - -// ExamplesIn counts the `it` blocks between a guard and its matching `end`. -// A LOWER BOUND: shared examples expand at run time and are invisible here. -func ExamplesIn(lines []string, start int) int { - depth, n := 1, 0 - for i := start + 1; i < len(lines) && depth > 0; i++ { - l := lines[i] - if strings.HasPrefix(strings.TrimSpace(l), "#") { - continue - } - if endKw.MatchString(l) || braceEnd.MatchString(l) { - depth-- - continue - } - if doKw.MatchString(l) || (openKw.MatchString(l) && !modifIf.MatchString(l)) { - depth++ - } - if itRe.MatchString(l) { - n++ - } - } - return n -} - -// Census reads one spec file. -func Census(path string, src []byte) []Site { - lines := strings.Split(strings.ReplaceAll(string(src), "\r\n", "\n"), "\n") - var out []Site - itDepth := 0 - for i, line := range lines { - if itRe.MatchString(line) { - itDepth++ - } else if strings.TrimSpace(line) == "end" && itDepth > 0 { - itDepth-- - } - s, ok := Classify(line) - if !ok { - continue - } - s.File, s.Line, s.InIt = path, i+1, itDepth > 0 - if s.Shape == block { - s.Examples = ExamplesIn(lines, i) - } - out = append(out, s) - } - return out -} - func main() { root := flag.String("root", ".", "the ruby/spec checkout") list := flag.Bool("list", false, "print every inline site and every guard the old anchor missed") + move := flag.String("move", "", "a version move FROM:TO, e.g. 3.4.1:4.0.9 — report what it opens, closes and flips") flag.Parse() - var sites []Site - files := 0 - walk := func(p string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - if d.IsDir() || !strings.HasSuffix(p, "_spec.rb") { - return nil - } - files++ - b, rerr := os.ReadFile(p) - if rerr != nil { - return rerr - } - sites = append(sites, Census(strings.TrimPrefix(strings.TrimPrefix(p, *root), "/"), b)...) - return nil - } - - // ⛔ language/ and core/ ONLY, named rather than filtered. A draft walked - // the checkout minus a denylist of directory names and counted 2207 files - // where the gate counts 2206: a denylist inherits whatever it has not - // heard of. The gate sweeps exactly two suites, so this walks exactly two. - var err error - for _, suite := range []string{"language", "core"} { - if err = filepath.WalkDir(filepath.Join(*root, suite), walk); err != nil { - break - } - } + // ⛔ language/ and core/ ONLY, named rather than filtered: the gate sweeps + // exactly two suites, so this walks exactly two. + sites, files, err := gc.Walk(*root, []string{"language", "core"}) if err != nil { fmt.Fprintf(os.Stderr, "guards: walking %s: %v\n", *root, err) os.Exit(2) @@ -228,12 +56,12 @@ func main() { os.Exit(2) } - var blocks, inlines, unk []Site + var blocks, inlines, unk []gc.Site for _, s := range sites { switch s.Shape { - case block: + case gc.Block: blocks = append(blocks, s) - case inline: + case gc.Inline: inlines = append(inlines, s) default: unk = append(unk, s) @@ -280,6 +108,15 @@ func main() { fmt.Printf(" %d inline sites sit inside an `it` (one example each);\n", inIt) fmt.Printf(" %d sit at describe level, where one site can steer SEVERAL examples\n\n", len(inlines)-inIt) + if *move != "" { + from, to, ok := strings.Cut(*move, ":") + if !ok || from == "" || to == "" { + fmt.Fprintf(os.Stderr, "guards: -move wants FROM:TO, e.g. 3.4.1:4.0.9; got %q\n", *move) + os.Exit(2) + } + reportMove(gc.ClassifyMove(sites, from, to), *list) + } + if *list { fmt.Println(`BLOCK guards at "4.0" a ` + "`do$`" + ` anchor does NOT find:`) for _, s := range blocks { @@ -307,7 +144,36 @@ func main() { } } -func tally(sites []Site, withExamples bool) { +// reportMove prints what the move does to the corpus's own population. The +// OPENED section comes first and names its files, because that is the half a +// pass-only delta cannot see: a file whose every example is guarded away on one +// side scores zero on both counters there and moves by nothing. +func reportMove(m gc.Move, list bool) { + fmt.Printf("MOVE %s -> %s -- what the CORPUS does, before any engine runs\n", m.From, m.To) + fmt.Printf(" OPENED %4d block guards in %3d files (>= %d examples appear; a lower bound)\n", + len(m.Opened), len(gc.Files(m.Opened)), m.ExamplesOpened()) + fmt.Printf(" CLOSED %4d block guards in %3d files (examples vanish)\n", + len(m.Closed), len(gc.Files(m.Closed))) + fmt.Printf(" FLIPPED %4d inline sites in %3d files (the same example asserts something else)\n\n", + len(m.Flipped), len(gc.Files(m.Flipped))) + if !list { + fmt.Println(" -list names the files.") + fmt.Println() + return + } + for _, s := range m.Opened { + fmt.Printf(" OPENED %s:%d >=%d examples %s\n", s.File, s.Line, s.Examples, s.Text) + } + for _, s := range m.Closed { + fmt.Printf(" CLOSED %s:%d %s\n", s.File, s.Line, s.Text) + } + for _, s := range m.Flipped { + fmt.Printf(" FLIPPED %s:%d %s\n", s.File, s.Line, s.Text) + } + fmt.Println() +} + +func tally(sites []gc.Site, withExamples bool) { type agg struct{ guards, examples int } by := map[string]*agg{} for _, s := range sites { diff --git a/scripts/conformance/rubyspec/cmd/guards/main_test.go b/scripts/conformance/rubyspec/cmd/guards/main_test.go deleted file mode 100644 index 481195c6..00000000 --- a/scripts/conformance/rubyspec/cmd/guards/main_test.go +++ /dev/null @@ -1,153 +0,0 @@ -package main - -import ( - "strings" - "testing" -) - -// ⛔ Every case below except the first two is a line that actually appears in -// the pinned corpus, and the two marked FOUND BY PRINTING are the ones a -// `do\s*$` anchor misses. They are the reason this classifier exists: the -// earlier census reported 56 guards at "4.0" where there are 61, and the five -// it could not see were not exotic — three carry a trailing comment and one is -// merely parenthesised. -func TestClassify(t *testing.T) { - cases := []struct { - name string - line string - want shape - from string - to string - inAnc bool // a `do$` anchor would have found it - }{ - { - name: "the plain block guard", - line: ` ruby_version_is "4.0" do`, - want: block, - from: "4.0", - inAnc: true, - }, - { - name: "a range block guard", - line: ` ruby_version_is ""..."4.0" do`, - want: block, from: "", to: "4.0", inAnc: true, - }, - { - // FOUND BY PRINTING THE SITES: core/data/initialize_spec.rb:83 - name: "a block guard with a trailing comment", - line: ` ruby_version_is "4.0" do # https://bugs.ruby-lang.org/issues/21844`, - want: block, from: "4.0", inAnc: false, - }, - { - // FOUND BY PRINTING THE SITES: core/set/pretty_print_cycle_spec.rb:9 - name: "a block guard that is parenthesised", - line: ` ruby_version_is("4.0") do`, - want: block, from: "4.0", inAnc: false, - }, - { - name: "a brace block in an argument list", - line: ` *ruby_version_is("4.0") { :linux },`, - want: block, from: "4.0", inAnc: false, - }, - { - name: "the ternary, which RUNS either way", - line: ` msg_class = ruby_version_is("4.0") ? "Module" : "module"`, - want: inline, from: "4.0", inAnc: false, - }, - { - name: "a ternary on the 3.4 boundary", - line: ` expected = ruby_version_is("3.4") ? "{a: 1}" : "{:a=>1}"`, - want: inline, from: "3.4", inAnc: false, - }, - { - name: "inside a lambda guard, which is neither", - line: ` guard -> { ruby_version_is "3.4" and !"test".equal?("test") } do`, - want: block, from: "3.4", inAnc: false, - }, - { - name: "a line that never mentions it", - line: ` it "returns a String" do`, - want: unknown, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - s, ok := Classify(c.line) - if c.want == unknown && c.line == ` it "returns a String" do` { - if ok { - t.Fatalf("a line with no mention was classified as %s", s.Shape) - } - return - } - if !ok { - t.Fatal("no mention found in a line that has one") - } - if s.Shape != c.want { - t.Errorf("shape %s, want %s", s.Shape, c.want) - } - if s.From != c.from { - t.Errorf("from %q, want %q", s.From, c.from) - } - if s.To != c.to { - t.Errorf("to %q, want %q", s.To, c.to) - } - if s.OldAnchor != c.inAnc { - t.Errorf("OldAnchor %v, want %v", s.OldAnchor, c.inAnc) - } - }) - } -} - -// ⛔ The example counter is a LOWER BOUND, and this pins the bound rather than -// pretending to an exact figure: nested blocks are followed, and a shared -// example is NOT counted because a static reader cannot expand one. -func TestExamplesIn(t *testing.T) { - src := strings.Split(`ruby_version_is "4.0" do - it "one" do - end - - describe "nested" do - it "two" do - end - end - - it_behaves_like :some_shared, :method -end -it "outside, must not be counted" do -end`, "\n") - - if got := ExamplesIn(src, 0); got != 2 { - t.Errorf("counted %d examples, want 2 (the shared example is invisible to a static reader)", got) - } -} - -// A guard with no examples at all still reports zero rather than swallowing -// the file. -func TestAGuardWithNoExamples(t *testing.T) { - src := strings.Split("ruby_version_is \"4.0\" do\n CONST = 1\nend\n", "\n") - if got := ExamplesIn(src, 0); got != 0 { - t.Errorf("counted %d, want 0", got) - } -} - -// Census ties the two together and must mark an inline inside an `it`. -func TestCensusMarksAnInlineInsideAnExample(t *testing.T) { - src := []byte(`describe "Hash#inspect" do - it "formats" do - expected = ruby_version_is("3.4") ? "{a: 1}" : "{:a=>1}" - expected.should be_kind_of(String) - end -end -`) - sites := Census("core/hash/inspect_spec.rb", src) - if len(sites) != 1 { - t.Fatalf("found %d sites, want 1", len(sites)) - } - if sites[0].Shape != inline { - t.Errorf("shape %s, want inline", sites[0].Shape) - } - if !sites[0].InIt { - t.Error("the site is inside an `it` and was not marked so") - } -} diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main.go b/scripts/conformance/rubyspec/cmd/specdelta/main.go index 106e14f9..8a67e894 100644 --- a/scripts/conformance/rubyspec/cmd/specdelta/main.go +++ b/scripts/conformance/rubyspec/cmd/specdelta/main.go @@ -22,17 +22,37 @@ // concern: reading the detail lines rather than the tally is what caught a // figure misattributed between two branches on 2026-11-01. // +// ⛔ -move IS REQUIRED, and that is the point of it. On 2026-11-01 this tool +// measured #799's version move and the figure was published as "ten examples of +// work". The real figure was about seventy-five. Three +// `core/binding/implicit_parameter*_spec.rb` files are `pass=0 fail=0 error=0` +// below 4.0 — every example behind a `ruby_version_is "4.0"` guard — and 37 +// failures above it, and `-dir` had never been pointed at `core/binding` at +// all. The corpus itself changes between the two engines, and a tool that +// compares RESULTS cannot see that from the results. +// +// A note saying "ask what the target version ADDS before measuring what moves" +// had been written down beforehand and was not consulted. So the question is no +// longer something to remember: the tool will not produce a table until the +// caller has declared which comparison this is, and when a move is declared it +// censuses the corpus (internal/guardcensus) and REFUSES, before running +// anything, if the move touches files outside the measured set. +// // 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" +// 1 at least one file regressed, or an EXAMPLE went red inside a file that +// improved on balance +// 2 refused: no files, an engine that would not start, output that could not +// be read, no -move declared, a declared move whose surface this run does +// not cover, or a pair of binaries that cannot be tied to two distinct +// commits — never reported as a regression, because "could not measure" // and "measured worse" need opposite responses package main import ( "bytes" + "debug/buildinfo" "errors" "flag" "fmt" @@ -44,6 +64,8 @@ import ( "strconv" "strings" "time" + + gc "github.com/go-embedded-ruby/ruby/scripts/conformance/rubyspec/internal/guardcensus" ) // result is one spec file's outcome under one engine. @@ -52,6 +74,13 @@ type result struct { details int // RBGO_DETAIL lines seen ran bool // a RESULT line was found at all killed bool + + // labels counts the DESCRIPTION of each detail line — the example's own + // name, without the message, which carries object addresses that differ + // between runs. A multiset and not a set: a shared example block included + // twice reports the same description twice, and a file that loses one of + // the two has lost an example. + labels map[string]int } // tallyDisagrees reports whether the subject printed FEWER detail lines than @@ -98,6 +127,11 @@ func main1(args []string, out, errOut io.Writer) int { // 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") + // ⛔ No default. A default here would be a guess about the one thing the + // results cannot reveal, and the wrong guess is silent. + move := fs.String("move", "", `required: "none" if both engines read the corpus at the same RUBY_VERSION, else FROM:TO (e.g. 3.4.1:4.0.9)`) + partial := fs.Bool("partial", false, "run a declared move anyway when its surface reaches outside -dir; the report is stamped PARTIAL") + unprovenanced := fs.Bool("unprovenanced", false, "compare engines that cannot be tied to two distinct commits; the report is stamped UNPROVENANCED") if err := fs.Parse(args); err != nil { return 2 } @@ -126,6 +160,17 @@ func main1(args []string, out, errOut io.Writer) int { fmt.Fprintln(errOut, "specdelta: -repeat must be at least 1") return 2 } + + // THE MOVE GATE, before a single engine is started: a run whose surface is + // already known to be incomplete must not cost an hour first. + mv, err := declaredMove(*move, *root, *dir, files, errOut, *partial) + if err != nil { + return 2 + } + pv, err := checkProvenance(engines, []buildFacts{readBuildFacts(engines[0]), readBuildFacts(engines[1])}, *unprovenanced, errOut) + if err != nil { + 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 @@ -168,7 +213,18 @@ func main1(args []string, out, errOut io.Writer) int { return rows[i].file < rows[j].file }) - var totalA, totalB, moved, regressed, suspect, appeared int + // The declaration prints ABOVE the table, so a pasted table carries the + // claim it was made under. "ten examples" was quoted from a table that said + // nothing about which comparison it was. + fmt.Fprint(out, "PROVENANCE") + fmt.Fprintln(out, pv.stamp) + for _, l := range pv.lines { + fmt.Fprintln(out, l) + } + fmt.Fprintln(out, mv.header()) + + var totalA, totalB, moved, regressed, suspect, appeared, wentRed int + var movePass, moveBad, moveFiles int fmt.Fprintf(out, "%-56s %7s %7s %6s\n", "file", "control", "treat", "delta") for _, r := range rows { d := r.b.pass - r.a.pass @@ -217,6 +273,40 @@ func main1(args []string, out, errOut io.Writer) int { if worse > 0 { note += fmt.Sprintf(" ⛔ fail+error %d → %d", badA, badB) } + // ⛔ A NET GAIN HIDES AN EXAMPLE THAT WENT RED. On 2026-11-02 + // core/thread/raise_spec.rb read `+17, 0 regressed` — true, and + // incomplete: nineteen examples turned green and TWO turned red, and no + // count of any kind can show a swap. The example that broke had been + // passing BECAUSE of a different defect. + // + // So the failure DESCRIPTIONS are compared, not their number, and every + // one that appears more often on the treatment is named. + if newlyRed := gainedLabels(r.a.labels, r.b.labels); len(newlyRed) > 0 && !r.unstable { + for _, l := range newlyRed { + // The COUNT, not just the name: a shared example block included + // twice in one file reports the same description twice, and + // losing both is two examples rather than one. + wentRed += l.gained + times := "" + if l.gained > 1 { + times = fmt.Sprintf(" (×%d)", l.gained) + } + note += "\n ⛔ WENT RED: " + strings.ReplaceAll(l.label, "\t", " ") + times + } + } + // A file the move itself changes is marked, because its delta is not a + // reading of the engines at all: the corpus handed the two runs + // different work. Reading `+3` on such a row as an improvement is + // exactly the mistake this flag exists to stop. + if n, ok := mv.opens[r.file]; ok { + note += fmt.Sprintf(" ⛔ THE MOVE OPENS ≥%d examples here", n) + movePass += d + moveBad += worse + moveFiles++ + // The note is non-empty from here, so -quiet cannot drop the row: a + // delta of zero on a file the move opens is precisely how 37 + // failures stayed invisible. + } if *quiet && d == 0 && note == "" { continue } @@ -225,23 +315,323 @@ func main1(args []string, out, errOut io.Writer) int { 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", moved, regressed) + if wentRed > 0 { + // Counted apart from `regressed`, which is a per-FILE verdict: a file + // can improve on balance and still contain an example that broke. + fmt.Fprintf(out, ", %d example(s) WENT RED", wentRed) + } if appeared > 0 { // Named separately because it is not a regression in the passing total // and would otherwise read as nothing at all. fmt.Fprintf(out, ", %d gained failures without losing passes", appeared) } fmt.Fprintln(out) + // ⛔ The sentence a reader will quote. It must not be a passing total alone: + // the published one was, and it read as a plan. + if mv.on { + fmt.Fprintf(out, "of which the declared move %s → %s accounts for %d file(s): %+d passing, %+d failing\n", + mv.from, mv.to, moveFiles, movePass, moveBad) + fmt.Fprintf(out, "the move also opens ≥%d examples in %d file(s) of the corpus in total%s\n", + mv.move.ExamplesOpened(), len(gcFiles(mv.move.Opened)), mv.partialNote()) + } 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 { + // ⛔ An example that went red IS a regression, whatever the file's total did. + // Letting a net gain carry it out would be the exact reading this check + // exists to stop. + if regressed > 0 || wentRed > 0 { return 1 } return 0 } +// gainedLabels lists the failure descriptions the TREATMENT reports more often +// than the control — examples that were passing (or failing less often) and are +// not any more. Sorted, so a report is stable between runs. +// +// It reads descriptions rather than messages because a message carries object +// addresses (`#`) that differ between two runs of the +// SAME binary, which would make every file look changed. +func gainedLabels(a, b map[string]int) []gainedLabel { + var out []gainedLabel + for label, n := range b { + if n > a[label] { + out = append(out, gainedLabel{label: label, gained: n - a[label]}) + } + } + sort.Slice(out, func(i, j int) bool { return out[i].label < out[j].label }) + return out +} + +// gainedLabel is one failure description and HOW MANY more times the treatment +// reports it than the control. +type gainedLabel struct { + label string + gained int +} + +// declared is what -move resolved to: either "the corpus is the same on both +// sides" or a census of what the version move does to it. +type declared struct { + on bool + from, to string + move gc.Move + opens map[string]int // measured file -> lower-bound examples the move opens there + partial bool +} + +func gcFiles(sites []gc.Site) []string { return gc.Files(sites) } + +func (d declared) header() string { + if !d.on { + return "MOVE none declared: both engines read the same corpus, so every delta below is the ENGINE\n" + } + return fmt.Sprintf( + "MOVE %s → %s declared: the CORPUS differs between the two runs — %d guards open, %d close, %d inline sites flip%s\n", + d.from, d.to, len(d.move.Opened), len(d.move.Closed), len(d.move.Flipped), d.partialNote()) +} + +func (d declared) partialNote() string { + if d.partial { + return " ⛔ PARTIAL: part of the move's surface was not measured" + } + return "" +} + +// declaredMove resolves -move and, for a real move, REFUSES a run whose +// measured set does not cover the surface the move touches. +// +// ⛔ The refusal is the whole feature. `specdelta -dir core/set` across a +// 3.4→4.0 move is a perfectly well-formed command that produces a perfectly +// readable table saying nothing about the move, because the three files the +// move hits hardest are under core/binding. There is no way to see that in the +// results; it is only visible in the corpus, before anything runs. +func declaredMove(spec, root, dir string, files []string, errOut io.Writer, allowPartial bool) (declared, error) { + if spec == "" { + fmt.Fprint(errOut, `specdelta: -move is required. + + -move none both engines read the corpus at the same RUBY_VERSION, so + every difference below belongs to the engine + -move 3.4.1:4.0.9 the engines read the corpus at DIFFERENT versions: the + guards move too, and the census says which files that hits + +There is no default, because the results cannot answer it. A version move was +measured here on 2026-11-01 and published as "ten examples of work"; the real +figure was about seventy-five, and the missing part was three spec files whose +every example is guarded away below 4.0 — zero passes and zero failures on one +side, 37 failures on the other, and a pass delta of zero either way. +`) + return declared{}, errRefused + } + if spec == "none" { + return declared{on: false}, nil + } + from, to, ok := strings.Cut(spec, ":") + if !ok || from == "" || to == "" { + fmt.Fprintf(errOut, "specdelta: -move wants \"none\" or FROM:TO, e.g. 3.4.1:4.0.9; got %q\n", spec) + return declared{}, errRefused + } + if gc.CompareVersions(from, to) == 0 { + fmt.Fprintf(errOut, "specdelta: -move %s:%s names the same version twice — that is -move none\n", from, to) + return declared{}, errRefused + } + + sites, read, err := gc.Walk(root, []string{"language", "core"}) + if err != nil { + fmt.Fprintf(errOut, "specdelta: censusing %s: %v\n", root, err) + return declared{}, errRefused + } + if read == 0 { + // ⛔ An unreadable corpus must not pass as a move that touches nothing: + // "found no guards" and "could not look" are the same number. + fmt.Fprintf(errOut, "specdelta: no *_spec.rb under %s/{language,core} — the move cannot be censused\n", root) + return declared{}, errRefused + } + + d := declared{on: true, from: from, to: to, move: gc.ClassifyMove(sites, from, to), opens: map[string]int{}} + + measured := make(map[string]bool, len(files)) + for _, f := range files { + measured[filepath.ToSlash(f)] = true + } + for _, s := range d.move.Opened { + if measured[s.File] { + d.opens[s.File] += s.Examples + } + } + var outside []string + for _, f := range d.move.Touched() { + if !measured[f] { + outside = append(outside, f) + } + } + if len(outside) == 0 { + return d, nil + } + sort.Strings(outside) + fmt.Fprintf(errOut, "specdelta: the move %s → %s touches %d file(s) that -dir %s does not measure:\n", from, to, len(outside), dir) + for i, f := range outside { + if i == 12 { + fmt.Fprintf(errOut, " … and %d more\n", len(outside)-i) + break + } + fmt.Fprintf(errOut, " %s\n", f) + } + if !allowPartial { + fmt.Fprintln(errOut, " A number from this run would not be a number about the move. Widen -dir, or") + fmt.Fprintln(errOut, " pass -partial to say so on purpose — the report is then stamped PARTIAL.") + return declared{}, errRefused + } + d.partial = true + return d, nil +} + +// errRefused carries nothing: the refusing function has already said why, in +// the words the reader needs rather than a wrapped error string. +var errRefused = errors.New("refused") + +// buildFacts is what can be learned about an engine binary WITHOUT running it. +type buildFacts struct { + known bool // Go build information was readable at all + rev string // vcs.revision, as `go build` stamps it + dirty bool // vcs.modified: the tree had uncommitted changes + treeRev string // HEAD of the git work tree the binary sits in, "" if none +} + +// prov is the provenance verdict: what to print, and whether it is attributable. +type prov struct { + lines []string + stamp string +} + +// ⛔ THE SECOND RULE THIS TOOL NOW ENFORCES, and it is a repeat offence. +// +// On 2026-11-01 a figure was published for PR #858 — `pass=118 fail=1` — from a +// binary built BEFORE that branch existed. It carried the previous branch's +// code. The true figure was 117/2, and the correction had to go on the record. +// The same shape is in the notes from an earlier arc: a control built from the +// WORKING TREE rather than from the commit, which attributed +3 to the wrong +// fix. Writing it down twice did not stop it a third time. +// +// `go build` already stamps `vcs.revision` and `vcs.modified` into every binary +// it produces from a repository, so the binary can be asked what it is. The +// three refusals below are the three ways a comparison is void before it starts: +// +// same revision nothing to compare: one of them was not rebuilt +// dirty tree the revision names code the binary does not contain +// stale against HEAD the tree moved on after the binary was built — #858 +// +// An engine with no Go build information at all (MRI, a shell fake) is NOT +// refused: it is legitimately not a Go binary. It is STAMPED, because a table +// nobody can attribute must say so rather than look like the others. +func checkProvenance(engines []string, facts []buildFacts, allow bool, errOut io.Writer) (prov, error) { + var p prov + unattributable := false + for i, f := range facts { + switch { + case !f.known: + p.lines = append(p.lines, fmt.Sprintf(" %-9s %s — no Go build information", role(i), engines[i])) + unattributable = true + case f.dirty: + p.lines = append(p.lines, fmt.Sprintf(" %-9s %s — built from %s with UNCOMMITTED changes", role(i), engines[i], short(f.rev))) + unattributable = true + default: + p.lines = append(p.lines, fmt.Sprintf(" %-9s %s — built from %s", role(i), engines[i], short(f.rev))) + } + } + + var why []string + if facts[0].known && facts[1].known && !facts[0].dirty && !facts[1].dirty && facts[0].rev == facts[1].rev { + why = append(why, fmt.Sprintf("both engines were built from %s — one of them was not rebuilt, and there is nothing to compare", short(facts[0].rev))) + } + for i, f := range facts { + if f.dirty { + why = append(why, fmt.Sprintf("the %s was built from a tree with uncommitted changes, so %s does not name the code it contains", role(i), short(f.rev))) + } + // ⛔ #858 exactly: the binary is older than the branch it is being + // quoted for. Nothing in the results can show this. + if f.known && !f.dirty && f.treeRev != "" && f.treeRev != f.rev { + why = append(why, fmt.Sprintf("the %s binary was built from %s but its work tree is now at %s — it is not the code you are measuring", + role(i), short(f.rev), short(f.treeRev))) + } + } + if len(why) == 0 { + if unattributable { + p.stamp = " ⛔ UNPROVENANCED" + } + return p, nil + } + if allow { + p.stamp = " ⛔ UNPROVENANCED" + return p, nil + } + fmt.Fprintln(errOut, "specdelta: this comparison cannot be attributed to any pair of commits:") + for _, w := range why { + fmt.Fprintf(errOut, " %s\n", w) + } + fmt.Fprintln(errOut, " Rebuild, or pass -unprovenanced to say so on purpose — the report is then stamped.") + return prov{}, errRefused +} + +func role(i int) string { + if i == 0 { + return "control" + } + return "treatment" +} + +func short(rev string) string { + if len(rev) > 12 { + return rev[:12] + } + if rev == "" { + return "(no revision)" + } + return rev +} + +// readBuildFacts asks a binary what it is. Replaceable so the policy above can +// be driven by a table rather than by building Go binaries in a test. +var readBuildFacts = func(path string) buildFacts { + var f buildFacts + bi, err := buildinfo.ReadFile(path) + if err != nil { + return f + } + f.known = true + for _, s := range bi.Settings { + switch s.Key { + case "vcs.revision": + f.rev = s.Value + case "vcs.modified": + f.dirty = s.Value == "true" + } + } + if f.rev == "" { + // Stamped with no revision is the same as not stamped: there is + // nothing to attribute it to. + f.known = false + return f + } + f.treeRev = headOf(filepath.Dir(path)) + return f +} + +// headOf asks git for a work tree's HEAD, and answers "" for anything it +// cannot establish — a binary outside a repository is not a defect. +func headOf(dir string) string { + cmd := exec.Command("git", "-C", dir, "rev-parse", "HEAD") + cmd.Stdin = nil + out, err := cmd.Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + func abs(n int) int { if n < 0 { return -n @@ -350,6 +740,13 @@ func parse(s string) (result, error) { switch { case strings.HasPrefix(line, "RBGO_DETAIL"): r.details++ + // RBGO_DETAIL \t kind \t class \t description \t message + if f := strings.Split(line, "\t"); len(f) >= 4 { + if r.labels == nil { + r.labels = map[string]int{} + } + r.labels[f[1]+"\t"+f[3]]++ + } case strings.HasPrefix(line, "RBGO_RESULT"): if r.ran { return r, fmt.Errorf("two RBGO_RESULT lines in one run") diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main_test.go b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go index 4d42a6a9..3c9c84c0 100644 --- a/scripts/conformance/rubyspec/cmd/specdelta/main_test.go +++ b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go @@ -125,7 +125,7 @@ func TestEmptyPopulationIsRefused(t *testing.T) { 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 { + if code := main1([]string{"-root", root, "-move", "none", "-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") { @@ -140,10 +140,10 @@ func TestUsageRefusals(t *testing.T) { 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"}}, + {"one engine", []string{"-root", root, "-move", "none", "-dir", "x", "/bin/echo"}}, + {"three engines", []string{"-root", root, "-move", "none", "-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"}}, + {"absent dir", []string{"-root", root, "-move", "none", "-dir", "nope", "/bin/echo", "/bin/echo"}}, } { t.Run(tc.name, func(t *testing.T) { var out, errOut bytes.Buffer @@ -172,7 +172,7 @@ func TestReportsMovementAndRegression(t *testing.T) { 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 { + if code := main1([]string{"-root", root, "-move", "none", "-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") { @@ -181,7 +181,7 @@ func TestReportsMovementAndRegression(t *testing.T) { out.Reset() errOut.Reset() - if code := main1([]string{"-root", root, "-dir", "core/x", three, two}, &out, &errOut); code != 1 { + if code := main1([]string{"-root", root, "-move", "none", "-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") { @@ -223,7 +223,7 @@ esac esac `) var out, errOut bytes.Buffer - if code := main1([]string{"-root", root, "-dir", "core/w", low, high}, &out, &errOut); code != 0 { + if code := main1([]string{"-root", root, "-move", "none", "-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") @@ -254,7 +254,7 @@ func TestAFileThatDidNotRunIsNamed(t *testing.T) { mute := fakeEngine(t, root, "mute", "exit 1\n") var out, errOut bytes.Buffer - main1([]string{"-root", root, "-dir", "core/y", ok, mute}, &out, &errOut) + main1([]string{"-root", root, "-move", "none", "-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()) } @@ -295,7 +295,7 @@ fi // 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 { + if code := main1([]string{"-root", root, "-move", "none", "-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()) } @@ -305,7 +305,7 @@ fi } out.Reset() errOut.Reset() - if code := main1([]string{"-root", root, "-dir", "core/z", "-repeat", "3", stable, flaky}, &out, &errOut); code != 0 { + if code := main1([]string{"-root", root, "-move", "none", "-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") { @@ -318,7 +318,7 @@ fi // 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 { + if code := main1([]string{"-root", root, "-move", "none", "-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") { @@ -328,7 +328,7 @@ fi // 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 { + if code := main1([]string{"-root", root, "-move", "none", "-dir", "core/z", "-repeat", "0", stable, stable}, &out, &errOut); code != 2 { t.Errorf("-repeat 0: exit = %d, want 2; err %q", code, errOut.String()) } } @@ -361,7 +361,7 @@ func TestAFileThatGainsFailuresWithoutLosingPassesIsNamed(t *testing.T) { "printf 'RBGO_DETAIL\\terror\\tY\\tc\\td\\n%.0s' 1 2\n") var out, errOut bytes.Buffer - main1([]string{"-root", root, "-dir", "core/v", "-quiet", guarded, opened}, &out, &errOut) + main1([]string{"-root", root, "-move", "none", "-dir", "core/v", "-quiet", guarded, opened}, &out, &errOut) if !strings.Contains(out.String(), "fail+error 0 → 5") { t.Errorf("the new failures are not named:\n%s", out.String()) } @@ -376,12 +376,368 @@ func TestAFileThatGainsFailuresWithoutLosingPassesIsNamed(t *testing.T) { // The control: a file that is quiet on both engines stays out of a -quiet // report, or every unchanged file would be printed. out.Reset() - main1([]string{"-root", root, "-dir", "core/v", "-quiet", guarded, guarded}, &out, &errOut) + main1([]string{"-root", root, "-move", "none", "-dir", "core/v", "-quiet", guarded, guarded}, &out, &errOut) if strings.Contains(out.String(), "d_spec.rb") { t.Errorf("an unchanged file was printed under -quiet:\n%s", out.String()) } } +// ⛔ THE GATE. -move has no default, because the results cannot answer it: a +// run across a version move and a run within one produce the same table, and +// only one of them is a reading of the engines. +// +// Without it the tool refuses, and the refusal has to come BEFORE the engines +// run — a sweep of core/ is an hour, and an hour spent on a figure already +// known to be incomplete is the cost this gate exists to avoid. The fake engine +// here records every call, so "did not run" is asserted rather than assumed. +func TestAMoveMustBeDeclaredAndTheRefusalPrecedesTheRun(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", "q"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "core", "q", "e_spec.rb"), []byte("#\n"), 0o644); err != nil { + t.Fatal(err) + } + ran := filepath.Join(root, "ran") + engine := fakeEngine(t, root, "counting", "echo x >> '"+ran+"'\necho 'RBGO_RESULT pass=1 fail=0 error=0 skip=0'\n") + + var out, errOut bytes.Buffer + if code := main1([]string{"-root", root, "-dir", "core/q", engine, engine}, &out, &errOut); code != 2 { + t.Errorf("a missing -move: exit = %d, want 2", code) + } + if !strings.Contains(errOut.String(), "-move is required") { + t.Errorf("the refusal does not say what is missing:\n%s", errOut.String()) + } + if _, err := os.Stat(ran); err == nil { + t.Error("an engine was started before the declaration was checked") + } + + // And the malformed spellings, each exit 2 rather than a silent "no move". + for _, bad := range []string{"3.4.1", "3.4.1:", ":4.0.9", "4.0.9:4.0.9", "4.0:4.0.0"} { + out.Reset() + errOut.Reset() + if code := main1([]string{"-root", root, "-move", bad, "-dir", "core/q", engine, engine}, &out, &errOut); code != 2 { + t.Errorf("-move %q: exit = %d, want 2; err %q", bad, code, errOut.String()) + } + } +} + +// ⛔ THE REFUSAL THAT WOULD HAVE CAUGHT THE PUBLISHED FIGURE. The command is +// well-formed and the table it produces is readable; it simply is not a table +// about the move, because the move's heaviest files are under a directory -dir +// never named. There is nothing in the RESULTS that could say so. +func TestAMoveReachingOutsideTheMeasuredSetIsRefused(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the fake engines are POSIX shell scripts") + } + root := t.TempDir() + for _, d := range []string{"core/set", "core/binding"} { + if err := os.MkdirAll(filepath.Join(root, d), 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(root, "core", "set", "divide_spec.rb"), []byte("#\n"), 0o644); err != nil { + t.Fatal(err) + } + // The file the move opens, and the one -dir core/set cannot see. + if err := os.WriteFile(filepath.Join(root, "core", "binding", "implicit_parameters_spec.rb"), []byte( + "describe \"Binding#implicit_parameters\" do\n"+ + " ruby_version_is \"4.0\" do\n"+ + " it \"exists\" do\n"+ + " end\n"+ + " end\n"+ + "end\n"), 0o644); err != nil { + t.Fatal(err) + } + engine := fakeEngine(t, root, "e", "echo 'RBGO_RESULT pass=1 fail=0 error=0 skip=0'\n") + + var out, errOut bytes.Buffer + if code := main1([]string{"-root", root, "-move", "3.4.1:4.0.9", "-dir", "core/set", engine, engine}, &out, &errOut); code != 2 { + t.Fatalf("exit = %d, want 2; err %q", code, errOut.String()) + } + if !strings.Contains(errOut.String(), "core/binding/implicit_parameters_spec.rb") { + t.Errorf("the refusal does not name the file it cannot see:\n%s", errOut.String()) + } + + // -partial is the named derogation, and it STAMPS the report rather than + // silencing the gate: an opt-in that only removes a message would leave the + // same number published with nothing attached to it. + out.Reset() + errOut.Reset() + if code := main1([]string{"-root", root, "-move", "3.4.1:4.0.9", "-partial", "-dir", "core/set", engine, engine}, &out, &errOut); code != 0 { + t.Fatalf("-partial: exit = %d, want 0; err %q", code, errOut.String()) + } + if !strings.Contains(out.String(), "PARTIAL") { + t.Errorf("-partial did not stamp the report:\n%s", out.String()) + } + + // And widening -dir to cover the surface is accepted, which is the control: + // without it the refusal above would only prove the gate always fires. + out.Reset() + errOut.Reset() + if code := main1([]string{"-root", root, "-move", "3.4.1:4.0.9", "-dir", "core", engine, engine}, &out, &errOut); code != 0 { + t.Fatalf("widened -dir: exit = %d, want 0; err %q", code, errOut.String()) + } + if strings.Contains(out.String(), "PARTIAL") { + t.Errorf("a complete run was stamped PARTIAL:\n%s", out.String()) + } + if !strings.Contains(out.String(), "THE MOVE OPENS") { + t.Errorf("the opened file is not marked in the table:\n%s", out.String()) + } +} + +// A file the move opens is marked and kept even when nothing moved, because a +// delta of zero on such a file is the signature of the defect, not of calm. +func TestAFileTheMoveOpensSurvivesQuiet(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", "binding"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "core", "binding", "implicit_parameters_spec.rb"), []byte( + "ruby_version_is \"4.0\" do\n it \"a\" do\n end\n it \"b\" do\n end\nend\n"), 0o644); err != nil { + t.Fatal(err) + } + // Both engines score the same: delta zero, and under -quiet the row would + // have been dropped before this gate existed. + engine := fakeEngine(t, root, "e", "echo 'RBGO_RESULT pass=0 fail=0 error=0 skip=0'\n") + + var out, errOut bytes.Buffer + if code := main1([]string{"-root", root, "-move", "3.4.1:4.0.9", "-quiet", "-dir", "core/binding", engine, engine}, &out, &errOut); code != 0 { + t.Fatalf("exit = %d, want 0; err %q", code, errOut.String()) + } + if !strings.Contains(out.String(), "implicit_parameters_spec.rb") { + t.Errorf("-quiet dropped a file the move opens:\n%s", out.String()) + } + if !strings.Contains(out.String(), "≥2 examples") { + t.Errorf("the opened example count is not reported:\n%s", out.String()) + } + if !strings.Contains(out.String(), "the declared move 3.4.1 → 4.0.9 accounts for 1 file(s)") { + t.Errorf("the summary does not separate the move's share:\n%s", out.String()) + } + + // The control: declared as no move, the same run under -quiet prints + // nothing about the file — which is what makes the line above a reading of + // the declaration rather than of the file name. + out.Reset() + if code := main1([]string{"-root", root, "-move", "none", "-quiet", "-dir", "core/binding", engine, engine}, &out, &errOut); code != 0 { + t.Fatalf("control: exit = %d, want 0; err %q", code, errOut.String()) + } + if strings.Contains(out.String(), "implicit_parameters_spec.rb") { + t.Errorf("a quiet file was printed with no move declared:\n%s", out.String()) + } +} + +// ⛔ THE REPEAT OFFENCE. On 2026-11-01 `pass=118 fail=1` was published for +// PR #858 from a binary built BEFORE that branch existed; it carried the +// previous branch's code, and the true figure was 117/2. The same shape is in +// the notes from an earlier arc — a control built from the working tree rather +// than from the commit, attributing +3 to the wrong fix. Written down twice, +// and it happened a third time. +// +// `go build` stamps vcs.revision and vcs.modified into every binary, so the +// binary can be asked. The policy is tested as a table because the alternative +// is building Go binaries inside a test to observe a decision that does not +// depend on building them. +func TestProvenanceRefusesWhatCannotBeAttributed(t *testing.T) { + clean := func(rev, head string) buildFacts { + return buildFacts{known: true, rev: rev, treeRev: head} + } + for _, tc := range []struct { + name string + facts []buildFacts + refuse bool + stamped bool + says string + }{{ + name: "two distinct commits, both clean — the only fully attributable case", + facts: []buildFacts{clean("aaaaaaaaaaaa1", "aaaaaaaaaaaa1"), clean("bbbbbbbbbbbb2", "bbbbbbbbbbbb2")}, + }, { + name: "the same revision twice: one of them was not rebuilt", + facts: []buildFacts{clean("aaaaaaaaaaaa1", ""), clean("aaaaaaaaaaaa1", "")}, + refuse: true, + says: "nothing to compare", + }, { + name: "a dirty tree: the revision does not name the code in the binary", + facts: []buildFacts{clean("aaaaaaaaaaaa1", ""), {known: true, rev: "bbbbbbbbbbbb2", dirty: true}}, + refuse: true, + says: "uncommitted changes", + }, { + // #858, exactly. + name: "the tree moved on after the binary was built", + facts: []buildFacts{clean("aaaaaaaaaaaa1", "aaaaaaaaaaaa1"), clean("bbbbbbbbbbbb2", "cccccccccccc3")}, + refuse: true, + says: "not the code you are measuring", + }, { + // MRI, or a shell fake: legitimately not a Go binary. Not a refusal, + // but the table must not look like an attributable one. + name: "an engine with no build information is stamped, not refused", + facts: []buildFacts{{}, clean("bbbbbbbbbbbb2", "bbbbbbbbbbbb2")}, + stamped: true, + }} { + t.Run(tc.name, func(t *testing.T) { + var errOut bytes.Buffer + p, err := checkProvenance([]string{"./a", "./b"}, tc.facts, false, &errOut) + if tc.refuse { + if err == nil { + t.Fatalf("not refused; printed %q", errOut.String()) + } + if !strings.Contains(errOut.String(), tc.says) { + t.Errorf("the refusal does not say why: %q", errOut.String()) + } + // ⛔ And the derogation STAMPS rather than silences: an opt-in + // that only removed a message would leave the same number + // published with nothing attached to it. + errOut.Reset() + p, err = checkProvenance([]string{"./a", "./b"}, tc.facts, true, &errOut) + if err != nil { + t.Fatalf("-unprovenanced still refused: %v", err) + } + if !strings.Contains(p.stamp, "UNPROVENANCED") { + t.Errorf("the derogation did not stamp the report: %q", p.stamp) + } + return + } + if err != nil { + t.Fatalf("refused: %q", errOut.String()) + } + if strings.Contains(p.stamp, "UNPROVENANCED") != tc.stamped { + t.Errorf("stamp = %q, want stamped=%v", p.stamp, tc.stamped) + } + // Both engines are always named, so a pasted table says what + // produced it. + if len(p.lines) != 2 { + t.Errorf("provenance lines = %v, want one per engine", p.lines) + } + }) + } +} + +// The header has to carry both revisions into the table a reader will paste, +// because the published figure that was wrong carried nothing at all. +func TestTheTableNamesWhatProducedIt(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", "p"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "core", "p", "f_spec.rb"), []byte("#\n"), 0o644); err != nil { + t.Fatal(err) + } + a := fakeEngine(t, root, "a", "echo 'RBGO_RESULT pass=1 fail=0 error=0 skip=0'\n") + b := fakeEngine(t, root, "b", "echo 'RBGO_RESULT pass=1 fail=0 error=0 skip=0'\n") + + saved := readBuildFacts + t.Cleanup(func() { readBuildFacts = saved }) + readBuildFacts = func(path string) buildFacts { + if strings.HasSuffix(path, "/a") { + return buildFacts{known: true, rev: "1111111111112222", treeRev: "1111111111112222"} + } + return buildFacts{known: true, rev: "3333333333334444", treeRev: "3333333333334444"} + } + + var out, errOut bytes.Buffer + if code := main1([]string{"-root", root, "-move", "none", "-dir", "core/p", a, b}, &out, &errOut); code != 0 { + t.Fatalf("exit = %d, want 0; err %q", code, errOut.String()) + } + for _, want := range []string{"PROVENANCE", "111111111111", "333333333333"} { + if !strings.Contains(out.String(), want) { + t.Errorf("the table does not carry %q:\n%s", want, out.String()) + } + } + if strings.Contains(out.String(), "UNPROVENANCED") { + t.Errorf("two clean distinct commits were stamped:\n%s", out.String()) + } +} + +// ⛔ A NET GAIN HIDES AN EXAMPLE THAT WENT RED, and this is the measurement +// that proved it rather than a hypothesis. On 2026-11-02 a control and a +// treatment built from the same commit minus one change read: +// +// core/thread/raise_spec.rb 51 → 68 +17 +// 53 file(s) … 1 file(s) moved, 0 regressed +// +// True, and incomplete. Nineteen examples turned green and TWO turned red — +// the two were found by comparing the failure DESCRIPTIONS of the two runs, and +// no count of passes, of failures, or of both can show a swap. +// +// The example that broke had been passing BECAUSE of a different defect, which +// is why it reads as a clean win: nothing about it looks wrong from the +// outside. +func TestAnExampleThatWentRedInsideAnImprovedFileIsNamed(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", "r"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "core", "r", "g_spec.rb"), []byte("#\n"), 0o644); err != nil { + t.Fatal(err) + } + // Control: 5 passing, 3 failing — "alpha", "beta", "gamma". + control := fakeEngine(t, root, "ctl", "echo 'RBGO_RESULT pass=5 fail=3 error=0 skip=0'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tSpecFail\\talpha\\tmsg one\\n'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tSpecFail\\tbeta\\tmsg two\\n'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tSpecFail\\tgamma\\tmsg three\\n'\n") + // Treatment: 7 passing, 1 failing — a clear improvement by every count, and + // the one failure is a DIFFERENT example: "delta" was green and is not. + treatment := fakeEngine(t, root, "treat", "echo 'RBGO_RESULT pass=7 fail=1 error=0 skip=0'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tSpecFail\\tdelta\\tmsg four\\n'\n") + + var out, errOut bytes.Buffer + code := main1([]string{"-root", root, "-move", "none", "-dir", "core/r", control, treatment}, &out, &errOut) + if !strings.Contains(out.String(), "WENT RED") || !strings.Contains(out.String(), "delta") { + t.Errorf("the example that broke is not named:\n%s", out.String()) + } + if !strings.Contains(out.String(), "1 example(s) WENT RED") { + t.Errorf("the summary does not count it:\n%s", out.String()) + } + // ⛔ And it is a REGRESSION, whatever the totals did: +2 passing and two + // fewer failures would otherwise carry it out on exit 0. + if code != 1 { + t.Errorf("exit = %d, want 1 (an example went red)", code) + } + + // THE CONTROL: the same shapes with no swap must stay silent, or every + // improved file would be flagged. Here the treatment simply fixes "gamma". + fixed := fakeEngine(t, root, "fixed", "echo 'RBGO_RESULT pass=7 fail=2 error=0 skip=0'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tSpecFail\\talpha\\tmsg one\\n'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tSpecFail\\tbeta\\tmsg two\\n'\n") + out.Reset() + errOut.Reset() + if code := main1([]string{"-root", root, "-move", "none", "-dir", "core/r", control, fixed}, &out, &errOut); code != 0 { + t.Errorf("a genuine improvement: exit = %d, want 0; out %q", code, out.String()) + } + if strings.Contains(out.String(), "WENT RED") { + t.Errorf("a file that only improved was flagged:\n%s", out.String()) + } + + // ⛔ And a MESSAGE that differs is not an example that broke: the same + // description carrying an object address prints differently between two + // runs of the same binary, and keying on it would make every file look + // changed. + sameLabels := fakeEngine(t, root, "addr", "echo 'RBGO_RESULT pass=5 fail=3 error=0 skip=0'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tSpecFail\\talpha\\texpected #\\n'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tSpecFail\\tbeta\\tmsg two\\n'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tSpecFail\\tgamma\\tmsg three\\n'\n") + out.Reset() + errOut.Reset() + if code := main1([]string{"-root", root, "-move", "none", "-dir", "core/r", control, sameLabels}, &out, &errOut); code != 0 { + t.Errorf("differing messages: exit = %d, want 0; out %q", code, out.String()) + } + if strings.Contains(out.String(), "WENT RED") { + t.Errorf("a differing message was read as an example that broke:\n%s", out.String()) + } +} + func fakeEngine(t *testing.T, dir, name, body string) string { t.Helper() p := filepath.Join(dir, name) diff --git a/scripts/conformance/rubyspec/internal/guardcensus/census.go b/scripts/conformance/rubyspec/internal/guardcensus/census.go new file mode 100644 index 00000000..c1fc58a6 --- /dev/null +++ b/scripts/conformance/rubyspec/internal/guardcensus/census.go @@ -0,0 +1,367 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +// Package guardcensus reads ruby/spec's `ruby_version_is` guards out of the +// corpus and says which ones a given version move OPENS, CLOSES or FLIPS. +// +// It exists as a package rather than as one command's internals because two +// commands need the same answer and must not drift: `cmd/guards` censuses the +// corpus on its own, and `cmd/specdelta` is now REFUSED a version-move verdict +// unless the same census says the run covered the surface the move touches. +// +// ⛔ WHY THE REFUSAL. On 2026-11-01 a version move was measured by comparing +// PASSING counts between two engines, and the figure was published. Three +// `core/binding/implicit_parameter*_spec.rb` files are `pass=0 fail=0 error=0` +// below 4.0 — every example behind a guard — and 37 failures above it. A +// pass-only delta showed nothing, and a note saying "ask what the target +// version ADDS before measuring what moves" had been written down months +// earlier and was not consulted. The note was not the missing part; a gate was. +// +// The version predicate here is spec_helper.rb's `_version_in_range` +// (scripts/conformance/rubyspec/spec_helper.rb:133), which is itself mspec's +// VersionGuard semantics: a bare string is a LOWER bound, a Range is bounded at +// both ends with `...` exclusive at the top, an empty endpoint is unbounded, +// and versions compare component by component with absent components read as +// zero (`_ver_cmp`, :119). It is reimplemented rather than called because the +// census must answer for versions the running interpreter is NOT, which is the +// whole question. +package guardcensus + +import ( + "io/fs" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" +) + +var ( + callRe = regexp.MustCompile(`ruby_version_is`) + argRe = regexp.MustCompile(`ruby_version_is\s*\(?\s*["']([^"']*)["']\s*(?:(\.\.\.?)\s*["']([^"']*)["']\s*)?\)?`) + opensRe = regexp.MustCompile(`(?:\bdo\b|\{)\s*(?:\|[^|]*\|)?\s*$`) + bracesRe = regexp.MustCompile(`ruby_version_is\s*\(?[^)]*\)?\s*\{`) + commentRe = regexp.MustCompile(`\s+#(?:[^{].*)?$`) + // OldAnchorRe is the earlier census's shape, kept ONLY so its number can + // be reproduced as a control. Nothing else should use it. + oldAnchorRe = regexp.MustCompile(`ruby_version_is\s+["'][^"']*["']\s*(\.\.\.?\s*["'][^"']*["']\s*)?do\s*$`) + itRe = regexp.MustCompile(`^\s*it[\s(]`) + openKw = regexp.MustCompile(`(^|\s)(def|class|module|case|begin|while|until|unless|if)\s`) + modifIf = regexp.MustCompile(`\S\s+(if|unless|while|until)\s`) + endKw = regexp.MustCompile(`^\s*end\b`) + doKw = regexp.MustCompile(`\bdo\b\s*(\|[^|]*\|)?\s*(#.*)?$`) + braceEnd = regexp.MustCompile(`^\s*\}`) +) + +// Shape is what a mention of ruby_version_is does. +// +// BLOCK ruby_version_is "4.0" do … end +// The examples inside are SKIPPED while the version is below the +// threshold. Raising it ADDS them to the population. +// +// INLINE msg_class = ruby_version_is("4.0") ? "Module" : "module" +// The example RUNS either way; the guard only picks a value. Raising +// the version changes what these ASSERT without changing how many +// there are. +type Shape int + +const ( + Block Shape = iota + Inline + Unknown +) + +func (s Shape) String() string { + switch s { + case Block: + return "block" + case Inline: + return "inline" + } + return "unclassified" +} + +// Site is one mention of ruby_version_is. +type Site struct { + File, Text string + Line int + From, To string + Ranged bool // the source wrote `..` or `...`, so To is meaningful + Excl bool // `...`: the upper bound is exclusive + Shape Shape + InIt bool // an inline inside an `it` moves exactly one example + Examples int // `it` blocks inside a block guard: a LOWER BOUND + OldAnchor bool // a `do$` anchor would have found it +} + +// Key is the version range a site guards, as it reads in the source. +func (s Site) Key() string { + if s.Ranged { + dots := ".." + if s.Excl { + dots = "..." + } + return s.From + dots + s.To + } + if s.From == "" { + return `"" (open lower bound)` + } + return s.From +} + +// OpenAt reports whether this guard's condition holds at the given version — +// spec_helper.rb's `_version_in_range` (:133), reimplemented so it can be asked +// about a version the running interpreter is not. +func (s Site) OpenAt(version string) bool { + if !s.Ranged { + // A bare string is a LOWER bound, and the empty string is always true. + return s.From == "" || CompareVersions(version, s.From) >= 0 + } + if s.From != "" && CompareVersions(version, s.From) < 0 { + return false + } + if s.To == "" { + return true + } + if s.Excl { + return CompareVersions(version, s.To) < 0 + } + return CompareVersions(version, s.To) <= 0 +} + +// CompareVersions is spec_helper.rb's `_ver_cmp` (:119): component by +// component, with a component the other side does not have read as zero, so +// "4.0" and "4.0.0" are equal and "3.4" < "3.4.1". +// +// ⛔ A component that is not a number reads as zero, exactly as Ruby's `to_i` +// does there. Diverging would make this census answer a question about a +// different corpus than the one the shim ran. +func CompareVersions(a, b string) int { + pa, pb := strings.Split(a, "."), strings.Split(b, ".") + n := len(pa) + if len(pb) > n { + n = len(pb) + } + for i := 0; i < n; i++ { + x, y := component(pa, i), component(pb, i) + if x != y { + if x < y { + return -1 + } + return 1 + } + } + return 0 +} + +func component(parts []string, i int) int { + if i >= len(parts) { + return 0 + } + // Ruby's String#to_i takes the leading digits and yields 0 for none. + end := 0 + for end < len(parts[i]) && parts[i][end] >= '0' && parts[i][end] <= '9' { + end++ + } + n, _ := strconv.Atoi(parts[i][:end]) + return n +} + +// Classify decides what a single line of Ruby does with ruby_version_is. +// +// ⛔ THE SHAPE IS DECIDED BY WHAT OPENS A BLOCK, NOT BY THE PARENTHESES. A +// first draft keyed on "no parentheses, and the line ends in do". It was wrong +// twice, and both were found by PRINTING THE SITES instead of only the totals: +// +// ruby_version_is "4.0" do # https://bugs.ruby-lang.org/issues/21844 +// a block guard whose trailing comment defeats a `do\s*$` anchor +// ruby_version_is("4.0") do +// a block guard that IS parenthesised +// +// Five guards at "4.0" hide in those two shapes, so a census anchored that way +// reports 56 where there are 61. +func Classify(line string) (Site, bool) { + if !callRe.MatchString(line) { + return Site{}, false + } + s := Site{Text: strings.TrimSpace(line)} + m := argRe.FindStringSubmatch(line) + if m == nil { + s.Shape = Unknown + return s, true + } + s.From = m[1] + if m[2] != "" { + s.Ranged = true + s.Excl = m[2] == "..." + s.To = m[3] + } + s.OldAnchor = oldAnchorRe.MatchString(line) + bare := commentRe.ReplaceAllString(line, "") + if opensRe.MatchString(bare) || bracesRe.MatchString(bare) { + s.Shape = Block + } else { + s.Shape = Inline + } + return s, true +} + +// ExamplesIn counts the `it` blocks between a guard and its matching `end`. +// +// ⛔ A LOWER BOUND, and it says so: shared examples (`it_behaves_like`) expand +// at run time and are invisible to a static count. cmd/guards keeps a control +// that reproduces the guard number while the example number does not, which +// places the difference in the counting rather than in the census. Tuning the +// heuristic until it reached the expected answer would be fitting the +// instrument to the answer. +func ExamplesIn(lines []string, start int) int { + depth, n := 1, 0 + for i := start + 1; i < len(lines) && depth > 0; i++ { + l := lines[i] + if strings.HasPrefix(strings.TrimSpace(l), "#") { + continue + } + if endKw.MatchString(l) || braceEnd.MatchString(l) { + depth-- + continue + } + if doKw.MatchString(l) || (openKw.MatchString(l) && !modifIf.MatchString(l)) { + depth++ + } + if itRe.MatchString(l) { + n++ + } + } + return n +} + +// Census reads one spec file. `path` is recorded on every Site as given. +func Census(path string, src []byte) []Site { + lines := strings.Split(strings.ReplaceAll(string(src), "\r\n", "\n"), "\n") + var out []Site + itDepth := 0 + for i, line := range lines { + if itRe.MatchString(line) { + itDepth++ + } else if strings.TrimSpace(line) == "end" && itDepth > 0 { + itDepth-- + } + s, ok := Classify(line) + if !ok { + continue + } + s.File, s.Line, s.InIt = path, i+1, itDepth > 0 + if s.Shape == Block { + s.Examples = ExamplesIn(lines, i) + } + out = append(out, s) + } + return out +} + +// Walk censuses every *_spec.rb under root/suite for each named suite, with +// Site.File relative to root. +// +// ⛔ The suites are NAMED by the caller rather than discovered. A draft walked +// the checkout minus a denylist of directory names and counted 2207 files where +// the gate counts 2206: a denylist inherits whatever it has not heard of. +// +// It returns the number of files read, so a caller can refuse an empty corpus +// rather than report an empty census — the two need opposite responses. A +// suite that is not present at all is skipped rather than refused, because a +// caller measuring one suite should not have to own the other; `files == 0` +// still distinguishes "nothing to read" from "read nothing". +func Walk(root string, suites []string) ([]Site, int, error) { + var sites []Site + files := 0 + for _, suite := range suites { + if fi, err := os.Stat(filepath.Join(root, suite)); err != nil || !fi.IsDir() { + continue + } + err := filepath.WalkDir(filepath.Join(root, suite), func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() || !strings.HasSuffix(p, "_spec.rb") { + return nil + } + files++ + b, rerr := os.ReadFile(p) + if rerr != nil { + return rerr + } + rel, rerr := filepath.Rel(root, p) + if rerr != nil { + return rerr + } + sites = append(sites, Census(filepath.ToSlash(rel), b)...) + return nil + }) + if err != nil { + return nil, files, err + } + } + return sites, files, nil +} + +// Move is what a version move does to the corpus's own population, before any +// engine is run. Every field is a reading of the SOURCE, not of a result. +type Move struct { + From, To string + Opened []Site // block guards shut at From and open at To: examples APPEAR + Closed []Site // block guards open at From and shut at To: examples VANISH + Flipped []Site // inline sites whose value changes: the SAME example asserts something else +} + +// ClassifyMove sorts a census into what the move opens, closes and flips. +func ClassifyMove(sites []Site, from, to string) Move { + m := Move{From: from, To: to} + for _, s := range sites { + was, now := s.OpenAt(from), s.OpenAt(to) + if was == now { + continue + } + switch s.Shape { + case Block: + if now { + m.Opened = append(m.Opened, s) + } else { + m.Closed = append(m.Closed, s) + } + case Inline: + m.Flipped = append(m.Flipped, s) + } + } + return m +} + +// Files lists, in corpus order and without repeats, the files holding the given +// sites. This is the unit the gate works in: a measurement covers FILES, and a +// file the run never touched cannot have its guards attributed to anything. +func Files(sites []Site) []string { + seen := map[string]bool{} + var out []string + for _, s := range sites { + if !seen[s.File] { + seen[s.File] = true + out = append(out, s.File) + } + } + return out +} + +// Touched is every file the move opens, closes or flips. +func (m Move) Touched() []string { + all := append(append(append([]Site{}, m.Opened...), m.Closed...), m.Flipped...) + return Files(all) +} + +// ExamplesOpened sums the lower-bound example counts of the opened guards. +func (m Move) ExamplesOpened() int { + n := 0 + for _, s := range m.Opened { + n += s.Examples + } + return n +} diff --git a/scripts/conformance/rubyspec/internal/guardcensus/census_test.go b/scripts/conformance/rubyspec/internal/guardcensus/census_test.go new file mode 100644 index 00000000..847d4823 --- /dev/null +++ b/scripts/conformance/rubyspec/internal/guardcensus/census_test.go @@ -0,0 +1,308 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package guardcensus + +import ( + "strings" + "testing" +) + +// ⛔ Every case below except the first two is a line that actually appears in +// the pinned corpus, and the two marked FOUND BY PRINTING are the ones a +// `do\s*$` anchor misses. They are the reason this classifier exists: the +// earlier census reported 56 guards at "4.0" where there are 61, and the five +// it could not see were not exotic — three carry a trailing comment and one is +// merely parenthesised. +func TestClassify(t *testing.T) { + cases := []struct { + name string + line string + want Shape + from string + to string + excl bool + inAnc bool // a `do$` anchor would have found it + }{ + { + name: "the plain block guard", + line: ` ruby_version_is "4.0" do`, + want: Block, + from: "4.0", + inAnc: true, + }, + { + name: "a range block guard", + line: ` ruby_version_is ""..."4.0" do`, + want: Block, from: "", to: "4.0", excl: true, inAnc: true, + }, + { + // FOUND BY PRINTING THE SITES: core/data/initialize_spec.rb:83 + name: "a block guard with a trailing comment", + line: ` ruby_version_is "4.0" do # https://bugs.ruby-lang.org/issues/21844`, + want: Block, from: "4.0", inAnc: false, + }, + { + // FOUND BY PRINTING THE SITES: core/set/pretty_print_cycle_spec.rb:9 + name: "a block guard that is parenthesised", + line: ` ruby_version_is("4.0") do`, + want: Block, from: "4.0", inAnc: false, + }, + { + name: "a brace block in an argument list", + line: ` *ruby_version_is("4.0") { :linux },`, + want: Block, from: "4.0", inAnc: false, + }, + { + name: "the ternary, which RUNS either way", + line: ` msg_class = ruby_version_is("4.0") ? "Module" : "module"`, + want: Inline, from: "4.0", inAnc: false, + }, + { + name: "a ternary on the 3.4 boundary", + line: ` expected = ruby_version_is("3.4") ? "{a: 1}" : "{:a=>1}"`, + want: Inline, from: "3.4", inAnc: false, + }, + { + name: "inside a lambda guard, which is neither", + line: ` guard -> { ruby_version_is "3.4" and !"test".equal?("test") } do`, + want: Block, from: "3.4", inAnc: false, + }, + { + name: "a line that never mentions it", + line: ` it "returns a String" do`, + want: Unknown, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + s, ok := Classify(c.line) + if c.want == Unknown && c.line == ` it "returns a String" do` { + if ok { + t.Fatalf("a line with no mention was classified as %s", s.Shape) + } + return + } + if !ok { + t.Fatal("no mention found in a line that has one") + } + if s.Shape != c.want { + t.Errorf("shape %s, want %s", s.Shape, c.want) + } + if s.From != c.from { + t.Errorf("from %q, want %q", s.From, c.from) + } + if s.To != c.to { + t.Errorf("to %q, want %q", s.To, c.to) + } + if s.Excl != c.excl { + t.Errorf("exclusive %v, want %v", s.Excl, c.excl) + } + if s.OldAnchor != c.inAnc { + t.Errorf("OldAnchor %v, want %v", s.OldAnchor, c.inAnc) + } + }) + } +} + +// ⛔ The example counter is a LOWER BOUND, and this pins the bound rather than +// pretending to an exact figure: nested blocks are followed, and a shared +// example is NOT counted because a static reader cannot expand one. +func TestExamplesIn(t *testing.T) { + src := strings.Split(`ruby_version_is "4.0" do + it "one" do + end + + describe "nested" do + it "two" do + end + end + + it_behaves_like :some_shared, :method +end +it "outside, must not be counted" do +end`, "\n") + + if got := ExamplesIn(src, 0); got != 2 { + t.Errorf("counted %d examples, want 2 (the shared example is invisible to a static reader)", got) + } +} + +// A guard with no examples at all still reports zero rather than swallowing +// the file. +func TestAGuardWithNoExamples(t *testing.T) { + src := strings.Split("ruby_version_is \"4.0\" do\n CONST = 1\nend\n", "\n") + if got := ExamplesIn(src, 0); got != 0 { + t.Errorf("counted %d, want 0", got) + } +} + +// Census ties the two together and must mark an inline inside an `it`. +func TestCensusMarksAnInlineInsideAnExample(t *testing.T) { + src := []byte(`describe "Hash#inspect" do + it "formats" do + expected = ruby_version_is("3.4") ? "{a: 1}" : "{:a=>1}" + expected.should be_kind_of(String) + end +end +`) + sites := Census("core/hash/inspect_spec.rb", src) + if len(sites) != 1 { + t.Fatalf("found %d sites, want 1", len(sites)) + } + if sites[0].Shape != Inline { + t.Errorf("shape %s, want inline", sites[0].Shape) + } + if !sites[0].InIt { + t.Error("the site is inside an `it` and was not marked so") + } +} + +// CompareVersions is spec_helper.rb's `_ver_cmp` and has to agree with it, or +// the census answers about a corpus the shim never ran. The cases that matter +// are the ones where the two spellings have different lengths — that is where a +// lexicographic comparison, the obvious wrong implementation, diverges. +func TestCompareVersionsMatchesTheShim(t *testing.T) { + for _, c := range []struct { + a, b string + want int + }{ + {"4.0", "4.0.0", 0}, // absent components read as zero + {"3.4", "3.4.1", -1}, // and so "3.4" is BELOW "3.4.1" + {"3.4.1", "4.0", -1}, // + {"4.0.9", "4.0", 1}, // + {"10.0", "9.0", 1}, // ⛔ lexicographically "10" < "9" + {"3.4.10", "3.4.9", 1}, // ⛔ and "10" < "9" again + {"4.0.0", "4.0.0", 0}, // + {"4.0.0-rc1", "4.0.0", 0}, // Ruby's to_i takes the leading digits + } { + if got := CompareVersions(c.a, c.b); got != c.want { + t.Errorf("CompareVersions(%q, %q) = %d, want %d", c.a, c.b, got, c.want) + } + if got := CompareVersions(c.b, c.a); got != -c.want { + t.Errorf("CompareVersions(%q, %q) = %d, want %d (not antisymmetric)", c.b, c.a, got, -c.want) + } + } +} + +// OpenAt is the predicate the whole gate rests on: a bare string is a LOWER +// bound, a range is bounded at both ends, `...` excludes the top, and an empty +// endpoint is unbounded. Each row below is a guard shape that occurs in the +// corpus, read at the two versions #799's move runs between. +func TestOpenAtReadsEachGuardShape(t *testing.T) { + for _, c := range []struct { + name string + line string + at341, at409 bool + opens, closes bool + }{ + { + name: `ruby_version_is "4.0" — shut at 3.4.1, open at 4.0.9`, + line: ` ruby_version_is "4.0" do`, + at341: false, at409: true, opens: true, + }, + { + name: `ruby_version_is ""..."4.0" — the inverse, and it CLOSES`, + line: ` ruby_version_is ""..."4.0" do`, + at341: true, at409: false, closes: true, + }, + { + name: `ruby_version_is "3.4" — open on both sides, so the move does nothing to it`, + line: ` ruby_version_is "3.4" do`, + at341: true, at409: true, + }, + { + name: `ruby_version_is "4.1" — shut on both sides`, + line: ` ruby_version_is "4.1" do`, + at341: false, at409: false, + }, + { + name: `ruby_version_is "3.4".."4.0" — INCLUSIVE, so 4.0.9 is outside it`, + line: ` ruby_version_is "3.4".."4.0" do`, + at341: true, at409: false, closes: true, + }, + { + name: `ruby_version_is ""..."4.1" — still open at 4.0.9`, + line: ` ruby_version_is ""..."4.1" do`, + at341: true, at409: true, + }, + } { + t.Run(c.name, func(t *testing.T) { + s, ok := Classify(c.line) + if !ok { + t.Fatal("not classified") + } + if got := s.OpenAt("3.4.1"); got != c.at341 { + t.Errorf("OpenAt(3.4.1) = %v, want %v", got, c.at341) + } + if got := s.OpenAt("4.0.9"); got != c.at409 { + t.Errorf("OpenAt(4.0.9) = %v, want %v", got, c.at409) + } + m := ClassifyMove([]Site{s}, "3.4.1", "4.0.9") + if (len(m.Opened) == 1) != c.opens { + t.Errorf("opened %d, want opens=%v", len(m.Opened), c.opens) + } + if (len(m.Closed) == 1) != c.closes { + t.Errorf("closed %d, want closes=%v", len(m.Closed), c.closes) + } + }) + } +} + +// ⛔ THE CASE THAT COST A PUBLISHED FIGURE. core/binding/implicit_parameters_spec.rb +// is one `describe` entirely inside a `ruby_version_is "4.0"` guard: at 3.4.1 +// the file is `pass=0 fail=0 error=0`, at 4.0.9 every example runs and fails. +// A pass-count delta over that file is ZERO in both directions, which is how +// 37 failures stayed out of a plan that was published as "ten examples". +// +// The census sees it from the SOURCE, before either engine runs, which is the +// only place it is visible at all. +func TestTheFileThatAPassDeltaCannotSee(t *testing.T) { + src := []byte(`require_relative '../../spec_helper' + +describe "Binding#implicit_parameters" do + ruby_version_is "4.0" do + it "returns the implicit parameters" do + -> { _1 }.call(1).should == 1 + end + + it "is empty outside a block" do + binding.implicit_parameters.should == [] + end + end +end +`) + sites := Census("core/binding/implicit_parameters_spec.rb", src) + m := ClassifyMove(sites, "3.4.1", "4.0.9") + if len(m.Opened) != 1 { + t.Fatalf("opened %d guards, want 1", len(m.Opened)) + } + if m.ExamplesOpened() != 2 { + t.Errorf("opened %d examples, want 2", m.ExamplesOpened()) + } + if files := m.Touched(); len(files) != 1 || files[0] != "core/binding/implicit_parameters_spec.rb" { + t.Errorf("Touched() = %v, want the one file", files) + } + // And the control: the same census at a move that does not cross the + // threshold must find nothing, or the gate would fire on every run. + if quiet := ClassifyMove(sites, "4.0.5", "4.0.9"); len(quiet.Opened)+len(quiet.Closed)+len(quiet.Flipped) != 0 { + t.Errorf("a move inside 4.0 touched %d sites, want 0", len(quiet.Touched())) + } +} + +// An inline site FLIPS: the example runs on both sides and asserts something +// different, which is neither an appearance nor a disappearance and has to be +// counted apart from both. +func TestAnInlineSiteFlipsRatherThanOpening(t *testing.T) { + sites := Census("core/module/to_s_spec.rb", []byte( + " msg_class = ruby_version_is(\"4.0\") ? \"Module\" : \"module\"\n")) + m := ClassifyMove(sites, "3.4.1", "4.0.9") + if len(m.Flipped) != 1 { + t.Fatalf("flipped %d, want 1", len(m.Flipped)) + } + if len(m.Opened) != 0 || len(m.Closed) != 0 { + t.Errorf("an inline was counted as opened/closed: %+v", m) + } +}