From 0730ef609216798e787ee8b6542912f5ec613a77 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:26:02 +0200 Subject: [PATCH] Keep the unit level cheap: the merge's real-git tests start processes, so they are integration tests; the places tests wait 10 ms, not 50 (issue 895) --- go/buildstep/merge_integration_test.go | 172 +++++++++++++++++++++++++ go/buildstep/merge_test.go | 158 ----------------------- go/delivery/buildrun/places_test.go | 4 +- 3 files changed, 174 insertions(+), 160 deletions(-) create mode 100644 go/buildstep/merge_integration_test.go diff --git a/go/buildstep/merge_integration_test.go b/go/buildstep/merge_integration_test.go new file mode 100644 index 0000000..88b6d11 --- /dev/null +++ b/go/buildstep/merge_integration_test.go @@ -0,0 +1,172 @@ +//go:build integration + +package buildstep + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "reflect" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +// The merge on real git repositories starts git processes, so it is an integration test (the unit level keeps its bound, +// issue 895); the unit tests of merge_test.go prove the commands and the refusals with a fake runner. + +func realGit(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", dir, "-c", "user.name=t", "-c", "user.email=t@example.invalid", "-c", "commit.gpgsign=false"}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +func writeFile(t *testing.T, dir, name, content string) { + t.Helper() + p := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +// mergeRig is a local bare repository standing in for the forge, a work repository that pushes to it, and an empty +// workspace: the fetch step's shape (FetchCommit of the head into the workspace, then MergeInto). +type mergeRig struct { + t *testing.T + bare, work, ws string + fork, trunk string // the commit both histories share, and the trunk's head + remote string +} + +func newMergeRig(t *testing.T) *mergeRig { + root := t.TempDir() + r := &mergeRig{t: t, bare: filepath.Join(root, "r.git"), work: filepath.Join(root, "work"), ws: filepath.Join(root, "ws")} + for _, d := range []string{r.bare, r.work, r.ws} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + r.remote = "file://" + r.bare + realGit(t, r.bare, "init", "-q", "--bare") + realGit(t, r.work, "init", "-q", "-b", "main") + writeFile(t, r.work, "a.txt", "one\ntwo\nthree\n") + writeFile(t, r.work, "modules/m/module.json", "{}\n") + realGit(t, r.work, "add", ".") + realGit(t, r.work, "commit", "-q", "-m", "fork point") + r.fork = realGit(t, r.work, "rev-parse", "HEAD") + return r +} + +// head makes a commit on a branch off the fork point, pushes it as the pull request's head ref and answers its sha. +func (r *mergeRig) head(files map[string]string) string { + realGit(r.t, r.work, "checkout", "-q", "-B", "pr", r.fork) + for n, c := range files { + writeFile(r.t, r.work, n, c) + } + realGit(r.t, r.work, "add", ".") + realGit(r.t, r.work, "commit", "-q", "-m", "head") + h := realGit(r.t, r.work, "rev-parse", "HEAD") + realGit(r.t, r.work, "push", "-q", r.bare, "HEAD:refs/pull/1/head") + return h +} + +// advance moves the trunk past the fork point with a commit changing files, pushes it and answers its sha. +func (r *mergeRig) advance(files map[string]string) string { + realGit(r.t, r.work, "checkout", "-q", "-B", "main", r.fork) + for n, c := range files { + writeFile(r.t, r.work, n, c) + } + realGit(r.t, r.work, "add", ".") + realGit(r.t, r.work, "commit", "-q", "-m", "trunk moves") + r.trunk = realGit(r.t, r.work, "rev-parse", "HEAD") + realGit(r.t, r.work, "push", "-q", "-f", r.bare, "HEAD:refs/heads/main") + return r.trunk +} + +func (r *mergeRig) fetch(head string) { + if err := FetchCommit(context.Background(), forgekey.ExecRunner, r.ws, r.remote, head, "refs/pull/1/head", AllowFileRemote()); err != nil { + r.t.Fatal(err) + } +} + +// 1674/AC1: a clean merge answers the merge commit and the changed files, and leaves the workspace at the merge, which +// holds both sides' changes. +func TestMergeIntoACleanMergeAnswersTheMergeCommitAndTheChangedFiles(t *testing.T) { + r := newMergeRig(t) + head := r.head(map[string]string{"b.txt": "head\n"}) + base := r.advance(map[string]string{"a.txt": "one\ntwo\nthree\nfour\n"}) + r.fetch(head) + res, err := MergeInto(context.Background(), forgekey.ExecRunner, r.ws, r.remote, base, head, AllowFileRemote()) + if err != nil { + t.Fatal(err) + } + if res.Base != base || res.Head != head || len(res.Merge) != 40 || res.Merge == base || res.Merge == head || len(res.Conflicts) != 0 { + t.Fatalf("result = %+v", res) + } + if !reflect.DeepEqual(res.Changed, []string{"b.txt"}) { + t.Errorf("changed = %v, want only what the head adds to the trunk: b.txt", res.Changed) + } + if at := realGit(t, r.ws, "rev-parse", "HEAD"); at != res.Merge { + t.Errorf("the workspace is at %s, not at the merge %s", at, res.Merge) + } + for f, want := range map[string]string{"b.txt": "head\n", "a.txt": "one\ntwo\nthree\nfour\n"} { + if got, _ := os.ReadFile(filepath.Join(r.ws, f)); string(got) != want { + t.Errorf("%s = %q, want %q", f, got, want) + } + } + if parents := strings.Fields(realGit(t, r.ws, "rev-list", "--parents", "-n", "1", res.Merge)); len(parents) != 3 || parents[1] != base || parents[2] != head { + t.Errorf("the merge commit's parents = %v, want the trunk commit and the head", parents) + } + if author := realGit(t, r.ws, "log", "-1", "--format=%an <%ae>", res.Merge); author != "mesh merge check " { + t.Errorf("the merge is made as %q", author) + } +} + +// 1674/AC2: a conflict is an answer: the files in conflict, no merge commit, and the workspace left at the trunk commit. +func TestMergeIntoAConflictAnswersTheFilesAndLeavesTheTreeAtTheTrunk(t *testing.T) { + r := newMergeRig(t) + head := r.head(map[string]string{"a.txt": "one\nHEAD\nthree\n", "c.txt": "c\n"}) + base := r.advance(map[string]string{"a.txt": "one\nTRUNK\nthree\n"}) + r.fetch(head) + res, err := MergeInto(context.Background(), forgekey.ExecRunner, r.ws, r.remote, base, head, AllowFileRemote()) + if err != nil { + t.Fatalf("a conflict is an answer, not an error: %v", err) + } + if !reflect.DeepEqual(res.Conflicts, []string{"a.txt"}) || res.Merge != "" || len(res.Changed) != 0 || res.Base != base || res.Head != head { + t.Fatalf("result = %+v", res) + } + if at := realGit(t, r.ws, "rev-parse", "HEAD"); at != base { + t.Errorf("the workspace is at %s, not at the trunk commit %s", at, base) + } + if got, _ := os.ReadFile(filepath.Join(r.ws, "a.txt")); string(got) != "one\nTRUNK\nthree\n" { + t.Errorf("a.txt = %q, want the trunk's", got) + } + if st := realGit(t, r.ws, "status", "--porcelain"); st != "" { + t.Errorf("the tree is not clean after the aborted merge: %q", st) + } + if _, err := os.Stat(filepath.Join(r.ws, "c.txt")); err == nil { + t.Error("a file of the head is in a tree left at the trunk") + } +} + +// A head already in the trunk merges into nothing: the merge is the trunk commit and no path changes. +func TestMergeIntoAHeadAlreadyInTheTrunkChangesNothing(t *testing.T) { + r := newMergeRig(t) + head := r.head(map[string]string{"b.txt": "head\n"}) + r.fork = head // the trunk continues from the head + base := r.advance(map[string]string{"a.txt": "one\ntwo\nthree\nfour\n"}) + r.fetch(head) + res, err := MergeInto(context.Background(), forgekey.ExecRunner, r.ws, r.remote, base, head, AllowFileRemote()) + if err != nil || res.Merge != base || len(res.Changed) != 0 || len(res.Conflicts) != 0 { + t.Errorf("result = %+v, %v", res, err) + } +} diff --git a/go/buildstep/merge_test.go b/go/buildstep/merge_test.go index 4c59610..4eb0a88 100644 --- a/go/buildstep/merge_test.go +++ b/go/buildstep/merge_test.go @@ -2,157 +2,12 @@ package buildstep import ( "context" - "os" - "os/exec" - "path/filepath" - "reflect" "strings" "testing" "git.novox.be/novox/mesh-sdk/go/forgekey" ) -func realGit(t *testing.T, dir string, args ...string) string { - t.Helper() - cmd := exec.Command("git", append([]string{"-C", dir, "-c", "user.name=t", "-c", "user.email=t@example.invalid", "-c", "commit.gpgsign=false"}, args...)...) - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v: %v\n%s", args, err, out) - } - return strings.TrimSpace(string(out)) -} - -func writeFile(t *testing.T, dir, name, content string) { - t.Helper() - p := filepath.Join(dir, name) - if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(p, []byte(content), 0o644); err != nil { - t.Fatal(err) - } -} - -// mergeRig is a local bare repository standing in for the forge, a work repository that pushes to it, and an empty -// workspace: the fetch step's shape (FetchCommit of the head into the workspace, then MergeInto). -type mergeRig struct { - t *testing.T - bare, work, ws string - fork, trunk string // the commit both histories share, and the trunk's head - remote string -} - -func newMergeRig(t *testing.T) *mergeRig { - root := t.TempDir() - r := &mergeRig{t: t, bare: filepath.Join(root, "r.git"), work: filepath.Join(root, "work"), ws: filepath.Join(root, "ws")} - for _, d := range []string{r.bare, r.work, r.ws} { - if err := os.MkdirAll(d, 0o755); err != nil { - t.Fatal(err) - } - } - r.remote = "file://" + r.bare - realGit(t, r.bare, "init", "-q", "--bare") - realGit(t, r.work, "init", "-q", "-b", "main") - writeFile(t, r.work, "a.txt", "one\ntwo\nthree\n") - writeFile(t, r.work, "modules/m/module.json", "{}\n") - realGit(t, r.work, "add", ".") - realGit(t, r.work, "commit", "-q", "-m", "fork point") - r.fork = realGit(t, r.work, "rev-parse", "HEAD") - return r -} - -// head makes a commit on a branch off the fork point, pushes it as the pull request's head ref and answers its sha. -func (r *mergeRig) head(files map[string]string) string { - realGit(r.t, r.work, "checkout", "-q", "-B", "pr", r.fork) - for n, c := range files { - writeFile(r.t, r.work, n, c) - } - realGit(r.t, r.work, "add", ".") - realGit(r.t, r.work, "commit", "-q", "-m", "head") - h := realGit(r.t, r.work, "rev-parse", "HEAD") - realGit(r.t, r.work, "push", "-q", r.bare, "HEAD:refs/pull/1/head") - return h -} - -// advance moves the trunk past the fork point with a commit changing files, pushes it and answers its sha. -func (r *mergeRig) advance(files map[string]string) string { - realGit(r.t, r.work, "checkout", "-q", "-B", "main", r.fork) - for n, c := range files { - writeFile(r.t, r.work, n, c) - } - realGit(r.t, r.work, "add", ".") - realGit(r.t, r.work, "commit", "-q", "-m", "trunk moves") - r.trunk = realGit(r.t, r.work, "rev-parse", "HEAD") - realGit(r.t, r.work, "push", "-q", "-f", r.bare, "HEAD:refs/heads/main") - return r.trunk -} - -func (r *mergeRig) fetch(head string) { - if err := FetchCommit(context.Background(), forgekey.ExecRunner, r.ws, r.remote, head, "refs/pull/1/head", AllowFileRemote()); err != nil { - r.t.Fatal(err) - } -} - -// 1674/AC1: a clean merge answers the merge commit and the changed files, and leaves the workspace at the merge, which -// holds both sides' changes. -func TestMergeIntoACleanMergeAnswersTheMergeCommitAndTheChangedFiles(t *testing.T) { - r := newMergeRig(t) - head := r.head(map[string]string{"b.txt": "head\n"}) - base := r.advance(map[string]string{"a.txt": "one\ntwo\nthree\nfour\n"}) - r.fetch(head) - res, err := MergeInto(context.Background(), forgekey.ExecRunner, r.ws, r.remote, base, head, AllowFileRemote()) - if err != nil { - t.Fatal(err) - } - if res.Base != base || res.Head != head || len(res.Merge) != 40 || res.Merge == base || res.Merge == head || len(res.Conflicts) != 0 { - t.Fatalf("result = %+v", res) - } - if !reflect.DeepEqual(res.Changed, []string{"b.txt"}) { - t.Errorf("changed = %v, want only what the head adds to the trunk: b.txt", res.Changed) - } - if at := realGit(t, r.ws, "rev-parse", "HEAD"); at != res.Merge { - t.Errorf("the workspace is at %s, not at the merge %s", at, res.Merge) - } - for f, want := range map[string]string{"b.txt": "head\n", "a.txt": "one\ntwo\nthree\nfour\n"} { - if got, _ := os.ReadFile(filepath.Join(r.ws, f)); string(got) != want { - t.Errorf("%s = %q, want %q", f, got, want) - } - } - if parents := strings.Fields(realGit(t, r.ws, "rev-list", "--parents", "-n", "1", res.Merge)); len(parents) != 3 || parents[1] != base || parents[2] != head { - t.Errorf("the merge commit's parents = %v, want the trunk commit and the head", parents) - } - if author := realGit(t, r.ws, "log", "-1", "--format=%an <%ae>", res.Merge); author != "mesh merge check " { - t.Errorf("the merge is made as %q", author) - } -} - -// 1674/AC2: a conflict is an answer: the files in conflict, no merge commit, and the workspace left at the trunk commit. -func TestMergeIntoAConflictAnswersTheFilesAndLeavesTheTreeAtTheTrunk(t *testing.T) { - r := newMergeRig(t) - head := r.head(map[string]string{"a.txt": "one\nHEAD\nthree\n", "c.txt": "c\n"}) - base := r.advance(map[string]string{"a.txt": "one\nTRUNK\nthree\n"}) - r.fetch(head) - res, err := MergeInto(context.Background(), forgekey.ExecRunner, r.ws, r.remote, base, head, AllowFileRemote()) - if err != nil { - t.Fatalf("a conflict is an answer, not an error: %v", err) - } - if !reflect.DeepEqual(res.Conflicts, []string{"a.txt"}) || res.Merge != "" || len(res.Changed) != 0 || res.Base != base || res.Head != head { - t.Fatalf("result = %+v", res) - } - if at := realGit(t, r.ws, "rev-parse", "HEAD"); at != base { - t.Errorf("the workspace is at %s, not at the trunk commit %s", at, base) - } - if got, _ := os.ReadFile(filepath.Join(r.ws, "a.txt")); string(got) != "one\nTRUNK\nthree\n" { - t.Errorf("a.txt = %q, want the trunk's", got) - } - if st := realGit(t, r.ws, "status", "--porcelain"); st != "" { - t.Errorf("the tree is not clean after the aborted merge: %q", st) - } - if _, err := os.Stat(filepath.Join(r.ws, "c.txt")); err == nil { - t.Error("a file of the head is in a tree left at the trunk") - } -} - // 1674/AC3: no base is refused before anything runs; so is a base or a commit that is not a full sha, a workspace that // is not at the commit, and a remote that is not ssh://. func TestMergeIntoRefusesWhatIsNotAMerge(t *testing.T) { @@ -217,16 +72,3 @@ func TestMergeIntoRunsTheCheckMergesCommands(t *testing.T) { } } } - -// A head already in the trunk merges into nothing: the merge is the trunk commit and no path changes. -func TestMergeIntoAHeadAlreadyInTheTrunkChangesNothing(t *testing.T) { - r := newMergeRig(t) - head := r.head(map[string]string{"b.txt": "head\n"}) - r.fork = head // the trunk continues from the head - base := r.advance(map[string]string{"a.txt": "one\ntwo\nthree\nfour\n"}) - r.fetch(head) - res, err := MergeInto(context.Background(), forgekey.ExecRunner, r.ws, r.remote, base, head, AllowFileRemote()) - if err != nil || res.Merge != base || len(res.Changed) != 0 || len(res.Conflicts) != 0 { - t.Errorf("result = %+v, %v", res, err) - } -} diff --git a/go/delivery/buildrun/places_test.go b/go/delivery/buildrun/places_test.go index 384c537..f7ff940 100644 --- a/go/delivery/buildrun/places_test.go +++ b/go/delivery/buildrun/places_test.go @@ -43,7 +43,7 @@ func TestThreeStepsOnANodeOfCapacityTwoRunTwoAtOnceAndTheThirdStartsWhenOneEnds( select { case <-third: t.Fatal("a third step started on a node of capacity 2 with two running") - case <-time.After(50 * time.Millisecond): + case <-time.After(10 * time.Millisecond): } if p.Running()["ace"] != 2 { t.Errorf("running = %v, want 2 on ace", p.Running()) @@ -65,7 +65,7 @@ func TestANodeWithNoSettingRunsOneStepAtATime(t *testing.T) { // round 3 select { case <-second: t.Fatal("two steps on a node without a setting") - case <-time.After(50 * time.Millisecond): + case <-time.After(10 * time.Millisecond): } first() r := <-second