From 2bdb8d0e00a20a20059203a7cc3746b3d7ead864 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 14:06:57 +0200 Subject: [PATCH] Ask the Go build module to run the candidate when a merge changes it, and say in the line which program judged (issue 1898 K3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A change to mesh-build-go was judged by the installed module it replaces, so such pull requests were merged by hand. ADR 0535 §1, §2, §4, §5. --- .../internal/check/checker.go | 15 +- .../internal/check/judge.go | 83 ++++++- .../internal/check/judged_test.go | 207 ++++++++++++++++++ .../internal/check/line.go | 56 +++-- .../internal/check/line_test.go | 38 ++-- .../internal/check/read.go | 2 +- .../internal/check/results.go | 5 + .../internal/check/try.go | 2 +- 8 files changed, 363 insertions(+), 45 deletions(-) create mode 100644 modules/mesh-delivery-checker/internal/check/judged_test.go diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 1236af4..78d6ba2 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -40,6 +40,9 @@ type Outcome struct { Builds []BuildNote // Slowest names the slowest tests of a level that went over its bound ("unit pkg.Test 1.2s"). Slowest []string + // Judged says, by pull request (pullKey), which program judged a change to a build module (ADR 0535 §5); a pull request + // that changes none is not named. + Judged map[string]string } // BuildNote is the state of a candidate's module build, by module: built, failed or waiting (ADR 0484 clause 7). @@ -311,7 +314,7 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { return } began := ch.now() - ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, nil, nil) + ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) out := ch.judge(ctx, cs) if ctx.Err() != nil { return @@ -342,11 +345,11 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { if ch.movedSince(cs) { // Judged on a trunk that moved while it ran: the verdict is kept as judged, and the status stays pending until the // check of the new trunk, which Reconsider starts, ends. - ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, nil, nil) + ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) ch.logf("%s: %s on a trunk that moved while it ran; the status stays pending", cs.Key, v.State) return } - ch.writeOwed(ctx, cs, v, out.Builds, out.Slowest) + ch.writeOwed(ctx, cs, v, out) ch.logf("%s: %s", cs.Key, v.State) } @@ -366,7 +369,7 @@ func failure(machine, words string) delivery.Verdict { // writeOwed writes, for each pull request judged, the status the forge bridge performs: pending while the check runs, success // or failure at its end (ADR 0484 clause 2). A key already holding the same value is not written again, so the bridge // performs one act for each change; an older head's key is never touched. -func (ch *Checker) writeOwed(ctx context.Context, cs *Case, v delivery.Verdict, builds []BuildNote, slowest []string) { +func (ch *Checker) writeOwed(ctx context.Context, cs *Case, v delivery.Verdict, out Outcome) { for _, p := range cs.Pulls { key, err := delivery.CheckerOwedKey(p.Repository, p.Number, p.Head) if err != nil { @@ -375,7 +378,7 @@ func (ch *Checker) writeOwed(ctx context.Context, cs *Case, v delivery.Verdict, } state := map[string]string{delivery.VerdictRunning: "pending", delivery.VerdictPass: "success", delivery.VerdictFail: "failure"}[v.State] o := delivery.Owed{Act: delivery.OwedStatus, Context: delivery.StatusContextCheck, State: state, - Target: strings.TrimSuffix(ch.Link, "/") + "/" + cs.Key, Text: Line(v, cs, p, builds, slowest)} + Target: strings.TrimSuffix(ch.Link, "/") + "/" + cs.Key, Text: Line(v, cs, p, out.Builds, out.Slowest, out.Judged)} data, err := delivery.EncodeCheckerOwed(o) if err != nil { ch.logf("%s: the status of %s cannot be written: %v", cs.Key, key, err) @@ -486,7 +489,7 @@ func (ch *Checker) TrunkMoved(ctx context.Context) { if v, err := delivery.DecodeVerdict(value); err == nil && sameTrunk(v.Trunk, cs.Trunk) { continue } - ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, nil, nil) + ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) } ch.Reconsider(ctx) } diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 5593b58..dcd07f0 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -17,6 +17,35 @@ import ( // goModule is the build module the Go repositories are judged by. const goModule = buildrun.GoBuildModule +// RunnerWorkspace is the runner the Go build module's digest and test steps are asked with when the merge changes the build +// module itself: the installed module compiles the candidate's program from the merged tree and runs the step as it, shut +// off (ADR 0535 §1). +const RunnerWorkspace = "workspace" + +// buildGoDir is the Go build module's directory in the catalogue: a merge changing a path under it is judged by its +// candidate (ADR 0535 §1). mesh-build-node's own directory is judged by the installed module until its part of issue 1898. +const buildGoDir = "modules/mesh-build-go/" + +// changesBuildGo says whether a merge of the repository with these changed paths changes the Go build module. +func changesBuildGo(repository string, changed []string) bool { + if repository != catalogueRepository { + return false + } + for _, p := range changed { + if strings.HasPrefix(p, buildGoDir) { + return true + } + } + return false +} + +// The words of the line on the program that judged a change to the Go build module (ADR 0535 §2, §5). +const ( + judgedByOldInstalled = "judged by the installed mesh-build-go: it does not run a candidate yet" + judgedByInstalled = "judged by the installed mesh-build-go" + judgedByCandidate = "judged by the candidate's mesh-build-go at " +) + // verdictPtr is a verdict of failure a step of the check ends with. type verdictPtr = *delivery.Verdict @@ -118,6 +147,34 @@ type target struct { // so a verdict is kept or taken only under a digest that says it folded exactly these in (lead decision L5). beside []string plans map[string]*goPlan // by level + // runner is RunnerWorkspace while the target's steps are asked to run the candidate (ADR 0535 §1), empty otherwise. + runner string + // judged is what the line says of the program that judged a change to the build module; empty for any other change. + judged string +} + +// judgedBy takes what a step under runner workspace answered of the program that ran it (judged_by): the candidate, or the +// installed module (a module that does not take the runner says nothing, and is asked without it from now on: ADR 0535 §2). +// It says whether the candidate ran. +func (t *target) judgedBy(answer string) bool { + switch { + case strings.HasPrefix(answer, "candidate "): + t.judged = judgedByCandidate + short12(strings.TrimPrefix(answer, "candidate ")) + return true + case answer == "": + t.judged, t.runner = judgedByOldInstalled, "" + default: + t.judged, t.runner = judgedByInstalled, "" + } + return false +} + +// stepArgs are a step's arguments with the target's runner. +func (t *target) stepArgs(args map[string]any) map[string]any { + if t.runner != "" { + args["runner"] = t.runner + } + return args } // goPlan is what a Go target has to run at one level: the digests, the packages without a kept verdict and the tests taken. @@ -216,7 +273,16 @@ func (st *state) outcome(v delivery.Verdict) Outcome { slow = append(slow, fmt.Sprintf("%s %s %.1fs", level, path.Base(s.Package)+"."+s.Test, s.Elapsed)) } } - return Outcome{Verdict: v, Slowest: slow} + out := Outcome{Verdict: v, Slowest: slow} + for _, t := range st.targets { + if t.judged != "" { + if out.Judged == nil { + out.Judged = map[string]string{} + } + out.Judged[pullKey(Pull{Repository: t.pull.Repository, Number: t.pull.Number})] = t.judged + } + } + return out } // fail is a verdict of failure at a machine. @@ -326,6 +392,9 @@ func (st *state) fetchAll(ctx context.Context) *delivery.Verdict { return fail(node, fmt.Sprintf("%s#%d does not merge into the trunk: files in conflict: %s", p.Repository, p.Number, strings.Join(t.fetched.Conflicts, ", "))) } t.changed = t.fetched.Changed + if bm == buildrun.GoBuildModule && changesBuildGo(p.Repository, t.changed) { + t.runner = RunnerWorkspace + } merged = append(merged, t.fetched) st.targets = append(st.targets, t) } @@ -467,7 +536,7 @@ func (st *state) planGo(ctx context.Context, t *target, level string) *delivery. return nil } raw, err := st.step(ctx, t, "digest", fmt.Sprintf("digest the %s level of %s#%d on %s", level, t.pull.Repository, t.pull.Number, t.node), - map[string]any{"workspace": t.fetched.Workspace, "level": level}) + t.stepArgs(map[string]any{"workspace": t.fetched.Workspace, "level": level})) var d Digested if err == nil { err = json.Unmarshal(raw, &d) @@ -475,6 +544,11 @@ func (st *state) planGo(ctx context.Context, t *target, level string) *delivery. if ctx.Err() != nil { return fail(t.node, ctx.Err().Error()) } + if err == nil && t.runner != "" && t.judgedBy(d.JudgedBy) { + // The candidate's digest decides which verdicts would be taken: every package runs and nothing is kept (ADR 0535 §4). + plan.all = true + return nil + } if err != nil || len(d.Packages) == 0 { // The build module named no digest (a level it cannot digest here): the level runs whole and keeps nothing. if err != nil { @@ -532,7 +606,7 @@ func (st *state) runGo(ctx context.Context, t *target, level string, priority bo return nil } } - args := map[string]any{"workspace": t.fetched.Workspace, "level": level} + args := t.stepArgs(map[string]any{"workspace": t.fetched.Workspace, "level": level}) if run != nil { args["packages"] = run } @@ -547,6 +621,9 @@ func (st *state) runGo(ctx context.Context, t *target, level string, priority bo if err := json.Unmarshal(raw, &res); err != nil { return fail(t.node, fmt.Sprintf("the %s level of %s#%d answered what is not a result: %v", level, t.pull.Repository, t.pull.Number, err)) } + if t.runner != "" { + t.judgedBy(res.JudgedBy) + } a := st.levels[level] a.run += res.Tests a.skipped += res.Skipped diff --git a/modules/mesh-delivery-checker/internal/check/judged_test.go b/modules/mesh-delivery-checker/internal/check/judged_test.go new file mode 100644 index 0000000..f90febd --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/judged_test.go @@ -0,0 +1,207 @@ +package check + +import ( + "context" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// The judged_by answer of mesh-build-go's digest and test steps (ADR 0535 §5; the milestone file's interface for issue +// 1898): "installed" for a step its installed program ran, "candidate " for one the candidate's program ran +// under runner workspace; an installed module older than the runner names nothing (it ignores the unknown argument: the +// SDK's tool server checks no argument against the input schema). + +const buildGoFile = "modules/mesh-build-go/cmd/mesh-build-go/digest.go" + +// candidateBench is a bench whose fetch changes the Go build module's directory, and whose build module runs the candidate +// when asked to (a module with the runner) or ignores the runner (one without). +func candidateBench(t *testing.T, takesRunner bool) *bench { + b := newBench(t) + b.fetch.Changed = []string{"README.md", buildGoFile} + judged := func(c stepCall) string { + if !takesRunner { + return "" + } + if c.args["runner"] == RunnerWorkspace { + return "candidate " + b.fetch.Merge + } + return "installed" + } + b.steps.handlers["build_go_digest"] = func(c stepCall) (any, error) { + d := b.digest[c.args["level"].(string)] + d.JudgedBy = judged(c) + return d, nil + } + b.test = func(c stepCall) (any, error) { + r := captured[Tested](t, "go-test-slow") + r.Level, _ = c.args["level"].(string) + r.JudgedBy = judged(c) + return r, nil + } + return b +} + +func TestAMergeChangingTheGoBuildModuleIsAskedWithTheCandidateRunner(t *testing.T) { // 1898/AC1 + b := candidateBench(t, true) + b.judge(t, caseOf(t, pullOf(catalogueRepository, 9, head1))) + asked := append(b.steps.tool("digest"), b.steps.tool("test")...) + if len(asked) < 3 { + t.Fatalf("asked %+v, want the digest and both levels", asked) + } + for _, c := range asked { + if c.args["runner"] != RunnerWorkspace { + t.Errorf("%s %v was asked with runner %v, want %s", c.tool, c.args["level"], c.args["runner"], RunnerWorkspace) + } + } +} + +func TestAMergeNotChangingTheGoBuildModuleIsAskedWithoutARunner(t *testing.T) { // 1898/AC1 + for _, tc := range []struct { + name, repo, path string + }{ + {"another module of the catalogue", catalogueRepository, "modules/mesh-build-node/cmd/mesh-build-node/main.go"}, + {"a look-alike path in another repository", repoA, buildGoFile}, + } { + t.Run(tc.name, func(t *testing.T) { + b := candidateBench(t, true) + b.fetch.Changed = []string{tc.path} + out := b.judge(t, caseOf(t, pullOf(tc.repo, 9, head1))) + for _, c := range append(b.steps.tool("digest"), b.steps.tool("test")...) { + if _, ok := c.args["runner"]; ok { + t.Errorf("%s was asked with runner %v", c.tool, c.args["runner"]) + } + } + if len(out.Judged) != 0 { + t.Errorf("judged = %v, want nothing said of the program", out.Judged) + } + }) + } +} + +func TestAnInstalledModuleWithoutTheRunnerJudgesAsTodayAndTheLineSaysSo(t *testing.T) { // 1898/AC2 + b := candidateBench(t, false) + cs := caseOf(t, pullOf(catalogueRepository, 9, head1)) + out := b.judge(t, cs) + if out.Verdict.State != delivery.VerdictPass { + t.Fatalf("verdict = %+v", out.Verdict) + } + // As today: the digests key kept verdicts, and the levels are asked without the runner the module ignored. + for _, c := range b.steps.tool("test") { + if _, ok := c.args["runner"]; ok { + t.Errorf("the %v level was asked again with the runner the module does not take", c.args["level"]) + } + } + if keys, _ := b.kept.Keys(context.Background()); len(keys) != 2 { + t.Errorf("kept %v, want the passing package at both levels, as today", keys) + } + p := cs.Pulls[0] + want := "judged by the installed mesh-build-go: it does not run a candidate yet" + if got := out.Judged[pullKey(p)]; got != want { + t.Fatalf("judged = %q, want %q", got, want) + } + if l := Line(out.Verdict, cs, p, out.Builds, out.Slowest, out.Judged); !strings.Contains(l, want) { + t.Errorf("line %q does not say %q", l, want) + } +} + +func TestACandidateJudgedCheckTakesNoKeptVerdictAndWritesNone(t *testing.T) { // 1898/AC3 + b := candidateBench(t, true) + if _, err := (Kept{Store: b.kept}).Keep(context.Background(), "unit", digA, keptValue("fixture/slow", 2)); err != nil { + t.Fatal(err) + } + out := b.judge(t, caseOf(t, pullOf(catalogueRepository, 9, head1))) + if out.Verdict.State != delivery.VerdictPass { + t.Fatalf("verdict = %+v", out.Verdict) + } + tests := b.steps.tool("test") + if len(tests) != 2 { + t.Fatalf("test calls = %+v, want both levels", tests) + } + for _, c := range tests { + if _, ok := c.args["packages"]; ok { + t.Errorf("the %v level was asked for %v, want it whole", c.args["level"], c.args["packages"]) + } + } + if u := out.Verdict.Levels[delivery.LevelUnit]; u.Kept != 0 { + t.Errorf("unit = %+v, want nothing taken", u) + } + if keys, _ := b.kept.Keys(context.Background()); len(keys) != 1 || keys[0] != "unit."+digA { + t.Errorf("kept keys = %v, want only the one kept before", keys) + } +} + +func TestTheLineNamesTheCandidateThatJudged(t *testing.T) { // 1898/AC6 + b := candidateBench(t, true) + cs := caseOf(t, pullOf(catalogueRepository, 9, head1)) + out := b.judge(t, cs) + p := cs.Pulls[0] + want := "judged by the candidate's mesh-build-go at " + b.fetch.Merge[:12] + for name, l := range map[string]string{ + "line": Line(out.Verdict, cs, p, out.Builds, out.Slowest, out.Judged), + "report": Report(out.Verdict, cs, p, out.Builds, out.Slowest, out.Judged), + } { + if !strings.Contains(l, want) { + t.Errorf("the %s %q does not say %q", name, l, want) + } + } + // A long failing line keeps the program it was judged by; only the lesser parts go. + v := out.Verdict + v.State = delivery.VerdictFail + v.Failed = delivery.FailedFirst{Test: "unit: git.novox.be/novox/mesh-catalog/modules/mesh-build-go/cmd/mesh-build-go.TestADigestFoldsTheSiblings", Words: strings.Repeat("x", 60)} + if l := Line(v, cs, p, out.Builds, out.Slowest, out.Judged); !strings.Contains(l, "judged by the candidate's") { + t.Errorf("the long line %q dropped the program", l) + } +} + +func TestInAGroupOnlyTheBuildModulesPullRequestNamesItsProgram(t *testing.T) { // 1898/AC6 + b := candidateBench(t, true) + cs := caseOf(t, pullOf(catalogueRepository, 9, head1), pullOf(repoA, 7, head1)) + out := b.judge(t, cs) + for _, p := range cs.Pulls { + l := Report(out.Verdict, cs, p, out.Builds, out.Slowest, out.Judged) + says := strings.Contains(l, "judged by") + if says != (p.Repository == catalogueRepository) { + t.Errorf("%s#%d: %q", p.Repository, p.Number, l) + } + } +} + +func TestACandidateThatDoesNotCompileFailsTheCheckWithTheCompilersWords(t *testing.T) { // ADR 0535 §3, checker side + b := candidateBench(t, true) + refusal := "the candidate's mesh-build-go does not compile: modules/mesh-build-go/cmd/mesh-build-go/digest.go:12:2: undefined: x" + b.steps.handlers["build_go_digest"] = func(stepCall) (any, error) { return nil, errorString(refusal) } + b.test = func(stepCall) (any, error) { return nil, errorString(refusal) } + out := b.judge(t, caseOf(t, pullOf(catalogueRepository, 9, head1))) + if out.Verdict.State != delivery.VerdictFail || !strings.Contains(out.Verdict.Failed.Words, "undefined: x") { + t.Fatalf("verdict = %+v, want a failure naming the compile error", out.Verdict) + } + if n := len(b.steps.tool("test")); n != 1 { + t.Errorf("%d test steps, want the first one's failure to end the check", n) + } +} + +type errorString string + +func (e errorString) Error() string { return string(e) } + +func TestTheStatusTheForgeShowsNamesTheProgramThatJudged(t *testing.T) { // 1898/AC6, the forge's line + ctx := context.Background() + words := judgedByCandidate + "fedcba987654" + j := &judgeFunc{f: func(context.Context, *Case, int) (Outcome, error) { + return Outcome{Verdict: passing(), Judged: map[string]string{repoA + "#7": words}}, nil + }} + r := newRig(t, j) + r.ch.Candidate(ctx, "feat/x", candidateOf(t, "feat/x", pullOf(repoA, 7, head1))) + r.ch.Wait() + key, _ := delivery.CheckerOwedKey(repoA, 7, head1) + raw, _, _, _ := r.owed.Get(ctx, key) + o, err := delivery.DecodeCheckerOwed(raw) + if err != nil { + t.Fatal(err) + } + if o.State != "success" || !strings.Contains(o.Text, words) { + t.Errorf("status %+v, want success saying %q", o, words) + } +} diff --git a/modules/mesh-delivery-checker/internal/check/line.go b/modules/mesh-delivery-checker/internal/check/line.go index cedef15..85e6830 100644 --- a/modules/mesh-delivery-checker/internal/check/line.go +++ b/modules/mesh-delivery-checker/internal/check/line.go @@ -52,15 +52,20 @@ func firstFailed(v delivery.Verdict) string { // pieces are the parts of a line, from the most to the least needed. type pieces struct { head, commits, counts, shortCounts, times, shortTimes, over, builds, composition, shortComposition, group string + // judged names the program that judged a change to a build module (ADR 0535 §5); never dropped. + judged string } +// pullKey names a pull request of a case: `#`. +func pullKey(p Pull) string { return fmt.Sprintf("%s#%d", p.Repository, p.Number) } + func (p pieces) join(drop int) string { - parts := []string{p.head, p.commits, p.counts} + parts := []string{p.head, p.commits, p.counts, p.judged} switch { case drop < 3 && p.times != "": parts = append(parts, p.times) case drop == 3 && p.shortTimes != "": - parts = []string{p.head, p.commits, p.shortCounts, p.shortTimes} + parts = []string{p.head, p.commits, p.shortCounts, p.judged, p.shortTimes} } if p.over != "" { parts = append(parts, p.over) @@ -77,13 +82,7 @@ func (p pieces) join(drop int) string { case p.composition != "": parts = append(parts, p.composition) } - out := make([]string, 0, len(parts)) - for _, s := range parts { - if s != "" { - out = append(out, s) - } - } - return strings.Join(out, " | ") + return joinSet(parts...) } func levelWords(name string, r delivery.LevelResult) string { @@ -139,8 +138,9 @@ func buildWords(builds []BuildNote) string { } // parts makes the pieces of the line of a pull request of a case. -func parts(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []string) pieces { +func parts(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []string, judged map[string]string) pieces { var pc pieces + pc.judged = judged[pullKey(p)] commits := short12(p.Head) + " on " + short12(p.Trunk) switch v.State { case delivery.VerdictRunning: @@ -221,22 +221,48 @@ func parts(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []s } // Report is the full line of a pull request's status, with no limit: what the read tools and the full report say. -func Report(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []string) string { - return parts(v, cs, p, builds, slowest).join(0) +func Report(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []string, judged map[string]string) string { + return parts(v, cs, p, builds, slowest, judged).join(0) } // Line is the words of a status for one pull request of a case (ADR 0447 clause 3, ADR 0484 clause 7): the verdict first (a // failure names the conflict, the refused step or the test before anything), the head and the trunk commit as twelve // characters each, the tests run, kept and skipped per level, the time of each part, a level over its bound with its slowest -// tests, the builds of the touched modules, and that the composition was not yet run. It is at most MaxLine characters: the +// tests, the builds of the touched modules, and that the composition was not yet run; for a change to a build module, the +// program that judged it (judged, by pullKey; ADR 0535 §5). It is at most MaxLine characters: the // builds, then the group drop first, then the times are given in their compact form, then dropped, and the line is cut at last. -func Line(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []string) string { - pc := parts(v, cs, p, builds, slowest) +// The program that judged is never dropped: a line naming it gives the counts in their compact form, then drops them and +// what follows, and is cut in its first part rather than lose it. +func Line(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []string, judged map[string]string) string { + pc := parts(v, cs, p, builds, slowest, judged) for drop := 0; drop <= 4; drop++ { if s := pc.join(drop); utf8.RuneCountInString(s) <= MaxLine { return s } } + if pc.judged != "" { + for _, counts := range []string{pc.shortCounts, ""} { + if s := joinSet(pc.head, pc.commits, counts, pc.judged); utf8.RuneCountInString(s) <= MaxLine { + return s + } + } + rest := " | " + joinSet(pc.commits, pc.judged) + room := MaxLine - utf8.RuneCountInString(rest) - 1 + if head := []rune(pc.head); room > 0 && len(head) > room { + return string(head[:room]) + "…" + rest + } + } runes := []rune(pc.join(4)) return string(runes[:MaxLine-1]) + "…" } + +// joinSet joins the parts that are not empty. +func joinSet(parts ...string) string { + out := make([]string, 0, len(parts)) + for _, s := range parts { + if s != "" { + out = append(out, s) + } + } + return strings.Join(out, " | ") +} diff --git a/modules/mesh-delivery-checker/internal/check/line_test.go b/modules/mesh-delivery-checker/internal/check/line_test.go index 662db1d..9f5824f 100644 --- a/modules/mesh-delivery-checker/internal/check/line_test.go +++ b/modules/mesh-delivery-checker/internal/check/line_test.go @@ -22,7 +22,7 @@ func passVerdict() delivery.Verdict { func TestAPassOpensWithTheVerdictAndSaysTheCommitsTheCountsTheTimesAndThatTheCompositionWasNotRun(t *testing.T) { // 1300/AC1 cs, p := lineCase(t, pullOf(repoA, 7, head1)) - full := Report(passVerdict(), cs, p, nil, nil) + full := Report(passVerdict(), cs, p, nil, nil, nil) for _, want := range []string{"pass 111111111111 on aaaaaaaaaaaa", "unit 12 run, 301 kept", "integration 3 run, 40 kept", "queue 800ms", "clone 4.1s", "unit 400ms", "int setup 3.2s", "int run 12s", "composition not yet run"} { if !strings.Contains(full, want) { t.Errorf("the report %q lacks %q", full, want) @@ -31,7 +31,7 @@ func TestAPassOpensWithTheVerdictAndSaysTheCommitsTheCountsTheTimesAndThatTheCom if !strings.HasPrefix(full, "pass ") || strings.Contains(full, "composition passed") { t.Errorf("report = %q", full) } - line := Line(passVerdict(), cs, p, nil, nil) + line := Line(passVerdict(), cs, p, nil, nil, nil) if n := utf8.RuneCountInString(line); n > MaxLine { t.Errorf("the line is %d characters, want at most %d: %q", n, MaxLine, line) } @@ -51,7 +51,7 @@ func TestAFailureNamesTheTestTheConflictOrTheRefusedStepFirst(t *testing.T) { // "conflict": {delivery.Verdict{State: delivery.VerdictFail, Failed: delivery.FailedFirst{Machine: "shanks", Words: "novox/mesh-delivery#7 does not merge into the trunk: files in conflict: go.mod"}}, "failed: shanks: novox/mesh-delivery#7 does not merge into the trunk: files in conflict: go.mod"}, "refused": {delivery.Verdict{State: delivery.VerdictFail, Failed: delivery.FailedFirst{Machine: "mesh-build-go", Words: "no go build node qualifies: builders disagree"}}, "failed: mesh-build-go: no go build node qualifies"}, } { - line := Line(c.v, cs, p, nil, nil) + line := Line(c.v, cs, p, nil, nil, nil) if !strings.HasPrefix(line, c.want) || !strings.Contains(line, "111111111111 on aaaaaaaaaaaa") { t.Errorf("%s: line = %q, want it to open with %q and name the commits", name, line, c.want) } @@ -62,8 +62,8 @@ func TestALevelOverItsBoundIsNamedWithItsSlowestTestsAndTheVerdictIsNotChanged(t 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"}) - if !strings.HasPrefix(line, "pass ") || !strings.Contains(Report(v, cs, p, nil, []string{"unit pkg.TestSlow 7.0s"}), "over bound: unit (slowest unit pkg.TestSlow 7.0s)") { + 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) } } @@ -72,16 +72,16 @@ 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); !strings.Contains(line, "unit 12 run, 301 kept, 2 skipped") { + 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) - b := Line(passVerdict(), cs, cs.Pulls[1], nil, nil) - if !strings.HasPrefix(a, "pass ") || !strings.HasPrefix(b, "pass ") || !strings.Contains(Report(passVerdict(), cs, cs.Pulls[0], nil, nil), "group feat/x of 2 pull requests") { + 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") { @@ -92,16 +92,16 @@ func TestAGroupsMembersEachCarryTheSameVerdict(t *testing.T) { // 1300/AC4 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); !strings.Contains(r, "builds: 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) + 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); !strings.Contains(l, "builds: avahi built") { + 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) } } @@ -109,14 +109,14 @@ func TestTheBuildsOfTheTouchedModulesAreSaidAndDroppedFirstWhenTheLineIsTooLong( 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); utf8.RuneCountInString(line) > MaxLine || !strings.HasPrefix(line, "failed: integration: ") { + 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); line != "checking 111111111111 on aaaaaaaaaaaa" { + if line := Line(delivery.Verdict{State: delivery.VerdictRunning}, cs, p, nil, nil, nil); line != "checking 111111111111 on aaaaaaaaaaaa" { t.Errorf("line = %q", line) } } @@ -125,11 +125,11 @@ func TestThePassedCompositionIsSaidOnlyWhenItWasRun(t *testing.T) { // ADR 0484 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); !strings.Contains(r, "composition passed with afbe371003a5") { + 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); !strings.Contains(r, "composition not yet run") || strings.Contains(r, "composition passed") { + 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) } } @@ -139,18 +139,18 @@ func TestTheCompositionsTimeAndMachinesAreSaidOnceTheAssignerReportsThem(t *test 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) + 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); !strings.Contains(l, "composition passed in 2.4s") || !strings.Contains(l, "on 4 machines") || utf8.RuneCountInString(l) > MaxLine { + 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); !strings.Contains(r, "composition failed in 2.4s on novox: novox: two seats claim dns") { + 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) } } diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index acb5ad3..165e406 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -82,7 +82,7 @@ func (r Reader) read(key string, raw json.RawMessage) (VerdictRead, error) { return out, nil } for _, p := range cs.Pulls { - out.Reports[fmt.Sprintf("%s#%d", p.Repository, p.Number)] = Report(v, cs, p, nil, nil) + out.Reports[pullKey(p)] = Report(v, cs, p, nil, nil, nil) } for _, l := range cs.Left { out.Reports[strings.SplitN(l, ": ", 2)[0]] = strings.SplitN(l, ": ", 2)[1] diff --git a/modules/mesh-delivery-checker/internal/check/results.go b/modules/mesh-delivery-checker/internal/check/results.go index 07be860..7eed537 100644 --- a/modules/mesh-delivery-checker/internal/check/results.go +++ b/modules/mesh-delivery-checker/internal/check/results.go @@ -87,6 +87,9 @@ type Tested struct { Failed []Failure `json:"failed,omitempty"` OverBound bool `json:"over_bound"` OverWhich string `json:"over_which,omitempty"` + // 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"` } // Digested is the answer of the Go build module's digest step: one digest per package that has tests at the level, by import @@ -98,4 +101,6 @@ type Digested struct { Level string `json:"level"` Packages map[string]string `json:"packages"` WithoutTests []string `json:"without_tests,omitempty"` + // JudgedBy is the program that ran the digest, as Tested.JudgedBy says. + JudgedBy string `json:"judged_by,omitempty"` } diff --git a/modules/mesh-delivery-checker/internal/check/try.go b/modules/mesh-delivery-checker/internal/check/try.go index ac5c6db..77e42d3 100644 --- a/modules/mesh-delivery-checker/internal/check/try.go +++ b/modules/mesh-delivery-checker/internal/check/try.go @@ -336,7 +336,7 @@ func (tr *Tryer) run(ctx context.Context, key string) { t.Ended = tr.now().UTC() t.Took = t.Ended.Sub(t.Started).Round(time.Second).String() t.Levels, t.Machines = v.Levels, v.Machines - t.Said = Report(v, cs, cs.Pulls[0], out.Builds, out.Slowest) + t.Said = Report(v, cs, cs.Pulls[0], out.Builds, out.Slowest, out.Judged) switch { case !judged || IsNodesFailure(v): t.State = TryError