From 06cb0cdd92bcfed667bbc480869c17b6773df1a1 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 6 Nov 2026 23:08:49 +0100 Subject: [PATCH] Merge the line, failure-words and package-cause tests into tables: the repository's unit count goes from 805 to 791 (issue 1921) Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- .../internal/check/failure_words_test.go | 155 +++++++------- .../internal/check/line_test.go | 191 +++++++++--------- .../internal/check/package_cause_test.go | 134 ++++++------ 3 files changed, 248 insertions(+), 232 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/failure_words_test.go b/modules/mesh-delivery-checker/internal/check/failure_words_test.go index 0d9edc4b..b5b59de3 100644 --- a/modules/mesh-delivery-checker/internal/check/failure_words_test.go +++ b/modules/mesh-delivery-checker/internal/check/failure_words_test.go @@ -21,81 +21,88 @@ func fatalAfterLog() Failure { } } -func TestAFailedTestsVerdictKeepsItsWholeFailureMessage(t *testing.T) { // 1901/AC1 - v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{fatalAfterLog()}}) - for _, want := range []string{"3 subscription(s) derive to a subject no emitter owns:", "a.b.c", "d.e.f", "g.h.i"} { - if !strings.Contains(v.Failed.Words, want) { - t.Errorf("words lack %q: %q", want, v.Failed.Words) - } - } - if len(v.Failed.Words) > rules.MaxWords+200 { - t.Errorf("words are %d bytes, bound %d", len(v.Failed.Words), rules.MaxWords) - } -} - -func TestTheStatusLineNamesTheFailureNotTheFirstLoggedLine(t *testing.T) { // 1901/AC2 - v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{fatalAfterLog()}}) - got := firstFailed(*v) - if !strings.Contains(got, "3 subscription(s) derive to a subject no emitter owns:") || strings.Contains(got, "judged against") || strings.Contains(got, "\n") { - t.Errorf("firstFailed = %q", got) - } - // With no cause told apart (an older build module), the words still start at the first line, as before. - f := fatalAfterLog() - f.Cause = "" - v = rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{f}}) - if !strings.HasPrefix(afterHid(v.Failed.Words), "agreement_catalogue_test.go:130") { - t.Errorf("words = %q", v.Failed.Words) - } -} - -func TestWordsLongerThanTheBoundKeepTheirLastLines(t *testing.T) { // 1901/AC1 - f := Failure{Output: strings.Repeat("log line of some length\n", 500) + "the cause is here"} - v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{f}}) - if !strings.HasSuffix(v.Failed.Words, "the cause is here") || len(v.Failed.Words) > rules.MaxWords+200 { - t.Errorf("%d bytes, ends %q", len(v.Failed.Words), v.Failed.Words[len(v.Failed.Words)-30:]) - } - _ = delivery.VerdictFail -} - -// An older build module sends no "cause": its answer is still accepted and the verdict's words are its output, as before -// (1901/AC3). A failure that hides its cause (none, or a passing line) is said loudly first in the words and in the status -// line, naming the build module and the version it says it is, and the level still fails; a failure that keeps the -// contract is worded as before (1901/AC8). -func TestAnOlderBuildModulesFailureIsAcceptedWithItsWholeOutput(t *testing.T) { // 1901/AC3, AC8 - old := `{"package":"p","test":"TestX","output":"x_test.go:1: logged\nx_test.go:2: the old kept lines"}` - var f Failure - if err := json.Unmarshal([]byte(old), &f); err != nil || f.Cause != "" { - t.Fatalf("1901/AC3: err = %v, cause = %q", err, f.Cause) - } - v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{f}}) - if afterHid(v.Failed.Words) != "x_test.go:1: logged\nx_test.go:2: the old kept lines" { - t.Errorf("1901/AC3: words = %q", v.Failed.Words) - } - if got := firstFailed(*v); !strings.Contains(got, "the build module hid the failure") || strings.Contains(got, "\n") { - t.Errorf("1901/AC3, AC8: status = %q", got) - } - - by := "mesh-build-node with mesh-sdk v0.1.14-0.20261026184535-e40dd0f56a78" - for _, c := range []struct { - name, module, by, cause, output, loud string +// A failed test's words are its whole failure message, kept within their bound by their last lines (issue 1901). +func TestAFailuresWordsAreItsWholeMessageWithinTheirBound(t *testing.T) { + for _, row := range []struct { + name string + test func(t *testing.T) }{ - {"no cause", "mesh-build-node", by, "", "test/b.test.ts: something", "the build module hid the failure (mesh-build-node at mesh-sdk v0.1.14-0.20261026184535-e40dd0f56a78): it sent no cause"}, - {"a passing line as its cause", "mesh-build-node", by, "\u2714 the same merge has the same id (8.006913ms)", "", "the build module hid the failure (mesh-build-node at mesh-sdk v0.1.14-0.20261026184535-e40dd0f56a78): its cause is a passing line"}, - {"from before the contract", "mesh-build-node", "", "", "\u2714 the same merge has the same id (8.006913ms)", "the build module hid the failure (mesh-build-node at a version before the failure contract): it sent no cause"}, - {"a real cause", "mesh-build-node", by, "one plus one is not three", "not ok 3 - fails", ""}, + {"a failed tests verdict keeps its whole failure message", func(t *testing.T) { // 1901/AC1 + v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{fatalAfterLog()}}) + for _, want := range []string{"3 subscription(s) derive to a subject no emitter owns:", "a.b.c", "d.e.f", "g.h.i"} { + if !strings.Contains(v.Failed.Words, want) { + t.Errorf("words lack %q: %q", want, v.Failed.Words) + } + } + if len(v.Failed.Words) > rules.MaxWords+200 { + t.Errorf("words are %d bytes, bound %d", len(v.Failed.Words), rules.MaxWords) + } + }}, + {"the status line names the failure not the first logged line", func(t *testing.T) { // 1901/AC2 + v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{fatalAfterLog()}}) + got := firstFailed(*v) + if !strings.Contains(got, "3 subscription(s) derive to a subject no emitter owns:") || strings.Contains(got, "judged against") || strings.Contains(got, "\n") { + t.Errorf("firstFailed = %q", got) + } + // With no cause told apart (an older build module), the words still start at the first line, as before. + f := fatalAfterLog() + f.Cause = "" + v = rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{f}}) + if !strings.HasPrefix(afterHid(v.Failed.Words), "agreement_catalogue_test.go:130") { + t.Errorf("words = %q", v.Failed.Words) + } + }}, + {"words longer than the bound keep their last lines", func(t *testing.T) { // 1901/AC1 + f := Failure{Output: strings.Repeat("log line of some length\n", 500) + "the cause is here"} + v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{f}}) + if !strings.HasSuffix(v.Failed.Words, "the cause is here") || len(v.Failed.Words) > rules.MaxWords+200 { + t.Errorf("%d bytes, ends %q", len(v.Failed.Words), v.Failed.Words[len(v.Failed.Words)-30:]) + } + _ = delivery.VerdictFail + }}, + // An older build module sends no "cause": its answer is still accepted and the verdict's words are its output, as before + // (1901/AC3). A failure that hides its cause (none, or a passing line) is said loudly first in the words and in the status + // line, naming the build module and the version it says it is, and the level still fails; a failure that keeps the + // contract is worded as before (1901/AC8). + {"an older build modules failure is accepted with its whole output", func(t *testing.T) { // 1901/AC3, AC8 + old := `{"package":"p","test":"TestX","output":"x_test.go:1: logged\nx_test.go:2: the old kept lines"}` + var f Failure + if err := json.Unmarshal([]byte(old), &f); err != nil || f.Cause != "" { + t.Fatalf("1901/AC3: err = %v, cause = %q", err, f.Cause) + } + v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{f}}) + if afterHid(v.Failed.Words) != "x_test.go:1: logged\nx_test.go:2: the old kept lines" { + t.Errorf("1901/AC3: words = %q", v.Failed.Words) + } + if got := firstFailed(*v); !strings.Contains(got, "the build module hid the failure") || strings.Contains(got, "\n") { + t.Errorf("1901/AC3, AC8: status = %q", got) + } + + by := "mesh-build-node with mesh-sdk v0.1.14-0.20261026184535-e40dd0f56a78" + for _, c := range []struct { + name, module, by, cause, output, loud string + }{ + {"no cause", "mesh-build-node", by, "", "test/b.test.ts: something", "the build module hid the failure (mesh-build-node at mesh-sdk v0.1.14-0.20261026184535-e40dd0f56a78): it sent no cause"}, + {"a passing line as its cause", "mesh-build-node", by, "\u2714 the same merge has the same id (8.006913ms)", "", "the build module hid the failure (mesh-build-node at mesh-sdk v0.1.14-0.20261026184535-e40dd0f56a78): its cause is a passing line"}, + {"from before the contract", "mesh-build-node", "", "", "\u2714 the same merge has the same id (8.006913ms)", "the build module hid the failure (mesh-build-node at a version before the failure contract): it sent no cause"}, + {"a real cause", "mesh-build-node", by, "one plus one is not three", "not ok 3 - fails", ""}, + } { + t.Run(c.name, func(t *testing.T) { + v := rules.LevelFailure("", c.module, "unit", Tested{By: c.by, Failed: []Failure{{Package: "modules/gitea", Test: "fails", Cause: c.cause, Output: c.output}}}) + if v.State != delivery.VerdictFail { + t.Errorf("1901/AC8: the verdict is %q, want it failed", v.State) + } + line := firstFailed(*v) + switch { + case c.loud == "" && strings.Contains(v.Failed.Words, "hid the failure"): + t.Errorf("1901/AC8: a failure that keeps the contract is said hidden: %q", v.Failed.Words) + case c.loud != "" && (!strings.HasPrefix(v.Failed.Words, c.loud) || !strings.Contains(line, c.loud) || !strings.Contains(v.Failures[0].Words, "hid the failure")): + t.Errorf("1901/AC8: words %q, status %q, want both to start with %q", v.Failed.Words, line, c.loud) + } + }) + } + }}, } { - t.Run(c.name, func(t *testing.T) { - v := rules.LevelFailure("", c.module, "unit", Tested{By: c.by, Failed: []Failure{{Package: "modules/gitea", Test: "fails", Cause: c.cause, Output: c.output}}}) - if v.State != delivery.VerdictFail { - t.Errorf("1901/AC8: the verdict is %q, want it failed", v.State) - } - line := firstFailed(*v) - switch { - case c.loud == "" && strings.Contains(v.Failed.Words, "hid the failure"): - t.Errorf("1901/AC8: a failure that keeps the contract is said hidden: %q", v.Failed.Words) - case c.loud != "" && (!strings.HasPrefix(v.Failed.Words, c.loud) || !strings.Contains(line, c.loud) || !strings.Contains(v.Failures[0].Words, "hid the failure")): - t.Errorf("1901/AC8: words %q, status %q, want both to start with %q", v.Failed.Words, line, c.loud) - } - }) + t.Run(row.name, row.test) } } diff --git a/modules/mesh-delivery-checker/internal/check/line_test.go b/modules/mesh-delivery-checker/internal/check/line_test.go index 66cbbdb4..9daa92c6 100644 --- a/modules/mesh-delivery-checker/internal/check/line_test.go +++ b/modules/mesh-delivery-checker/internal/check/line_test.go @@ -66,100 +66,103 @@ func TestAFailureNamesTheTestTheConflictOrTheRefusedStepFirst(t *testing.T) { // } } -func TestALevelOverItsBoundIsNamedWithItsSlowestTestsAndTheVerdictIsNotChanged(t *testing.T) { // 1300/AC3 - cs, p := lineCase(t, pullOf(repoA, 7, head1)) - v := passVerdict() - v.OverBound = []string{"unit"} - line := Line(v, cs, p, nil, []string{"unit pkg.TestSlow 7.0s"}, nil) - if !strings.HasPrefix(line, "pass ") || !strings.Contains(Report(v, cs, p, nil, []string{"unit pkg.TestSlow 7.0s"}, nil), "over bound: unit (slowest unit pkg.TestSlow 7.0s)") { - t.Errorf("line = %q", line) - } -} - -func TestSkippedTestsAreSaidAndNeverAFlatPass(t *testing.T) { // 1300/AC6 - cs, p := lineCase(t, pullOf(repoA, 7, head1)) - v := passVerdict() - v.Levels[delivery.LevelUnit] = delivery.LevelResult{Run: 12, Kept: 301, Skipped: 2} - if line := Line(v, cs, p, nil, nil, nil); !strings.Contains(line, "unit 12 run, 301 kept, 2 skipped") { - t.Errorf("line = %q", line) - } -} - -func TestAGroupsMembersEachCarryTheSameVerdict(t *testing.T) { // 1300/AC4 - cs := caseOf(t, pullOf(repoA, 7, head1), pullOf(repoSDK, 12, head2)) - a := Line(passVerdict(), cs, cs.Pulls[0], nil, nil, nil) - b := Line(passVerdict(), cs, cs.Pulls[1], nil, nil, nil) - if !strings.HasPrefix(a, "pass ") || !strings.HasPrefix(b, "pass ") || !strings.Contains(Report(passVerdict(), cs, cs.Pulls[0], nil, nil, nil), "group feat/x of 2 pull requests") { - t.Errorf("lines %q and %q", a, b) - } - if !strings.Contains(a, "111111111111") || !strings.Contains(b, "222222222222") { - t.Errorf("each member's line names its own head: %q and %q", a, b) - } -} - -func TestTheBuildsOfTheTouchedModulesAreSaidAndDroppedFirstWhenTheLineIsTooLong(t *testing.T) { // ADR 0484 clause 7 - cs, p := lineCase(t, pullOf(repoA, 7, head1)) - builds := []BuildNote{{"avahi", "built"}, {"mesh-tools", "waiting"}} - if r := Report(passVerdict(), cs, p, builds, nil, nil); !strings.Contains(r, "builds: avahi built, mesh-tools waiting") { - t.Errorf("report = %q", r) - } - line := Line(passVerdict(), cs, p, builds, nil, nil) - if utf8.RuneCountInString(line) > MaxLine { - t.Errorf("line %q is too long", line) - } - short := passVerdict() - short.Took = delivery.Took{} - if l := Line(short, cs, p, builds[:1], nil, nil); !strings.Contains(l, "builds: avahi built") { - t.Errorf("a short line keeps its builds: %q", l) - } -} - -func TestALineIsNeverLongerThanTheLimit(t *testing.T) { // 1300/AC1 - cs, p := lineCase(t, pullOf(repoA, 7, head1)) - v := delivery.Verdict{State: delivery.VerdictFail, Failed: delivery.FailedFirst{Test: "integration: " + strings.Repeat("pkg/", 50) + ".TestSomething", Words: strings.Repeat("because ", 40)}} - if line := Line(v, cs, p, nil, nil, nil); utf8.RuneCountInString(line) > MaxLine || !strings.HasPrefix(line, "failed: integration: ") { - t.Errorf("line = %q (%d)", line, utf8.RuneCountInString(line)) - } -} - -func TestARunningCheckIsPending(t *testing.T) { - cs, p := lineCase(t, pullOf(repoA, 7, head1)) - if line := Line(delivery.Verdict{State: delivery.VerdictRunning}, cs, p, nil, nil, nil); line != "checking 111111111111 on aaaaaaaaaaaa" { - t.Errorf("line = %q", line) - } -} - -func TestThePassedCompositionIsSaidOnlyWhenItWasRun(t *testing.T) { // ADR 0484 clause 6 - cs, p := lineCase(t, pullOf(repoA, 7, head1)) - v := passVerdict() - v.Composition = &delivery.Composition{State: delivery.CompositionPass, SDK: "afbe371003a570f088be4a394d83aa7a9458f7ab"} - if r := Report(v, cs, p, nil, nil, nil); !strings.Contains(r, "composition passed with afbe371003a5") { - t.Errorf("report = %q", r) - } - v.Composition = nil - if r := Report(v, cs, p, nil, nil, nil); !strings.Contains(r, "composition not yet run") || strings.Contains(r, "composition passed") { - t.Errorf("a verdict with no composition: %q", r) - } -} - -func TestTheCompositionsTimeAndMachinesAreSaidOnceTheAssignerReportsThem(t *testing.T) { // 1300 (C10), the assigner's verdict (issue 1297) - cs, p := lineCase(t, pullOf(repoA, 7, head1)) - v := passVerdict() - v.Took.Composition = 2400 - v.Composition = &delivery.Composition{State: delivery.CompositionPass, SDK: "afbe371003a570f088be4a394d83aa7a9458f7ab", Machines: []string{"g14", "ace", "novox", "shanks"}} - r := Report(v, cs, p, nil, nil, nil) - for _, want := range []string{"composition passed in 2.4s with afbe371003a5 on 4 machines: ace, g14, novox, shanks"} { - if !strings.Contains(r, want) { - t.Errorf("report %q lacks %q", r, want) - } - } - if l := Line(v, cs, p, nil, nil, nil); !strings.Contains(l, "composition passed in 2.4s") || !strings.Contains(l, "on 4 machines") || utf8.RuneCountInString(l) > MaxLine { - t.Errorf("line = %q", l) - } - v.State, v.Failed = delivery.VerdictFail, delivery.FailedFirst{Machine: "novox", Words: "composition refused"} - v.Composition = &delivery.Composition{State: delivery.CompositionFail, Words: "novox: two seats claim dns", Machines: []string{"novox"}} - if r := Report(v, cs, p, nil, nil, nil); !strings.Contains(r, "composition failed in 2.4s on novox: novox: two seats claim dns") { - t.Errorf("failed composition report = %q", r) +// The status line and its full report say each part of a verdict, within the forge's limit (issue 1300, ADR 0484). +func TestTheLineAndTheReportSayEachPartOfTheVerdict(t *testing.T) { + for _, row := range []struct { + name string + test func(t *testing.T) + }{ + {"a level over its bound is named with its slowest tests and the verdict is not changed", func(t *testing.T) { // 1300/AC3 + cs, p := lineCase(t, pullOf(repoA, 7, head1)) + v := passVerdict() + v.OverBound = []string{"unit"} + line := Line(v, cs, p, nil, []string{"unit pkg.TestSlow 7.0s"}, nil) + if !strings.HasPrefix(line, "pass ") || !strings.Contains(Report(v, cs, p, nil, []string{"unit pkg.TestSlow 7.0s"}, nil), "over bound: unit (slowest unit pkg.TestSlow 7.0s)") { + t.Errorf("line = %q", line) + } + }}, + {"skipped tests are said and never a flat pass", func(t *testing.T) { // 1300/AC6 + cs, p := lineCase(t, pullOf(repoA, 7, head1)) + v := passVerdict() + v.Levels[delivery.LevelUnit] = delivery.LevelResult{Run: 12, Kept: 301, Skipped: 2} + if line := Line(v, cs, p, nil, nil, nil); !strings.Contains(line, "unit 12 run, 301 kept, 2 skipped") { + t.Errorf("line = %q", line) + } + }}, + {"a groups members each carry the same verdict", func(t *testing.T) { // 1300/AC4 + cs := caseOf(t, pullOf(repoA, 7, head1), pullOf(repoSDK, 12, head2)) + a := Line(passVerdict(), cs, cs.Pulls[0], nil, nil, nil) + b := Line(passVerdict(), cs, cs.Pulls[1], nil, nil, nil) + if !strings.HasPrefix(a, "pass ") || !strings.HasPrefix(b, "pass ") || !strings.Contains(Report(passVerdict(), cs, cs.Pulls[0], nil, nil, nil), "group feat/x of 2 pull requests") { + t.Errorf("lines %q and %q", a, b) + } + if !strings.Contains(a, "111111111111") || !strings.Contains(b, "222222222222") { + t.Errorf("each member's line names its own head: %q and %q", a, b) + } + }}, + {"the builds of the touched modules are said and dropped first when the line is too long", func(t *testing.T) { // ADR 0484 clause 7 + cs, p := lineCase(t, pullOf(repoA, 7, head1)) + builds := []BuildNote{{"avahi", "built"}, {"mesh-tools", "waiting"}} + if r := Report(passVerdict(), cs, p, builds, nil, nil); !strings.Contains(r, "builds: avahi built, mesh-tools waiting") { + t.Errorf("report = %q", r) + } + line := Line(passVerdict(), cs, p, builds, nil, nil) + if utf8.RuneCountInString(line) > MaxLine { + t.Errorf("line %q is too long", line) + } + short := passVerdict() + short.Took = delivery.Took{} + if l := Line(short, cs, p, builds[:1], nil, nil); !strings.Contains(l, "builds: avahi built") { + t.Errorf("a short line keeps its builds: %q", l) + } + }}, + {"a line is never longer than the limit", func(t *testing.T) { // 1300/AC1 + cs, p := lineCase(t, pullOf(repoA, 7, head1)) + v := delivery.Verdict{State: delivery.VerdictFail, Failed: delivery.FailedFirst{Test: "integration: " + strings.Repeat("pkg/", 50) + ".TestSomething", Words: strings.Repeat("because ", 40)}} + if line := Line(v, cs, p, nil, nil, nil); utf8.RuneCountInString(line) > MaxLine || !strings.HasPrefix(line, "failed: integration: ") { + t.Errorf("line = %q (%d)", line, utf8.RuneCountInString(line)) + } + }}, + {"a running check is pending", func(t *testing.T) { + cs, p := lineCase(t, pullOf(repoA, 7, head1)) + if line := Line(delivery.Verdict{State: delivery.VerdictRunning}, cs, p, nil, nil, nil); line != "checking 111111111111 on aaaaaaaaaaaa" { + t.Errorf("line = %q", line) + } + }}, + {"the passed composition is said only when it was run", func(t *testing.T) { // ADR 0484 clause 6 + cs, p := lineCase(t, pullOf(repoA, 7, head1)) + v := passVerdict() + v.Composition = &delivery.Composition{State: delivery.CompositionPass, SDK: "afbe371003a570f088be4a394d83aa7a9458f7ab"} + if r := Report(v, cs, p, nil, nil, nil); !strings.Contains(r, "composition passed with afbe371003a5") { + t.Errorf("report = %q", r) + } + v.Composition = nil + if r := Report(v, cs, p, nil, nil, nil); !strings.Contains(r, "composition not yet run") || strings.Contains(r, "composition passed") { + t.Errorf("a verdict with no composition: %q", r) + } + }}, + {"the compositions time and machines are said once the assigner reports them", func(t *testing.T) { // 1300 (C10), the assigner's verdict (issue 1297) + cs, p := lineCase(t, pullOf(repoA, 7, head1)) + v := passVerdict() + v.Took.Composition = 2400 + v.Composition = &delivery.Composition{State: delivery.CompositionPass, SDK: "afbe371003a570f088be4a394d83aa7a9458f7ab", Machines: []string{"g14", "ace", "novox", "shanks"}} + r := Report(v, cs, p, nil, nil, nil) + for _, want := range []string{"composition passed in 2.4s with afbe371003a5 on 4 machines: ace, g14, novox, shanks"} { + if !strings.Contains(r, want) { + t.Errorf("report %q lacks %q", r, want) + } + } + if l := Line(v, cs, p, nil, nil, nil); !strings.Contains(l, "composition passed in 2.4s") || !strings.Contains(l, "on 4 machines") || utf8.RuneCountInString(l) > MaxLine { + t.Errorf("line = %q", l) + } + v.State, v.Failed = delivery.VerdictFail, delivery.FailedFirst{Machine: "novox", Words: "composition refused"} + v.Composition = &delivery.Composition{State: delivery.CompositionFail, Words: "novox: two seats claim dns", Machines: []string{"novox"}} + if r := Report(v, cs, p, nil, nil, nil); !strings.Contains(r, "composition failed in 2.4s on novox: novox: two seats claim dns") { + t.Errorf("failed composition report = %q", r) + } + }}, + } { + t.Run(row.name, row.test) } } diff --git a/modules/mesh-delivery-checker/internal/check/package_cause_test.go b/modules/mesh-delivery-checker/internal/check/package_cause_test.go index 2e1e3ad1..5e1ada4e 100644 --- a/modules/mesh-delivery-checker/internal/check/package_cause_test.go +++ b/modules/mesh-delivery-checker/internal/check/package_cause_test.go @@ -20,75 +20,81 @@ const ( oldPassThenRace = "PASS\n==================\nWARNING: DATA RACE\nWrite at 0x000000836428 by goroutine 9:\n example.com/racemain.TestMain.func1()\n /tmp/fix-1901b/racemain/p_test.go:15 +0x24\nPrevious write at 0x000000836428 by main goroutine:\n example.com/racemain.TestMain()\n /tmp/fix-1901b/racemain/p_test.go:16 +0x44\n main.main()\n _testmain.go:48 +0x171\nGoroutine 9 (running) created at:\n example.com/racemain.TestMain()\n /tmp/fix-1901b/racemain/p_test.go:15 +0x37\n main.main()\n _testmain.go:48 +0x171\n==================\nFound 1 data race(s)\nFAIL\texample.com/racemain\t1.031s" ) -// A failure with no cause told apart never has "PASS" or "ok" as its words: the checker takes the panic line from the -// output, or says that the tests passed and the program failed after them (1901/AC5). -func TestAPackageFailureIsNeverWordedPass(t *testing.T) { // 1901/AC5 - cases := []struct { - name, output, want string +// A package's failure is worded by its cause, cut to its bound, and never as pass (issue 1901, AC5). +func TestAPackageFailuresCauseIsWordedNeverPass(t *testing.T) { + for _, row := range []struct { + name string + test func(t *testing.T) }{ - {"panic after the tests", oldPassThenPanic, "panic: the bus closed after the tests: nats: connection closed"}, - {"exit after the tests", oldPassThenExit, "the tests passed, then the test program exited with a failure"}, - {"race after the tests", oldPassThenRace, "WARNING: DATA RACE: Write at 0x000000836428 by goroutine 9 in example.com/racemain.TestMain.func1()"}, - {"a panic deep in a long output", "PASS\npanic: the seat row vanished\n" + strings.Repeat("goroutine 7 [chan receive]:\n\tsome/frame.go:1 +0x1\n", 300) + "FAIL\tp\t2.0s", "panic: the seat row vanished"}, - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - v := rules.LevelFailure("", "", "integration", Tested{Failed: []Failure{{Package: "github.com/novox/mesh-controller/cmd/mesh-controller", Output: c.output}}}) - for _, l := range strings.Split(v.Failed.Words, "\n") { - if l = strings.TrimSpace(l); l == "PASS" || strings.HasPrefix(l, "ok ") || strings.HasPrefix(l, "ok\t") { - t.Errorf("1901/AC5: the words keep %q: %q", l, v.Failed.Words) + // A failure with no cause told apart never has "PASS" or "ok" as its words: the checker takes the panic line from the + // output, or says that the tests passed and the program failed after them (1901/AC5). + {"a package failure is never worded pass", func(t *testing.T) { // 1901/AC5 + cases := []struct { + name, output, want string + }{ + {"panic after the tests", oldPassThenPanic, "panic: the bus closed after the tests: nats: connection closed"}, + {"exit after the tests", oldPassThenExit, "the tests passed, then the test program exited with a failure"}, + {"race after the tests", oldPassThenRace, "WARNING: DATA RACE: Write at 0x000000836428 by goroutine 9 in example.com/racemain.TestMain.func1()"}, + {"a panic deep in a long output", "PASS\npanic: the seat row vanished\n" + strings.Repeat("goroutine 7 [chan receive]:\n\tsome/frame.go:1 +0x1\n", 300) + "FAIL\tp\t2.0s", "panic: the seat row vanished"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + v := rules.LevelFailure("", "", "integration", Tested{Failed: []Failure{{Package: "github.com/novox/mesh-controller/cmd/mesh-controller", Output: c.output}}}) + for _, l := range strings.Split(v.Failed.Words, "\n") { + if l = strings.TrimSpace(l); l == "PASS" || strings.HasPrefix(l, "ok ") || strings.HasPrefix(l, "ok\t") { + t.Errorf("1901/AC5: the words keep %q: %q", l, v.Failed.Words) + } + } + if !strings.HasPrefix(afterHid(v.Failed.Words), c.want) { + t.Errorf("1901/AC5: words after the loud line start %q, want %q", rules.FirstLine(afterHid(v.Failed.Words)), c.want) + } + if got := firstFailed(*v); !strings.Contains(got, "the build module hid the failure") || strings.HasSuffix(got, ": PASS") { + t.Errorf("1901/AC5, AC8: status = %q", got) + } + }) + } + }}, + // A cause the build module told apart is cut to a line's bound, so the words keep their bound (the 1901 review's note). + {"a long cause is cut to a lines bound", func(t *testing.T) { // 1901/AC5 + f := Failure{Package: "p", Test: "TestX", Cause: strings.Repeat("c", 5000), Output: "x_test.go:1: something"} + v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{f}}) + if len(v.Failed.Words) > rules.MaxWords+200 { + t.Errorf("1901/AC5: words are %d bytes, bound %d", len(v.Failed.Words), rules.MaxWords) + } + }}, + // The build module on main sends a package that stopped (a timeout, an exit, a kill) as only go test's last line, with no + // cause: the words then say so, never nothing (review round 1). Each output is what that build module sends for the + // capture of the same name (go-test-json-pkg-timeout, -exiting, -killed): the package's own output, its "FAIL" line. + {"a package stopped with only its fail line is still worded", func(t *testing.T) { // 1901/AC5 + for _, out := range []string{"FAIL\texample.com/timeout\t2.017s", "FAIL\texample.com/exiting\t0.006s", "FAIL\texample.com/killed\t0.006s"} { + v := rules.LevelFailure("", "", "integration", Tested{Failed: []Failure{{Package: "example.com/x", Output: out}}}) + if !strings.HasPrefix(afterHid(v.Failed.Words), "the package failed with no test to name and no panic, signal or build error in its output") { + t.Errorf("1901/AC5: %q is worded %q", out, v.Failed.Words) } } - if !strings.HasPrefix(afterHid(v.Failed.Words), c.want) { - t.Errorf("1901/AC5: words after the loud line start %q, want %q", rules.FirstLine(afterHid(v.Failed.Words)), c.want) + }}, + // A line of the test's own that starts as go's summary does, or that holds a runtime's words indented, is not taken for + // either (review round 1): go prints its summary in exact forms and the runtime's lines at column 0. + {"only gos exact forms are summary or cause", func(t *testing.T) { // 1901/AC5 + out := " x_test.go:3: ok so far\nok then: the seat is free\n panic: child retried and recovered\npanic: the real one\nFAIL\tp\t1.0s" + v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{{Package: "p", Output: out}}}) + if !strings.HasPrefix(afterHid(v.Failed.Words), "panic: the real one") || !strings.Contains(v.Failed.Words, "ok then: the seat is free") { + t.Errorf("1901/AC5: words = %q", v.Failed.Words) } - if got := firstFailed(*v); !strings.Contains(got, "the build module hid the failure") || strings.HasSuffix(got, ": PASS") { - t.Errorf("1901/AC5, AC8: status = %q", got) + }}, + // The words keep rules.MaxWords exactly, counting the cause, the marker and the newlines (review round 1). + {"the words keep their bound exactly", func(t *testing.T) { // 1901/AC5 + for _, cause := range []string{"", "a cause", strings.Repeat("c", 3000)} { + for _, n := range []int{100, 170, 171, 172, 500} { + f := Failure{Package: "p", Test: "TestX", Cause: cause, Output: strings.Repeat("a log line of twenty\n", n) + "the end"} + if w := rules.FailureWords(f); len(w) > rules.MaxWords { + t.Errorf("1901/AC5: cause %d bytes, %d lines: words are %d bytes, bound %d", len(cause), n, len(w), rules.MaxWords) + } + } } - }) - } -} - -// A cause the build module told apart is cut to a line's bound, so the words keep their bound (the 1901 review's note). -func TestALongCauseIsCutToALinesBound(t *testing.T) { // 1901/AC5 - f := Failure{Package: "p", Test: "TestX", Cause: strings.Repeat("c", 5000), Output: "x_test.go:1: something"} - v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{f}}) - if len(v.Failed.Words) > rules.MaxWords+200 { - t.Errorf("1901/AC5: words are %d bytes, bound %d", len(v.Failed.Words), rules.MaxWords) - } -} - -// The build module on main sends a package that stopped (a timeout, an exit, a kill) as only go test's last line, with no -// cause: the words then say so, never nothing (review round 1). Each output is what that build module sends for the -// capture of the same name (go-test-json-pkg-timeout, -exiting, -killed): the package's own output, its "FAIL" line. -func TestAPackageStoppedWithOnlyItsFailLineIsStillWorded(t *testing.T) { // 1901/AC5 - for _, out := range []string{"FAIL\texample.com/timeout\t2.017s", "FAIL\texample.com/exiting\t0.006s", "FAIL\texample.com/killed\t0.006s"} { - v := rules.LevelFailure("", "", "integration", Tested{Failed: []Failure{{Package: "example.com/x", Output: out}}}) - if !strings.HasPrefix(afterHid(v.Failed.Words), "the package failed with no test to name and no panic, signal or build error in its output") { - t.Errorf("1901/AC5: %q is worded %q", out, v.Failed.Words) - } - } -} - -// A line of the test's own that starts as go's summary does, or that holds a runtime's words indented, is not taken for -// either (review round 1): go prints its summary in exact forms and the runtime's lines at column 0. -func TestOnlyGosExactFormsAreSummaryOrCause(t *testing.T) { // 1901/AC5 - out := " x_test.go:3: ok so far\nok then: the seat is free\n panic: child retried and recovered\npanic: the real one\nFAIL\tp\t1.0s" - v := rules.LevelFailure("", "", "unit", Tested{Failed: []Failure{{Package: "p", Output: out}}}) - if !strings.HasPrefix(afterHid(v.Failed.Words), "panic: the real one") || !strings.Contains(v.Failed.Words, "ok then: the seat is free") { - t.Errorf("1901/AC5: words = %q", v.Failed.Words) - } -} - -// The words keep rules.MaxWords exactly, counting the cause, the marker and the newlines (review round 1). -func TestTheWordsKeepTheirBoundExactly(t *testing.T) { // 1901/AC5 - for _, cause := range []string{"", "a cause", strings.Repeat("c", 3000)} { - for _, n := range []int{100, 170, 171, 172, 500} { - f := Failure{Package: "p", Test: "TestX", Cause: cause, Output: strings.Repeat("a log line of twenty\n", n) + "the end"} - if w := rules.FailureWords(f); len(w) > rules.MaxWords { - t.Errorf("1901/AC5: cause %d bytes, %d lines: words are %d bytes, bound %d", len(cause), n, len(w), rules.MaxWords) - } - } + }}, + } { + t.Run(row.name, row.test) } }