From b519c8dde2da4a52283f2b43a1e334363d44c8b4 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 21:52:41 +0200 Subject: [PATCH] Keep nothing for a workspace given siblings, place the siblings where the tests state them, decide the hold from the listed plan keys (review D1-D3) --- .../cmd/mesh-delivery-checker/forge.go | 19 +++++ .../cmd/mesh-delivery-checker/forge_test.go | 15 ++++ .../cmd/mesh-delivery-checker/hold.go | 61 +++++++++---- .../cmd/mesh-delivery-checker/hold_test.go | 35 ++++++++ .../cmd/mesh-delivery-checker/judge.go | 14 ++- .../cmd/mesh-delivery-checker/judge_test.go | 3 + .../internal/check/beside.go | 85 ++++++++++++++----- .../internal/check/examiner_test.go | 35 +++++++- .../internal/check/hold.go | 35 +------- .../internal/check/hold_test.go | 30 +------ .../internal/check/judge.go | 19 ++++- .../internal/check/pins.go | 2 + .../internal/check/pins_test.go | 57 +++++++++++++ .../internal/check/testdata/captured/README | 2 + .../testdata/captured/forge-commits.json | 14 +++ .../mesh-delivery-checker/manifest_test.go | 2 +- modules/mesh-delivery-checker/module.json | 1 + 17 files changed, 322 insertions(+), 107 deletions(-) create mode 100644 modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold_test.go create mode 100644 modules/mesh-delivery-checker/internal/check/testdata/captured/forge-commits.json diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge.go index 9245ad5..9cf6715 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge.go @@ -13,6 +13,7 @@ import ( const ( getFileTool = "gitea.gitea_get_file" compareTool = "gitea.gitea_compare" + commitsTool = "gitea.gitea_commits" ) // forgeReads reads the forge through call (mesh-sdk process.Conn.Call as the checker's own account). @@ -59,3 +60,21 @@ func (f forgeReads) OnTrunk(_ context.Context, repository, commit string) (bool, } return *a.Total == 0, nil } + +// FullSHA is the whole sha of a commit named by its first digits: the newest commit of the history at that ref. +func (f forgeReads) FullSHA(_ context.Context, repository, commit string) (string, error) { + owner, repo := ownerRepo(repository) + raw, err := f.call(commitsTool, map[string]any{"owner": owner, "repo": repo, "ref": commit, "limit": 1}) + if err != nil { + return "", err + } + var a struct { + Commits []struct { + SHA string `json:"sha"` + } `json:"commits"` + } + if err := json.Unmarshal(raw, &a); err != nil || len(a.Commits) == 0 || len(a.Commits[0].SHA) != 40 || !strings.HasPrefix(a.Commits[0].SHA, commit) { + return "", fmt.Errorf("the answer of %s for %s is not a commit that starts with it: %v", commitsTool, commit, err) + } + return a.Commits[0].SHA, nil +} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge_test.go index fbb84d8..2b1f2b1 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge_test.go @@ -53,3 +53,18 @@ func TestTheForgeReadsParseWhatTheForgeAnswered(t *testing.T) { // 1624/AC4 t.Error("an answer with no total was taken") } } + +func TestTheForgeNamesTheWholeShaOfAShortCommit(t *testing.T) { // review D2 + f := forgeReads{call: func(key string, body any) (json.RawMessage, error) { + if key != commitsTool { + t.Errorf("called %s", key) + } + return capturedAnswer(t, "forge-commits"), nil + }} + if sha, err := f.FullSHA(context.Background(), "novox/mesh-sdk", "cb54756c3865"); err != nil || sha != "cb54756c3865df5ac9c0dc0481ad9ebe2bf013a1" { + t.Errorf("sha = %q, %v", sha, err) + } + if _, err := f.FullSHA(context.Background(), "novox/mesh-sdk", "deadbeefdead"); err == nil { + t.Error("a commit that does not start with the pin was taken") + } +} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold.go index dbbbfc7..1518bd7 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold.go @@ -11,15 +11,18 @@ import ( "mesh-delivery-checker/internal/check" ) -// holdWatch holds the checker back while a delivery builds (ADR 0484 clause 12): it watches the controller's bucket of the -// delivery plans in their build phase and reads it as empty where it is absent (issue 1652 makes it) or cannot be read, -// saying so once. The bucket may come into being after the checker started and the checker holds no timer (ADR 0437 §8), so -// each time a check is about to start and the bucket is not watched yet, the checker looks for it. +// holdWatch holds the checker back while a delivery builds (ADR 0484 clause 12): whether one does is the list of the keys of +// the controller's bucket of the delivery plans in their build phase, read each time a check is about to start. Its keys age +// out (a time to live, and no delete marker), which a watch never hears, so a watch cannot decide it (mesh-controller +// internal/link/holds.go lists the keys too); the watch is only the wake-up, for a plan that ended by a delete. The bucket +// is read as empty where it is absent (issue 1652 makes it) or cannot be read, saying so once. It may come into being after +// the checker started and the checker holds no timer (ADR 0437 §8), so each time a check is about to start and the bucket is +// not known yet, the checker looks for it. type holdWatch struct { ctx context.Context js bucketOpener - hold check.PlanHold - changed func(ctx context.Context) // the last plan ended: the checks that waited may start + keys func(ctx context.Context) ([]string, error) + changed func(ctx context.Context) // a plan key changed: the checks that waited may start logger *log.Logger errs chan error mu sync.Mutex @@ -34,23 +37,37 @@ func newHoldWatch(ctx context.Context, js bucketOpener, changed func(context.Con // Held says whether a delivery builds now. func (w *holdWatch) Held() bool { w.ensure() - return w.hold.Held() + w.mu.Lock() + keys := w.keys + w.mu.Unlock() + if keys == nil { + return false + } + list, err := keys(w.ctx) + if err != nil { + w.say("the keys of %s cannot be listed, so it is read as empty: %v", check.PlansBuildingBucket, err) + return false + } + return len(list) > 0 } func (w *holdWatch) say(format string, args ...any) { + w.mu.Lock() + defer w.mu.Unlock() if !w.said { w.said = true w.logger.Printf(format, args...) } } -// ensure starts the watch if the bucket exists and is not watched. +// ensure finds the bucket if it is not known and exists, and starts the wake-up watch. func (w *holdWatch) ensure() { w.mu.Lock() - defer w.mu.Unlock() - if w.watched { + if w.watched || w.js == nil { + w.mu.Unlock() return } + w.mu.Unlock() kv, err := w.js.KeyValue(w.ctx, check.PlansBuildingBucket) switch { case errors.Is(err, jetstream.ErrBucketNotFound): @@ -60,17 +77,25 @@ func (w *holdWatch) ensure() { w.say("the bucket %s cannot be read, so it is read as empty: %v", check.PlansBuildingBucket, err) return } - watcher, err := settle(w.ctx, kv, func(e jetstream.KeyValueEntry) { w.hold.Put(e.Key(), !opOf(e)) }) - if err != nil { - w.say("the bucket %s cannot be watched, so it is read as empty: %v", check.PlansBuildingBucket, err) + w.mu.Lock() + defer w.mu.Unlock() + if w.watched { return } w.watched, w.said = true, false + w.keys = func(ctx context.Context) ([]string, error) { + list, err := kv.Keys(ctx) + if errors.Is(err, jetstream.ErrNoKeysFound) { + return nil, nil + } + return list, err + } go func() { - w.errs <- follow(w.ctx, watcher, func(e jetstream.KeyValueEntry) { - if w.hold.Put(e.Key(), !opOf(e)) && !w.hold.Held() { - w.changed(w.ctx) - } - }) + wake, err := kv.WatchAll(w.ctx) + if err != nil { + w.errs <- err + return + } + w.errs <- follow(w.ctx, wake, func(jetstream.KeyValueEntry) { w.changed(w.ctx) }) }() } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold_test.go new file mode 100644 index 0000000..3f15b31 --- /dev/null +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold_test.go @@ -0,0 +1,35 @@ +package main + +import ( + "context" + "errors" + "io" + "log" + "testing" +) + +// The controller's plans-building keys age out of the bucket (a time to live, no delete marker), which a watch never hears: +// whether a delivery builds is the list of the keys now, and the watch is only a wake-up (review D3). +func TestADeliveryHoldsWhileItsKeyIsListedAndNotWhenItAgedOutWithNoEvent(t *testing.T) { + listed := []string{"plan-1"} + w := &holdWatch{watched: true, logger: log.New(io.Discard, "", 0), keys: func(context.Context) ([]string, error) { return listed, nil }} + if !w.Held() { + t.Fatal("a plan key is listed and nothing holds") + } + listed = nil // the key aged out: no delete reached the watch + if w.Held() { + t.Error("the key is gone from the bucket and the checker is still held") + } +} + +func TestABucketThatCannotBeListedIsReadAsEmpty(t *testing.T) { + w := &holdWatch{watched: true, logger: log.New(io.Discard, "", 0), keys: func(context.Context) ([]string, error) { return []string{"plan-1"}, errors.New("the bus is down") }} + if w.Held() { + t.Error("an unreadable bucket held the checker") + } + var absent holdWatch + absent.logger = log.New(io.Discard, "", 0) + if absent.Held() { + t.Error("a watch with no bucket held the checker") + } +} 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 b2a909f..6dc5381 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -31,7 +31,7 @@ type judgeDeps struct { func newJudge(d judgeDeps) check.Judge { return &check.Examiner{Steps: d.runner, Toolchains: d.tools, Presence: d.presence, Health: d.have, Places: d.places, - Kept: check.Kept{Store: d.kept}, Previews: d.previews, Modules: d.repos, Forge: d.forge, Builds: d.builds, Announce: d.announce, Trunks: d.trunks, Logf: d.logf} + Kept: check.Kept{Store: d.kept}, Previews: d.previews, Modules: d.repos, Forge: d.forge, Runs: d.repos, Builds: d.builds, Announce: d.announce, Trunks: d.trunks, Logf: d.logf} } // manifestKV reads a manifest by its digest. @@ -49,6 +49,7 @@ type repoModules struct { mu sync.Mutex langs map[string]map[string]bool // by manifest digest: the languages of its artifacts repoOf map[string]string // by module: its repository + commit map[string]string // by module: the commit its record was computed at (the commit the mesh runs) typed map[string]string // by module: its manifest digest } @@ -56,11 +57,12 @@ type repoModules struct { func (r *repoModules) Put(ctx context.Context, key string, value []byte, deleted bool) { r.mu.Lock() if r.repoOf == nil { - r.repoOf, r.typed, r.langs = map[string]string{}, map[string]string{}, map[string]map[string]bool{} + r.repoOf, r.typed, r.langs, r.commit = map[string]string{}, map[string]string{}, map[string]map[string]bool{}, map[string]string{} } if deleted { delete(r.repoOf, key) delete(r.typed, key) + delete(r.commit, key) r.mu.Unlock() return } @@ -72,6 +74,7 @@ func (r *repoModules) Put(ctx context.Context, key string, value []byte, deleted r.mu.Lock() r.repoOf[key] = m.Repository r.typed[key] = m.Manifest + r.commit[key] = m.Commit _, known := r.langs[m.Manifest] r.mu.Unlock() if m.Manifest == "" || known { @@ -128,3 +131,10 @@ func (r buildReader) Build(ctx context.Context, module, fp2 string) (delivery.Bu b, err := delivery.DecodeBuilderBuild(value) return b, err == nil, err } + +// CommitOf is the commit the indexer's record of the module was computed at: the commit of its repository the mesh runs. +func (r *repoModules) CommitOf(module string) string { + r.mu.Lock() + defer r.mu.Unlock() + return r.commit[module] +} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go index ba3c1c0..28329bf 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go @@ -66,6 +66,9 @@ func TestARepositoryWithATypeScriptModuleIsJudgedByTheNodeBuildModuleToo(t *test if got := r.BuildModules("novox/mesh-catalog"); strings.Join(got, ",") != buildrun.GoBuildModule+","+buildrun.NodeBuildModule { t.Errorf("mesh-catalog: %v, want both", got) } + if got := r.CommitOf("mesh-delivery-checker"); got != strings.Repeat("a", 40) || r.CommitOf("gone") != "" { + t.Errorf("the commit the mesh runs = %q", got) + } if got := r.BuildModules("novox/mesh-sdk"); strings.Join(got, ",") != buildrun.GoBuildModule { t.Errorf("a repository with no module: %v, want the Go build module", got) } diff --git a/modules/mesh-delivery-checker/internal/check/beside.go b/modules/mesh-delivery-checker/internal/check/beside.go index f18ae1a..32ba99a 100644 --- a/modules/mesh-delivery-checker/internal/check/beside.go +++ b/modules/mesh-delivery-checker/internal/check/beside.go @@ -1,6 +1,10 @@ package check -import "strconv" +import ( + "context" + "fmt" + "strconv" +) // BesideRef is a sibling repository a repository's tests read, placed beside the fetched tree as a plain directory (ADR // 0484 clause 4). Ref names the pull request's head when the commit is only there. @@ -10,37 +14,74 @@ type BesideRef struct { Ref string `json:"ref,omitempty"` } +const ( + catalogueRepository = "novox/mesh-catalog" + nodeEngineRepo = "novox/mesh-host" + nodeEngineModule = "mesh-host" + controllerRepo = "novox/mesh-controller" +) + // besideRules are the repositories each core repository's tests read, as the tests state it (mesh-sdk go/internal/beside -// beside.go): the catalogue at its trunk, the node-engine, the SDK as the controller pins it, and the delivery's pinned -// forge dependencies (mesh-catalog checks/deps). The checker chooses which sibling at which commit; the build module only -// places what it is told. +// beside.go): the catalogue, the node-engine, the SDK, and the delivery's pinned forge dependencies (mesh-catalog +// checks/deps). Which commit of each is besideCommit's rule. var besideRules = map[string][]string{ - "novox/mesh-controller": {"novox/mesh-catalog", "novox/mesh-host", "novox/mesh-sdk"}, - "novox/mesh-host": {"novox/mesh-catalog", "novox/mesh-sdk"}, - "novox/mesh-sdk": {"novox/mesh-catalog", "novox/mesh-host"}, - "novox/mesh-catalog": {"novox/mesh-sdk", "novox/mesh-controller"}, - "novox/mesh-delivery": {"novox/mesh-sdk", "novox/mesh-catalog"}, + controllerRepo: {catalogueRepository, nodeEngineRepo, SDKRepository}, + nodeEngineRepo: {catalogueRepository, SDKRepository}, + SDKRepository: {catalogueRepository, nodeEngineRepo}, + catalogueRepository: {SDKRepository, controllerRepo}, + "novox/mesh-delivery": {SDKRepository, catalogueRepository}, } -// Beside are the siblings a pull request's tests read: each at its trunk head, and at the head of the group's pull request -// when the group holds one of that repository (a group's members are checked together, ADR 0482 clause 2) — except the -// catalogue, which the tests read at its trunk even when the group holds a catalogue pull request (beside.go). -func (j *Examiner) Beside(c *Case, p Pull) []BesideRef { +// besideFor are the siblings a pull request's tests read (ADR 0484 clause 4): the catalogue at its trunk, always, even in a +// group holding a catalogue pull request; the node-engine at the commit the mesh runs (the commit of the indexer's module +// mesh-host, its trunk head when none is known); mesh-sdk, for the controller's tests, at the commit the controller's go.mod +// pins; any other sibling at the head of the group's pull request for it when the group holds one, else at its trunk. +func (j *Examiner) besideFor(ctx context.Context, c *Case, p Pull) ([]BesideRef, error) { var heads map[string]string if j.Trunks != nil { heads = j.Trunks.Heads() } var out []BesideRef for _, repo := range besideRules[p.Repository] { - if member := c.member(repo); member != nil && repo != "novox/mesh-catalog" { - out = append(out, BesideRef{Repository: repo, Commit: member.Head, Ref: refOf(member.Number)}) - continue - } - if head := heads[repo]; head != "" { - out = append(out, BesideRef{Repository: repo, Commit: head}) + switch { + case repo == catalogueRepository: + if head := heads[repo]; head != "" { + out = append(out, BesideRef{Repository: repo, Commit: head}) + } + case repo == nodeEngineRepo: + commit := "" + if j.Runs != nil { + commit = j.Runs.CommitOf(nodeEngineModule) + } + if commit == "" { + commit = heads[repo] + } + if commit != "" { + out = append(out, BesideRef{Repository: repo, Commit: commit}) + } + case repo == SDKRepository && p.Repository == controllerRepo && j.Forge != nil: + text, err := j.Forge.File(ctx, p.Repository, "go.mod", p.Head) + if err != nil { + return nil, fmt.Errorf("go.mod of %s#%d cannot be read to place the mesh-sdk its tests read: %w", p.Repository, p.Number, err) + } + pin, ok := SDKPin(text) + if !ok { + return nil, fmt.Errorf("go.mod of %s#%d names no mesh-sdk commit, so the mesh-sdk its tests read cannot be placed", p.Repository, p.Number) + } + full, err := j.Forge.FullSHA(ctx, SDKRepository, pin) + if err != nil { + return nil, fmt.Errorf("mesh-sdk %s, which %s#%d pins, cannot be found: %w", pin, p.Repository, p.Number, err) + } + out = append(out, BesideRef{Repository: repo, Commit: full}) + default: + if member := c.member(repo); member != nil { + out = append(out, BesideRef{Repository: repo, Commit: member.Head, Ref: refOf(member.Number)}) + } else if head := heads[repo]; head != "" { + out = append(out, BesideRef{Repository: repo, Commit: head}) + } } } - return out + return out, nil } func (c *Case) member(repository string) *Pull { @@ -56,7 +97,5 @@ func refOf(number int) string { if number <= 0 { return "" } - return "refs/pull/" + itoa(number) + "/head" + return "refs/pull/" + strconv.Itoa(number) + "/head" } - -func itoa(n int) string { return strconv.Itoa(n) } diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index ec6c3aa..66edd34 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -150,7 +150,7 @@ func caseOf(t testing.TB, pulls ...delivery.CandidatePull) *Case { if err != nil { t.Fatal(err) } - cs := CaseOf(c, map[string]string{repoA: trunkA, repoSDK: trunkS}) + cs := CaseOf(c, map[string]string{repoA: trunkA, repoSDK: trunkS, "novox/mesh-controller": trunkA, "novox/mesh-catalog": trunkA}) if cs == nil { t.Fatal("no case") } @@ -552,3 +552,36 @@ func TestTheOutcomeSaysIfEachCandidatesModuleIsBuiltFailedOrWaiting(t *testing.T t.Errorf("builds = %v, want one note for each module that has a fingerprint, sorted", got) } } + +// trunksOf are trunk heads of every repository, as the gate's source names them. +func trunksOf(t *testing.T, heads map[string]string) *Trunks { + t.Helper() + data, err := delivery.EncodeSource(delivery.Source{Group: "g", Heads: heads}) + if err != nil { + t.Fatal(err) + } + tr := &Trunks{} + tr.Put(data, false) + return tr +} + +func TestNothingIsKeptForAWorkspaceGivenSiblingsBeside(t *testing.T) { // review D1, ADR 0444, issue 1287 + b := newBench(t) + b.ex.Trunks = trunksOf(t, map[string]string{repoSDK: trunkS, "novox/mesh-catalog": trunkA}) + b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) + if got := len(b.steps.tool("fetch")); got != 1 || b.steps.tool("fetch")[0].args["beside"] == nil { + t.Fatalf("the fetch was not given siblings: %+v", b.steps.tool("fetch")) + } + if keys, _ := b.kept.Keys(context.Background()); len(keys) != 0 { + t.Errorf("kept %v for a package whose tests may read a sibling the digest does not cover", keys) + } + // And a kept verdict of the same digest from elsewhere is not taken for it: everything runs. + if _, err := (Kept{Store: b.kept}).Keep(context.Background(), "unit", digA, keptValue("fixture/slow", 2)); err != nil { + t.Fatal(err) + } + b.steps.calls = nil + b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) + if n := len(b.steps.tool("test")); n != 2 { + t.Errorf("%d test steps, want both levels run whole", n) + } +} diff --git a/modules/mesh-delivery-checker/internal/check/hold.go b/modules/mesh-delivery-checker/internal/check/hold.go index eb8984b..5725a8d 100644 --- a/modules/mesh-delivery-checker/internal/check/hold.go +++ b/modules/mesh-delivery-checker/internal/check/hold.go @@ -1,40 +1,7 @@ package check -import "sync" - // PlansBuildingBucket is the controller's bucket of the delivery plans in their build phase, one key for each plan, written // by the controller alone (issue 1652): a plan key there means a delivery builds on the build nodes, and no new check starts -// meanwhile (ADR 0484 clause 12, ADR 0483 clause 6). The bucket may be absent: the controller's issue 1652 is delivered +// meanwhile (ADR 0484 clause 12, ADR 0483 clause 6). The keys age out with no delete marker, so the keys are listed, not watched. The bucket may be absent: the controller's issue 1652 is delivered // before R4c-5b, and if it is not, the bucket is read as empty. const PlansBuildingBucket = "mesh-controller_plans-building" - -// PlanHold holds which delivery plans build now, from the controller's bucket (a watch puts the changes in). Safe for use -// from the watch and from a check at once. -type PlanHold struct { - mu sync.Mutex - plans map[string]bool -} - -// Put takes a change of the bucket in: a put of a plan's key, or its delete. It answers whether a delivery builds now -// differently than before, so a watch wakes the checker when the last plan ended. -func (h *PlanHold) Put(key string, deleted bool) (changed bool) { - h.mu.Lock() - defer h.mu.Unlock() - before := len(h.plans) > 0 - if deleted { - delete(h.plans, key) - } else { - if h.plans == nil { - h.plans = map[string]bool{} - } - h.plans[key] = true - } - return before != (len(h.plans) > 0) -} - -// Held says whether any delivery plan builds now. -func (h *PlanHold) Held() bool { - h.mu.Lock() - defer h.mu.Unlock() - return len(h.plans) > 0 -} diff --git a/modules/mesh-delivery-checker/internal/check/hold_test.go b/modules/mesh-delivery-checker/internal/check/hold_test.go index 38cc8bc..35f6ef1 100644 --- a/modules/mesh-delivery-checker/internal/check/hold_test.go +++ b/modules/mesh-delivery-checker/internal/check/hold_test.go @@ -7,39 +7,17 @@ import ( "git.novox.be/novox/mesh-sdk/go/delivery" ) -func TestADeliveryThatBuildsHoldsTheChecksAndTheEndOfTheLastPlanSaysSo(t *testing.T) { // N8/AC1 - var h PlanHold - if h.Held() { - t.Fatal("an empty bucket holds") - } - if !h.Put("plan-1", false) || !h.Held() { - t.Error("a first plan did not hold") - } - if h.Put("plan-2", false) { - t.Error("a second plan changed whether a delivery builds") - } - if h.Put("plan-1", true) || !h.Held() { - t.Error("one of two plans ending released the hold") - } - if !h.Put("plan-2", true) || h.Held() { - t.Error("the last plan ending did not release the hold") - } - if h.Put("plan-3", true) { - t.Error("the delete of a plan never put changed the hold") - } -} - func TestARunningCheckIsNotStoppedWhenADeliveryStartsToBuild(t *testing.T) { // ADR 0484 clause 12, ADR 0483 clause 4 ctx := context.Background() - var h PlanHold + held := false j := &judgeFunc{f: func(_ context.Context, _ *Case, call int) (Outcome, error) { if call == 1 { - h.Put("plan-1", false) // a delivery starts while this check runs + held = true // a delivery starts while this check runs } return Outcome{Verdict: passing()}, nil }} r := newRig(t, j) - r.ch.Hold = h.Held + r.ch.Hold = func() bool { return held } r.ch.Candidate(ctx, "feat/x", candidateOf(t, "feat/x", pullOf(repoA, 7, head1))) r.ch.Wait() if v := r.verdict(t, keyOf(t, "feat/x", candidateOf(t, "feat/x", pullOf(repoA, 7, head1)))); v.State != delivery.VerdictPass { @@ -51,7 +29,7 @@ func TestARunningCheckIsNotStoppedWhenADeliveryStartsToBuild(t *testing.T) { // if j.count() != 1 { t.Errorf("judged %d times while a delivery builds, want only the one already running", j.count()) } - h.Put("plan-1", true) + held = false r.ch.Reconsider(ctx) r.ch.Wait() if j.count() != 2 { diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 49af8a4..8cdeb58 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -58,6 +58,8 @@ type Examiner struct { // Previews is the previews bucket; nil writes none. Previews KV Modules BuildModules + // Runs says which commit the mesh runs of a module (the indexer's modules); nil knows none. + Runs interface{ CommitOf(module string) string } // Forge reads the go.mod files of the Go members and says whether a mesh-sdk commit is on its trunk (ADR 0484 clause 9); // nil checks no pin. Forge Forge @@ -94,7 +96,9 @@ type target struct { fetched Fetched // priority are the Go packages (import paths) or the Node module directories whose test files the merge changes. changed []string - plans map[string]*goPlan // by level + // beside says the workspace was given siblings: its tests may read what the digest does not cover, so nothing is kept for it. + beside bool + plans map[string]*goPlan // by level } // goPlan is what a Go target has to run at one level: the digests, the packages without a kept verdict and the tests taken. @@ -238,8 +242,13 @@ func (st *state) fetchAll(ctx context.Context) *delivery.Verdict { st.nodes[node] = true args := map[string]any{"repository": p.Repository, "commit": p.Head, "ref": refOf(p.Number), "base": p.Trunk} if bm == buildrun.GoBuildModule { - if beside := st.j.Beside(st.c, p); len(beside) > 0 { + beside, err := st.j.besideFor(ctx, st.c, p) + if err != nil { + return fail(node, err.Error()) + } + if len(beside) > 0 { args["beside"] = beside + t.beside = true } } raw, err := st.step(ctx, t, "fetch", fmt.Sprintf("fetch %s#%d on %s", p.Repository, p.Number, node), args) @@ -364,6 +373,12 @@ func (st *state) planGo(ctx context.Context, t *target, level string) *delivery. } plan := &goPlan{digests: map[string]string{}} t.plans[level] = plan + if t.beside { + // The digest covers the package's own files, the toolchain and the header, not the siblings placed beside the tree: a + // package whose tests read one must never be kept, nor taken from a verdict kept for other inputs (ADR 0444). + plan.all = true + 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}) var d Digested diff --git a/modules/mesh-delivery-checker/internal/check/pins.go b/modules/mesh-delivery-checker/internal/check/pins.go index 592195d..7ce5df6 100644 --- a/modules/mesh-delivery-checker/internal/check/pins.go +++ b/modules/mesh-delivery-checker/internal/check/pins.go @@ -29,6 +29,8 @@ type Forge interface { // OnTrunk says whether commit (a full or a 12-digit sha) is already merged into the trunk of the repository: it holds no // commit the trunk does not. OnTrunk(ctx context.Context, repository, commit string) (bool, error) + // FullSHA is the 40-digit sha of a commit named by its first 12 digits. + FullSHA(ctx context.Context, repository, commit string) (string, error) } var pseudoVersion = regexp.MustCompile(`-([0-9a-f]{12})$`) diff --git a/modules/mesh-delivery-checker/internal/check/pins_test.go b/modules/mesh-delivery-checker/internal/check/pins_test.go index 3ffdc06..065dc56 100644 --- a/modules/mesh-delivery-checker/internal/check/pins_test.go +++ b/modules/mesh-delivery-checker/internal/check/pins_test.go @@ -29,6 +29,13 @@ func (f *fakeForge) File(_ context.Context, repository, path, ref string) (strin return "", fmt.Errorf("no file %s of %s at %s", path, repository, ref) } +func (f *fakeForge) FullSHA(_ context.Context, _ string, commit string) (string, error) { + if f.err != nil { + return "", f.err + } + return commit + strings.Repeat("0", 40-len(commit)), nil +} + func (f *fakeForge) OnTrunk(_ context.Context, _ string, commit string) (bool, error) { if f.err != nil { return false, f.err @@ -145,3 +152,53 @@ func TestAPinThatCannotBeCheckedFailsAndNeverPasses(t *testing.T) { // ADR 0453 t.Errorf("verdict = %+v", v) } } + +type runsAt map[string]string + +func (r runsAt) CommitOf(module string) string { return r[module] } + +func besideOf(t *testing.T, b *bench, repository string) map[string]BesideRef { + t.Helper() + out := map[string]BesideRef{} + for _, f := range b.steps.tool("fetch") { + if f.args["repository"] == repository { + refs, _ := f.args["beside"].([]BesideRef) + for _, r := range refs { + out[r.Repository] = r + } + } + } + return out +} + +func TestTheSiblingsAreWhereTheTestsStateThem(t *testing.T) { // review D2, ADR 0484 clause 4 + const controller, catalogue, host = "novox/mesh-controller", "novox/mesh-catalog", "novox/mesh-host" + hostRuns := strings.Repeat("7", 40) + b := newBench(t) + b.ex.Trunks = trunksOf(t, map[string]string{controller: trunkA, catalogue: trunkS, host: trunkA2, SDKRepository: "9999999999999999999999999999999999999999"}) + b.ex.Runs = runsAt{"mesh-host": hostRuns} + forge := &fakeForge{goMods: map[string]string{controller + " go.mod " + head1: capturedGoMod(t, "cb54756c3865")}, onTrunk: map[string]bool{"cb54756c3865": true}} + b.ex.Forge = forge + // A group of the controller and the catalogue: the controller's tests read the catalogue's trunk, not the group's head. + b.judge(t, caseOf(t, pullOf(controller, 7, head1), pullOf(catalogue, 12, head2))) + got := besideOf(t, b, controller) + if c := got[catalogue]; c.Commit != trunkS || c.Ref != "" { + t.Errorf("the catalogue beside the controller = %+v, want its trunk %s", c, trunkS) + } + if h := got[host]; h.Commit != hostRuns { + t.Errorf("the node-engine beside the controller = %+v, want the commit the mesh runs %s", h, hostRuns) + } + if s := got[SDKRepository]; s.Commit != "cb54756c3865"+strings.Repeat("0", 28) { + t.Errorf("mesh-sdk beside the controller = %+v, want the commit its go.mod pins, whole", s) + } +} + +func TestThePinOfTheControllerThatCannotBeReadFailsTheCheck(t *testing.T) { // review D2 + b := newBench(t) + b.ex.Trunks = trunksOf(t, map[string]string{"novox/mesh-controller": trunkA}) + b.ex.Forge = &fakeForge{goMods: map[string]string{}, onTrunk: map[string]bool{}} + v := b.judge(t, caseOf(t, pullOf("novox/mesh-controller", 7, head1))).Verdict + if v.State != delivery.VerdictFail || !strings.Contains(v.Failed.Words, "go.mod") { + t.Errorf("verdict = %+v", v) + } +} diff --git a/modules/mesh-delivery-checker/internal/check/testdata/captured/README b/modules/mesh-delivery-checker/internal/check/testdata/captured/README index 9c809ea..58a30ed 100644 --- a/modules/mesh-delivery-checker/internal/check/testdata/captured/README +++ b/modules/mesh-delivery-checker/internal/check/testdata/captured/README @@ -17,3 +17,5 @@ novox/mesh-delivery at main, and forge-compare-trunk.json the answer of gitea_co trunk: no commit main does not hold); both called through the mesh on 2026-10-21 for issue 1624. forge-compare-branch.json is the answer of the same call for a head that is not on the trunk (novox/mesh-sdk base main, head afbe371003a5, the branch feat/r4c5-sdk), shortened to two of its seven commits and one of its twenty-six files, with the head and the totals set to match. + +forge-commits.json is the answer of the forge's gitea_commits for novox/mesh-sdk, ref cb54756c3865, limit 1, called through the mesh on 2026-10-21 (review D2). diff --git a/modules/mesh-delivery-checker/internal/check/testdata/captured/forge-commits.json b/modules/mesh-delivery-checker/internal/check/testdata/captured/forge-commits.json new file mode 100644 index 0000000..813ec62 --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/testdata/captured/forge-commits.json @@ -0,0 +1,14 @@ +{ + "commits": [ + { + "author": "mesh-tools", + "date": "2026-10-21T10:02:09Z", + "sha": "cb54756c3865df5ac9c0dc0481ad9ebe2bf013a1", + "subject": "Merge pull request 'Give delivery.Module the toolchains its fingerprint was computed with, and '.' as a root module's path, so the builder builds a key only with its own toolchain (issue 1615)' (#55) from fix/1615-module-toolchains-root into main" + } + ], + "more": true, + "page": 1, + "ref": "cb54756c3865", + "repository": "novox/mesh-sdk" +} diff --git a/modules/mesh-delivery-checker/manifest_test.go b/modules/mesh-delivery-checker/manifest_test.go index 5370e5c..c3d4592 100644 --- a/modules/mesh-delivery-checker/manifest_test.go +++ b/modules/mesh-delivery-checker/manifest_test.go @@ -100,7 +100,7 @@ func TestManifestDeclarations(t *testing.T) { // 1678/AC1 "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules", "mesh-delivery-watchman.presence", "node-engine.have"}; !reflect.DeepEqual(sorted(m.Reads), want) { t.Errorf("reads = %v, want %v", m.Reads, want) } - if want := []string{"gitea.gitea_compare", "gitea.gitea_get_file", "mesh-build-go.build_go_digest", "mesh-build-go.build_go_fetch", "mesh-build-go.build_go_test", "mesh-build-node.build_node_fetch", "mesh-build-node.build_node_test"}; !reflect.DeepEqual(sorted(m.Invokes), want) { + if want := []string{"gitea.gitea_commits", "gitea.gitea_compare", "gitea.gitea_get_file", "mesh-build-go.build_go_digest", "mesh-build-go.build_go_fetch", "mesh-build-go.build_go_test", "mesh-build-node.build_node_fetch", "mesh-build-node.build_node_test"}; !reflect.DeepEqual(sorted(m.Invokes), want) { t.Errorf("invokes = %v, want only the build modules' fetch, digest and test (no run step before issue 1309, ADR 0484 clause 5) and the forge's two reads for a mesh-sdk pin (clause 9)", m.Invokes) } // The checker is woken by a build module's report of a toolchain, as the indexer is: the bucket may not exist when it starts. diff --git a/modules/mesh-delivery-checker/module.json b/modules/mesh-delivery-checker/module.json index 80b7351..87c1005 100644 --- a/modules/mesh-delivery-checker/module.json +++ b/modules/mesh-delivery-checker/module.json @@ -35,6 +35,7 @@ "checker_verdicts" ], "invokes": [ + "gitea.gitea_commits", "gitea.gitea_compare", "gitea.gitea_get_file", "mesh-build-go.build_go_digest",