From 4f5fab81fe9e0b34005f7ab7f2c06c94ba6e9b34 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 03:25:27 +0200 Subject: [PATCH] Read the SDK fixtures at the pin of the tree under check, not the running controller's (issue 449 review) A pull request moving the SDK passed its check against the old SDK and then failed everywhere once it rolled out. In a merge check the test now reads the clone's history at the tree's own go.mod pin, and holds the captured copy to the clone byte for byte. --- cmd/mesh-controller/checks.go | 6 +- internal/link/conformance_sources_test.go | 128 ++++++++++++++++++ internal/link/conformance_test.go | 152 +++++++++++++++++++--- 3 files changed, 264 insertions(+), 22 deletions(-) create mode 100644 internal/link/conformance_sources_test.go diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index 4cbb75b3..bb78c718 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -276,8 +276,10 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop // builds from; and beside the controller its main, for a judge the running controller predates, and the // lab's main, whose replays every check runs. **One rule, read by the check the controller asks for and by // the facts snapshot** (Facts.Beside), so a check run by hand clones what the build seat clones. Beside the -// controller also the SDK, at the commit the controller's go.mod pins (sdkPinned): its conformance fixtures -// are judged against the SDK the controller is built with, not one a desktop holds (novox/hq issue 449). +// controller also the SDK, checked out at the commit the running controller's go.mod pins (sdkPinned), not +// one a desktop holds (novox/hq issue 449). The checkout only places the clone: the conformance test reads the +// fixtures at the pin of the tree under check, from the clone's history, so a pull request moving the SDK is +// judged against the SDK it moves to (internal/link/conformance_test.go). func besideRefs(dir, running string) map[string]string { switch dir { case "mesh-catalog": diff --git a/internal/link/conformance_sources_test.go b/internal/link/conformance_sources_test.go new file mode 100644 index 00000000..53e40484 --- /dev/null +++ b/internal/link/conformance_sources_test.go @@ -0,0 +1,128 @@ +package link + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// Where the conformance fixtures are read from, held on a small SDK repository of the test's own: two +// commits, the clone checked out at the older as the build seat leaves it at the running controller's pin +// (novox/hq issue 449). The contents are the test's, because what is judged is which commit is read. +type sdkBed struct { + root, goMod, captured, capturedLog string + older, newer string +} + +const fixtureName = "events/module-event.json" + +func newSDKBed(t *testing.T) sdkBed { + t.Helper() + if _, err := exec.LookPath("git"); err != nil { + t.Fatalf("git is needed to read the SDK at a pin, as a merge check does: %v", err) + } + b := sdkBed{root: t.TempDir()} + clone := filepath.Join(b.root, "mesh-sdk") + git := func(args ...string) string { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", clone, "-c", "user.name=t", "-c", "user.email=t@t", + "-c", "commit.gpgsign=false"}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + write := func(dir, body string) { + t.Helper() + file := filepath.Join(dir, "conformance", filepath.FromSlash(fixtureName)) + if err := os.MkdirAll(filepath.Dir(file), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(file, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + if err := os.MkdirAll(clone, 0o755); err != nil { + t.Fatal(err) + } + git("init", "--quiet") + write(clone, "older\n") + git("add", "-A") + git("commit", "--quiet", "-m", "older") + b.older = git("rev-parse", "HEAD") + write(clone, "newer\n") + git("commit", "--quiet", "-am", "newer") + b.newer = git("rev-parse", "HEAD") + git("checkout", "--quiet", b.older) + + other := t.TempDir() + b.captured = filepath.Join(other, "mesh-sdk") + write(b.captured, "older\n") + b.capturedLog = filepath.Join(other, "CAPTURED") + if err := os.WriteFile(b.capturedLog, []byte("mesh-sdk "+b.older+" conformance/events\n"), 0o644); err != nil { + t.Fatal(err) + } + b.goMod = filepath.Join(other, "go.mod") + b.pin(t, b.newer) + return b +} + +func (b sdkBed) pin(t *testing.T, commit string) { + t.Helper() + mod := "module x\n\nrequire (\n\t" + sdkModule + " v0.1.11-0.20261009143344-" + commit[:12] + "\n)\n" + if err := os.WriteFile(b.goMod, []byte(mod), 0o644); err != nil { + t.Fatal(err) + } +} + +// A pull request moving the SDK is judged against the SDK it moves to: the clone is read at the tree's own +// pin, not at what it is checked out at, the running controller's. +func TestAMergeCheckReadsTheSDKAtTheTreesOwnPin(t *testing.T) { + b := newSDKBed(t) + raw, from, err := sdkFixture(b.root, b.goMod, b.captured, b.capturedLog, fixtureName) + if err != nil { + t.Fatal(err) + } + if string(raw) != "newer\n" { + t.Errorf("read %q from %s; the tree pins the newer commit", raw, from) + } +} + +// A captured copy that is not the SDK's at the commit CAPTURED names fails a merge check. +func TestAMergeCheckFailsACapturedCopyEditedByHand(t *testing.T) { + b := newSDKBed(t) + if err := os.WriteFile(filepath.Join(b.captured, "conformance", filepath.FromSlash(fixtureName)), + []byte("edited\n"), 0o644); err != nil { + t.Fatal(err) + } + if _, _, err := sdkFixture(b.root, b.goMod, b.captured, b.capturedLog, fixtureName); err == nil || + !strings.Contains(err.Error(), "is not mesh-sdk's at") { + t.Errorf("a hand-edited captured copy was accepted: %v", err) + } +} + +// A pin the clone does not hold, or no clone at all, fails loudly and names what is missing. +func TestAMergeCheckFailsWithoutTheSDKAtItsPin(t *testing.T) { + b := newSDKBed(t) + b.pin(t, "0123456789ab0123456789ab0123456789ab0123") + if _, _, err := sdkFixture(b.root, b.goMod, b.captured, b.capturedLog, fixtureName); err == nil || + !strings.Contains(err.Error(), "0123456789ab") { + t.Errorf("a pin the clone lacks was not named: %v", err) + } + if _, _, err := sdkFixture(t.TempDir(), b.goMod, b.captured, b.capturedLog, fixtureName); err == nil || + !strings.Contains(err.Error(), "not beside this check") { + t.Errorf("a missing clone was not refused: %v", err) + } +} + +// Away from a merge check the captured copy is read, and nothing else. +func TestAwayFromACheckTheCapturedCopyIsRead(t *testing.T) { + b := newSDKBed(t) + raw, _, err := sdkFixture("", b.goMod, b.captured, b.capturedLog, fixtureName) + if err != nil || string(raw) != "older\n" { + t.Errorf("read %q, %v; the captured copy holds the older", raw, err) + } +} diff --git a/internal/link/conformance_test.go b/internal/link/conformance_test.go index edc3f148..66c7471e 100644 --- a/internal/link/conformance_test.go +++ b/internal/link/conformance_test.go @@ -1,8 +1,12 @@ package link import ( + "bytes" "encoding/json" + "fmt" "os" + "os/exec" + "path" "path/filepath" "regexp" "strings" @@ -16,10 +20,19 @@ import ( // // **Read from the SDK's own conformance directory, never copied by hand**: a fixture written into each // implementation is two fixtures, and two fixtures drift, which is the failure the suite exists to prevent. -// Which SDK is read is named, never the one a desktop happens to hold beside this checkout (novox/hq issue -// 449): in a merge check the SDK the build seat clones beside it, at the commit this controller's go.mod -// pins, and a missing clone fails the test; elsewhere the copy captured in testdata/beside at that same -// commit (internal/beside, testdata/beside/CAPTURED), held to go.mod by TestTheCapturedSDKIsTheOneGoModPins. +// Which SDK is read is the one this tree's go.mod pins, never the one a desktop happens to hold beside this +// checkout (novox/hq issue 449): +// +// - in a merge check, the fixture as it stands at that pin in the mesh-sdk clone the build seat places +// beside the check — read with `git show`, because the clone is checked out at the *running* +// controller's pin, and a pull request moving the SDK must be judged against the SDK it moves to, or it +// passes the check and fails everywhere after it rolls out. A missing clone or a pin the clone lacks +// fails the test; +// - elsewhere, the copy captured in testdata/beside at the commit testdata/beside/CAPTURED names, held to +// go.mod by TestTheCapturedSDKIsTheOneGoModPins. +// +// In a merge check the captured copy is also compared with the clone at the captured commit, byte for byte, +// so a hand-edited copy cannot pass for the SDK's. type fixture struct { Name string `json:"name"` Given struct { @@ -36,14 +49,24 @@ type fixture struct { } `json:"wire"` } +// sdkModule is the Go module of the SDK the controller is built against. +const sdkModule = "git.novox.be/novox/mesh-sdk/go" + +// The tree's go.mod, and the captured copy, as this package finds them. +var ( + goModFile = filepath.Join("..", "..", "go.mod") + capturedSDK = filepath.Join(beside.Captured(), "mesh-sdk") + capturedLog = filepath.Join(beside.Captured(), "CAPTURED") +) + func loadFixture(t *testing.T, name string) fixture { t.Helper() - path := filepath.Join(beside.Dir(t, "mesh-sdk"), "conformance", name) - raw, err := os.ReadFile(path) + raw, from, err := sdkFixture(os.Getenv(beside.Env), goModFile, capturedSDK, capturedLog, name) if err != nil { // Failed, never skipped: a skip here passed the suite with nothing judged. - t.Fatalf("the SDK's conformance fixture %s: %v", name, err) + t.Fatal(err) } + t.Logf("%s: %s", name, from) var f fixture if err := json.Unmarshal(raw, &f); err != nil { t.Fatalf("%s: %v", name, err) @@ -51,30 +74,119 @@ func loadFixture(t *testing.T, name string) fixture { return f } +// sdkFixture is one conformance fixture and where it was read: from the mesh-sdk clone in root (a merge +// check's MESH_CHECK_BESIDE) at the pin of goMod, or, with no root, from the captured copy. +func sdkFixture(root, goMod, captured, capturedLog, name string) ([]byte, string, error) { + file := path.Join("conformance", name) + if root == "" { + raw, err := os.ReadFile(filepath.Join(captured, filepath.FromSlash(file))) + if err != nil { + return nil, "", fmt.Errorf("the captured SDK's %s: %w", file, err) + } + return raw, "the copy captured in testdata/beside (see CAPTURED); a merge check reads the SDK's clone " + + "at this tree's pin", nil + } + clone := filepath.Join(root, "mesh-sdk") + if _, err := os.Stat(clone); err != nil { + return nil, "", fmt.Errorf("mesh-sdk is not beside this check in %s=%s, so the SDK this controller "+ + "pins cannot be judged here: %w", beside.Env, root, err) + } + ref, err := sdkRef(goMod) + if err != nil { + return nil, "", err + } + raw, err := gitShow(clone, ref, file) + if err != nil { + return nil, "", fmt.Errorf("the mesh-sdk clone beside this check has no %s at %s, the SDK this tree's "+ + "go.mod pins: %w", file, ref, err) + } + // The captured copy is the SDK's own, never edited by hand: the clone at the captured commit says so. + at, err := capturedCommit(capturedLog) + if err != nil { + return nil, "", err + } + theirs, err := gitShow(clone, at, file) + if err != nil { + return nil, "", fmt.Errorf("the mesh-sdk clone has no %s at %s, the commit CAPTURED names: %w", file, at, err) + } + ours, err := os.ReadFile(filepath.Join(captured, filepath.FromSlash(file))) + if err != nil { + return nil, "", fmt.Errorf("the captured SDK's %s: %w", file, err) + } + if !bytes.Equal(ours, theirs) { + return nil, "", fmt.Errorf("the captured %s is not mesh-sdk's at %s, the commit CAPTURED names: it was "+ + "edited or captured wrong; capture it again as CAPTURED says", file, at) + } + return raw, "mesh-sdk cloned beside this check, at " + ref + " (this tree's go.mod)", nil +} + +// gitShow is one file of a repository at a ref. safe.directory, because the clone is the build seat's and +// the test may run as another user. +func gitShow(repository, ref, file string) ([]byte, error) { + cmd := exec.Command("git", "-c", "safe.directory=*", "-C", repository, "show", ref+":"+file) + var stderr bytes.Buffer + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("git show %s:%s: %v: %s", ref, file, err, strings.TrimSpace(stderr.String())) + } + return out, nil +} + +// pseudoCommit is the commit a Go pseudo-version names: v0.1.11-0.20261009143344-f047d0a4a970 → f047d0a4a970. +var pseudoCommit = regexp.MustCompile(`-([0-9a-f]{12})$`) + +// sdkRef is the SDK repository's ref a go.mod pins: a pseudo-version's commit, or a release's tag (the SDK +// tags its Go module under go/). +func sdkRef(goMod string) (string, error) { + raw, err := os.ReadFile(goMod) + if err != nil { + return "", err + } + version := "" + for _, line := range strings.Split(string(raw), "\n") { + fields := strings.Fields(strings.TrimPrefix(strings.TrimSpace(line), "require ")) + if len(fields) >= 2 && fields[0] == sdkModule { + version = fields[1] + } + } + if m := pseudoCommit.FindStringSubmatch(version); m != nil { + return m[1], nil + } + if strings.HasPrefix(version, "v") { + return "go/" + version, nil + } + return "", fmt.Errorf("%s pins no version of %s that names a commit or a tag", goMod, sdkModule) +} + +// capturedCommit is the commit testdata/beside/CAPTURED names for mesh-sdk. +func capturedCommit(capturedLog string) (string, error) { + raw, err := os.ReadFile(capturedLog) + if err != nil { + return "", err + } + at := regexp.MustCompile(`(?m)^mesh-sdk\s+([0-9a-f]{40})\s`).FindSubmatch(raw) + if at == nil { + return "", fmt.Errorf("%s names no commit for mesh-sdk", capturedLog) + } + return string(at[1]), nil +} + // The captured SDK is the one this controller is built against: when go.mod moves the SDK, the copy moves // with it, or the tests away from a merge check judge an SDK the controller no longer uses (novox/hq issue // 449). func TestTheCapturedSDKIsTheOneGoModPins(t *testing.T) { - mod, err := os.ReadFile(filepath.Join("..", "..", "go.mod")) + pinned, err := sdkRef(goModFile) if err != nil { t.Fatal(err) } - pinned := regexp.MustCompile(`(?m)^\s*git\.novox\.be/novox/mesh-sdk/go v\S+-([0-9a-f]{12})$`).FindSubmatch(mod) - if pinned == nil { - t.Fatal("go.mod pins no commit of git.novox.be/novox/mesh-sdk/go as a pseudo-version; say here how a " + - "release's tag is matched against testdata/beside/CAPTURED") - } - captured, err := os.ReadFile(filepath.Join(beside.Captured(), "CAPTURED")) + at, err := capturedCommit(capturedLog) if err != nil { t.Fatal(err) } - at := regexp.MustCompile(`(?m)^mesh-sdk\s+([0-9a-f]{40})\s`).FindSubmatch(captured) - if at == nil { - t.Fatal("testdata/beside/CAPTURED names no commit for mesh-sdk") - } - if !strings.HasPrefix(string(at[1]), string(pinned[1])) { + if strings.HasPrefix(pinned, "go/") || !strings.HasPrefix(at, pinned) { t.Errorf("the SDK is captured at %s but go.mod pins %s: capture it again at the pinned commit, as "+ - "testdata/beside/CAPTURED says", at[1], pinned[1]) + "testdata/beside/CAPTURED says", at, pinned) } }