Fix the six tests the merge gate refused: assignments beside pulled-in modules, migration tests on the tables, the event compared as JSON, the restarted move settled to the table (issues 1090, 1092, 1093, 1095)

cmd/mesh-controller/chain_integration_test.go

@@ -108,10 +108,14 @@ func TestOneReadOfAnAssignmentNamesItsCommitBuildAndRolloutAndOneReadOfTheRollou
 
 	// One read of the assignment.
 	on, err := inv.AssignmentsOn(ctx, "anchor")
-	if err != nil || len(on) != 1 {
+	a, ok := assignmentOf(on, "notes")
+	if err != nil || !ok {
 		t.Fatalf("the assignments of anchor: %+v %v", on, err)
 	}
-	a := on[0]
+	// Beside it a module pulled in by the assignment, which no send of this walk put anywhere.
+	if wireguard, pulled := assignmentOf(on, "mesh-wireguard"); pulled && (wireguard.Build != "" || wireguard.Rollout != "") {
+		t.Fatalf("a module this rollout did not send names a build or a rollout: %+v", wireguard)
+	}
 	if a.Module != "notes" || a.Commit != "c-notes" || a.Build != buildID || a.Rollout != rollout || a.Artifact != digest ||
 		a.Since.IsZero() || a.Assigned.IsZero() || !a.Stands() {
 		t.Fatalf("the assignment names %+v, not commit c-notes, build %s, artifact %s, rollout %s", a, buildID, digest, rollout)
@@ -169,21 +173,37 @@ func TestASendOfWhatAMachineRunsAlreadyChangesNoLinkOfItsAssignment(t *testing.T
 		t.Fatal(err)
 	}
 	on, _ := inv.AssignmentsOn(ctx, "anchor")
-	if len(on) != 1 || on[0].Rollout != "plan-1" || on[0].Build != "build-1" || on[0].Commit != "c1" {
+	if a, _ := assignmentOf(on, "notes"); a.Rollout != "plan-1" || a.Build != "build-1" || a.Commit != "c1" {
 		t.Fatalf("the assignment names %+v", on)
 	}
 	// A declaration sent by hand with no builds known says nothing of them.
 	if err := inv.RecordSent(ctx, node, "d3", nil); err != nil {
 		t.Fatal(err)
 	}
-	if on, _ = inv.AssignmentsOn(ctx, "anchor"); len(on) != 1 || on[0].Commit != "c1" {
+	if on, _ = inv.AssignmentsOn(ctx, "anchor"); runOf(on, "notes").Commit != "c1" {
 		t.Fatalf("a send of unknown builds changed %+v", on)
 	}
 	// A module sent a commit no build is kept of is named by its commit alone.
 	if err := inv.RecordSent(ctx, node, "d4", map[string]string{"notes": "c2"}); err != nil {
 		t.Fatal(err)
 	}
-	if on, _ = inv.AssignmentsOn(ctx, "anchor"); len(on) != 1 || on[0].Commit != "c2" || on[0].Build != "" || on[0].Rollout != "" {
+	if on, _ = inv.AssignmentsOn(ctx, "anchor"); runOf(on, "notes").Commit != "c2" || runOf(on, "notes").Build != "" ||
+		runOf(on, "notes").Rollout != "" {
 		t.Fatalf("the assignment after a push of c2 by hand names %+v", on)
 	}
 }
+
+// assignmentOf is one module's assignment among those of a machine: assigning a module assigns what it requires beside it.
+func assignmentOf(on []chain.Assignment, module string) (chain.Assignment, bool) {
+	for _, a := range on {
+		if a.Module == module {
+			return a, true
+		}
+	}
+	return chain.Assignment{}, false
+}
+
+func runOf(on []chain.Assignment, module string) chain.Assignment {
+	a, _ := assignmentOf(on, module)
+	return a
+}

internal/inventory/chain_move_test.go

@@ -3,6 +3,7 @@
 package inventory
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
@@ -10,6 +11,7 @@ import (
 	"slices"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -35,6 +37,15 @@ func aBuildRow(t *testing.T, inv *Inventory, id, module, commit, failed string)
 		id, module, commit, failed)
 }
 
+// inTheTables is an inventory before the move: its builds and assignments are rows of the store, which a test of a data
+// migration of the store (that reads those rows) needs.
+func inTheTables(t *testing.T) *Inventory {
+	t.Helper()
+	inv, finish := ForTestBefore(t, 105)
+	finish()
+	return inv
+}
+
 // reportOf is the report of one kind among those a move made, which are one for each kind in the order they are moved.
 func reportOf(t *testing.T, reports []MoveReport, kind string) MoveReport {
 	t.Helper()
@@ -131,8 +142,24 @@ func TestTheMoveCopiesTheBuildRowsAndAgainWritesNothing(t *testing.T) {
 	}
 }
 
-// **A move that stopped half way is finished by the next start, and never lays a row over a newer bucket value** (ADR
+// failingToCreate is a bucket that fails to make one key while told to: the bus going away half way through a move.
-// 0408 §8): the keys that exist are present, the others are written.
+type failingToCreate struct {
+	chain.Backend
+	key  string
+	fail *atomic.Bool
+}
+
+func (f failingToCreate) Create(ctx context.Context, key string, value []byte) (uint64, error) {
+	if key == f.key && f.fail.Load() {
+		return 0, errors.New("the bus went away")
+	}
+	return f.Backend.Create(ctx, key, value)
+}
+
+// **A controller restarted half way through the move finishes it: the bucket equals the table, with no key made twice and
+// no refusal** (ADR 0408 §8). Before a kind is recorded as moved nothing but the move writes its keys, and every write of
+// the kind goes to its table (each method of a kind asks ChainReads first and writes the store until it says moved), so the
+// table is the only truth: a key the first start made is the row it was, and the second start settles it to the row it is.
 func TestAControllerRestartedMidMoveFinishesItAndOverwritesNothing(t *testing.T) {
 	inv, finish := ForTestBefore(t, 105)
 	finish()
@@ -140,25 +167,54 @@ func TestAControllerRestartedMidMoveFinishesItAndOverwritesNothing(t *testing.T)
 	aBuildRow(t, inv, "build-1", "notes", "c1", "")
 	aBuildRow(t, inv, "build-2", "notes", "c2", "")
 	aBuildRow(t, inv, "build-3", "wiki", "c3", "")
-	// An earlier start wrote build-1, and the bucket has had a newer value since.
+	var away atomic.Bool
-	newer := chain.Build{ID: "build-1", Repository: "novox/notes", Module: "notes", Commit: "newer"}
+	away.Store(true)
-	if _, err := inv.chain.Builds.Create(ctx, "build-1", newer); err != nil {
+	builds := chain.NewMemory()
-		t.Fatal(err)
+	inv.UseChain(chain.New(failingToCreate{builds, "build-3", &away}, chain.NewMemory(), chain.NewMemory(),
+		func(context.Context) error { return nil }))
+	// The first start made two of the three keys and the bus went away.
+	_, err := inv.MoveChain(ctx, nil)
+	var refusal *Refusal
+	if err == nil || errors.As(err, &refusal) {
+		t.Fatalf("the first start answered %v: it should fail to be tried again, not be refused", err)
 	}
+	if moved, _ := inv.ChainMoved(ctx); len(moved) != 0 {
+		t.Fatalf("a move that did not finish recorded %v", moved)
+	}
+	if keys, _ := builds.Keys(ctx); len(keys) != 2 {
+		t.Fatalf("the first start made %v", keys)
+	}
+	// The controller is started again, the bus is back, and the move finishes.
+	away.Store(false)
 	reports, err := inv.MoveChain(ctx, nil)
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("the second start was refused or failed: %v", err)
 	}
-	if b := reportOf(t, reports, KindBuilds); b.Written != 2 || b.Present != 1 || b.Rows != 3 {
+	if b := reportOf(t, reports, KindBuilds); b.Written != 1 || b.Present != 2 || b.Rows != 3 {
 		t.Fatalf("the move reported %+v", b)
 	}
-	got, _, _ := inv.chain.Builds.Get(ctx, "build-1")
-	if !reflect.DeepEqual(got.Value, newer) {
-		t.Fatalf("the newer value was laid over: %+v", got.Value)
-	}
 	if moved, _ := inv.ChainMoved(ctx); moved[KindBuilds] != 3 {
 		t.Fatalf("chain_moved says %v", moved)
 	}
+	// The bucket is the table: three keys, each made once and not again, each the row's own.
+	for _, id := range []string{"build-1", "build-2", "build-3"} {
+		history, err := builds.History(ctx, id)
+		if err != nil || len(history) != 1 {
+			t.Fatalf("%s has %d values: %v", id, len(history), err)
+		}
+	}
+	rows, err := inv.allBuildRows(ctx)
+	if err != nil || len(rows) != 3 {
+		t.Fatalf("the table holds %d builds: %v", len(rows), err)
+	}
+	for _, row := range rows {
+		got, found, err := inv.chain.Builds.Get(ctx, row.ID)
+		want, _ := json.Marshal(toChainBuild(row))
+		have, _ := json.Marshal(got.Value)
+		if err != nil || !found || string(want) != string(have) {
+			t.Fatalf("%s in the bucket is %s, the row says %s (%v)", row.ID, have, want, err)
+		}
+	}
 }
 
 // **A value over the bus's largest message stops the move with nothing written and nothing switched** (ADR 0408 §6):
@@ -528,7 +584,7 @@ func TestTheMoveCarriesThePlansAndTheirMergesAndTheyReadTheSame(t *testing.T) {
 	}
 	nowMerge, known, _ := inv.MergeOf(ctx, "novox/mesh-catalog", "m-walk")
 	if !known || nowMerge.Plan != "plan-2" || nowMerge.Branch != wasMerge.Branch || !nowMerge.Merged.Equal(wasMerge.Merged) ||
-		!nowMerge.Heard.Equal(wasMerge.Heard) || string(nowMerge.Event) != string(wasMerge.Event) {
+		!nowMerge.Heard.Equal(wasMerge.Heard) || !sameEvent(nowMerge.Event, wasMerge.Event) {
 		t.Fatalf("a merge was %+v and is %+v", wasMerge, nowMerge)
 	}
 	alone, _ := inv.AloneMerges(ctx)
@@ -884,3 +940,10 @@ func TestARowChangedAfterTheFirstPassIsInTheBucketWhenTheKindIsRecordedMoved(t *
 		t.Fatalf("the next save of the plan is refused: %v", err)
 	}
 }
+
+// sameEvent says two encodings of the forge's announcement are one: the store keeps it as jsonb, which spells it with its own
+// white space, and whoever reads it (the cut, mesh-delivery) decodes it, so the white space is no part of what it says.
+func sameEvent(a, b json.RawMessage) bool {
+	var x, y bytes.Buffer
+	return json.Compact(&x, a) == nil && json.Compact(&y, b) == nil && x.String() == y.String()
+}

internal/inventory/mirrored_test.go

@@ -151,7 +151,7 @@ func TestAPersonRecordsACopyAndTheFirstRecordStands(t *testing.T) {
 // The migration records the copies the build records already name: only a module's own on-<argument>
 // repository, and only once, by the earliest build naming it.
 func TestTheCopiesTheRecordsAlreadyNameAreRecorded(t *testing.T) {
-	inv := fresh(t)
+	inv := inTheTables(t)
 	ctx := context.Background()
 	copyRef := mirrorRef("route-proxy/on-go_base", 1)
 	notACopy := ref("go-base", "build", 2)

internal/inventory/pin_completed_test.go

@@ -34,7 +34,7 @@ func completeEarlierPins(t *testing.T, inv *Inventory) {
 }
 
 func TestAPinMadeBeforeIsCompletedWhenTheNodeAnswersOnce(t *testing.T) {
-	inv := fresh(t)
+	inv := inTheTables(t)
 	ctx := context.Background()
 	for _, n := range []string{"user", "provider"} {
 		if _, err := inv.AddNode(ctx, n); err != nil {
@@ -61,7 +61,7 @@ func TestAPinMadeBeforeIsCompletedWhenTheNodeAnswersOnce(t *testing.T) {
 }
 
 func TestAPinMadeBeforeIsLeftOpenWhenTheNodeAnswersTwice(t *testing.T) {
-	inv := fresh(t)
+	inv := inTheTables(t)
 	ctx := context.Background()
 	for _, n := range []string{"user", "provider"} {
 		if _, err := inv.AddNode(ctx, n); err != nil {