diff --git a/checks/failurekit/failurekit.go b/checks/failurekit/failurekit.go new file mode 100644 index 00000000..72e1ed25 --- /dev/null +++ b/checks/failurekit/failurekit.go @@ -0,0 +1,115 @@ +// Package failurekit refuses a build module whose tests do not run mesh-sdk's conformance kit (issue 1901, its thorough +// fix): a module that claims a node-build-* seat and serves a test step must hold a test that calls conformance.Run, so +// its parse of the test runner's output is held to the failure contract (a failing test named with its cause, never a +// passing line in its place). A build module with no test step (mesh-build-image builds images and runs no tests) is +// exempt, and said so. +package failurekit + +import ( + "encoding/json" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" +) + +// Judged is what the check says of one build module: Refused says why it fails the check, "" when it passes; Exempt +// says why the kit does not apply. +type Judged struct { + Module, Refused, Exempt string +} + +type manifest struct { + Claims []struct { + Name string `json:"name"` + } `json:"claims"` + Tools []json.RawMessage `json:"tools"` +} + +// Judge reads every modules//module.json of the catalogue at root and judges each module that claims a node-build-* +// seat. +func Judge(root string) ([]Judged, error) { + manifests, err := filepath.Glob(filepath.Join(root, "modules", "*", "module.json")) + if err != nil { + return nil, err + } + sort.Strings(manifests) + var out []Judged + for _, p := range manifests { + raw, err := os.ReadFile(p) + if err != nil { + return nil, err + } + var m manifest + if err := json.Unmarshal(raw, &m); err != nil { + return nil, fmt.Errorf("%s: %w", p, err) + } + seat := "" + for _, c := range m.Claims { + if strings.HasPrefix(c.Name, "node-build-") { + seat = c.Name + } + } + if seat == "" { + continue + } + dir := filepath.Dir(p) + j := Judged{Module: filepath.Base(dir)} + test := testTool(m.Tools) + switch { + case test == "": + j.Exempt = fmt.Sprintf("it claims %s and serves no test step", seat) + default: + runs, err := runsTheKit(dir) + if err != nil { + return nil, err + } + if !runs { + j.Refused = fmt.Sprintf("it claims %s and serves the test step %s, and no test of it calls conformance.Run "+ + "(mesh-sdk go/buildstep/conformance): run the kit with the module's own parse path (issue 1901)", seat, test) + } + } + out = append(out, j) + } + return out, nil +} + +// testTool is the name of the module's test step tool (_test), or "". +func testTool(tools []json.RawMessage) string { + for _, raw := range tools { + var name string + if json.Unmarshal(raw, &name) != nil { + var t struct { + Name string `json:"name"` + } + if json.Unmarshal(raw, &t) != nil { + continue + } + name = t.Name + } + if strings.HasSuffix(name, "_test") { + return name + } + } + return "" +} + +// runsTheKit says a _test.go file under dir calls conformance.Run. +func runsTheKit(dir string) (bool, error) { + found := false + err := filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { + if err != nil || found || d.IsDir() || !strings.HasSuffix(p, "_test.go") { + return err + } + raw, err := os.ReadFile(p) + if err != nil { + return err + } + found = strings.Contains(string(raw), "conformance.Run(") && + strings.Contains(string(raw), `"git.novox.be/novox/mesh-sdk/go/buildstep/conformance"`) + return nil + }) + return found, err +} diff --git a/checks/failurekit/failurekit_test.go b/checks/failurekit/failurekit_test.go new file mode 100644 index 00000000..2b2e9d92 --- /dev/null +++ b/checks/failurekit/failurekit_test.go @@ -0,0 +1,70 @@ +package failurekit + +import ( + "os" + "path/filepath" + "testing" +) + +// 1901/AC7: a build module with a test step and no kit is refused, one that runs the kit passes, one with no test step is +// exempt, and a module that claims no node-build-* seat is not judged. +func TestABuildModuleWithATestStepMustRunTheKit(t *testing.T) { + root := t.TempDir() + kit := "package main\n\nimport \"git.novox.be/novox/mesh-sdk/go/buildstep/conformance\"\n\nfunc TestKit(t *testing.T) { conformance.Run(t, conformance.GoTest, nil) }\n" + for path, body := range map[string]string{ + "modules/a/module.json": `{"claims":[{"name":"node-build-a"}],"tools":["build_a_fetch","build_a_test"]}`, + "modules/a/cmd/a/check_test.go": "package main\n", + "modules/b/module.json": `{"claims":[{"name":"node-build-b"}],"tools":[{"name":"build_b_test"}]}`, + "modules/b/cmd/b/kit_test.go": kit, + "modules/c/module.json": `{"claims":[{"name":"node-build-c"}],"tools":["build_c_build"]}`, + "modules/d/module.json": `{"claims":[{"name":"other"}],"tools":["d_test"]}`, + "modules/e/module.json": `{"claims":[{"name":"node-build-e"}],"tools":["build_e_test"]}`, + "modules/e/cmd/e/kit.go": kit, // not a test file + } { + p := filepath.Join(root, path) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + got, err := Judge(root) + if err != nil { + t.Fatal(err) + } + want := map[string]string{"a": "refused", "b": "passes", "c": "exempt", "e": "refused"} + if len(got) != len(want) { + t.Fatalf("judged %+v, want a, b, c and e", got) + } + for _, j := range got { + is := "passes" + if j.Refused != "" { + is = "refused" + } else if j.Exempt != "" { + is = "exempt" + } + if is != want[j.Module] { + t.Errorf("%s %s (%+v), want it %s", j.Module, is, j, want[j.Module]) + } + } +} + +// Every build module of this catalogue with a test step runs the kit. +func TestTheCataloguesBuildModulesRunTheKit(t *testing.T) { + got, err := Judge(filepath.Join("..", "..")) + if err != nil { + t.Fatal(err) + } + if len(got) == 0 { + t.Fatal("no module claims a node-build-* seat: the check reads the wrong tree") + } + for _, j := range got { + switch { + case j.Refused != "": + t.Errorf("%s: %s", j.Module, j.Refused) + case j.Exempt != "": + t.Logf("%s is exempt: %s", j.Module, j.Exempt) + } + } +} diff --git a/checks/failurekit/go.mod b/checks/failurekit/go.mod new file mode 100644 index 00000000..5245863c --- /dev/null +++ b/checks/failurekit/go.mod @@ -0,0 +1,3 @@ +module git.novox.be/novox/mesh-catalog/checks/failurekit + +go 1.22 diff --git a/checks/failurekit/reads-outside b/checks/failurekit/reads-outside new file mode 100644 index 00000000..f6b2d871 --- /dev/null +++ b/checks/failurekit/reads-outside @@ -0,0 +1 @@ +These tests read files outside this directory (the whole catalogue, the repository, or its root files): mesh-build-go never keeps this package's verdict (issue 1935). diff --git a/modules/mesh-build-go/cmd/mesh-build-go/failure_words_test.go b/modules/mesh-build-go/cmd/mesh-build-go/failure_words_test.go deleted file mode 100644 index 9f82aa72..00000000 --- a/modules/mesh-build-go/cmd/mesh-build-go/failure_words_test.go +++ /dev/null @@ -1,112 +0,0 @@ -package main - -import ( - "encoding/json" - "os" - "strings" - "testing" -) - -// A failed test keeps the lines that say why it failed, not the lines it logged first (issue 1901). The fixture is -// the real `go test -json` of a test that logs a line and then fails with a multi-line t.Fatalf. -func TestAFailedTestKeepsItsFatalMessageAfterItsLogLines(t *testing.T) { - raw, err := os.ReadFile("testdata/captured/go-test-json-log-then-fatal.jsonl") - if err != nil { - t.Fatal(err) - } - res := &TestResult{} - rd := newReader(res, ".") - rd.read(string(raw)) - rd.finish() - if len(res.Failed) != 1 { - t.Fatalf("failures = %+v, want one", res.Failed) - } - f := res.Failed[0] - 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(f.Output, want) { - t.Errorf("1901/AC1: output lacks %q: %q", want, f.Output) - } - } - if want := "3 subscription(s) derive to a subject no emitter owns:"; f.Cause != want && !strings.HasSuffix(f.Cause, want) { - t.Errorf("1901/AC1: cause = %q, want the t.Fatalf line, not the t.Logf line", f.Cause) - } -} - -// A failure longer than the bound keeps its last lines, where the cause is, and says how many earlier lines it dropped. -func TestAFailureLongerThanTheBoundKeepsItsLastLines(t *testing.T) { - var lines []string - for i := 0; i < 400; i++ { - lines = append(lines, " x_test.go:1: log line "+strings.Repeat("y", 40)) - } - lines = append(lines, " x_test.go:9: the real cause", "--- FAIL: TestX (0.00s)") - out := clip(lines, "") - if len(out) > outputBytes+100 { - t.Errorf("1901/AC1: kept %d bytes, bound is %d", len(out), outputBytes) - } - if !strings.HasSuffix(out, "the real cause\n--- FAIL: TestX (0.00s)") || !strings.Contains(out, "\n... (") { - t.Errorf("1901/AC1: out = %q...%q", out[:40], out[len(out)-60:]) - } -} - -// An older checker, which knows no "cause" field, still reads the new build module's answer, and the output it gets -// holds at least what the older build module kept (its first lines, for an output shorter than the bound). -func TestAnOlderCheckerReadsTheNewFailureAndGetsAtLeastWhatItKept(t *testing.T) { // 1901/AC3 - raw, err := os.ReadFile("testdata/captured/go-test-json-log-then-fatal.jsonl") - if err != nil { - t.Fatal(err) - } - res := &TestResult{} - rd := newReader(res, ".") - rd.read(string(raw)) - rd.finish() - b, err := json.Marshal(res.Failed) - if err != nil { - t.Fatal(err) - } - var old []struct { - Package string `json:"package"` - Test string `json:"test,omitempty"` - Output string `json:"output,omitempty"` - } - if err := json.Unmarshal(b, &old); err != nil || len(old) != 1 { - t.Fatalf("1901/AC3: an older checker refused the answer: %v, %d failures", err, len(old)) - } - first := strings.SplitN(old[0].Output, "\n", 2)[0] - if first == "" || !strings.Contains(old[0].Output, "judged against") || !strings.Contains(old[0].Output, "3 subscription(s)") { - t.Errorf("1901/AC3: older checker's output = %q", old[0].Output) - } -} - -// A long failure, whose earlier lines are left out, still starts with a real line for an older checker, which keeps only the -// first non-empty line of the output cut to 120 characters, and not with the marker. -func TestALongFailureStillStartsWithARealLineForAnOlderChecker(t *testing.T) { // 1901/AC3 - var lines []string - for i := 0; i < 400; i++ { - lines = append(lines, " x_test.go:1: log line "+strings.Repeat("y", 40)) - } - lines = append(lines, " x_test.go:9: the real cause", "--- FAIL: TestX (0.00s)") - oldFirst := func(out string) string { // the older checker's rule (judge.go firstLine) - for _, l := range strings.Split(out, "\n") { - if l = strings.TrimSpace(l); l != "" { - if len(l) > 120 { - l = l[:120] - } - return l - } - } - return "" - } - for _, cause := range []string{"the real cause", ""} { - out := clip(lines, cause) - got := oldFirst(out) - if strings.HasPrefix(got, "... (") || got == "" { - t.Errorf("1901/AC3: cause %q: an older checker would read %q", cause, got) - } - if cause != "" && got != cause { - t.Errorf("1901/AC3: older checker reads %q, want the cause", got) - } - if !strings.HasSuffix(out, "--- FAIL: TestX (0.00s)") { - t.Errorf("1901/AC3: the last lines are lost: %q", out[len(out)-40:]) - } - } -} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/kit_test.go b/modules/mesh-build-go/cmd/mesh-build-go/kit_test.go new file mode 100644 index 00000000..5aff31a8 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/kit_test.go @@ -0,0 +1,22 @@ +package main + +import ( + "testing" + + "git.novox.be/novox/mesh-sdk/go/buildstep" + "git.novox.be/novox/mesh-sdk/go/buildstep/conformance" +) + +// 1901/AC7: the module's own parse path (readGoTest, then the level's judging) keeps the failure contract on every captured +// go test -json case of the SDK's conformance kit, and its result names the module that sent it. +func TestTheTestStepKeepsTheFailureContract(t *testing.T) { + conformance.Run(t, conformance.GoTest, func(o conformance.Output) []buildstep.Failure { + res := &TestResult{Level: LevelUnit, By: buildstep.By("mesh-build-go")} + readGoTest(res, ".", o.Stdout) + res.judge(Bounds{Run: UnitBound}) + if res.Passed { + t.Errorf("a failed run judged passed: %+v", res) + } + return res.Failed + }) +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/levels.go b/modules/mesh-build-go/cmd/mesh-build-go/levels.go index eeab6dd0..7a5c83cc 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/levels.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/levels.go @@ -6,14 +6,12 @@ package main // step that could not run. import ( - "bufio" - "encoding/json" "fmt" - "regexp" - "slices" "sort" "strings" "time" + + "git.novox.be/novox/mesh-sdk/go/buildstep" ) // The two levels (ADR 0445 §2). @@ -32,39 +30,14 @@ type Bounds struct { Setup, Run time.Duration } -// PackageResult is one package's part of a level. -type PackageResult struct { - Dir string `json:"dir"` - Package string `json:"package"` - Passed bool `json:"passed"` - Elapsed float64 `json:"elapsed"` - Tests int `json:"tests"` - // NoTests is a package with no test files (at this level: a file with the other level's build tag is none). - NoTests bool `json:"no_tests,omitempty"` - // Skipped is how many of the package's tests skipped: a pass that skipped proved less than it says, so the checker keeps no verdict for it. - Skipped int `json:"skipped,omitempty"` -} - -// TestTime is one test and the time it took. -type TestTime struct { - Package string `json:"package"` - Test string `json:"test"` - Elapsed float64 `json:"elapsed"` -} - -// Failure is a test that failed, or a package that failed with no test to name (a build that failed, a TestMain that -// panicked); Output is the last lines it wrote, up to outputBytes (issue 1901), and Cause the first line of what the -// test reported as its error (t.Error, t.Fatal), without its file and line, when go test said which lines those were. -// **A failure that is not one test's assertion carries its real cause too** (issue 1901, its third meeting): the -// "panic:" line (a timeout's with the tests it found running), a "fatal error:" or "signal:" line, a build's first -// error, or in words that the test program exited, while which tests ran or after its tests passed. Test then names the -// tests that were running when the program stopped. A "PASS" or "ok" line is never a failure's words. -type Failure struct { - Package string `json:"package"` - Test string `json:"test,omitempty"` - Output string `json:"output,omitempty"` - Cause string `json:"cause,omitempty"` -} +// PackageResult, TestTime and Failure are the SDK's failure contract (buildstep, issue 1901), with the JSON names this +// module always sent: a Failure names its test, its file, its cause (never empty, never a passing line) and its own +// output up to 4096 bytes, cause first. +type ( + PackageResult = buildstep.GoPackage + TestTime = buildstep.TestTime + Failure = buildstep.Failure +) // TestResult is what a test step passes with. type TestResult struct { @@ -98,368 +71,23 @@ type TestResult struct { // JudgedBy is the program that ran the level (ADR 0535 §5): "installed", or "candidate " under the // runner workspace. JudgedBy string `json:"judged_by,omitempty"` + // By is the build module and the SDK it was built with (buildstep.By): the checker names it when a failure hides + // its cause. + By string `json:"by,omitempty"` } -// event is one line of `go test -json` (test2json, and the build events go test adds). -type event struct { - Action string - Package string - Test string - Output string - OutputType string // "error" for a t.Error or t.Fatal message, "error-continue" for its further lines (go test -json) - ImportPath string - FailedBuild string - Elapsed float64 -} - -// outputBytes is the most a failure keeps of its output, and outputLineBytes the most of one line. **A failure keeps its -// last lines** (issue 1901): a test logs before it fails, so the lines that say why are at the end, and the first -// fifteen lines kept before lost them (the verdict of group r4g kept a t.Logf line, not the t.Fatalf after it). Four -// kilobytes holds a long assertion message and stays far under a bus message's or a verdict's bound. -const ( - outputBytes = 4096 - outputLineBytes = 1024 -) - -// reader collects the events of one go.mod directory's `go test -json` into a result. -type reader struct { - res *TestResult - dir string - pkgs map[string]*PackageResult - order []string - builds map[string][]string // build output by import path - pkgOut map[string][]string - testOut map[string][]string // by package + "\x00" + test - causes map[string]string // the first error line of a test, by the same key - times []TestTime - failedIn map[string]bool // packages with a failed test - // running is the tests that started and have not ended, by package, in the order they started: when a package - // fails with no failed test, these were running when its test program stopped (issue 1901). - running map[string][]string - // passed says a package's test program printed "PASS" (its tests passed) before it failed; built says its test - // program failed to build. - passed, built map[string]bool -} - -func newReader(res *TestResult, dir string) *reader { - return &reader{res: res, dir: dir, pkgs: map[string]*PackageResult{}, builds: map[string][]string{}, - pkgOut: map[string][]string{}, testOut: map[string][]string{}, causes: map[string]string{}, failedIn: map[string]bool{}, - running: map[string][]string{}, passed: map[string]bool{}, built: map[string]bool{}} -} - -// read takes go test's output. Lines that are not events are kept for a failure that has nothing else to say. -func (r *reader) read(out string) (noise []string) { - sc := bufio.NewScanner(strings.NewReader(out)) - sc.Buffer(make([]byte, 1<<20), 1<<26) - for sc.Scan() { - var ev event - if err := json.Unmarshal(sc.Bytes(), &ev); err != nil { - if l := strings.TrimSpace(sc.Text()); l != "" { - noise = append(noise, l) - } - continue - } - r.event(ev) - } - return noise -} - -func (r *reader) pkg(name string) *PackageResult { - p := r.pkgs[name] - if p == nil { - p = &PackageResult{Dir: r.dir, Package: name} - r.pkgs[name] = p - r.order = append(r.order, name) - } - return p -} - -func keep(lines []string, out string) []string { - if t := strings.TrimRight(out, "\r\n"); t != "" && !strings.HasPrefix(t, "=== ") { - lines = append(lines, t) - } - return lines -} - -func (r *reader) event(ev event) { - switch { - case ev.Package == "" && ev.ImportPath != "": - if ev.Action == "build-output" { - r.builds[ev.ImportPath] = keep(r.builds[ev.ImportPath], ev.Output) - } - case ev.Package == "": - case ev.Test == "": - p := r.pkg(ev.Package) - switch ev.Action { - case "output": - // go test's own summary of a package ("PASS", "FAIL", "ok ", "FAIL ") is never a failure's - // words: a "PASS" before a "FAIL " says only that the tests passed and the program failed after them. - switch l := strings.TrimRight(ev.Output, "\r\n"); { - case l == "PASS": - r.passed[ev.Package] = true - case summaryLine(l): - default: - r.pkgOut[ev.Package] = keep(r.pkgOut[ev.Package], ev.Output) - } - case "pass": - p.Passed, p.Elapsed = true, ev.Elapsed - case "fail": - p.Passed, p.Elapsed = false, ev.Elapsed - if ev.FailedBuild != "" { - r.built[ev.Package] = true - r.pkgOut[ev.Package] = append(r.builds[ev.FailedBuild], r.pkgOut[ev.Package]...) - } - case "skip": - p.Passed, p.NoTests = true, true - } - default: - key := ev.Package + "\x00" + ev.Test - switch ev.Action { - case "output": - r.testOut[key] = keep(r.testOut[key], ev.Output) - if ev.OutputType == "error" && r.causes[key] == "" { - r.causes[key] = errorWords(ev.Output) - } - case "run": - r.running[ev.Package] = append(r.running[ev.Package], ev.Test) - case "pass", "fail", "skip": - r.running[ev.Package] = slices.DeleteFunc(r.running[ev.Package], func(t string) bool { return t == ev.Test }) - p := r.pkg(ev.Package) - p.Tests++ - r.res.Tests++ - if ev.Action == "skip" { - r.res.Skipped++ - p.Skipped++ - } - r.times = append(r.times, TestTime{Package: ev.Package, Test: ev.Test, Elapsed: ev.Elapsed}) - if ev.Action == "fail" { - r.failedIn[ev.Package] = true - cause := r.causes[key] - if race := raceCause(r.testOut[key]); race != "" && (cause == "" || strings.HasPrefix(cause, "race detected")) { - cause = race // the race itself, not testing's "race detected during execution of test" - } else if cause == "" { - cause = causeOf(r.testOut[key]) - } - r.res.Failed = append(r.res.Failed, Failure{Package: ev.Package, Test: ev.Test, Output: clip(r.testOut[key], cause), Cause: cause}) - } - } - } -} - -// clip joins the last lines of a failure's output that fit outputBytes, each cut to outputLineBytes, and says how many -// earlier lines it left out. **Its first line is the failure's cause** (issue 1901/AC3, AC4), put ahead when the kept -// lines do not start with it, or, with no cause and lines left out, the first non-empty line of the output, then the -// marker. An older checker takes only the first non-empty line of the output, and a marker there would be less than the -// older pair kept. -func clip(lines []string, cause string) string { - size, from := 0, len(lines) - for from > 0 { - n := len(cutLine(lines[from-1])) + 1 - if size+n > outputBytes && from < len(lines) { - break - } - size += n - from-- - } - kept := make([]string, 0, len(lines)-from+2) - head := strings.TrimSpace(cause) - for i := 0; head == "" && i < from; i++ { - head = strings.TrimSpace(lines[i]) - } - if head != "" && (from > 0 || len(lines) == 0 || strings.TrimSpace(lines[from]) != head) { - kept = append(kept, cutLine(head)) - } - if from > 0 { - kept = append(kept, fmt.Sprintf("... (%d earlier lines)", from)) - } - for _, l := range lines[from:] { - kept = append(kept, cutLine(l)) - } - return strings.Join(kept, "\n") -} - -// summaryLine says a line is go test's summary of a package, in go's exact forms only: "PASS" or "FAIL" alone, -// "ok \t\t