From 4dfb6af6ef340734a397b818669cab88cc190dbb Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 6 Nov 2026 23:06:21 +0100 Subject: [PATCH] The begin bound of a run ask is the checker's setting; a dropped run ask and a program that does not build fail the whole check naming the program (issue 1921) Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- .../mesh-delivery-checker/integration_test.go | 96 ++++++++++++++----- .../cmd/mesh-delivery-checker/judge.go | 16 ++-- .../cmd/mesh-delivery-checker/main.go | 7 +- 3 files changed, 88 insertions(+), 31 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go index 05fe877e..9b53cce8 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go @@ -98,6 +98,30 @@ type fakeBuildModules struct { inputs jetstream.KeyValue runs atomic.Int32 broken atomic.Value + // drop, when set, drops every run ask unanswered and counts it in dropped, as a build module that does not know the job + // run (the lead's ruling on milestone 47/B0). + drop atomic.Bool + dropped atomic.Int32 +} + +// droppingAsks is the seat's asks as a build module that does not know the job run takes them: a run ask is acknowledged and +// never begun while the fake drops them. +type droppingAsks struct { + gobuild.AskSource + f *fakeBuildModules +} + +func (d droppingAsks) Next(ctx context.Context, kind string, wait time.Duration) (*gobuild.Pulled, error) { + msg, err := d.AskSource.Next(ctx, kind, wait) + if err != nil || msg == nil || !d.f.drop.Load() { + return msg, err + } + var a delivery.StepAsk + if json.Unmarshal(msg.Data, &a) == nil && a.Run != nil { + d.f.dropped.Add(1) + return nil, msg.Ack() + } + return msg, nil } // run answers a run ask as mesh-build-go does: the checking program's work (rules.Program) over the input the ask names, @@ -220,7 +244,7 @@ func (f *fakeBuildModules) serve(t *testing.T, module string, kept jetstream.Key raw, _ := value.(json.RawMessage) return kv.Put(f.ctx, key, raw) } - p := &gobuild.Puller{Source: &gobuild.JetStreamAsks{JS: f.js, Language: language}, Language: language, Node: "n1", Put: put, + p := &gobuild.Puller{Source: droppingAsks{&gobuild.JetStreamAsks{JS: f.js, Language: language}, f}, Language: language, Node: "n1", Put: put, Capacity: func() int { return 2 }, PullWait: 200 * time.Millisecond, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { if a.Check != nil { @@ -792,28 +816,54 @@ func keptCount(t *testing.T, b *bed) int { return len(keys) } -// TestAHeadThatDoesNotMergeFailsNamingTheFilesAndRunsNoTest: the fetch answers the files in conflict; the verdict fails naming -// them, no test or digest step is run, a preview of the conflicts alone is written and the status is owed as failure. -func TestAHeadThatDoesNotMergeFailsNamingTheFilesAndRunsNoTest(t *testing.T) { // 1289/AC4 - b := startBed(t) - b.fake.conflict.Store(true) - key := b.candidate(t, "feat/x", pull(repo, 7, head1)) - b.start(t, nil) - v := b.verdict(t, key, ended) - if v.State != delivery.VerdictFail || !strings.Contains(v.Failed.Words, "files in conflict: a.go, sub/b.go") { - t.Fatalf("verdict = %+v", v) - } - if o := b.owed(t, 7, head1, "failure"); !strings.HasPrefix(o.Text, "failed: n1: ") || !strings.Contains(o.Text, "files in conflict: a.go, sub/b.go") { - t.Errorf("owed = %+v", o) - } - if n := b.fake.count("mesh-build-go.build_go_digest") + b.fake.count("mesh-build-go.build_go_test"); n != 0 { - t.Errorf("%d digest and test steps ran after a conflict", n) - } - pk, _ := delivery.PreviewKey(repo, head1, trunk) - if e, err := b.buckets[delivery.PreviewsBucket].Get(b.ctx, pk); err != nil { - t.Errorf("previews.%s: %v", pk, err) - } else if pv, err := delivery.DecodePreview(e.Value()); err != nil || strings.Join(pv.Conflicts, ",") != "a.go,sub/b.go" || pv.Merge != "" { - t.Errorf("preview = %+v, %v, want the files in conflict alone", pv, err) +// TestACheckThatCannotBeJudgedFailsNamingWhy: a head that does not merge fails naming the files in conflict and runs no +// test (1289/AC4); a checking program whose run asks no build node begins, and one that does not build, fail the check +// naming the checking program, never judged by the running rules (issue 1921, the lead's ruling on milestone 47/B0: the +// begin bound is the checker's setting, shortened here). +func TestACheckThatCannotBeJudgedFailsNamingWhy(t *testing.T) { // 1289/AC4, 1921 + for _, row := range []struct { + name string + set func(f *fakeBuildModules) + env map[string]string + words string + tests bool // the digest and test steps ran + dropped int32 // the run asks dropped: each put twice, the seat list's and the verdict's + program bool // the checking program judged it + }{ + {name: "a head that does not merge", set: func(f *fakeBuildModules) { f.conflict.Store(true) }, words: "files in conflict: a.go, sub/b.go", program: true}, + {name: "a run ask no build node begins", set: func(f *fakeBuildModules) { f.drop.Store(true) }, env: map[string]string{"MESH_CHECKER_BEGIN_BOUND": "500ms"}, + words: "no build node began the job run of the asks", tests: true, dropped: 4}, + {name: "a program that does not build", set: func(f *fakeBuildModules) { f.broken.Store("go build: undefined: rules.Nothing") }, + words: "by the checking program: its run ask", tests: true}, + } { + t.Run(row.name, func(t *testing.T) { + b := startBed(t) + row.set(b.fake) + key := b.candidate(t, "feat/x", pull(repo, 7, head1)) + b.start(t, row.env) + v := b.verdict(t, key, ended) + if v.State != delivery.VerdictFail || !strings.Contains(v.Failed.Words, row.words) || (v.JudgedBy != nil) != row.program { + t.Fatalf("verdict = %+v, want a fail saying %q, judged by the program %v", v, row.words, row.program) + } + if o := b.owed(t, 7, head1, "failure"); !strings.HasPrefix(o.Text, "failed") || + !strings.Contains(o.Text, map[bool]string{true: row.words, false: "the checking program"}[row.program]) { + t.Errorf("owed = %+v", o) + } + if n := b.fake.count("mesh-build-go.build_go_digest") + b.fake.count("mesh-build-go.build_go_test"); (n != 0) != row.tests { + t.Errorf("%d digest and test steps ran, want them run %v", n, row.tests) + } + if n := b.fake.dropped.Load(); n != row.dropped { + t.Errorf("%d run asks dropped, want %d", n, row.dropped) + } + if !row.tests { + pk, _ := delivery.PreviewKey(repo, head1, trunk) + if e, err := b.buckets[delivery.PreviewsBucket].Get(b.ctx, pk); err != nil { + t.Errorf("previews.%s: %v", pk, err) + } else if pv, err := delivery.DecodePreview(e.Value()); err != nil || strings.Join(pv.Conflicts, ",") != "a.go,sub/b.go" || pv.Merge != "" { + t.Errorf("preview = %+v, %v, want the files in conflict alone", pv, err) + } + } + }) } } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go index 863eb35d..8c2c3430 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -25,12 +25,14 @@ import ( // judgeDeps are the parts the judge of a check is made of. type judgeDeps struct { // inputs keeps the checking program's inputs (check.InputsState). - inputs check.Inputs - asks check.Asks - tools *buildrun.ToolchainSet - trunks *check.Trunks - kept check.KV - previews check.KV + inputs check.Inputs + // beginBound is how long a run ask may wait for a build node to begin it; check.DefaultBeginBound when 0. + beginBound time.Duration + asks check.Asks + tools *buildrun.ToolchainSet + trunks *check.Trunks + kept check.KV + previews check.KV // candidates answers the indexer's candidate of a try's group (try_candidates.go); nil leaves a whole try with no // manifest or composition judged, said. candidates check.TryCandidates @@ -162,7 +164,7 @@ func newJudge(d judgeDeps) check.Judge { nodes = d.nodes } ex := &check.Examiner{Nodes: nodes, Compositions: d.compositions, Asks: d.asks, Toolchains: d.tools, - Kept: check.Kept{Store: d.kept}, Previews: d.previews, Candidates: d.candidates, Modules: d.repos, Packages: d.repos, Published: d.published, Forge: d.forge, Runs: d.repos, Builds: d.builds, BuildEnds: d.ends, BuildWait: d.buildWait, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Read: d.repos, Predefined: d.predefined, Inputs: d.inputs, Logf: d.logf} + Kept: check.Kept{Store: d.kept}, Previews: d.previews, Candidates: d.candidates, Modules: d.repos, Packages: d.repos, Published: d.published, Forge: d.forge, Runs: d.repos, Builds: d.builds, BuildEnds: d.ends, BuildWait: d.buildWait, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Read: d.repos, Predefined: d.predefined, Inputs: d.inputs, BeginBound: d.beginBound, Logf: d.logf} if d.nodes != nil && d.builds != nil && d.repos != nil { ex.Serving = runningServes{have: d.nodes, builds: d.builds, read: d.repos, logf: d.logf} } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index 8dfe2f28..375cf192 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -128,6 +128,11 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro if err != nil { return err } + // The begin bound of a run ask of the checking program (check.DefaultBeginBound, issue 1921). + beginBound, err := durationEnv(getenv, "MESH_CHECKER_BEGIN_BOUND", check.DefaultBeginBound) + if err != nil { + return err + } link := strings.TrimSpace(getenv("MESH_CHECKER_LINK")) if link == "" { link = defaultLink @@ -230,7 +235,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro asks: buildrun.Asker{Publisher: asks, Waiter: waiter, Bound: stepBound}, tools: tools, trunks: trunks, kept: kvStore{buckets[delivery.CheckerKeptBucket]}, previews: kvStore{buckets[delivery.PreviewsBucket]}, candidates: tryCandidates{buckets[delivery.CandidatesBucket]}, repos: repos, published: published, builds: builds, ends: ends, buildWait: set.buildWait, forge: forgeReads{call: caller.Call}, logf: logger.Printf, - announce: announce, compositions: compositions, inputs: runInputs{awaitedKV{inputsKV}}, + announce: announce, compositions: compositions, inputs: runInputs{awaitedKV{inputsKV}}, beginBound: beginBound, nodes: openNodeNames(ctx, js, logger.Printf), predefined: keptPredefined{process.AwaitBucket(ctx, js, conn, delivery.IndexerModule+"_"+delivery.IndexerPredefinedState, "a change's manifests are judged with no predefined keys, each fallback as its value declares it", logger.Printf)},