Build a change once: the check builds what its merge moves, the walk takes it by key

A merge built every moved module a second time although its check had judged the same tree (issue 649). The check now builds into the scratch namespace under a reuse key (source fingerprint plus the builder's own digest), and the walk of the trunk commit copies a build of the same key by digest instead of building, saying per module why when it cannot (novox/hq ADR 0350).
2026-10-13 23:04:40 +02:00
parent bff5b7f0e4
commit cd49b50c40
23 changed files with 2195 additions and 13 deletions
@@ -0,0 +1,190 @@
+package main
+
+import (
+	"context"
+	"encoding/json"
+	"reflect"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/builder"
+	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/inventory"
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// One build per change (novox/hq ADR 0350), as the controller asks it and takes it in.
+
+// The check of a change is asked to build what a merge of it would move, in the walk's order: a module of the
+// repository checked from the merge, one standing on it from its own repository as the walk builds it, and one
+// that reads the repository through a context not at all. A change that moves no module builds nothing.
+func TestACheckIsAskedToBuildWhatTheMergeWouldMove(t *testing.T) {
+	entries := []inventory.Entry{
+		{Manifest: catalogue.Manifest{Module: "app"}, Source: inventory.Source{Repository: "novox/shop", Seat: "git", Path: "modules/app"}},
+		{Manifest: catalogue.Manifest{Module: "web"}, Source: inventory.Source{Repository: "novox/web", Seat: "git", Ref: "main"}},
+		{Manifest: catalogue.Manifest{Module: "docs"}, Source: inventory.Source{Repository: "novox/docs", Seat: "git"}},
+	}
+	p := link.PullUpdated{Owner: "novox", Repo: "shop", Number: 7, Base: "main", Commit: "abc"}
+	scope := checkScope{Reach: mergeReach{Packaging: entries[2:], Plan: inventory.Plan{Tiers: [][]string{{"app", "docs"}, {"web"}}}}}
+	clone := func(s inventory.Source) (string, error) { return "http://forge.invalid/" + s.Repository + ".git", nil }
+
+	got := checkBuildsOf(p, scope, entries, clone)
+	want := []link.CheckBuild{
+		{Module: "app", Path: "modules/app", Merged: true, Tier: 0},
+		{Module: "docs", Tier: 0, Skip: "its build reads novox/shop through a context, which a check cannot hand the merge"},
+		{Module: "web", Repository: "http://forge.invalid/novox/web.git", Ref: "main", Tier: 1},
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("the check is asked to build\n%+v\nwant\n%+v", got, want)
+	}
+	if got := checkBuildsOf(p, checkScope{}, entries, clone); got != nil {
+		t.Fatalf("a change that moves no module asks for builds: %+v", got)
+	}
+}
+
+// A walk's ask carries its leave to take a check's build, naming the pull requests of the merges it answers; a dry
+// run never does, and neither does an ask that is not a walk's.
+func TestOnlyAWalksAskTakesAChecksBuildAndADryRunNever(t *testing.T) {
+	p := &inventory.Plan{Delivery: &inventory.PlanDelivery{Merges: []inventory.PlanMerge{
+		{Repository: "novox/shop", Number: 7}, {Repository: "http://forge.invalid/novox/web.git", Number: 3},
+		{Repository: "novox/shop", Number: 7}, {Repository: "novox/shop"}}}}
+	asked := reuseAskOf(p)
+	if want := []link.PullRef{{Owner: "novox", Repo: "shop", Number: 7}, {Owner: "novox", Repo: "web", Number: 3}}; !reflect.DeepEqual(asked.Pulls, want) {
+		t.Fatalf("the walk names %+v", asked.Pulls)
+	}
+	walk := withReuse(t.Context(), asked)
+	if reuseAsked(walk, false) != asked {
+		t.Fatal("a walk's ask does not carry its leave")
+	}
+	if reuseAsked(walk, true) != nil {
+		t.Fatal("a dry run carries a leave to take a check's build")
+	}
+	if reuseAsked(t.Context(), false) != nil {
+		t.Fatal("an ask that is not a walk's carries a leave")
+	}
+}
+
+// A walk ended lets go of its merges' check builds — only once it ended, after it took what it reuses.
+func TestAWalkEndedLetsGoOfItsMergesCheckBuilds(t *testing.T) {
+	var forgot []builder.PullRef
+	was := forgetChecks
+	forgetChecks = func(_ context.Context, _ *inventory.Inventory, pull builder.PullRef) (string, error) {
+		forgot = append(forgot, pull)
+		return "", nil
+	}
+	t.Cleanup(func() { forgetChecks = was })
+	p := inventory.Plan{ID: "plan-1", Delivery: &inventory.PlanDelivery{Merges: []inventory.PlanMerge{
+		{Repository: "novox/shop", Number: 7}}}}
+
+	p.WalkSaid = inventory.WalkStarted
+	forgetChecksOfWalk(t.Context(), nil, p)
+	p.WalkSaid = ""
+	forgetChecksOfWalk(t.Context(), nil, p)
+	if len(forgot) != 0 {
+		t.Fatalf("a walk not ended let go of %v, which it may still take", forgot)
+	}
+	p.WalkSaid = inventory.WalkEnded
+	forgetChecksOfWalk(t.Context(), nil, p)
+	if want := []builder.PullRef{{Owner: "novox", Repo: "shop", Number: 7}}; !reflect.DeepEqual(forgot, want) {
+		t.Fatalf("the walk ended let go of %v", forgot)
+	}
+}
+
+// A build in the scratch namespace is never registered: only the walk's copy in the module's own namespace is.
+func TestABuildInTheScratchNamespaceIsNeverRegistered(t *testing.T) {
+	if got := inScratch([]link.MadeArtifact{{Name: "server", Reference: "store.internal:5000/app/server@sha256:aa"}}); got != "" {
+		t.Fatalf("a module's own build reads as a scratch build: %s", got)
+	}
+	scratch := "store.internal:5000/scratch/app/server@sha256:aa"
+	if got := inScratch([]link.MadeArtifact{{Name: "server", Reference: scratch}}); got != scratch {
+		t.Fatalf("a scratch build was not found: %q", got)
+	}
+	if got := inScratch([]link.MadeArtifact{{Name: "look", Reference: "http://store.internal:5000/v2/scratch/app/look/blobs/sha256:bb"}}); got == "" {
+		t.Fatal("a scratch archive was not found")
+	}
+}
+
+// The walk says per module what it did: "reused from …" or "built: <why>" — and a build seat from before says
+// neither, which is said.
+func TestAWalksBuildSaysWhetherItReusedOrBuilt(t *testing.T) {
+	if got := (link.BuildResult{Reused: "reused from the check of novox/shop#7"}).MadeWords(); got != "reused from the check of novox/shop#7" {
+		t.Fatalf("said %q", got)
+	}
+	if got := (link.BuildResult{BuiltWhy: "no check built it"}).MadeWords(); got != "built: no check built it" {
+		t.Fatalf("said %q", got)
+	}
+	if got := (link.BuildResult{}).MadeWords(); !strings.Contains(got, "predates ADR 0350") {
+		t.Fatalf("said %q", got)
+	}
+}
+
+// A walk's module the check's build stood in for is registered by the digest the check built — the copy in the
+// module's namespace — and the walk keeps "reused from …" for it; its first-node gate judges that registered
+// build as it judges one the walk built. A scratch reference is refused. Needs the store (MESH_TEST_POSTGRES), and
+// is named in testdata/store-tests.
+func TestAReusedCheckBuildIsRegisteredByItsDigestAndGatedAsABuiltOne(t *testing.T) {
+	g := aGateMesh(t)
+	ctx := t.Context()
+	inv := g.open.inventory
+	gateEvery = 0
+
+	digest := "sha256:" + strings.Repeat("c", 64)
+	reused := func(id, reference string) link.BuildResult {
+		manifest := mustJSON(t, catalogue.Manifest{Module: "app", Version: "c3", Resources: []map[string]any{
+			{"id": "svc", "type": "container", "name": "app", "image": reference}}})
+		return link.BuildResult{ID: id, Repository: "novox/mesh-catalog", Path: "modules/app", Ref: "main", On: "anchor",
+			Commit: "c3", Manifest: manifest, Trunk: "main", OnTrunk: true, Branches: []string{"main"},
+			Made:   []link.MadeArtifact{{Name: "server", Kind: "image", Reference: reference}},
+			Source: &link.SourceOnSeat{Seat: "git", Repository: "novox/mesh-catalog"},
+			Reused: "reused from the check of novox/mesh-catalog#9 at head abcdefabcdef (merged into trunk 123456123456): server " + digest,
+			ReusedFrom: &link.ReusedFrom{Pull: link.PullRef{Owner: "novox", Repo: "mesh-catalog", Number: 9},
+				Head: "abcdefabcdef", Trunk: "123456123456", Key: "r1:" + strings.Repeat("1", 64)}}
+	}
+	if _, _, err := takeIn(ctx, inv, reused(link.NewBuildID(time.Now()), "store.internal:5000/scratch/app/server@"+digest)); err == nil ||
+		!strings.Contains(err.Error(), "scratch") {
+		t.Fatalf("a build in the scratch namespace was registered: %v", err)
+	}
+	id := link.NewBuildID(time.Now().Add(time.Second))
+	result := reused(id, "store.internal:5000/app/server@"+digest)
+	if _, _, err := takeIn(ctx, inv, result); err != nil {
+		t.Fatal(err)
+	}
+	current, err := inv.CurrentBuilds(ctx)
+	if err != nil || current["app"].Commit != "c3" {
+		t.Fatalf("the reused build is not what app is registered at: %+v %v", current["app"], err)
+	}
+	shelf, err := inv.Catalogue(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	raw, _ := json.Marshal(shelf["app"].Resources)
+	if !strings.Contains(string(raw), "app/server@"+digest) {
+		t.Fatalf("app is registered as %s, not by the check's digest in its own namespace", raw)
+	}
+
+	// The walk asked it: the walk keeps what its build did, and its gate judges the registered build.
+	asked := time.Now().UTC()
+	p := g.plan(t)
+	p.Commit = "c3"
+	p.Modules["app"] = &inventory.PlanModule{State: "asked", AskedAt: &asked, Build: id}
+	if err := inv.SavePlan(ctx, &p); err != nil {
+		t.Fatal(err)
+	}
+	planBuiltSaying(ctx, g.open, "app", "c3", "", asked, id, result.MadeWords())
+	p = g.plan(t)
+	if m := p.Modules["app"]; m.State != "built" || !strings.HasPrefix(m.Made, "reused from the check of novox/mesh-catalog#9") {
+		t.Fatalf("the walk keeps %+v", m)
+	}
+	for i := 0; i < 6; i++ {
+		advancePlans(ctx, g.open)
+	}
+	p = g.plan(t)
+	gate := p.Modules["app"].Gate
+	if gate == nil || gate.To != "c3" || gate.Verdict != inventory.GatePassed {
+		t.Fatalf("the gate judged %+v, not the reused build", gate)
+	}
+	if sent, _, _ := inv.SentBuilds(ctx, "anchor"); sent["app"] != "c3" {
+		t.Fatalf("the first machine was sent %v, not the reused build", sent)
+	}
+}