Skip to content

Commit 5fc0868

Browse files
aledbfclaude
andcommitted
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 <tmp>/devcontainercli-<user>/container-features/<version>-<unix ms>: 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 <noreply@anthropic.com>
1 parent e6be72e commit 5fc0868

7 files changed

Lines changed: 74 additions & 13 deletions

File tree

‎internal/mutate/cover.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ func Cover(dir, timeout string, slots chan *Tree) (*Coverage, error) {
120120
exceeded bool
121121
runErr error
122122
)
123-
if err := tree.in(dir, func(at string) {
124-
run.Dir = at
123+
if err := tree.in(dir, func(at, tmp string) {
124+
run.Dir, run.Env = at, append(run.Env, "TMPDIR="+tmp)
125125
said, exceeded, runErr = bounded("cover: run one test", run)
126126
}); err != nil {
127127
mu.Lock()

‎internal/mutate/mutate.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -787,8 +787,8 @@ func runPackage(tree *Tree, path, pkgDir, dir, what string, only []string, timeo
787787
run := exec.Command(bin, args...) //nolint:gosec // a test binary this run built
788788
run.Env = testEnv()
789789
var runErr error
790-
if err := tree.in(pkgDir, func(at string) {
791-
run.Dir = at
790+
if err := tree.in(pkgDir, func(at, tmp string) {
791+
run.Dir, run.Env = at, append(run.Env, "TMPDIR="+tmp)
792792
out, exceeded, runErr = bounded("alone: run "+what, run)
793793
}); err != nil {
794794
return 0, err

‎internal/mutate/runner_test.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,49 @@ func layered(t *testing.T) {
283283
}
284284
t.Chdir(dir)
285285
}
286+
287+
// Each test process has a temporary directory of its own, emptied after it: the same tests run
288+
// in many processes at once here, which go test never does, and a path a test or the code makes
289+
// under os.TempDir() - a cache folder named by the millisecond - was one they all shared, so a
290+
// test failed beside another with nothing broken, was asked about no edit, and the edits only it
291+
// reached were let through. Here each test claims a file there and leaves it: shared, Half's edit,
292+
// which nothing checks, is refused by a claim another run made, not let through.
293+
func TestEachTestProcessHasATemporaryDirectoryOfItsOwn(t *testing.T) {
294+
outer := t.TempDir()
295+
t.Setenv("TMPDIR", outer)
296+
dir := t.TempDir()
297+
claim := "\tf, err := os.OpenFile(filepath.Join(os.TempDir(), \"claimed-\"+t.Name()), os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600)\n" +
298+
"\tif err != nil {\n\t\tt.Fatal(\"the temporary directory is shared: \", err)\n\t}\n\t_ = f.Close()\n"
299+
for name, body := range map[string]string{
300+
"go.mod": "module example.com/claims\n\ngo 1.24\n",
301+
"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",
302+
"code_test.go": "package claims\n\nimport (\n\t\"os\"\n\t\"path/filepath\"\n\t\"testing\"\n)\n\n" +
303+
"func TestDouble(t *testing.T) {\n" + claim + "\tif Double(2) != 4 {\n\t\tt.Fatal(\"not doubled\")\n\t}\n}\n\n" +
304+
"func TestHalf(t *testing.T) {\n" + claim + "\t_ = Half(4)\n}\n",
305+
} {
306+
require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600))
307+
}
308+
plan, err := mutate.Plan(filepath.Join(dir, "code.go"), map[int]bool{4: true, 8: true})
309+
require.NoError(t, err)
310+
var double, half int
311+
for i := range plan {
312+
plan[i].ID = i + 1
313+
if plan[i].Func == "Double" {
314+
double++
315+
} else {
316+
half++
317+
}
318+
}
319+
require.Positive(t, double)
320+
require.Positive(t, half)
321+
var said strings.Builder
322+
tally, err := mutate.RunAll(plan, nil, "2m", 2, &said)
323+
require.NoError(t, err)
324+
assert.Equal(t, double, tally.Killed, "only Double's edits are refused: %s", said.String())
325+
assert.Len(t, tally.Survived, half, "Half's edits, which nothing checks, were refused by a claim another run made: %s", said.String())
326+
left, err := os.ReadDir(outer)
327+
require.NoError(t, err)
328+
for _, e := range left {
329+
assert.NotContains(t, e.Name(), "claimed-", "a test's temporary file was left in the temporary directory mutate was given")
330+
}
331+
}

‎internal/mutate/schema.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,8 @@ func Ask(tree *Tree, bin, dir string, id int, timeout string, only []string) (Ou
384384
exceeded bool
385385
runErr error
386386
)
387-
if err := tree.in(dir, func(at string) {
388-
cmd.Dir = at
387+
if err := tree.in(dir, func(at, tmp string) {
388+
cmd.Dir, cmd.Env = at, append(cmd.Env, "TMPDIR="+tmp)
389389
out, exceeded, runErr = bounded("schema: run one edit", cmd)
390390
}); err != nil {
391391
return 0, "", err

‎internal/mutate/tree.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import (
1414
"time"
1515
)
1616

17-
// Tree is a private copy of the modules a run's tests are run in, held by one test process at a
18-
// time and put back as its source is after each. A test that writes into its directory - a
17+
// Tree is a private copy of the modules a run's tests are run in, with a temporary directory of its
18+
// 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
1919
// Dockerfile it generates, a file it means to read back - writes into a copy no other run reads.
2020
// Run where the package's sources are, edits asked side by side shared one directory: what one
2121
// 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 {
5454
}
5555

5656
// in calls run with where dir, a package's directory, is in the tree, copying its module the
57-
// first time, and puts the copy back as the source is once run returns.
58-
func (t *Tree) in(dir string, run func(at string)) error {
57+
// first time, and with an empty temporary directory, the process's TMPDIR; once run returns, the
58+
// copy is put back as the source is and the temporary directory removed. A test's temporary
59+
// directory is shared as its working directory was: a cache folder under os.TempDir() named by
60+
// the millisecond was one directory for two tests started at once, and each failed with nothing
61+
// broken - asked about no edit, so the edits only they reached were let through.
62+
func (t *Tree) in(dir string, run func(at, tmp string)) error {
5963
abs, err := filepath.Abs(dir)
6064
if err != nil {
6165
return err
@@ -76,7 +80,14 @@ func (t *Tree) in(dir string, run func(at string)) error {
7680
if err != nil {
7781
return err
7882
}
79-
run(filepath.Join(c.dst, rel))
83+
tmp := filepath.Join(t.dir, "tmp")
84+
if err := os.MkdirAll(tmp, 0o700); err != nil {
85+
return err
86+
}
87+
run(filepath.Join(c.dst, rel), tmp)
88+
if err := os.RemoveAll(tmp); err != nil {
89+
return fmt.Errorf("mutate: removing the temporary directory of %s's tests: %w", root, err)
90+
}
8091
if err := c.reset(func(rel string) { t.wrote[filepath.Join(c.src, rel)] = true }); err != nil {
8192
return fmt.Errorf("mutate: putting the copy of %s back after its tests: %w", root, err)
8293
}

‎internal/mutate/tree_internal_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func TestATreeIsTheModuleAsGitSeesItPutBackAfterEachRun(t *testing.T) {
1818
var at string
1919
ran := func(run func(at string)) {
2020
t.Helper()
21-
if err := tree.in(filepath.Join(root, "p"), run); err != nil {
21+
if err := tree.in(filepath.Join(root, "p"), func(at, _ string) { run(at) }); err != nil {
2222
t.Fatal(err)
2323
}
2424
}

‎site/src/content/docs/mutate.mdx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,11 @@ need besides the code, like a database, comes from that environment. spin runs `
191191
can't: one edit's tests would read the files another's wrote, and the verdict would depend on which
192192
edits ran side by side. Instead, every test process runs in a private copy of the module, one per
193193
`-j`, and the copy is reset after each run. Tests still read `testdata/` and paths like
194-
`../../docs` as they would in place, and nothing they write reaches your checkout.
194+
`../../docs` as they would in place, and nothing they write reaches your checkout. Each process
195+
also gets an empty temporary directory of its own as `TMPDIR`, removed after the run. Without it,
196+
two runs of the same tests could share a path under `os.TempDir()`, such as a cache folder named
197+
by the current time. One of them would fail with nothing broken, and the edits only that test
198+
reaches would be let through.
195199
196200
The copy holds what git sees: tracked files and files git doesn't ignore. Directories git ignores
197201
(`node_modules`, a build's output) and repositories nested in the module are linked rather than

0 commit comments

Comments
 (0)