From 40f048415f4a6efdd36edb83e04c659b2db1727c Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 2 Nov 2026 16:27:32 +0100 Subject: [PATCH 1/4] specdelta: a file can get worse without its pass count moving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tool compared **passing** counts. A file that goes from *no examples* to *many failing examples* has a pass delta of zero and never appeared in the report — not even without `-quiet`. That is not hypothetical: it cost a published figure. Measuring the 4.0 version move, these were invisible: ```text core/binding/implicit_parameter_get_spec.rb 0 → 0 +0 fail+error 0 → 14 core/binding/implicit_parameter_defined_spec.rb 0 → 0 +0 fail+error 0 → 13 core/binding/implicit_parameters_spec.rb 0 → 0 +0 fail+error 0 → 10 core/marshal/dump_spec.rb 191 → 191 +0 fail+error 17 → 20 language/predefined_spec.rb 141 → 141 +0 fail+error 20 → 24 ``` The three `core/binding/` files are `pass=0 fail=0 error=0` at 3.4.1 — every example behind a `ruby_version_is "4.0"` guard — and 37 failures at 4.0.9, where the guard opens and three new Binding methods turn out not to exist. "Ten examples of work" was short by about sixty-five, and #799 is corrected. Six more files were reported as **gains** while also gaining failures. The worst reads `+3` in the old output: ```text core/thread/raise_spec.rb 48 → 51 +3 ⛔ fail+error 10 → 26 ``` Nineteen examples arrive at 4.0; three pass and sixteen fail. A pass-only delta cannot say that, and it was quoted as though it could. ## The change A row prints whenever **either** the passes moved or the failures did, carrying `⛔ fail+error A → B`, and the summary counts the pass-neutral ones separately: `N gained failures without losing passes`. They are named apart because such a file is not a regression in the passing total and would otherwise read as nothing at all. The test asserts that `-quiet` does **not** suppress them, since a delta of zero is exactly how they hid, and keeps the control beside it: a file quiet on both engines must still stay out of a `-quiet` report, or every unchanged file would print. ⛔ The lesson behind it was already written down and not applied: ask what the target version **adds** before measuring what moves. A delta cannot see an API absent on both sides of the comparison when the specs for it are guarded away on one of them. Refs #799 Co-authored-by: Claude Opus 5 --- .../rubyspec/cmd/specdelta/main.go | 27 +++++++++- .../rubyspec/cmd/specdelta/main_test.go | 49 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main.go b/scripts/conformance/rubyspec/cmd/specdelta/main.go index 45222648..106e14f9 100644 --- a/scripts/conformance/rubyspec/cmd/specdelta/main.go +++ b/scripts/conformance/rubyspec/cmd/specdelta/main.go @@ -168,13 +168,27 @@ func main1(args []string, out, errOut io.Writer) int { return rows[i].file < rows[j].file }) - var totalA, totalB, moved, regressed, suspect int + var totalA, totalB, moved, regressed, suspect, appeared int fmt.Fprintf(out, "%-56s %7s %7s %6s\n", "file", "control", "treat", "delta") for _, r := range rows { d := r.b.pass - r.a.pass + // ⛔ A file can get WORSE without its pass count moving. Three + // `core/binding/implicit_parameter*_spec.rb` files report + // `pass=0 fail=0 error=0` under one engine — every example behind a + // version guard — and 37 failures under the other, where the guard opens + // and the methods do not exist. Delta 0, invisible, and the measurement + // that missed them was used to size a plan. + // + // So the row is printed whenever EITHER the passes moved or the + // failures did. + badA, badB := r.a.fail+r.a.errs, r.b.fail+r.b.errs + worse := badB - badA if d != 0 { moved++ } + if d == 0 && worse > 0 { + appeared++ + } // ⛔ 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 { @@ -200,6 +214,9 @@ func main1(args []string, out, errOut io.Writer) int { note += " ⛔ fewer detail lines than its own fail+error tally" suspect++ } + if worse > 0 { + note += fmt.Sprintf(" ⛔ fail+error %d → %d", badA, badB) + } if *quiet && d == 0 && note == "" { continue } @@ -207,7 +224,13 @@ 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\n", moved, regressed) + fmt.Fprintf(out, "%d file(s) moved, %d regressed", moved, regressed) + 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) 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) diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main_test.go b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go index 5aefe3f7..4d42a6a9 100644 --- a/scripts/conformance/rubyspec/cmd/specdelta/main_test.go +++ b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go @@ -333,6 +333,55 @@ fi } } +// ⛔ A file can get WORSE without its pass count moving, and the first version +// of this tool could not see it. +// +// Three `core/binding/implicit_parameter*_spec.rb` files report +// `pass=0 fail=0 error=0` at 3.4.1 — every example behind a `ruby_version_is +// "4.0"` guard — and **37 failures** at 4.0.9, where the guard opens and the +// three new Binding methods do not exist. Delta 0, absent from the report, and +// the measurement that missed them was used to size a plan: "ten examples of +// work" was short by those 37. +func TestAFileThatGainsFailuresWithoutLosingPassesIsNamed(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", "v"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "core", "v", "d_spec.rb"), []byte("#\n"), 0o644); err != nil { + t.Fatal(err) + } + // Guarded out on the control: no examples at all, nothing to pass or fail. + guarded := fakeEngine(t, root, "guarded", "echo 'RBGO_RESULT pass=0 fail=0 error=0 skip=0'\n") + // The guard opens on the treatment and every example fails. + opened := fakeEngine(t, root, "opened", "echo 'RBGO_RESULT pass=0 fail=3 error=2 skip=0'\n"+ + "printf 'RBGO_DETAIL\\tfail\\tX\\ta\\tb\\n%.0s' 1 2 3\n"+ + "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) + if !strings.Contains(out.String(), "fail+error 0 → 5") { + t.Errorf("the new failures are not named:\n%s", out.String()) + } + if !strings.Contains(out.String(), "1 gained failures without losing passes") { + t.Errorf("the summary does not count it:\n%s", out.String()) + } + // ⛔ And -quiet must NOT hide it: a delta of 0 was exactly how it hid before. + if !strings.Contains(out.String(), "d_spec.rb") { + t.Errorf("-quiet suppressed a file that got worse:\n%s", out.String()) + } + + // 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) + if strings.Contains(out.String(), "d_spec.rb") { + t.Errorf("an unchanged file was printed under -quiet:\n%s", out.String()) + } +} + func fakeEngine(t *testing.T, dir, name, body string) string { t.Helper() p := filepath.Join(dir, name) From acc31a7cf31e787267ebbaef4cc3088f50eefd86 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 2 Nov 2026 18:07:11 +0100 Subject: [PATCH 2/4] specdelta: the question that was written down and not asked is now a refusal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The census of what a version move does to the corpus already existed, in cmd/guards, and its own README said what it was: "an analysis tool, not a gate: nothing fails when it is not run." On 2026-11-01 it was not run. The 4.0 move was measured by comparing PASSING counts between two engines and published as "ten examples of work". The real figure is 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 note saying "ask what the target version ADDS before measuring what moves" had been written months earlier. The note was not the missing part. ## internal/guardcensus The reading moves out of cmd/guards into a package both commands use, so the two cannot drift. It gains the part a census of one version never needed: a version PREDICATE, which is spec_helper.rb's `_version_in_range` (:133) and `_ver_cmp` (:119) reimplemented in Go -- a bare string is a lower bound, `...` excludes the top, an empty endpoint is unbounded, absent components read as zero. Reimplemented rather than called because the census must answer for versions the running interpreter is not, which is the whole question. `ClassifyMove(sites, from, to)` then sorts every site into what the move OPENS (examples appear), CLOSES (they vanish) and FLIPS (the same example asserts something else). Against the pinned corpus, 3.4.1 -> 4.0.9 opens 67 block guards in 50 files, closes 45 in 37, and flips 5 inline sites in 3. ## The gate `specdelta -move` is REQUIRED and 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 is a reading of the engines. -move none both engines read the corpus at the same RUBY_VERSION -move 3.4.1:4.0.9 they do not With a move declared, specdelta censuses the corpus and REFUSES -- before starting either engine -- when the move touches a file `-dir` does not measure. On the real corpus: 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 … The three files that cost the published figure are named, and nothing ran. The refusal comes first on purpose: a core/ sweep is an hour, and an hour spent on a figure already known to be incomplete is the cost this exists to avoid. `-partial` is the named derogation and it STAMPS the report rather than silencing the gate -- an opt-in that only removed a message would leave the same number published with nothing attached to it. 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. The summary reports the move's share apart from the engines'. ## Tests The gate's test asserts the refusal AND that no engine was started (the fake engine appends to a file; the file must not exist). The out-of-reach refusal is tested against the real shape -- core/set measured, core/binding opened -- with two controls beside it: widening `-dir` is accepted, and a move declared `none` leaves the same quiet file out of a `-quiet` report. Without those controls the refusal would only prove the gate always fires. cmd/guards keeps its own control: 56 guards at "4.0" under a `do$` anchor still REPRODUCES. Refs #799 Co-Authored-By: Claude Opus 5 --- scripts/conformance/rubyspec/README.md | 75 +++- .../conformance/rubyspec/cmd/guards/main.go | 240 +++--------- .../rubyspec/cmd/guards/main_test.go | 153 -------- .../rubyspec/cmd/specdelta/main.go | 183 ++++++++- .../rubyspec/cmd/specdelta/main_test.go | 179 ++++++++- .../rubyspec/internal/guardcensus/census.go | 367 ++++++++++++++++++ .../internal/guardcensus/census_test.go | 308 +++++++++++++++ 7 files changed, 1143 insertions(+), 362 deletions(-) delete mode 100644 scripts/conformance/rubyspec/cmd/guards/main_test.go create mode 100644 scripts/conformance/rubyspec/internal/guardcensus/census.go create mode 100644 scripts/conformance/rubyspec/internal/guardcensus/census_test.go diff --git a/scripts/conformance/rubyspec/README.md b/scripts/conformance/rubyspec/README.md index 952f2827..d714cdc7 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,39 @@ 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'. + **`-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 @@ -250,8 +313,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..fcc221b8 100644 --- a/scripts/conformance/rubyspec/cmd/specdelta/main.go +++ b/scripts/conformance/rubyspec/cmd/specdelta/main.go @@ -22,13 +22,30 @@ // 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" -// and "measured worse" need opposite responses +// 2 refused: no files, an engine that would not start, output that could not +// be read, no -move declared, or a declared move whose surface this run +// does not cover — never reported as a regression, because "could not +// measure" and "measured worse" need opposite responses package main import ( @@ -44,6 +61,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. @@ -98,6 +117,10 @@ 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") if err := fs.Parse(args); err != nil { return 2 } @@ -126,6 +149,13 @@ 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 + } 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 +198,13 @@ func main1(args []string, out, errOut io.Writer) int { return rows[i].file < rows[j].file }) + // 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.Fprintln(out, mv.header()) + var totalA, totalB, moved, regressed, suspect, appeared 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 +253,19 @@ func main1(args []string, out, errOut io.Writer) int { if worse > 0 { note += fmt.Sprintf(" ⛔ fail+error %d → %d", badA, badB) } + // 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 } @@ -231,6 +280,14 @@ func main1(args []string, out, errOut io.Writer) int { 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) @@ -242,6 +299,126 @@ func main1(args []string, out, errOut io.Writer) int { return 0 } +// 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: declaredMove has already said why, in the words +// the reader needs rather than a wrapped error string. +var errRefused = errors.New("refused") + func abs(n int) int { if n < 0 { return -n diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main_test.go b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go index 4d42a6a9..5ff8784b 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,163 @@ 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()) + } +} + 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) + } +} From a658e8efbbfd5f3165bc3eaf82a74ddb98b4a301 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 2 Nov 2026 18:11:18 +0100 Subject: [PATCH 3/4] specdelta: a binary can be asked which commit it is, so it is asked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second rule this tool now enforces, and it is a repeat offence. On 2026-11-01 `pass=118 fail=1` was published for #858 from a binary built BEFORE that branch existed; it carried the previous branch's code, and the true figure was 117/2. The correction had to go on the record. The same shape was already 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. Written down twice, and it happened a third time. `go build` stamps `vcs.revision` and `vcs.modified` into every binary it produces from a repository. Verified on a real one rather than from the documentation: $ go version -m ./sd build vcs.revision=acc31a7cf31e787267ebbaef4cc3088f50eefd86 build vcs.modified=true So the binary can be asked what it is, through debug/buildinfo, before either engine runs. Three refusals, each a way the comparison is void before it starts: the same revision twice one of them was not rebuilt vcs.modified=true the revision does not name the code inside the tree moved past it the binary is not the code being measured (#858) Run against this worktree while it was dirty, the gate answers: specdelta: this comparison cannot be attributed to any pair of commits: the control was built from a tree with uncommitted changes, so acc31a7cf31e does not name the code it contains An engine with no Go build information -- MRI, or the shell fakes the tests use -- is NOT refused: it is legitimately not a Go binary. It 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, for the same reason. Both revisions print above the table, so a pasted figure carries what produced it. The one that was wrong carried nothing at all. The policy is tested as a table, through a replaceable `readBuildFacts`, rather than by building Go binaries inside a test to observe a decision that does not depend on building them. The #858 row is in it by name. Refs #799 Co-Authored-By: Claude Opus 5 --- scripts/conformance/rubyspec/README.md | 25 +++ .../rubyspec/cmd/specdelta/main.go | 161 +++++++++++++++++- .../rubyspec/cmd/specdelta/main_test.go | 124 ++++++++++++++ 3 files changed, 305 insertions(+), 5 deletions(-) diff --git a/scripts/conformance/rubyspec/README.md b/scripts/conformance/rubyspec/README.md index d714cdc7..ede76f6e 100644 --- a/scripts/conformance/rubyspec/README.md +++ b/scripts/conformance/rubyspec/README.md @@ -298,6 +298,31 @@ 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 diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main.go b/scripts/conformance/rubyspec/cmd/specdelta/main.go index fcc221b8..0247e8bc 100644 --- a/scripts/conformance/rubyspec/cmd/specdelta/main.go +++ b/scripts/conformance/rubyspec/cmd/specdelta/main.go @@ -43,13 +43,15 @@ // 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, output that could not -// be read, no -move declared, or a declared move whose surface this run -// does not cover — never reported as a regression, because "could not -// measure" and "measured worse" need opposite responses +// 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" @@ -121,6 +123,7 @@ func main1(args []string, out, errOut io.Writer) int { // 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 } @@ -156,6 +159,10 @@ func main1(args []string, out, errOut io.Writer) int { 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 @@ -201,6 +208,11 @@ func main1(args []string, out, errOut io.Writer) 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 int @@ -415,10 +427,149 @@ side, 37 failures on the other, and a pass delta of zero either way. return d, nil } -// errRefused carries nothing: declaredMove has already said why, in the words -// the reader needs rather than a wrapped error string. +// 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 diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main_test.go b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go index 5ff8784b..8677b64f 100644 --- a/scripts/conformance/rubyspec/cmd/specdelta/main_test.go +++ b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go @@ -533,6 +533,130 @@ func TestAFileTheMoveOpensSurvivesQuiet(t *testing.T) { } } +// ⛔ 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()) + } +} + func fakeEngine(t *testing.T, dir, name, body string) string { t.Helper() p := filepath.Join(dir, name) From fb161f7eb1534c1f1cbf2ef6f368ba05543acbda Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 2 Nov 2026 21:59:04 +0100 Subject: [PATCH 4/4] specdelta: a net gain hides an example that went red MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured today, control and treatment built from the same commit minus one change: core/thread/raise_spec.rb 51 → 68 +17 53 file(s): control 328 passing, treatment 345 passing, +17 1 file(s) moved, 0 regressed Exit 0. True, and incomplete: NINETEEN examples turned green and TWO turned red. No count of passes, of failures, or of both can show a swap -- the two movements partly cancel and the remainder reads as a clean win. They were found by hand, by comparing the two runs' failure DESCRIPTIONS rather than their number. The example that broke had been passing BECAUSE of a different defect (Thread#raise ignored cause: entirely, so there was no trailing Hash to mispop), which is why nothing about it looked wrong. ## The change The detail lines' DESCRIPTIONS are collected as a multiset -- a shared example block included twice in one file reports the same description twice, and losing both is two examples, not one -- and every description the treatment reports MORE OFTEN than the control is named, with its count: 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 ⛔ And it EXITS 1. An example that went red is a regression whatever the file's total did; letting a per-file "0 regressed" carry it out on exit 0 is the exact reading this exists to stop. It keys on the description and NOT the message, because a message carries object addresses (`#`) that differ between two runs of the SAME binary -- keying on those would make every file look changed. That is one of the three cases the test pins. ## Verified against the run that produced the defect Rebuilt and pointed at the same two binaries, the tool names the same two examples I had found by hand, and the exit status goes 0 → 1. ## Tests The swap, plus two controls: a file that ONLY improves must stay silent (or every improved file would be flagged), and a differing MESSAGE under the same description must not read as an example that broke. Refs #799 Co-Authored-By: Claude Opus 5 --- scripts/conformance/rubyspec/README.md | 30 +++++++ .../rubyspec/cmd/specdelta/main.go | 75 ++++++++++++++++- .../rubyspec/cmd/specdelta/main_test.go | 81 +++++++++++++++++++ 3 files changed, 183 insertions(+), 3 deletions(-) diff --git a/scripts/conformance/rubyspec/README.md b/scripts/conformance/rubyspec/README.md index ede76f6e..e271e206 100644 --- a/scripts/conformance/rubyspec/README.md +++ b/scripts/conformance/rubyspec/README.md @@ -330,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 diff --git a/scripts/conformance/rubyspec/cmd/specdelta/main.go b/scripts/conformance/rubyspec/cmd/specdelta/main.go index 0247e8bc..8a67e894 100644 --- a/scripts/conformance/rubyspec/cmd/specdelta/main.go +++ b/scripts/conformance/rubyspec/cmd/specdelta/main.go @@ -41,7 +41,8 @@ // Exit status: // // 0 every file ran and no file passed fewer examples on the second engine -// 1 at least one file regressed +// 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 @@ -73,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 @@ -215,7 +223,7 @@ func main1(args []string, out, errOut io.Writer) int { } fmt.Fprintln(out, mv.header()) - var totalA, totalB, moved, regressed, suspect, appeared int + 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 { @@ -265,6 +273,27 @@ 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 @@ -286,6 +315,11 @@ 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. @@ -305,12 +339,40 @@ func main1(args []string, out, errOut io.Writer) int { 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 { @@ -678,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 8677b64f..3c9c84c0 100644 --- a/scripts/conformance/rubyspec/cmd/specdelta/main_test.go +++ b/scripts/conformance/rubyspec/cmd/specdelta/main_test.go @@ -657,6 +657,87 @@ func TestTheTableNamesWhatProducedIt(t *testing.T) { } } +// ⛔ 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)