From 40f048415f4a6efdd36edb83e04c659b2db1727c Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 2 Nov 2026 16:27:32 +0100 Subject: [PATCH] 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)