diff --git a/modules/mesh-delivery-checker/go.mod b/modules/mesh-delivery-checker/go.mod index e753fa3..6540a88 100644 --- a/modules/mesh-delivery-checker/go.mod +++ b/modules/mesh-delivery-checker/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-checker go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261023213200-403a5d72ab93 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024214154-bc16740feb0d github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-checker/go.sum b/modules/mesh-delivery-checker/go.sum index 744af4c..e514275 100644 --- a/modules/mesh-delivery-checker/go.sum +++ b/modules/mesh-delivery-checker/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261023213200-403a5d72ab93 h1:FAiFY2KBKEoZSH49OZTt7/zRBIQxyl5g/KhYrpggbN4= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261023213200-403a5d72ab93/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024214154-bc16740feb0d h1:oJREw0ggKqJWJ5DEISSL0i2ZC5XqdXAidxo9NJ74TCc= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024214154-bc16740feb0d/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-delivery-checker/internal/check/every_failure_test.go b/modules/mesh-delivery-checker/internal/check/every_failure_test.go new file mode 100644 index 0000000..99d85d7 --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/every_failure_test.go @@ -0,0 +1,90 @@ +package check + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// failingResult is a level's test result in which the named tests failed, each in its own package. +func failingResult(t *testing.T, tests int, failed ...Failure) json.RawMessage { + t.Helper() + raw, err := json.Marshal(Tested{Level: delivery.LevelIntegration, Tests: tests, Elapsed: 2, Failed: failed, Passed: len(failed) == 0}) + if err != nil { + t.Fatal(err) + } + return raw +} + +func judgeLevel(t *testing.T, tests ...json.RawMessage) (*delivery.Verdict, *state) { + t.Helper() + st := &state{j: &Examiner{}, c: &Case{}, levels: map[string]*acc{delivery.LevelUnit: {}, delivery.LevelIntegration: {}}, nodes: map[string]bool{}} + v, _ := st.level(t.Context(), &target{pull: Pull{Repository: "novox/x"}}, delivery.LevelAnswer{Level: delivery.LevelIntegration, Tests: tests}) + return v, st +} + +// Issue 1921: two failing tests in two packages are two entries of the verdict's Failures and two lines of a try's +// failed list, each with its own first line (its cause, issue 1901), whether the build module answered them in one test +// result (one go test run over both packages) or in two (the changed packages first, then the rest); every result is +// counted, the ones after a failure too; and Failed keeps the first one's words whole. +func TestTwoFailingTestsInTwoPackagesAreTwoEntries(t *testing.T) { + a := Failure{Package: "x/a", Test: "TestA", Cause: "a_test.go:9: want 1, got 2", Output: " a_test.go:3: logged\n a_test.go:9: want 1, got 2\n--- FAIL: TestA (0.00s)"} + b := Failure{Package: "x/b", Test: "TestB", Cause: "b_test.go:4: boom", Output: " b_test.go:4: boom\n--- FAIL: TestB (0.00s)"} + for name, tests := range map[string][]json.RawMessage{ + "one result": {failingResult(t, 10, a, b)}, + "two results": {failingResult(t, 4, a), failingResult(t, 3), failingResult(t, 3, b)}, + } { + v, st := judgeLevel(t, tests...) + if v == nil { + t.Fatalf("%s: the level passed", name) + } + want := []string{"integration: x/a.TestA: a_test.go:9: want 1, got 2", "integration: x/b.TestB: b_test.go:4: boom"} + if got := failedLines(*v); strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Errorf("%s: failed lines\n%s\nwant\n%s", name, strings.Join(got, "\n"), strings.Join(want, "\n")) + } + if len(v.Failures) != 2 || v.Failed.Test != "integration: x/a.TestA" || !strings.Contains(v.Failed.Words, "a_test.go:3: logged") { + t.Errorf("%s: verdict %+v", name, v) + } + if run := st.levels[delivery.LevelIntegration].run; run != 10 { + t.Errorf("%s: %d tests counted, want all 10", name, run) + } + if _, err := delivery.EncodeVerdict(delivery.Verdict{Group: "g", Heads: "0123456789ab", State: v.State, Failed: v.Failed, Failures: v.Failures}); err != nil { + t.Errorf("%s: the verdict is not written: %v", name, err) + } + } +} + +// Issue 1921: past delivery.MaxFailures the list names the first fifty and how many more, the status line says how many +// failed, and each entry's line is cut to MaxFailureLine. +func TestFailuresAreCappedWithHowManyMore(t *testing.T) { + var fs []Failure + for i := range 60 { + fs = append(fs, Failure{Package: fmt.Sprintf("x/p%d", i), Test: "TestT", Cause: strings.Repeat("y", 500)}) + } + v, _ := judgeLevel(t, failingResult(t, 60, fs...)) + if len(v.Failures) != delivery.MaxFailures+1 || v.Failures[delivery.MaxFailures].Test != "and 10 more" { + t.Fatalf("%d entries, last %+v", len(v.Failures), v.Failures[len(v.Failures)-1]) + } + if len(v.Failures[0].Words) > MaxFailureLine { + t.Errorf("an entry's line is %d bytes", len(v.Failures[0].Words)) + } + if n := failedCount(*v); n != 60 { + t.Errorf("failedCount = %d, want 60", n) + } + cs := &Case{Pulls: []Pull{{Repository: "novox/x", Head: "0123456789abcdef", Trunk: "fedcba9876543210"}}} + if line := Line(*v, cs, cs.Pulls[0], nil, nil, nil); !strings.HasPrefix(line, "failed (60 tests): integration: x/p0.TestT") { + t.Errorf("line %q", line) + } +} + +// A verdict that names no list (a conflict, a refused step, a verdict written before issue 1921) is one failed line, as +// before. +func TestAVerdictWithoutFailuresIsOneLine(t *testing.T) { + v := delivery.Verdict{State: delivery.VerdictFail, Failed: delivery.FailedFirst{Machine: "g14", Words: "files in conflict: go.mod\nmore"}} + if got := failedLines(v); len(got) != 1 || got[0] != "g14: files in conflict: go.mod" { + t.Fatalf("%q", got) + } +} diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index c8c4e17..25c9c1e 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -341,7 +341,9 @@ func TestANodePackageIsNeverKeptAndRunsEveryCheck(t *testing.T) { // 1288/AC4, 1 } } -func TestTheChangesOwnFailingTestRunsFirstAndNothingElseRuns(t *testing.T) { // 1289/AC1, AC2 +// 1289/AC1, AC2 as issue 1921 changed it: the change's own failing test runs first, the level's other packages still run +// after it (so one check names every failure), and no later level runs. +func TestTheChangesOwnFailingTestRunsFirstThenTheRestAndNoLaterLevel(t *testing.T) { b := newBench(t) b.fetch.Changed = []string{"modules/m/internal/b/b_test.go"} b.digest[delivery.LevelUnit] = Digested{Level: "unit", Packages: map[string]string{"m/internal/a": digA, "m/internal/b": digB}} @@ -357,8 +359,8 @@ func TestTheChangesOwnFailingTestRunsFirstAndNothingElseRuns(t *testing.T) { // t.Fatalf("verdict = %+v", out.Verdict) } tests := b.steps.tool("test") - if len(tests) != 1 || strings.Join(packagesOf(tests[0]), ",") != "m/internal/b" { - t.Fatalf("test calls = %+v, want only m/internal/b, once", tests) + if len(tests) != 2 || strings.Join(packagesOf(tests[0]), ",") != "m/internal/b" || strings.Join(packagesOf(tests[1]), ",") != "m/internal/a" { + t.Fatalf("test calls = %+v, want m/internal/b, then m/internal/a, each once", tests) } if n := len(b.steps.tool("digest")); n != 1 { t.Errorf("%d digest calls, want the unit level's alone (the check stopped)", n) diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 0567a4c..cbf731d 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -438,8 +438,9 @@ func (st *state) ask(ctx context.Context, t *target, merged *[]Fetched) *deliver } // level judges one level of a check ask's answer as the tools' answers were judged: the digest keys the kept verdicts -// of the packages that passed, the kept ones taken are counted, and the first test result that did not pass is the -// verdict. It answers the level's running time (its tests' and setup's). +// of the packages that passed, the kept ones taken are counted, and every test result is read and counted, also after +// one that did not pass, so the verdict names every failed test of the level (issue 1921). It answers the level's +// running time (its tests' and setup's). func (st *state) level(ctx context.Context, t *target, la delivery.LevelAnswer) (*delivery.Verdict, float64) { level := la.Level a := st.levels[level] @@ -461,6 +462,7 @@ func (st *state) level(ctx context.Context, t *target, la delivery.LevelAnswer) a.kept += k.Tests } elapsed := 0.0 + var failedResults []Tested for _, raw := range la.Tests { var res Tested if err := json.Unmarshal(raw, &res); err != nil { @@ -489,9 +491,12 @@ func (st *state) level(ctx context.Context, t *target, la delivery.LevelAnswer) } } if !res.Passed { - return levelFailure(t, level, res), elapsed + failedResults = append(failedResults, res) } } + if len(failedResults) > 0 { + return levelFailure(t, level, failedResults...), elapsed + } return nil, elapsed } @@ -558,23 +563,60 @@ func firstLine(s string) string { return "" } -// failure turns a level that did not pass into a verdict naming what failed first. -func levelFailure(t *target, level string, res Tested) *delivery.Verdict { +// MaxFailureLine is the most bytes of a failure's line in a verdict's Failures: the first line of its words, which is +// its cause where the build module told one apart (issue 1901). Failed keeps the first failure's words whole. +const MaxFailureLine = 300 + +// levelFailure turns a level that did not pass into a verdict: Failed names what failed first with its words whole, +// and Failures every failed test of the level's results with its first line, at most delivery.MaxFailures and an entry +// saying how many more (issue 1921). +func levelFailure(t *target, level string, results ...Tested) *delivery.Verdict { v := delivery.Verdict{State: delivery.VerdictFail} - switch { - case len(res.Failed) > 0: - f := res.Failed[0] - name := failedName(f) - if t.module == buildrun.NodeBuildModule && f.Package == "" { - name = res.Module + var all []delivery.FailedFirst + for _, res := range results { + if len(res.Failed) == 0 { + why := res.Why + if why == "" { + why = "the " + level + " level did not pass" + } + all = append(all, delivery.FailedFirst{Test: level + ": " + t.pull.Repository, Words: why}) + continue } - v.Failed = delivery.FailedFirst{Test: level + ": " + name, Words: failureWords(f)} - default: - why := res.Why - if why == "" { - why = "the " + level + " level did not pass" + for _, f := range res.Failed { + name := failedName(f) + if t.module == buildrun.NodeBuildModule && f.Package == "" { + name = res.Module + } + all = append(all, delivery.FailedFirst{Test: level + ": " + name, Words: failureWords(f)}) } - v.Failed = delivery.FailedFirst{Test: level + ": " + t.pull.Repository, Words: why} } + if len(all) == 0 { // no result: a level said failed with nothing to name + all = append(all, delivery.FailedFirst{Test: level + ": " + t.pull.Repository, Words: "the " + level + " level did not pass"}) + } + v.Failed = all[0] + v.Failures = failureLines(all) return &v } + +// failureLines is every failure with the first line of its words, cut to MaxFailureLine, at most delivery.MaxFailures +// and then one entry saying how many more failed. +func failureLines(all []delivery.FailedFirst) []delivery.FailedFirst { + n := min(len(all), delivery.MaxFailures) + out := make([]delivery.FailedFirst, 0, n+1) + for _, f := range all[:n] { + out = append(out, delivery.FailedFirst{Test: f.Test, Machine: f.Machine, Words: clipLine(firstLine(f.Words), MaxFailureLine)}) + } + if more := len(all) - n; more > 0 { + out = append(out, delivery.FailedFirst{Test: fmt.Sprintf("and %d more", more)}) + } + return out +} + +// clipLine cuts a line to at most n bytes on a whole character, marking the cut. +func clipLine(s string, n int) string { + if len(s) <= n { + return s + } + s = strings.ToValidUTF8(s[:n-3], "") + return s + "..." +} diff --git a/modules/mesh-delivery-checker/internal/check/line.go b/modules/mesh-delivery-checker/internal/check/line.go index f847823..a105441 100644 --- a/modules/mesh-delivery-checker/internal/check/line.go +++ b/modules/mesh-delivery-checker/internal/check/line.go @@ -49,6 +49,34 @@ func firstFailed(v delivery.Verdict) string { return firstWordsLine(f.Words) } +// failedLines is everything a verdict names as failed, one line each: every failed test with its first line (issue +// 1921), or what failed first when the verdict names no list (a conflict, a refused step, an older verdict). +func failedLines(v delivery.Verdict) []string { + if len(v.Failures) == 0 { + if f := firstFailed(v); f != "" { + return []string{f} + } + return nil + } + lines := make([]string, 0, len(v.Failures)) + for _, f := range v.Failures { + lines = append(lines, firstFailed(delivery.Verdict{Failed: f})) + } + return lines +} + +// failedCount is how many tests a verdict names as failed, the ones after delivery.MaxFailures counted too. +func failedCount(v delivery.Verdict) int { + n := len(v.Failures) + if n > delivery.MaxFailures { + var more int + if _, err := fmt.Sscanf(v.Failures[n-1].Test, "and %d more", &more); err == nil { + return delivery.MaxFailures + more + } + } + return n +} + // firstWordsLine is the first line of a failure's words: the words are several lines (issue 1901) and the status line one. func firstWordsLine(w string) string { return firstLine(w) } @@ -155,6 +183,9 @@ func parts(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []s pc.head = "pass " + commits default: pc.head = "failed: " + firstFailed(v) + if n := failedCount(v); n > 1 { + pc.head = fmt.Sprintf("failed (%d tests): %s", n, firstFailed(v)) + } pc.commits = commits } var counts []string diff --git a/modules/mesh-delivery-checker/internal/check/try.go b/modules/mesh-delivery-checker/internal/check/try.go index 5f0be8b..7b9cb54 100644 --- a/modules/mesh-delivery-checker/internal/check/try.go +++ b/modules/mesh-delivery-checker/internal/check/try.go @@ -67,7 +67,9 @@ type Try struct { // Said is the try's report in words: what failed first, the tests run, kept and skipped per level, the time of each part. Said string `json:"said,omitempty"` // Took is the time from its start to its end, as Go writes a duration. - Took string `json:"took,omitempty"` + Took string `json:"took,omitempty"` + // Failed is every test that failed, each with its first line, at most delivery.MaxFailures and then how many more + // (issue 1921); or what else failed first (a conflict, a refused step). Failed []string `json:"failed,omitempty"` Levels map[string]delivery.LevelResult `json:"levels,omitempty"` Machines []string `json:"machines,omitempty"` @@ -339,9 +341,7 @@ func (tr *Tryer) run(ctx context.Context, key string) { t.State = TryPass default: t.State = TryFail - if f := firstFailed(v); f != "" { - t.Failed = []string{f} - } + t.Failed = failedLines(v) } if t.Tests != "" { t.Said += " | only the tests " + t.Tests + " (Go); nothing kept"