diff --git a/modules/mesh-delivery-checker/go.mod b/modules/mesh-delivery-checker/go.mod index 3b83a31..d202053 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.20261026202658-805b4d9a3536 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026233910-a2759ec92ec9 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 c6d0c7b..918a889 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.20261026202658-805b4d9a3536 h1:+SoDdDXF7Tgj38WP4kRpUSps6DJVl6P+w5u/KfdtwnQ= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026233910-a2759ec92ec9 h1:MS7i4NVNQHMAMqRRFMdzbto31BHZY+BuEZY1ql3p4Rs= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026233910-a2759ec92ec9/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/failure_words_test.go b/modules/mesh-delivery-checker/internal/check/failure_words_test.go index ef95b47..9c01774 100644 --- a/modules/mesh-delivery-checker/internal/check/failure_words_test.go +++ b/modules/mesh-delivery-checker/internal/check/failure_words_test.go @@ -42,7 +42,7 @@ func TestTheStatusLineNamesTheFailureNotTheFirstLoggedLine(t *testing.T) { // 19 f := fatalAfterLog() f.Cause = "" v = levelFailure(&target{}, "unit", Tested{Failed: []Failure{f}}) - if !strings.HasPrefix(v.Failed.Words, "agreement_catalogue_test.go:130") { + if !strings.HasPrefix(afterHid(v.Failed.Words), "agreement_catalogue_test.go:130") { t.Errorf("words = %q", v.Failed.Words) } } @@ -56,18 +56,45 @@ func TestWordsLongerThanTheBoundKeepTheirLastLines(t *testing.T) { // 1901/AC1 _ = delivery.VerdictFail } -// An older build module sends no "cause": its answer is still accepted and the verdict's words are its output, as before. -func TestAnOlderBuildModulesFailureIsAcceptedWithItsWholeOutput(t *testing.T) { // 1901/AC3 +// 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 := levelFailure(&target{}, "unit", Tested{Failed: []Failure{f}}) - if v.Failed.Words != "x_test.go:1: logged\nx_test.go:2: the old kept lines" { + 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, "logged") || strings.Contains(got, "\n") { - t.Errorf("1901/AC3: status = %q", got) + 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 := levelFailure(&target{module: 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) + } + }) } } diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 4a66e0d..b6a1ed8 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -663,6 +663,12 @@ const MaxWords = 4096 // no cause told apart (an older build module) the cause is the output's first "panic:", "fatal error:" or "signal:" // line, read before the output is cut, or the words say the tests passed and the program failed after them. func failureWords(f Failure) string { + return failureWordsAfter(f, "") +} + +// failureWordsAfter is failureWords with the line loud put first, within the same MaxWords: what the checker itself says of +// the failure (that the build module hid it), which the status line then names. +func failureWordsAfter(f Failure, loud string) string { out, passed := dropSummary(strings.TrimSpace(f.Output)) cause := strings.TrimSpace(f.Cause) if cause == "" { @@ -678,11 +684,14 @@ func failureWords(f Failure) string { if len(cause) > MaxCause { cause = strings.ToValidUTF8(cause[:MaxCause], "") + "..." } - // The words keep MaxWords exactly: the cause and its newline, the marker and the kept lines (review round 1). + // The words keep MaxWords exactly: the loud line, the cause and their newlines, the marker and the kept lines. budget := MaxWords if cause != "" { budget -= len(cause) + 1 } + if loud != "" { + budget -= len(loud) + 1 + } if len(out) > budget { const marker = "... " tail := out[len(out)-(budget-len(marker)):] @@ -695,9 +704,31 @@ func failureWords(f Failure) string { if cause != "" && !strings.HasPrefix(firstLine(out), cause) { out = strings.TrimRight(cause+"\n"+out, "\n") } + if loud != "" { + out = strings.TrimRight(loud+"\n"+out, "\n") + } return out } +// hidWords is the checker's loud line for a failure the build module sent hiding its cause (issue 1901, its thorough +// fix): with no cause, or a passing line as its cause, under the failure contract (buildstep.Hidden). It names the build +// module and the version it says it is (Tested.By, " with mesh-sdk "); one from before the contract +// says nothing of itself. "" when the failure keeps the contract. The check fails either way: only its words change. +func hidWords(module, by string, f Failure) string { + why := buildstep.Hidden(f) + if why == "" { + return "" + } + version := "a version before the failure contract" + if module == "" { + module = "an unnamed build module" + } + if m, v, ok := strings.Cut(by, " with "); ok && m != "" { + module, version = m, v + } + return fmt.Sprintf("the build module hid the failure (%s at %s): %s", module, version, why) +} + // MaxCause is the most a cause in a failure's words holds: one long line (the 1901 review: a cause of a kilobyte could // push the words past MaxWords). const MaxCause = 1024 @@ -784,7 +815,7 @@ func levelFailure(t *target, level string, results ...Tested) *delivery.Verdict if t.module == buildrun.NodeBuildModule && f.Package == "" { name = res.Module } - all = append(all, delivery.FailedFirst{Test: level + ": " + name, Words: failureWords(f)}) + all = append(all, delivery.FailedFirst{Test: level + ": " + name, Words: failureWordsAfter(f, hidWords(t.module, res.By, f))}) } } if len(all) == 0 { // no result: a level said failed with nothing to name 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 344fed8..64b50ce 100644 --- a/modules/mesh-delivery-checker/internal/check/package_cause_test.go +++ b/modules/mesh-delivery-checker/internal/check/package_cause_test.go @@ -38,11 +38,11 @@ func TestAPackageFailureIsNeverWordedPass(t *testing.T) { // 1901/AC5 t.Errorf("1901/AC5: the words keep %q: %q", l, v.Failed.Words) } } - if !strings.HasPrefix(v.Failed.Words, c.want) { - t.Errorf("1901/AC5: words start %q, want %q", firstLine(v.Failed.Words), c.want) + if !strings.HasPrefix(afterHid(v.Failed.Words), c.want) { + t.Errorf("1901/AC5: words after the loud line start %q, want %q", firstLine(afterHid(v.Failed.Words)), c.want) } - if got := firstFailed(*v); !strings.Contains(got, c.want) || strings.HasSuffix(got, ": PASS") { - t.Errorf("1901/AC5: status = %q", got) + 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) } }) } @@ -63,7 +63,7 @@ func TestALongCauseIsCutToALinesBound(t *testing.T) { // 1901/AC5 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 := levelFailure(&target{}, "integration", Tested{Failed: []Failure{{Package: "example.com/x", Output: out}}}) - if !strings.HasPrefix(v.Failed.Words, "the package failed with no test to name and no panic, signal or build error in its output") { + 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) } } @@ -74,7 +74,7 @@ func TestAPackageStoppedWithOnlyItsFailLineIsStillWorded(t *testing.T) { // 1901 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 := levelFailure(&target{}, "unit", Tested{Failed: []Failure{{Package: "p", Output: out}}}) - if !strings.HasPrefix(v.Failed.Words, "panic: the real one") || !strings.Contains(v.Failed.Words, "ok then: the seat is free") { + 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) } } @@ -90,3 +90,13 @@ func TestTheWordsKeepTheirBoundExactly(t *testing.T) { // 1901/AC5 } } } + +// afterHid is a verdict's words after the checker's loud line, which a failure with no cause from an older build module +// now starts with (1901/AC8). +func afterHid(w string) string { + if strings.HasPrefix(w, "the build module hid the failure") { + _, rest, _ := strings.Cut(w, "\n") + return rest + } + return w +} diff --git a/modules/mesh-delivery-checker/internal/check/results.go b/modules/mesh-delivery-checker/internal/check/results.go index 12597a7..768e7c3 100644 --- a/modules/mesh-delivery-checker/internal/check/results.go +++ b/modules/mesh-delivery-checker/internal/check/results.go @@ -4,6 +4,7 @@ import ( "encoding/json" "strings" + "git.novox.be/novox/mesh-sdk/go/buildstep" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -69,14 +70,9 @@ type GoPackage struct { Skipped int `json:"skipped,omitempty"` } -// Failure is a test that failed, or a package that failed with no test to name. -type Failure struct { - Package string `json:"package"` - Test string `json:"test,omitempty"` - Output string `json:"output,omitempty"` - // Cause is the first line of what the test reported as its error, when the build module told it apart (issue 1901). - Cause string `json:"cause,omitempty"` -} +// Failure is a test that failed, or a package that failed with no test to name: the build modules' failure contract +// (buildstep.Failure, issue 1901), its cause never empty and never a passing line from a build module that keeps it. +type Failure = buildstep.Failure // TestTime is one test and the seconds it took. type TestTime struct { @@ -109,6 +105,9 @@ type Tested struct { // JudgedBy is the program that ran the step (ADR 0535 ยง5): "installed", or "candidate " under runner // workspace; absent from a build module that does not take the runner. JudgedBy string `json:"judged_by,omitempty"` + // By is the build module that answered and the SDK it was built with (buildstep.By); absent from a build module from + // before the failure contract. + By string `json:"by,omitempty"` } // Digested is the answer of the Go build module's digest step: one digest per package that has tests at the level, by import