From ce6038f397af91d31a3eddd503fe5b0abcc9cd61 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 21:54:33 -0300 Subject: [PATCH 1/2] fix(mutate): run each test process in a private copy of the module Every test process ran with its working directory in the package's source directory: a schema binary (Ask), each coverage run (Cover) and go test for an edit built alone (Run). Edits asked side by side shared that directory, so a file one edit's tests wrote was read by another's: in github.com/devcontainers/cli, -j 4 let a different set of extendImageWithFeatures edits through on each run, every one refused at -j 1, and Dockerfile-with-features and its siblings were left in the checkout afterwards. The run's budget of test processes is now a pool of trees, one per -j. A tree is a copy of the module as git sees it (tracked and not ignored files, with ignored directories and nested repositories linked), made the first time a slot is used and put back as the source is after every process. Tests are still built where the sources are; Run builds with go test -c and runs the binary in the copy, since go test always runs it in the package's directory. Whatever the tests left, changed or removed is named in the run's report (Tally.Wrote). Co-Authored-By: Claude Opus 5.5 --- internal/mutate/cover.go | 33 ++- internal/mutate/helpers_internal_test.go | 23 ++ internal/mutate/helpers_test.go | 27 ++ internal/mutate/mutate.go | 126 ++++++--- internal/mutate/mutate_test.go | 22 +- internal/mutate/runner.go | 69 +++-- internal/mutate/runner_internal_test.go | 12 +- internal/mutate/runner_test.go | 124 +++++++++ internal/mutate/schema.go | 28 +- internal/mutate/schema_test.go | 14 +- internal/mutate/share_test.go | 6 +- internal/mutate/tree.go | 319 +++++++++++++++++++++++ internal/mutate/tree_internal_test.go | 148 +++++++++++ site/src/content/docs/mutate.mdx | 17 ++ 14 files changed, 864 insertions(+), 104 deletions(-) create mode 100644 internal/mutate/helpers_internal_test.go create mode 100644 internal/mutate/helpers_test.go create mode 100644 internal/mutate/tree.go create mode 100644 internal/mutate/tree_internal_test.go diff --git a/internal/mutate/cover.go b/internal/mutate/cover.go index 2a98b65..0e63969 100644 --- a/internal/mutate/cover.go +++ b/internal/mutate/cover.go @@ -2,6 +2,7 @@ package mutate import ( "bufio" + "cmp" "fmt" "os" "os/exec" @@ -55,12 +56,12 @@ func inAny(spans []span, line int) bool { } // Cover measures the package in dir: its test binary built once with coverage, then each of its -// tests run alone into a profile of its own, each holding one of slots while it runs. The slots +// tests run alone into a profile of its own, each in one of slots' trees while it runs. The slots // are the caller's whole budget of test processes, shared with the mutations it runs: a budget // of its own per package multiplied with theirs, and a database package's tests start a // PostgreSQL per process. Nil where the package has no tests, or they would not build - Run then // asks every test, as it would without this. -func Cover(dir, timeout string, slots chan struct{}) (*Coverage, error) { +func Cover(dir, timeout string, slots chan *Tree) (*Coverage, error) { work, err := os.MkdirTemp("", "mutate-cover-") if err != nil { return nil, err @@ -69,9 +70,9 @@ func Cover(dir, timeout string, slots chan struct{}) (*Coverage, error) { bin := filepath.Join(work, "pkg.test") build := exec.Command("go", "test", "-c", "-cover", "-covermode=set", "-o", bin, ".") build.Dir = dir - slots <- struct{}{} + tree := <-slots out, err := combined("cover: build the package's tests", build) - <-slots + slots <- tree if err != nil { return nil, fmt.Errorf("mutate: building %s's tests with coverage: %w: %s", dir, err, out) } @@ -100,21 +101,34 @@ func Cover(dir, timeout string, slots chan struct{}) (*Coverage, error) { c := &Coverage{reached: map[string]map[string][]span{}, blocks: map[string][]span{}, failing: map[string]bool{}} var mu sync.Mutex var wg sync.WaitGroup + var treeErr error for i, test := range tests { wg.Add(1) - slots <- struct{}{} + tree := <-slots go func() { defer wg.Done() - defer func() { <-slots }() + defer func() { slots <- tree }() profile := filepath.Join(work, strconv.Itoa(i)+".cov") run := exec.Command(bin, "-test.run", "^"+test+"$", "-test.count=1", //nolint:gosec // the binary this built "-test.timeout", timeout, "-test.coverprofile", profile) - run.Dir = dir run.Env = testEnv() // A failure here is the package's own tests' to report, not this gate's; what this // gate does with it is ask the test about no edit, since it fails whatever the edit - // and say so, since an edit only it reaches then reads as one nothing holds. - said, exceeded, runErr := bounded("cover: run one test", run) + var ( + said []byte + exceeded bool + runErr error + ) + if err := tree.in(dir, func(at string) { + run.Dir = at + said, exceeded, runErr = bounded("cover: run one test", run) + }); err != nil { + mu.Lock() + treeErr = cmp.Or(treeErr, err) + mu.Unlock() + return + } if exceeded { runErr = fmt.Errorf("held over the %s a test process may, and stopped", bound()) } @@ -147,6 +161,9 @@ func Cover(dir, timeout string, slots chan struct{}) (*Coverage, error) { }() } wg.Wait() + if treeErr != nil { + return nil, treeErr + } return c, nil } diff --git a/internal/mutate/helpers_internal_test.go b/internal/mutate/helpers_internal_test.go new file mode 100644 index 0000000..f9311c3 --- /dev/null +++ b/internal/mutate/helpers_internal_test.go @@ -0,0 +1,23 @@ +package mutate + +import "testing" + +// runTests is Run, failing t where the tests could not be asked at all. +func runTests(t *testing.T, m Mutation, a Asked) (Outcome, string) { + t.Helper() + outcome, out, err := Run(m, a) + if err != nil { + t.Fatal(err) + } + return outcome, out +} + +// askTests is Ask in a tree of its own, failing t where the tests could not be asked at all. +func askTests(t *testing.T, bin, dir string, id int, timeout string, only []string) (Outcome, string) { + t.Helper() + outcome, out, err := Ask(NewTree(t.TempDir()), bin, dir, id, timeout, only) + if err != nil { + t.Fatal(err) + } + return outcome, out +} diff --git a/internal/mutate/helpers_test.go b/internal/mutate/helpers_test.go new file mode 100644 index 0000000..b346dff --- /dev/null +++ b/internal/mutate/helpers_test.go @@ -0,0 +1,27 @@ +package mutate_test + +import ( + "testing" + + "github.com/spin-stack/go-tools/internal/mutate" +) + +// runTests is mutate.Run, failing t where the tests could not be asked at all. +func runTests(t *testing.T, m mutate.Mutation, a mutate.Asked) (mutate.Outcome, string) { + t.Helper() + outcome, out, err := mutate.Run(m, a) + if err != nil { + t.Fatal(err) + } + return outcome, out +} + +// askTests is mutate.Ask in a tree of its own, failing t where the tests could not be asked at all. +func askTests(t *testing.T, bin, dir string, id int, timeout string, only []string) (mutate.Outcome, string) { + t.Helper() + outcome, out, err := mutate.Ask(mutate.NewTree(t.TempDir()), bin, dir, id, timeout, only) + if err != nil { + t.Fatal(err) + } + return outcome, out +} diff --git a/internal/mutate/mutate.go b/internal/mutate/mutate.go index 34b3845..cfeebdc 100644 --- a/internal/mutate/mutate.go +++ b/internal/mutate/mutate.go @@ -668,9 +668,6 @@ const ( Exceeded ) -// buildFailed is go test's word that a package it was to test did not build. -var buildFailed = regexp.MustCompile(`(?m)^FAIL\s+\S+\s+\[build failed\]$`) - // targetedTimeout is how long the tests that reach a mutated line may take. const targetedTimeout = "1m" @@ -686,35 +683,50 @@ type Asked struct { Reaching []string // Importers are the packages asked when the package's own tests let the edit through. Importers []string + // Tree is where the tests run, the caller's to hold for the call; nil is one of the call's + // own. Never the package's own directory (Tree says why). + Tree *Tree } -// Run builds and runs the tests of the package the mutated file belongs to, stopping at the -// first that fails. +// Run builds the tests of the package the mutated file belongs to where its sources are, and runs +// them in the tree, stopping at the first that fails. // // An edit its own package's tests let through is asked of the packages that import that one // directly, when importers names any: a package's behaviour is often held by its caller's tests - // the QMP client's file passing by the chain manager's - and counting those as survivors made the // gate report tests that exist as missing. Only a survivor pays for the second run. -func Run(m Mutation, a Asked) (Outcome, string) { +// +// The binaries are built with go test -c and run here rather than by go test, which runs each in +// its package's directory, where every edit asked at once would read what the others wrote. +func Run(m Mutation, a Asked) (Outcome, string, error) { dir := filepath.Dir(m.File) tests := hasTests(dir) if !tests && len(a.Importers) == 0 { - return Untested, "" + return Untested, "", nil + } + tree := a.Tree + if tree == nil { + work, err := os.MkdirTemp("", "mutate-tree-") + if err != nil { + return 0, "", err + } + defer func() { _ = os.RemoveAll(work) }() + tree = NewTree(work) } - // The package's own tests are run in its directory; its importers, named from the module's + // The package's own tests are named from its directory; its importers, from the module's // root, from where this runs. type testRun struct { dir string pkgs []string only []string - what string // what its process is counted under + what string // what its processes are counted under } var runs []testRun if tests && (a.Reaching == nil || len(a.Reaching) > 0) { - runs = append(runs, testRun{dir: dir, pkgs: []string{"."}, only: a.Reaching, what: "alone: build and run the package's tests"}) + runs = append(runs, testRun{dir: dir, pkgs: []string{"."}, only: a.Reaching, what: "the package's tests"}) } if len(a.Importers) > 0 { - runs = append(runs, testRun{pkgs: a.Importers, what: "alone: build and run the importers' tests"}) + runs = append(runs, testRun{pkgs: a.Importers, what: "the importers' tests"}) } var said strings.Builder for _, set := range runs { @@ -727,35 +739,75 @@ func Run(m Mutation, a Asked) (Outcome, string) { // package's whole allowance. timeout = targetedTimeout } - // -p 1: this run is one of the caller's budget of test processes, and go test over several - // packages would otherwise run a binary per core, each with a PostgreSQL of its own. - args := append(append([]string{"test"}, set.pkgs...), "-p", "1", "-count=1", "-failfast", "-v", "-timeout", timeout) - if a.Overlay != "" { - args = append(args, "-overlay", a.Overlay) - } - if len(set.only) > 0 { - args = append(args, "-run", "^("+strings.Join(set.only, "|")+")$") - } - cmd := exec.Command("go", args...) //nolint:gosec // go test over this repository's packages - cmd.Dir = set.dir - cmd.Env = testEnv() - out, exceeded, err := bounded(set.what, cmd) - countTests("", out) - said.Write(out) - if exceeded { - return Exceeded, said.String() - } - if err == nil { - continue + list := exec.Command("go", append([]string{"list", "-f", "{{.ImportPath}} {{.Dir}}"}, set.pkgs...)...) //nolint:gosec // go list over this repository's packages + list.Dir = set.dir + listed, err := output("listing", list) + if err != nil { + return 0, said.String(), fmt.Errorf("mutate: listing %s: %w", strings.Join(set.pkgs, " "), err) } - // go test's own line, not the words anywhere: a test that fails may print them - this - // package's do, about the modules they build - and that is an edit refused. - if buildFailed.Match(out) || strings.Contains(string(out), "build constraints exclude") { - return Unbuildable, said.String() + for _, line := range strings.Split(strings.TrimSpace(string(listed)), "\n") { + path, pkgDir, _ := strings.Cut(line, " ") + outcome, err := runPackage(tree, path, pkgDir, set.dir, set.what, set.only, timeout, a.Overlay, &said) + if err != nil || outcome != Survived { + return outcome, said.String(), err + } } - return Killed, said.String() } - return Survived, said.String() + return Survived, said.String(), nil +} + +// runPackage builds the tests of the package at path, from dir, and runs them in the tree's copy +// of pkgDir, writing what they said to said. Survived where it has no test this build would run. +func runPackage(tree *Tree, path, pkgDir, dir, what string, only []string, timeout, overlay string, said *strings.Builder) (Outcome, error) { + bin, err := tree.bin() + if err != nil { + return 0, err + } + args := []string{"test", "-c", "-vet=off", "-o", bin} + if overlay != "" { + args = append(args, "-overlay", overlay) + } + build := exec.Command("go", append(args, path)...) //nolint:gosec // go test over this repository's packages + build.Dir = dir + out, exceeded, err := bounded("alone: build "+what, build) + said.Write(out) + if exceeded { + return Exceeded, nil + } + if err != nil { + return Unbuildable, nil //nolint:nilerr // a build that fails is the edit's answer: it asks nothing + } + if _, err := os.Stat(bin); err != nil { + return Survived, nil //nolint:nilerr // no test binary: the package has no tests + } + args = []string{"-test.count=1", "-test.failfast", "-test.v", "-test.timeout", timeout} + if len(only) > 0 { + args = append(args, "-test.run", "^("+strings.Join(only, "|")+")$") + } + run := exec.Command(bin, args...) //nolint:gosec // a test binary this run built + run.Env = testEnv() + var runErr error + if err := tree.in(pkgDir, func(at string) { + run.Dir = at + out, exceeded, runErr = bounded("alone: run "+what, run) + }); err != nil { + return 0, err + } + countTests(path, out) + said.Write(out) + // go test's own line for the package, so what is said names who answered. + verdict := "ok " + if runErr != nil { + verdict = "FAIL" + } + fmt.Fprintf(said, "%s\t%s\n", verdict, path) + switch { + case exceeded: + return Exceeded, nil + case runErr != nil: + return Killed, nil //nolint:nilerr // a test that fails is the edit refused, not this run failing + } + return Survived, nil } // hasTests is whether the package in dir has a test go would build: one whose tests are all of diff --git a/internal/mutate/mutate_test.go b/internal/mutate/mutate_test.go index 5165b69..fb4a236 100644 --- a/internal/mutate/mutate_test.go +++ b/internal/mutate/mutate_test.go @@ -369,25 +369,25 @@ func TestTheLinesAChangeTouchesAreTheOnesItChanged(t *testing.T) { func TestAPackageWithNoTestsIsSaidSoAndNotCountedAsASurvivor(t *testing.T) { // Not this package: running its own tests from inside them is a test that runs forever. A // command with no tests of its own: cmd/mutate has the site's example now. - outcome, _ := mutate.Run(mutate.Mutation{File: filepath.Join("..", "..", "cmd", "ctxlife", "main.go")}, mutate.Asked{Timeout: "1m"}) + outcome, _ := runTests(t, mutate.Mutation{File: filepath.Join("..", "..", "cmd", "ctxlife", "main.go")}, mutate.Asked{Timeout: "1m"}) assert.Equal(t, mutate.Untested, outcome, "a package with no test files was asked anyway") // And one that has tests is run: unmutated, they pass, which is what a survivor looks like. - outcome, _ = mutate.Run(mutate.Mutation{File: filepath.Join("..", "fetch", "fetch.go")}, mutate.Asked{Timeout: "2m"}) + outcome, _ = runTests(t, mutate.Mutation{File: filepath.Join("..", "fetch", "fetch.go")}, mutate.Asked{Timeout: "2m"}) assert.Equal(t, mutate.Survived, outcome, "a package whose tests all pass was not run, or was misread") // A package with no tests of its own whose importer has some is asked through the importer, // not reported as untested: that is where its behaviour is held. importersRan := func() int { for _, c := range mutate.Costs() { - if c.What == "alone: build and run the importers' tests" { + if c.What == "alone: run the importers' tests" { return c.N } } return 0 } before := importersRan() - outcome, out := mutate.Run(mutate.Mutation{File: filepath.Join("..", "..", "cmd", "ctxlife", "main.go")}, + outcome, out := runTests(t, mutate.Mutation{File: filepath.Join("..", "..", "cmd", "ctxlife", "main.go")}, mutate.Asked{Timeout: "2m", Importers: []string{"../fetch"}}) assert.Equal(t, mutate.Survived, outcome, "the importer's tests were not run: %s", out) assert.Contains(t, out, "internal/fetch", "the run does not say which package answered") @@ -402,7 +402,7 @@ func TestAnEditATestRefusesIsCountedRefused(t *testing.T) { m := doubling(t, dir) overlay, err := mutate.Overlay(m, t.TempDir()) require.NoError(t, err) - outcome, out := mutate.Run(m, mutate.Asked{Overlay: overlay, Timeout: "2m"}) + outcome, out := runTests(t, m, mutate.Asked{Overlay: overlay, Timeout: "2m"}) assert.Equal(t, mutate.Killed, outcome, "a package whose test fails was not counted as refusing: %s", out) assert.Contains(t, out, "not doubled", "what refused it was not the package's own test") } @@ -418,7 +418,7 @@ func TestAPackageWhoseTestsAreOfAnotherBuildIsUntested(t *testing.T) { m := doubling(t, dir) overlay, err := mutate.Overlay(m, t.TempDir()) require.NoError(t, err) - outcome, out := mutate.Run(m, mutate.Asked{Overlay: overlay, Timeout: "2m"}) + outcome, out := runTests(t, m, mutate.Asked{Overlay: overlay, Timeout: "2m"}) assert.Equal(t, mutate.Untested, outcome, "tests of another build were counted as asked: %s", out) } @@ -559,7 +559,7 @@ func G() int { // says which, per test. func TestAnEditIsPutToTheTestsThatReachIt(t *testing.T) { dir := refuses(t) - c, err := mutate.Cover(dir, "2m", make(chan struct{}, 2)) + c, err := mutate.Cover(dir, "2m", mutate.Slots(t.TempDir(), 2)) require.NoError(t, err) require.NotNil(t, c) assert.Equal(t, []string{"TestDouble"}, c.Reaching(filepath.Join(dir, "code.go"), 4)) @@ -571,19 +571,19 @@ func TestAnEditIsPutToTheTestsThatReachIt(t *testing.T) { require.NoError(t, err) // Asked of the one test that reaches it, it is refused; of a test that does not, it survives: // only the named test ran. - outcome, out := mutate.Run(m, mutate.Asked{Overlay: overlay, Timeout: "2m", Reaching: []string{"TestDouble"}}) + outcome, out := runTests(t, m, mutate.Asked{Overlay: overlay, Timeout: "2m", Reaching: []string{"TestDouble"}}) assert.Equal(t, mutate.Killed, outcome, "the one test that reaches the edit was not run: %s", out) - outcome, out = mutate.Run(m, mutate.Asked{Overlay: overlay, Timeout: "2m", Reaching: []string{"TestHalf"}}) + outcome, out = runTests(t, m, mutate.Asked{Overlay: overlay, Timeout: "2m", Reaching: []string{"TestHalf"}}) assert.Equal(t, mutate.Survived, outcome, "a test that was not named ran: %s", out) // An edit that does not compile asks nothing. broken := m broken.With = "@" overlay, err = mutate.Overlay(broken, t.TempDir()) require.NoError(t, err) - outcome, out = mutate.Run(broken, mutate.Asked{Overlay: overlay, Timeout: "2m"}) + outcome, out = runTests(t, broken, mutate.Asked{Overlay: overlay, Timeout: "2m"}) assert.Equal(t, mutate.Unbuildable, outcome, "an edit that does not build was counted: %s", out) // And reached by none, it is not run at all. - outcome, out = mutate.Run(m, mutate.Asked{Overlay: overlay, Timeout: "2m", Reaching: []string{}}) + outcome, out = runTests(t, m, mutate.Asked{Overlay: overlay, Timeout: "2m", Reaching: []string{}}) assert.Equal(t, mutate.Survived, outcome) assert.Empty(t, out, "the package's tests ran for a line none of them reaches") } diff --git a/internal/mutate/runner.go b/internal/mutate/runner.go index 2ef9439..4b0b1a4 100644 --- a/internal/mutate/runner.go +++ b/internal/mutate/runner.go @@ -4,10 +4,13 @@ import ( "cmp" "fmt" "io" + "maps" "os" "path/filepath" + "slices" "sort" "strconv" + "strings" "sync" "time" ) @@ -17,6 +20,9 @@ type Tally struct { Survived, Refused []Mutation Killed, Unbuildable, Untested int UntestedPackages map[string]bool + // Wrote are the files the tests left, changed or removed where they ran, by their paths in + // the source, which none of them was written to (Tree). + Wrote []string } // result is what the tests did about one mutation. @@ -40,7 +46,8 @@ func RunAll(chosen []Mutation, importers map[string][]string, timeout string, jo // One budget of test processes for the whole run, which measuring coverage draws on too: a // mutation's run and each package's coverage each had one of their own, and ten mutations // measuring ten tests each were a hundred test processes and as many PostgreSQL containers. - slots := make(chan struct{}, max(jobs, 1)) // mutate-exempt: how many run at once, not what they say + // Each slot is a tree of its own (Tree), which the process holding it runs in. + slots := Slots(filepath.Join(work, "trees"), max(jobs, 1)) // mutate-exempt: how many run at once, not what they say // Builds are what cost: go parallelises one across the machine already, so they go one at a // time, beside the test processes. b := &builds{sem: make(chan struct{}, 1), work: work, bins: map[string]*importerBinary{}} @@ -97,13 +104,25 @@ func RunAll(chosen []Mutation, importers map[string][]string, timeout string, jo } } sort.Slice(t.Survived, func(i, j int) bool { return t.Survived[i].String() < t.Survived[j].String() }) + // Every process is done, so every tree is back. + wrote := map[string]bool{} + for range cap(slots) { + for _, path := range (<-slots).Wrote() { + wrote[path] = true + } + } + t.Wrote = slices.Sorted(maps.Keys(wrote)) + if len(t.Wrote) > 0 { + _, _ = fmt.Fprintf(w, " the tests left or changed these in their copy of the module, and not in the source: %s\n", + strings.Join(t.Wrote, ", ")) + } return t, firstErr } // pkgRun is one package's mutations put to the tests. type pkgRun struct { dir, work, timeout string - slots chan struct{} + slots chan *Tree builds *builds importers []string results chan<- result @@ -131,7 +150,8 @@ func (p pkgRun) run(ms []Mutation, covers *coverages) { wg.Add(1) go func() { defer wg.Done() - p.results <- result{m: m, outcome: p.built(m, s, c)} + outcome, err := p.built(m, s, c) + p.results <- result{m: m, outcome: outcome, err: err} }() } for i, m := range s.Alone { @@ -141,9 +161,9 @@ func (p pkgRun) run(ms []Mutation, covers *coverages) { // Measured before a slot is taken: the measuring takes slots of its own, and holding // one while waiting on the others is how a budget of one never finishes. reaching := covers.of(p.dir) - p.slots <- struct{}{} - defer func() { <-p.slots }() - outcome, err := runAlone(m, filepath.Join(p.work, "alone-"+strconv.Itoa(i)), p.timeout, reaching, p.importers, p.out) + tree := <-p.slots + defer func() { p.slots <- tree }() + outcome, err := runAlone(m, filepath.Join(p.work, "alone-"+strconv.Itoa(i)), p.timeout, reaching, p.importers, tree, p.out) p.results <- result{m: m, outcome: outcome, err: err} }() } @@ -152,15 +172,15 @@ func (p pkgRun) run(ms []Mutation, covers *coverages) { // built puts m, which is in s's binary, to the package's tests that reach its line, then to each // importer's tests, built once with the same schema: a survivor pays for the second. -func (p pkgRun) built(m Mutation, s *Schema, c *Coverage) Outcome { +func (p pkgRun) built(m Mutation, s *Schema, c *Coverage) (Outcome, error) { if s.Binary == "" && len(p.importers) == 0 { - return Untested + return Untested, nil } - ask := func(bin, dir string, only []string) Outcome { - p.slots <- struct{}{} - defer func() { <-p.slots }() - outcome, _ := Ask(bin, dir, m.ID, p.timeout, only) - return outcome + ask := func(bin, dir string, only []string) (Outcome, error) { + tree := <-p.slots + defer func() { p.slots <- tree }() + outcome, _, err := Ask(tree, bin, dir, m.ID, p.timeout, only) + return outcome, err } if s.Binary != "" { var reaching []string @@ -168,8 +188,8 @@ func (p pkgRun) built(m Mutation, s *Schema, c *Coverage) Outcome { reaching = c.Reaching(m.File, m.Line) } if reaching == nil || len(reaching) > 0 { - if outcome := ask(s.Binary, p.dir, reaching); outcome != Survived { - return outcome + if outcome, err := ask(s.Binary, p.dir, reaching); err != nil || outcome != Survived { + return outcome, err } } } @@ -178,11 +198,11 @@ func (p pkgRun) built(m Mutation, s *Schema, c *Coverage) Outcome { if bin == "" { continue } - if outcome := ask(bin, dir, nil); outcome != Survived { - return outcome + if outcome, err := ask(bin, dir, nil); err != nil || outcome != Survived { + return outcome, err } } - return Survived + return Survived, nil } // builds is the run's one build at a time, and the importers' binaries it made, each once. @@ -223,8 +243,8 @@ func (b *builds) importer(dir, importer, overlay string) (string, string) { } // runAlone builds the mutation in through an overlay under dir, and runs the package's tests that -// reach its line and then its importers'. The file itself is never written. -func runAlone(m Mutation, dir, timeout string, c *Coverage, importers []string, out io.Writer) (Outcome, error) { +// reach its line and then its importers', in tree. The file itself is never written. +func runAlone(m Mutation, dir, timeout string, c *Coverage, importers []string, tree *Tree, out io.Writer) (Outcome, error) { if err := os.MkdirAll(dir, 0o700); err != nil { return 0, err } @@ -232,12 +252,15 @@ func runAlone(m Mutation, dir, timeout string, c *Coverage, importers []string, if err != nil { return 0, err } - asked := Asked{Overlay: overlay, Timeout: timeout, Importers: importers} + asked := Asked{Overlay: overlay, Timeout: timeout, Importers: importers, Tree: tree} if c != nil { asked.Reaching = c.Reaching(m.File, m.Line) } started := time.Now() - outcome, _ := Run(m, asked) + outcome, _, err := Run(m, asked) + if err != nil { + return 0, err + } if took := time.Since(started); took > time.Minute { _, _ = fmt.Fprintf(out, " (%s took %s)\n", filepath.Dir(m.File), took.Round(time.Second)) } @@ -249,7 +272,7 @@ type coverages struct { mu sync.Mutex by map[string]*coverage timeout string - slots chan struct{} + slots chan *Tree } type coverage struct { diff --git a/internal/mutate/runner_internal_test.go b/internal/mutate/runner_internal_test.go index 8a1eb5d..04e2184 100644 --- a/internal/mutate/runner_internal_test.go +++ b/internal/mutate/runner_internal_test.go @@ -35,10 +35,10 @@ func TestAnEditWithNoCoverageIsAskedOfEveryTest(t *testing.T) { if err != nil || s.Binary == "" || len(s.Built) == 0 { t.Fatalf("schema: %+v, %v", s, err) } - p := pkgRun{dir: dir, timeout: "2m", slots: make(chan struct{}, 1)} + p := pkgRun{dir: dir, timeout: "2m", slots: Slots(t.TempDir(), 1)} for _, m := range s.Built { - if got := p.built(m, s, nil); got != Killed { - t.Errorf("%s: %v with no coverage, want refused by the test nobody measured", m, got) + if got, err := p.built(m, s, nil); err != nil || got != Killed { + t.Errorf("%s: %v (%v) with no coverage, want refused by the test nobody measured", m, got, err) } } } @@ -77,7 +77,7 @@ func TestAnEditThatAllocatesForEverIsStoppedAndRefused(t *testing.T) { plan[0].ID = 1 // Unbounded, which builds what the package imports: on a cold cache that is the standard // library, compiled a package a core, which is no test's memory. - if outcome, out := Run(plan[0], Asked{Timeout: "2m"}); outcome != Survived { + if outcome, out := runTests(t, plan[0], Asked{Timeout: "2m"}); outcome != Survived { t.Fatalf("the package as it is was %v: %s", outcome, out) } memLimit = 256 << 20 @@ -101,10 +101,10 @@ func TestAnEditThatAllocatesForEverIsStoppedAndRefused(t *testing.T) { } // The package as it is, built and run the same way under the same bound, is let be: it is the // edit's loop that is stopped, not the go command and its compiler. - if outcome, out := Run(plan[0], Asked{Timeout: "2m"}); outcome != Survived { + if outcome, out := runTests(t, plan[0], Asked{Timeout: "2m"}); outcome != Survived { t.Fatalf("the package as it is was %v under the bound: %s", outcome, out) } - if outcome, out := Run(plan[0], Asked{Overlay: overlay, Timeout: "2m"}); outcome != Exceeded { + if outcome, out := runTests(t, plan[0], Asked{Overlay: overlay, Timeout: "2m"}); outcome != Exceeded { t.Errorf("the edit built alone was %v, not stopped at its bound: %s", outcome, out) } // Killed, it is gone once the kernel has reaped it; left running, it sleeps for an hour. diff --git a/internal/mutate/runner_test.go b/internal/mutate/runner_test.go index 3d2810f..5384925 100644 --- a/internal/mutate/runner_test.go +++ b/internal/mutate/runner_test.go @@ -88,6 +88,7 @@ func TestEveryEditIsCountedAsTheTestsAnswered(t *testing.T) { assert.Contains(t, said.String(), bare+": 2 edit(s) in one binary, 1 built alone", "Wide's int64 literal is built alone, and a run of one is what is timed") assert.NotContains(t, said.String(), " took ", "a run under a minute was said to be slow") + assert.NotContains(t, said.String(), "in their copy of the module", "tests that wrote nothing were said to have") } // A test that fails with nothing broken refuses no edit: the coverage the run measures first says @@ -159,3 +160,126 @@ func TestAnEditThatCannotBeAskedIsAnError(t *testing.T) { _, err := mutate.RunAll([]mutate.Mutation{gone}, nil, "1m", 1, &strings.Builder{}) assert.Error(t, err) } + +// A test that writes into its directory writes into a copy of the module of its own, put back +// after every run, and never into the source. Run in the package's directory, the note the +// unmutated test leaves behind - while its coverage is measured - was there when each edit that +// stops Save writing it was asked, so every one of them was let through; with edits asked side by +// side, what one wrote was read by another's tests, a different set let through each run. And the +// note stayed in the checkout. The copy is the whole module, so the test still reads testdata/ +// and ../doc.txt as it would in place. +func TestATestThatWritesInItsDirectoryWritesInACopyOfItsOwn(t *testing.T) { + dir := notes(t) + pkg := filepath.Join(dir, "p") + plan, err := mutate.Plan(filepath.Join(pkg, "code.go"), map[int]bool{7: true}) + require.NoError(t, err) + require.Len(t, plan, 2, "Save's comparison and its bound: %v", plan) + for i := range plan { + plan[i].ID = i + 1 + } + var said strings.Builder + tally, err := mutate.RunAll(plan, nil, "2m", 2, &said) + require.NoError(t, err) + assert.Equal(t, len(plan), tally.Killed, "an edit that stops Save writing the note was let through: %s", said.String()) + assert.Empty(t, tally.Survived, said.String()) + assert.NoFileExists(t, filepath.Join(pkg, "note"), "a test's file was written into the source") + assert.Equal(t, []string{filepath.Join(pkg, "note")}, tally.Wrote, "what the tests left is not said") + assert.Contains(t, said.String(), "in their copy of the module, and not in the source: "+filepath.Join(pkg, "note")+"\n", + "the report does not say what the tests left") +} + +// The same of an edit built alone: its package's tests are built where the sources are, and run +// in the copy. Asked after a run that left the note, it is refused, and the note is in no source. +func TestAnEditBuiltAloneIsAskedInACopyOfItsOwn(t *testing.T) { + dir := notes(t) + code := filepath.Join(dir, "p", "code.go") + tree := mutate.NewTree(t.TempDir()) + outcome, out, err := mutate.Run(mutate.Mutation{File: code}, mutate.Asked{Timeout: "2m", Tree: tree}) + require.NoError(t, err) + require.Equal(t, mutate.Survived, outcome, "the package's test fails unmutated: %s", out) + assert.Contains(t, out, "ok \texample.com/notes/p\n", "the package's answer is not named") + + plan, err := mutate.Plan(code, map[int]bool{7: true}) + require.NoError(t, err) + require.NotEmpty(t, plan) + overlay, err := mutate.Overlay(plan[0], t.TempDir()) + require.NoError(t, err) + outcome, out, err = mutate.Run(plan[0], mutate.Asked{Overlay: overlay, Timeout: "2m", Tree: tree}) + require.NoError(t, err) + assert.Equal(t, mutate.Killed, outcome, "%s was let through by the note a run before it left: %s", plan[0], out) + assert.Contains(t, out, "no note was written", "what refused it was not the package's own test") + assert.Contains(t, out, "FAIL\texample.com/notes/p\n", "the package's refusal is not named") + assert.NoFileExists(t, filepath.Join(dir, "p", "note"), "a test's file was written into the source") + assert.Equal(t, []string{filepath.Join(dir, "p", "note")}, tree.Wrote()) +} + +// notes is a module whose package p has Save, which writes a note into the directory it runs in, +// and a test that reads what is around it and leaves the note behind, as many tests do. +func notes(t *testing.T) string { + t.Helper() + dir := t.TempDir() + for name, body := range map[string]string{ + "go.mod": "module example.com/notes\n\ngo 1.24\n", + "doc.txt": "read from the package as ../doc.txt\n", + "p/testdata/in.txt": "read from the package as testdata/in.txt\n", + "p/code.go": "package p\n\nimport \"os\"\n\n// Save writes the note when there is something to put in it.\nfunc Save(n int) error {\n" + + "\tif n > 0 {\n\t\treturn os.WriteFile(\"note\", []byte(\"noted\"), 0o600)\n\t}\n\treturn nil\n}\n", + "p/code_test.go": "package p\n\nimport (\n\t\"os\"\n\t\"testing\"\n)\n\nfunc TestSaveWritesTheNote(t *testing.T) {\n" + + "\tfor _, path := range []string{\"testdata/in.txt\", \"../doc.txt\"} {\n\t\tif _, err := os.Stat(path); err != nil {\n\t\t\tt.Fatal(err)\n\t\t}\n\t}\n" + + "\tif err := Save(1); err != nil {\n\t\tt.Fatal(err)\n\t}\n" + + "\tif _, err := os.Stat(\"note\"); err != nil {\n\t\tt.Fatal(\"no note was written\")\n\t}\n}\n", + } { + path := filepath.Join(dir, filepath.FromSlash(name)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o700)) + require.NoError(t, os.WriteFile(path, []byte(body), 0o600)) + } + return dir +} + +// An edit the package's own tests let through is asked of every importer in turn, until one +// refuses it: lib's test calls Double and checks nothing, also's the same, and use's holds it. In +// the package's binary and its importers', and built alone. +func TestAnEditItsOwnTestsLetThroughIsAskedOfEachImporterInTurn(t *testing.T) { + layered(t) + importers := []string{"./also", "./use"} + plan, err := mutate.Plan(filepath.Join("lib", "lib.go"), map[int]bool{4: true}) + require.NoError(t, err) + require.NotEmpty(t, plan) + for i := range plan { + plan[i].ID = i + 1 + } + var said strings.Builder + tally, err := mutate.RunAll(plan, map[string][]string{"lib": importers}, "2m", 2, &said) + require.NoError(t, err) + assert.Equal(t, len(plan), tally.Killed, "Double's edits are refused by use, after lib and also let them through: %s", said.String()) + + overlay, err := mutate.Overlay(plan[0], t.TempDir()) + require.NoError(t, err) + outcome, out := runTests(t, plan[0], mutate.Asked{Overlay: overlay, Timeout: "2m", Importers: importers}) + assert.Equal(t, mutate.Killed, outcome, "built alone, %s was not refused by use: %s", plan[0], out) + // Each package is named as go test names it, with what it said. + for _, verdict := range []string{"ok \texample.com/layered/lib\n", "ok \texample.com/layered/also\n", "FAIL\texample.com/layered/use\n"} { + assert.Contains(t, out, verdict, "the packages asked are not each named with their answer") + } +} + +// layered is a module, and the directory this runs from, whose lib's Double is called by the tests +// of lib and of its importer also, which check nothing, and held by those of its importer use. +func layered(t *testing.T) { + t.Helper() + dir := t.TempDir() + runs := "\n\nimport (\n\t\"testing\"\n\n\t\"example.com/layered/lib\"\n)\n\nfunc TestRuns(t *testing.T) {\n\t_ = lib.Double(2)\n}\n" + for name, body := range map[string]string{ + "go.mod": "module example.com/layered\n\ngo 1.24\n", + "lib/lib.go": "package lib\n\nfunc Double(n int) int {\n\treturn n + n\n}\n", + "lib/lib_test.go": "package lib_test" + runs, + "also/also.go": "package also\n", + "also/also_test.go": "package also_test" + runs, + "use/use.go": "package use\n\nimport \"example.com/layered/lib\"\n\nfunc Four() int { return lib.Double(2) }\n", + "use/use_test.go": "package use\n\nimport \"testing\"\n\nfunc TestFour(t *testing.T) {\n\tif Four() != 4 {\n\t\tt.Fatal(\"not four\")\n\t}\n}\n", + } { + require.NoError(t, os.MkdirAll(filepath.Join(dir, filepath.Dir(name)), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600)) + } + t.Chdir(dir) +} diff --git a/internal/mutate/schema.go b/internal/mutate/schema.go index 7b91a26..701aa3f 100644 --- a/internal/mutate/schema.go +++ b/internal/mutate/schema.go @@ -364,9 +364,10 @@ func writeSchema(dir string, ms []Mutation, work string) (string, error) { return path, os.WriteFile(path, overlay, 0o600) } -// Ask runs a test binary in dir with mutation id switched on, stopping at the first test that -// fails: only, when it names any, and every test when it is nil. -func Ask(bin, dir string, id int, timeout string, only []string) (Outcome, string) { +// Ask runs a test binary in the tree's copy of dir, a package's directory, with mutation id +// switched on, stopping at the first test that fails: only, when it names any, and every test +// when it is nil. +func Ask(tree *Tree, bin, dir string, id int, timeout string, only []string) (Outcome, string, error) { // mutate-exempt: which allowance a run gets; only a test an edit hangs tells them apart, and // only after a minute of it. if len(only) > 0 { @@ -377,17 +378,26 @@ func Ask(bin, dir string, id int, timeout string, only []string) (Outcome, strin args = append(args, "-test.run", "^("+strings.Join(only, "|")+")$") } cmd := exec.Command(bin, args...) //nolint:gosec // a test binary this run built - cmd.Dir = dir cmd.Env = testEnv(switchEnv + "=" + strconv.Itoa(id)) - out, exceeded, err := bounded("schema: run one edit", cmd) + var ( + out []byte + exceeded bool + runErr error + ) + if err := tree.in(dir, func(at string) { + cmd.Dir = at + out, exceeded, runErr = bounded("schema: run one edit", cmd) + }); err != nil { + return 0, "", err + } countTests(dir, out) if exceeded { - return Exceeded, string(out) + return Exceeded, string(out), nil } - if err != nil { - return Killed, string(out) + if runErr != nil { + return Killed, string(out), nil //nolint:nilerr // a test that fails is the edit refused, not this run failing } - return Survived, string(out) + return Survived, string(out), nil } // ImporterBinary builds the tests of importer, a package named from the module's root, at out, diff --git a/internal/mutate/schema_test.go b/internal/mutate/schema_test.go index b1ff38f..86a5fc8 100644 --- a/internal/mutate/schema_test.go +++ b/internal/mutate/schema_test.go @@ -167,21 +167,21 @@ func TestAnEditInTheSchemaIsAnsweredAsItIsAlone(t *testing.T) { assert.Equal(t, "6", wide(t, plan).With, "the literal alone is not what its edit says") assert.Len(t, s.Built, len(plan)-len(s.Alone)) - outcome, out := mutate.Ask(s.Binary, dir, 0, "2m", nil) + outcome, out := askTests(t, s.Binary, dir, 0, "2m", nil) require.Equal(t, mutate.Survived, outcome, "with no edit on, the package failed its own tests: %s", out) - covered, err := mutate.Cover(dir, "2m", make(chan struct{}, 2)) + covered, err := mutate.Cover(dir, "2m", mutate.Slots(t.TempDir(), 2)) require.NoError(t, err) killed := 0 for _, m := range s.Built { - inSchema, said := mutate.Ask(s.Binary, dir, m.ID, "2m", nil) - reaching, _ := mutate.Ask(s.Binary, dir, m.ID, "2m", covered.Reaching(m.File, m.Line)) + inSchema, said := askTests(t, s.Binary, dir, m.ID, "2m", nil) + reaching, _ := askTests(t, s.Binary, dir, m.ID, "2m", covered.Reaching(m.File, m.Line)) assert.Equal(t, inSchema, reaching, "%s: the tests that reach its line in the file answer otherwise in the schema", m) - nobody, _ := mutate.Ask(s.Binary, dir, m.ID, "2m", []string{"TestNothingIsCalledThis"}) + nobody, _ := askTests(t, s.Binary, dir, m.ID, "2m", []string{"TestNothingIsCalledThis"}) assert.Equal(t, mutate.Survived, nobody, "%s: tests that were not named ran", m) overlay, err := mutate.Overlay(m, t.TempDir()) require.NoError(t, err) - built, by := mutate.Run(m, mutate.Asked{Overlay: overlay, Timeout: "2m"}) + built, by := runTests(t, m, mutate.Asked{Overlay: overlay, Timeout: "2m"}) assert.Equal(t, built, inSchema, "%s: alone %s, in the schema %s", m, by, said) assert.Equal(t, firstFailure(by), firstFailure(said), "%s: another test refused it in the schema", m) if inSchema == mutate.Killed { @@ -278,7 +278,7 @@ func TestAnEditIsAskedInItsPackagesDirectory(t *testing.T) { build.Dir = dir out, err := build.CombinedOutput() require.NoError(t, err, "%s", out) - outcome, said := mutate.Ask(bin, dir, 0, "2m", []string{"TestHalf"}) + outcome, said := askTests(t, bin, dir, 0, "2m", []string{"TestHalf"}) assert.Equal(t, mutate.Survived, outcome, "TestHalf did not run beside code.go: %s", said) } diff --git a/internal/mutate/share_test.go b/internal/mutate/share_test.go index e96353e..23e563d 100644 --- a/internal/mutate/share_test.go +++ b/internal/mutate/share_test.go @@ -65,7 +65,7 @@ func TestEveryTestProcessIsHeldToItsShare(t *testing.T) { dir := oneProc(t) share(t, 1<<20) - c, err := Cover(dir, "1m", make(chan struct{}, 1)) + c, err := Cover(dir, "1m", Slots(t.TempDir(), 1)) if err != nil { t.Fatal(err) } @@ -79,11 +79,11 @@ func TestEveryTestProcessIsHeldToItsShare(t *testing.T) { if out, err := build.CombinedOutput(); err != nil { t.Fatalf("%v: %s", err, out) } - if outcome, out := Ask(bin, dir, 0, "1m", nil); outcome != Survived { + if outcome, out := askTests(t, bin, dir, 0, "1m", nil); outcome != Survived { t.Errorf("a schema binary was asked on every core: %s", out) } - if outcome, out := Run(Mutation{File: filepath.Join(dir, "code.go")}, Asked{Timeout: "1m"}); outcome != Survived { + if outcome, out := runTests(t, Mutation{File: filepath.Join(dir, "code.go")}, Asked{Timeout: "1m"}); outcome != Survived { t.Errorf("an edit built alone was asked on every core: %s", out) } diff --git a/internal/mutate/tree.go b/internal/mutate/tree.go new file mode 100644 index 0000000..c9165f3 --- /dev/null +++ b/internal/mutate/tree.go @@ -0,0 +1,319 @@ +package mutate + +import ( + "errors" + "fmt" + "io/fs" + "maps" + "os" + "os/exec" + "path/filepath" + "slices" + "strconv" + "strings" + "time" +) + +// Tree is a private copy of the modules a run's tests are run in, held by one test process at a +// time and put back as its source is after each. A test that writes into its directory - a +// Dockerfile it generates, a file it means to read back - writes into a copy no other run reads. +// Run where the package's sources are, edits asked side by side shared one directory: what one +// wrote, another's tests read, so an edit refused alone was let through beside another, and the +// files stayed in the checkout when the run was over. +// +// The copy is the module as git sees it: its tracked files and those it does not ignore, so a +// test reads testdata/ and ../../docs as it would in place. A directory git ignores (node_modules, +// a build's output) and a repository nested in the module are linked, not copied: read as they +// are, and shared. Outside a git work tree it is every file under the module's root. The tests +// are built where the sources are, and only run here. +type Tree struct { + dir string + copies map[string]*treeCopy // by the root of the module each copies + wrote map[string]bool // what runs left or changed, by its path in the source +} + +// NewTree is a tree under dir, which it makes when it is first used. +func NewTree(dir string) *Tree { + return &Tree{dir: dir, copies: map[string]*treeCopy{}, wrote: map[string]bool{}} +} + +// Slots is n trees under dir, each a directory of its own: a run's budget of test processes, and +// where each runs. +func Slots(dir string, n int) chan *Tree { + slots := make(chan *Tree, n) + for i := range n { + slots <- NewTree(filepath.Join(dir, strconv.Itoa(i))) + } + return slots +} + +// Wrote is every file the tests left, changed or removed in the tree, by its path in the source, +// sorted. None of them was written there. +func (t *Tree) Wrote() []string { + return slices.Sorted(maps.Keys(t.wrote)) +} + +// in calls run with where dir, a package's directory, is in the tree, copying its module the +// first time, and puts the copy back as the source is once run returns. +func (t *Tree) in(dir string, run func(at string)) error { + abs, err := filepath.Abs(dir) + if err != nil { + return err + } + root, err := moduleRoot(abs) + if err != nil { + return err + } + c := t.copies[root] + if c == nil { + c, err = copyModule(root, filepath.Join(t.dir, strconv.Itoa(len(t.copies)))) + if err != nil { + return fmt.Errorf("mutate: copying %s to run its tests in: %w", root, err) + } + t.copies[root] = c + } + rel, err := filepath.Rel(root, abs) + if err != nil { + return err + } + run(filepath.Join(c.dst, rel)) + if err := c.reset(func(rel string) { t.wrote[filepath.Join(c.src, rel)] = true }); err != nil { + return fmt.Errorf("mutate: putting the copy of %s back after its tests: %w", root, err) + } + return nil +} + +// bin is where a test binary the tree's holder builds goes, beside the copies: one at a time. +func (t *Tree) bin() (string, error) { + if err := os.MkdirAll(t.dir, 0o700); err != nil { + return "", err + } + bin := filepath.Join(t.dir, "pkg.test") + // A package with no tests builds none, and the last one's must not be run in its place. + if err := os.Remove(bin); err != nil && !errors.Is(err, fs.ErrNotExist) { + return "", err + } + return bin, nil +} + +// treeCopy is one module copied: each file as it was made, and every directory that holds one. +type treeCopy struct { + src, dst string + files map[string]treeFile + dirs map[string]bool +} + +// treeFile is a file of the copy as it was made: a symlink's target, or a regular file's stat. +type treeFile struct { + link string + size int64 + mode fs.FileMode + modTime time.Time +} + +func copyModule(src, dst string) (*treeCopy, error) { + files, err := moduleFiles(src) + if err != nil { + return nil, err + } + c := &treeCopy{src: src, dst: dst, files: map[string]treeFile{}, dirs: map[string]bool{}} + if err := os.MkdirAll(dst, 0o700); err != nil { + return nil, err + } + for _, rel := range files { + if err := c.put(rel); err != nil { + return nil, err + } + } + return c, nil +} + +// moduleFiles is what of the module under root is copied, by its path from root: what git +// tracks or does not ignore, and each directory it ignores whole, which is linked. +func moduleFiles(root string) ([]string, error) { + inside := exec.Command("git", "rev-parse", "--is-inside-work-tree") + inside.Dir = root + if _, err := output("listing", inside); err != nil { + return walkFiles(root) + } + var out []string + for _, args := range [][]string{ + {"ls-files", "-z", "--cached", "--others", "--exclude-standard"}, + {"ls-files", "-z", "--others", "--ignored", "--exclude-standard", "--directory"}, + } { + list := exec.Command("git", args...) + list.Dir = root + said, err := output("listing", list) + if err != nil { + return nil, fmt.Errorf("git %s: %w", strings.Join(args, " "), err) + } + ignored := slices.Contains(args, "--ignored") + for _, path := range strings.Split(string(said), "\x00") { + dir := strings.HasSuffix(path, "/") + // An ignored file is a build's or a run's output, which no test reads in place of + // the source; a directory may be what one does read, node_modules or a checkout. + if path == "" || ignored && !dir { + continue + } + out = append(out, filepath.FromSlash(strings.TrimSuffix(path, "/"))) + } + } + return out, nil +} + +// walkFiles is every file under root but git's own. +func walkFiles(root string) ([]string, error) { + var out []string + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if d.Name() == ".git" { + return fs.SkipDir + } + return nil + } + rel, err := filepath.Rel(root, path) + out = append(out, rel) + return err + }) + return out, err +} + +// put makes rel in the copy as it is in the source: a regular file copied, a symlink with the +// same target, and a directory - a nested repository, an ignored one - linked to the source's. +func (c *treeCopy) put(rel string) error { + src, dst := filepath.Join(c.src, rel), filepath.Join(c.dst, rel) + if err := os.RemoveAll(dst); err != nil { + return err + } + info, err := os.Lstat(src) + if errors.Is(err, fs.ErrNotExist) { + // Tracked and since deleted, or gone while the run went on: the copy has it no more. + delete(c.files, rel) + return nil + } + if err != nil { + return err + } + if err := os.MkdirAll(filepath.Dir(dst), 0o700); err != nil { + return err + } + var f treeFile + switch { + case info.IsDir(): + f.link = src + case info.Mode()&fs.ModeSymlink != 0: + if f.link, err = os.Readlink(src); err != nil { + return err + } + case info.Mode().IsRegular(): + if f, err = copyFile(src, dst, info); err != nil { + return err + } + default: + return nil // a socket or a device: nothing a test reads as a file + } + if f.link != "" { + if err := os.Symlink(f.link, dst); err != nil { + return err + } + } + c.files[rel] = f + for d := filepath.Dir(rel); d != "."; d = filepath.Dir(d) { + c.dirs[d] = true + } + return nil +} + +// copyFile copies src to dst with its mode and its modification time, which is in the past, so +// any write after is seen by the time alone; the stat it answers is the copy's. +func copyFile(src, dst string, info fs.FileInfo) (treeFile, error) { + body, err := os.ReadFile(src) //nolint:gosec // a file of the module being copied + if err != nil { + return treeFile{}, err + } + if err := os.WriteFile(dst, body, info.Mode().Perm()); err != nil { + return treeFile{}, err + } + // WriteFile's mode is under the umask; the copy's is the source's. + if err := os.Chmod(dst, info.Mode().Perm()); err != nil { + return treeFile{}, err + } + if err := os.Chtimes(dst, info.ModTime(), info.ModTime()); err != nil { + return treeFile{}, err + } + made, err := os.Lstat(dst) + if err != nil { + return treeFile{}, err + } + return treeFile{size: made.Size(), mode: made.Mode(), modTime: made.ModTime()}, nil +} + +// reset puts the copy back as it was made, from the source, and says each file a run left, +// changed or removed. +func (c *treeCopy) reset(wrote func(rel string)) error { + // What is removed is removed within the copy, whatever a run put in it: a link it made is + // removed, never followed. + root, err := os.OpenRoot(c.dst) + if err != nil { + return err + } + defer func() { _ = root.Close() }() + seen := map[string]bool{} + err = filepath.WalkDir(c.dst, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel, err := filepath.Rel(c.dst, path) + if err != nil || rel == "." { + return err + } + if d.IsDir() { + if c.dirs[rel] { + return nil + } + wrote(rel) + if err := root.RemoveAll(rel); err != nil { + return err + } + return fs.SkipDir + } + f, ok := c.files[rel] + seen[rel] = true + if ok && c.unchanged(path, f) { + return nil + } + wrote(rel) + if !ok { + return root.Remove(rel) + } + return c.put(rel) + }) + if err != nil { + return err + } + for rel := range c.files { + if !seen[rel] { + wrote(rel) + if err := c.put(rel); err != nil { + return err + } + } + } + return nil +} + +// unchanged is whether the file at path is still as f says it was made. +func (c *treeCopy) unchanged(path string, f treeFile) bool { + info, err := os.Lstat(path) + if err != nil { + return false + } + if f.link != "" { + target, err := os.Readlink(path) + return err == nil && target == f.link + } + return info.Mode() == f.mode && info.Size() == f.size && info.ModTime().Equal(f.modTime) +} diff --git a/internal/mutate/tree_internal_test.go b/internal/mutate/tree_internal_test.go new file mode 100644 index 0000000..50d4724 --- /dev/null +++ b/internal/mutate/tree_internal_test.go @@ -0,0 +1,148 @@ +package mutate + +import ( + "io/fs" + "os" + "os/exec" + "path/filepath" + "slices" + "testing" +) + +// A tree copies the module as git sees it - what it tracks and what it does not ignore - links a +// directory it ignores, leaves out a file it ignores, and after each run puts back whatever the +// run left, changed or removed, naming each by its path in the source, which is never written. +func TestATreeIsTheModuleAsGitSeesItPutBackAfterEachRun(t *testing.T) { + root := gitModule(t) + tree := NewTree(t.TempDir()) + var at string + ran := func(run func(at string)) { + t.Helper() + if err := tree.in(filepath.Join(root, "p"), run); err != nil { + t.Fatal(err) + } + } + ran(func(dir string) { + at = dir + copiedAsGitSeesIt(t, dir) + // What a run leaves: a file and a directory; a file rewritten at the same size, so only its + // time says so, and one whose mode alone changed; two removed; a link pointed elsewhere. + for _, err := range []error{ + os.WriteFile(filepath.Join(dir, "left.txt"), []byte("left"), 0o600), + os.MkdirAll(filepath.Join(dir, "made/under"), 0o700), + os.WriteFile(filepath.Join(dir, "tracked.go"), []byte("package q\n"), 0o600), + os.Chmod(filepath.Join(dir, "mode.txt"), 0o400), + os.Remove(filepath.Join(dir, "untracked.txt")), + os.Remove(filepath.Join(dir, "testdata/x.txt")), + os.Remove(filepath.Join(dir, "link")), + os.Symlink("elsewhere", filepath.Join(dir, "link")), + } { + if err != nil { + t.Fatal(err) + } + } + }) + var want []string + for _, name := range []string{"left.txt", "link", "made", "mode.txt", "testdata/x.txt", "tracked.go", "untracked.txt"} { + want = append(want, filepath.Join(root, "p", name)) + } + if got := tree.Wrote(); !slices.Equal(got, want) { + t.Errorf("wrote %v, want %v", got, want) + } + putBack(t, root, at) + + // The next run finds it as it was made, and leaves nothing to name. + ran(func(dir string) { + if dir != at { + t.Errorf("the second run is in %s, not the copy already made at %s", dir, at) + } + }) + if got := tree.Wrote(); !slices.Equal(got, want) { + t.Errorf("a run that wrote nothing added to what was written: %v", got) + } +} + +// gitModule is a module in a git work tree: files tracked, one neither tracked nor ignored, an +// ignored file in its package p, and a directory it ignores. +func gitModule(t *testing.T) string { + t.Helper() + root := t.TempDir() + for name, body := range map[string]string{ + "go.mod": "module example.com/tree\n\ngo 1.24\n", + ".gitignore": "out/\n*.log\n", + "doc.txt": "tracked, beside the package\n", + "p/tracked.go": "package p\n", + "p/untracked.txt": "not committed, and not ignored\n", + "p/skipped.log": "ignored\n", + "out/built.txt": "in an ignored directory\n", + "p/testdata/x.txt": "a fixture\n", + "p/mode.txt": "its mode is changed\n", + } { + path := filepath.Join(root, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink("tracked.go", filepath.Join(root, "p/link")); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"init", "-q"}, {"add", "go.mod", ".gitignore", "doc.txt", "p/tracked.go", "p/link", "p/testdata"}} { + git := exec.Command("git", args...) + git.Dir = root + if out, err := git.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + } + return root +} + +// copiedAsGitSeesIt checks the copy of gitModule's package p at dir. +func copiedAsGitSeesIt(t *testing.T, dir string) { + t.Helper() + for _, name := range []string{"tracked.go", "untracked.txt", "testdata/x.txt", "../doc.txt", "../go.mod"} { + if info, err := os.Lstat(filepath.Join(dir, name)); err != nil || !info.Mode().IsRegular() { + t.Errorf("%s is not copied: %v", name, err) + } + } + if info, err := os.Lstat(filepath.Join(dir, "../out")); err != nil || info.Mode()&fs.ModeSymlink == 0 { + t.Errorf("the ignored directory is not linked: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "../out/built.txt")); err != nil { + t.Errorf("the ignored directory is not read through its link: %v", err) + } + if _, err := os.Lstat(filepath.Join(dir, "skipped.log")); err == nil { + t.Error("an ignored file was copied") + } + if target, err := os.Readlink(filepath.Join(dir, "link")); err != nil || target != "tracked.go" { + t.Errorf("the link is %q (%v), want the source's own, to tracked.go", target, err) + } +} + +// putBack checks the copy at dir of root's package p is as it was made, and the source as it was. +func putBack(t *testing.T, root, dir string) { + t.Helper() + for _, gone := range []string{"left.txt", "made"} { + if _, err := os.Lstat(filepath.Join(dir, gone)); err == nil { + t.Errorf("%s was left in the copy", gone) + } + if _, err := os.Lstat(filepath.Join(root, "p", gone)); err == nil { + t.Errorf("%s was written into the source", gone) + } + } + if target, err := os.Readlink(filepath.Join(dir, "link")); err != nil || target != "tracked.go" { + t.Errorf("the link is %q (%v) after the run, want it put back to tracked.go", target, err) + } + if info, err := os.Stat(filepath.Join(dir, "mode.txt")); err != nil || info.Mode().Perm() != 0o600 { + t.Errorf("mode.txt is %v (%v) after the run, want its source's 0600", info, err) + } + for name, body := range map[string]string{ + "tracked.go": "package p\n", "untracked.txt": "not committed, and not ignored\n", "testdata/x.txt": "a fixture\n", + } { + if got, err := os.ReadFile(filepath.Join(dir, name)); err != nil || string(got) != body { + t.Errorf("%s is %q (%v) in the copy after the run, want it put back", name, got, err) + } + } +} diff --git a/site/src/content/docs/mutate.mdx b/site/src/content/docs/mutate.mdx index 2af4604..db26278 100644 --- a/site/src/content/docs/mutate.mdx +++ b/site/src/content/docs/mutate.mdx @@ -185,6 +185,23 @@ It exits 0 when every edit was refused, 1 when one survived, and 2 when it could need besides the code, like a database, comes from that environment. spin runs `mutate` under its `hack/testpg`, which starts one PostgreSQL for every test process. +### Tests that write files + +`go test` runs a package's tests in its source directory. `mutate` runs many edits at once, so it +can't: one edit's tests would read the files another's wrote, and the verdict would depend on which +edits ran side by side. Instead, every test process runs in a private copy of the module, one per +`-j`, and the copy is reset after each run. Tests still read `testdata/` and paths like +`../../docs` as they would in place, and nothing they write reaches your checkout. + +The copy holds what git sees: tracked files and files git doesn't ignore. Directories git ignores +(`node_modules`, a build's output) and repositories nested in the module are linked rather than +copied, so tests can read them, but those are shared. Outside a git work tree, the copy is every +file under the module's root. Packages are still built from your checkout, and only their tests run +in the copy. + +When the tests leave files behind, or change or remove one, the run's report lists them. Those +are files the same tests would leave in your checkout under `go test`. + ### Keeping a broken edit from taking the machine An edit can turn a loop into one that never ends or a slice into one that grows forever. Every From 49c8d8f6f4f30b9bd4c19eed2e9396d39ed7927e Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 22:04:01 -0300 Subject: [PATCH 2/2] fix(mutate): give each test process a temporary directory of its own mutate runs the same package's tests in many processes at once, which go test never does, and they all shared os.TempDir(). In github.com/devcontainers/cli the code under test makes /devcontainercli-/container-features/-: two coverage runs started in the same millisecond broke each other, both tests were marked as failing with nothing broken and asked about no edit, and a different edit only they reached survived at -j 4 on some runs. Each tree now hands the process it runs an empty TMPDIR under it, removed after the process, beside the copy of the module. Co-Authored-By: Claude Opus 5.5 --- internal/mutate/cover.go | 4 +-- internal/mutate/mutate.go | 4 +-- internal/mutate/runner_test.go | 46 +++++++++++++++++++++++++++ internal/mutate/schema.go | 4 +-- internal/mutate/tree.go | 21 +++++++++--- internal/mutate/tree_internal_test.go | 2 +- site/src/content/docs/mutate.mdx | 6 +++- 7 files changed, 74 insertions(+), 13 deletions(-) diff --git a/internal/mutate/cover.go b/internal/mutate/cover.go index 0e63969..6daa860 100644 --- a/internal/mutate/cover.go +++ b/internal/mutate/cover.go @@ -120,8 +120,8 @@ func Cover(dir, timeout string, slots chan *Tree) (*Coverage, error) { exceeded bool runErr error ) - if err := tree.in(dir, func(at string) { - run.Dir = at + if err := tree.in(dir, func(at, tmp string) { + run.Dir, run.Env = at, append(run.Env, "TMPDIR="+tmp) said, exceeded, runErr = bounded("cover: run one test", run) }); err != nil { mu.Lock() diff --git a/internal/mutate/mutate.go b/internal/mutate/mutate.go index cfeebdc..47362f5 100644 --- a/internal/mutate/mutate.go +++ b/internal/mutate/mutate.go @@ -787,8 +787,8 @@ func runPackage(tree *Tree, path, pkgDir, dir, what string, only []string, timeo run := exec.Command(bin, args...) //nolint:gosec // a test binary this run built run.Env = testEnv() var runErr error - if err := tree.in(pkgDir, func(at string) { - run.Dir = at + if err := tree.in(pkgDir, func(at, tmp string) { + run.Dir, run.Env = at, append(run.Env, "TMPDIR="+tmp) out, exceeded, runErr = bounded("alone: run "+what, run) }); err != nil { return 0, err diff --git a/internal/mutate/runner_test.go b/internal/mutate/runner_test.go index 5384925..6a858ff 100644 --- a/internal/mutate/runner_test.go +++ b/internal/mutate/runner_test.go @@ -283,3 +283,49 @@ func layered(t *testing.T) { } t.Chdir(dir) } + +// Each test process has a temporary directory of its own, emptied after it: the same tests run +// in many processes at once here, which go test never does, and a path a test or the code makes +// under os.TempDir() - a cache folder named by the millisecond - was one they all shared, so a +// test failed beside another with nothing broken, was asked about no edit, and the edits only it +// reached were let through. Here each test claims a file there and leaves it: shared, Half's edit, +// which nothing checks, is refused by a claim another run made, not let through. +func TestEachTestProcessHasATemporaryDirectoryOfItsOwn(t *testing.T) { + outer := t.TempDir() + t.Setenv("TMPDIR", outer) + dir := t.TempDir() + claim := "\tf, err := os.OpenFile(filepath.Join(os.TempDir(), \"claimed-\"+t.Name()), os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600)\n" + + "\tif err != nil {\n\t\tt.Fatal(\"the temporary directory is shared: \", err)\n\t}\n\t_ = f.Close()\n" + for name, body := range map[string]string{ + "go.mod": "module example.com/claims\n\ngo 1.24\n", + "code.go": "package claims\n\nfunc Double(n int) int {\n\treturn n + n\n}\n\nfunc Half(n int) int {\n\treturn n / 2\n}\n", + "code_test.go": "package claims\n\nimport (\n\t\"os\"\n\t\"path/filepath\"\n\t\"testing\"\n)\n\n" + + "func TestDouble(t *testing.T) {\n" + claim + "\tif Double(2) != 4 {\n\t\tt.Fatal(\"not doubled\")\n\t}\n}\n\n" + + "func TestHalf(t *testing.T) {\n" + claim + "\t_ = Half(4)\n}\n", + } { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600)) + } + plan, err := mutate.Plan(filepath.Join(dir, "code.go"), map[int]bool{4: true, 8: true}) + require.NoError(t, err) + var double, half int + for i := range plan { + plan[i].ID = i + 1 + if plan[i].Func == "Double" { + double++ + } else { + half++ + } + } + require.Positive(t, double) + require.Positive(t, half) + var said strings.Builder + tally, err := mutate.RunAll(plan, nil, "2m", 2, &said) + require.NoError(t, err) + assert.Equal(t, double, tally.Killed, "only Double's edits are refused: %s", said.String()) + assert.Len(t, tally.Survived, half, "Half's edits, which nothing checks, were refused by a claim another run made: %s", said.String()) + left, err := os.ReadDir(outer) + require.NoError(t, err) + for _, e := range left { + assert.NotContains(t, e.Name(), "claimed-", "a test's temporary file was left in the temporary directory mutate was given") + } +} diff --git a/internal/mutate/schema.go b/internal/mutate/schema.go index 701aa3f..671393e 100644 --- a/internal/mutate/schema.go +++ b/internal/mutate/schema.go @@ -384,8 +384,8 @@ func Ask(tree *Tree, bin, dir string, id int, timeout string, only []string) (Ou exceeded bool runErr error ) - if err := tree.in(dir, func(at string) { - cmd.Dir = at + if err := tree.in(dir, func(at, tmp string) { + cmd.Dir, cmd.Env = at, append(cmd.Env, "TMPDIR="+tmp) out, exceeded, runErr = bounded("schema: run one edit", cmd) }); err != nil { return 0, "", err diff --git a/internal/mutate/tree.go b/internal/mutate/tree.go index c9165f3..ac63500 100644 --- a/internal/mutate/tree.go +++ b/internal/mutate/tree.go @@ -14,8 +14,8 @@ import ( "time" ) -// Tree is a private copy of the modules a run's tests are run in, held by one test process at a -// time and put back as its source is after each. A test that writes into its directory - a +// Tree is a private copy of the modules a run's tests are run in, with a temporary directory of its +// own, held by one test process at a time and put back as its source is after each. A test that writes into its directory - a // Dockerfile it generates, a file it means to read back - writes into a copy no other run reads. // Run where the package's sources are, edits asked side by side shared one directory: what one // wrote, another's tests read, so an edit refused alone was let through beside another, and the @@ -54,8 +54,12 @@ func (t *Tree) Wrote() []string { } // in calls run with where dir, a package's directory, is in the tree, copying its module the -// first time, and puts the copy back as the source is once run returns. -func (t *Tree) in(dir string, run func(at string)) error { +// first time, and with an empty temporary directory, the process's TMPDIR; once run returns, the +// copy is put back as the source is and the temporary directory removed. A test's temporary +// directory is shared as its working directory was: a cache folder under os.TempDir() named by +// the millisecond was one directory for two tests started at once, and each failed with nothing +// broken - asked about no edit, so the edits only they reached were let through. +func (t *Tree) in(dir string, run func(at, tmp string)) error { abs, err := filepath.Abs(dir) if err != nil { return err @@ -76,7 +80,14 @@ func (t *Tree) in(dir string, run func(at string)) error { if err != nil { return err } - run(filepath.Join(c.dst, rel)) + tmp := filepath.Join(t.dir, "tmp") + if err := os.MkdirAll(tmp, 0o700); err != nil { + return err + } + run(filepath.Join(c.dst, rel), tmp) + if err := os.RemoveAll(tmp); err != nil { + return fmt.Errorf("mutate: removing the temporary directory of %s's tests: %w", root, err) + } if err := c.reset(func(rel string) { t.wrote[filepath.Join(c.src, rel)] = true }); err != nil { return fmt.Errorf("mutate: putting the copy of %s back after its tests: %w", root, err) } diff --git a/internal/mutate/tree_internal_test.go b/internal/mutate/tree_internal_test.go index 50d4724..928b9fb 100644 --- a/internal/mutate/tree_internal_test.go +++ b/internal/mutate/tree_internal_test.go @@ -18,7 +18,7 @@ func TestATreeIsTheModuleAsGitSeesItPutBackAfterEachRun(t *testing.T) { var at string ran := func(run func(at string)) { t.Helper() - if err := tree.in(filepath.Join(root, "p"), run); err != nil { + if err := tree.in(filepath.Join(root, "p"), func(at, _ string) { run(at) }); err != nil { t.Fatal(err) } } diff --git a/site/src/content/docs/mutate.mdx b/site/src/content/docs/mutate.mdx index db26278..5b67602 100644 --- a/site/src/content/docs/mutate.mdx +++ b/site/src/content/docs/mutate.mdx @@ -191,7 +191,11 @@ need besides the code, like a database, comes from that environment. spin runs ` can't: one edit's tests would read the files another's wrote, and the verdict would depend on which edits ran side by side. Instead, every test process runs in a private copy of the module, one per `-j`, and the copy is reset after each run. Tests still read `testdata/` and paths like -`../../docs` as they would in place, and nothing they write reaches your checkout. +`../../docs` as they would in place, and nothing they write reaches your checkout. Each process +also gets an empty temporary directory of its own as `TMPDIR`, removed after the run. Without it, +two runs of the same tests could share a path under `os.TempDir()`, such as a cache folder named +by the current time. One of them would fail with nothing broken, and the edits only that test +reaches would be let through. The copy holds what git sees: tracked files and files git doesn't ignore. Directories git ignores (`node_modules`, a build's output) and repositories nested in the module are linked rather than