mutate: a run's temporary directory is removed when a killed test left it read-only

internal/mutate/tree.go

@@ -88,7 +88,7 @@ func (t *Tree) in(dir string, run func(at, tmp string)) error {
 		return err
 	}
 	run(filepath.Join(c.dst, rel), tmp)
-	if err := os.RemoveAll(tmp); err != nil {
+	if err := 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 {
@@ -97,6 +97,28 @@ func (t *Tree) in(dir string, run func(at, tmp string)) error {
 	return nil
 }
 
+// removeAll removes dir and everything under it, read-only or not. A test that makes a directory
+// read-only puts it back in its t.Cleanup, which never runs when the test binary is killed at its
+// timeout - as an edit that hangs it is - and os.RemoveAll cannot unlink a file from a directory
+// its owner may not write. Each directory is made its owner's to write and search first, as the
+// go command's own removeAll does for a read-only module cache. WalkDir does not follow a link,
+// so nothing outside dir has its mode changed.
+func removeAll(dir string) error {
+	err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() {
+			return os.Chmod(path, 0o700) //nolint:gosec // a directory, searched to remove what is in it
+		}
+		return nil
+	})
+	if err != nil && !errors.Is(err, fs.ErrNotExist) {
+		return err
+	}
+	return os.RemoveAll(dir)
+}
+
 // 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 {

internal/mutate/tree_internal_test.go

@@ -100,6 +100,80 @@ func TestARunsTemporaryDirectoryTakesASocketFromATestWithALongName(t *testing.T)
 	}
 }
 
+// What a killed test leaves in its temporary directory is removed: a read-only directory of
+// read-only files, as a test that restores its modes in t.Cleanup leaves it when it never got
+// there. A link out of the directory is removed, never followed: what it points at keeps its
+// files and its mode. A directory already gone is no error.
+func TestATemporaryDirectoryAKilledTestLeftReadOnlyIsRemoved(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		make func(t *testing.T, dir string)
+	}{
+		{"read-only directory of read-only files", func(t *testing.T, dir string) {
+			t.Helper()
+			ro := filepath.Join(dir, "TestAnArtifact1715224689", "002")
+			readOnlyDir(t, ro)
+		}},
+		{"a link to a read-only directory outside it", func(t *testing.T, dir string) {
+			t.Helper()
+			outside := filepath.Join(t.TempDir(), "outside")
+			readOnlyDir(t, outside)
+			t.Cleanup(func() { _ = os.Chmod(outside, 0o700) }) //nolint:gosec // a directory, made writable so t.TempDir can remove it
+			if err := os.Symlink(outside, filepath.Join(dir, "link")); err != nil {
+				t.Fatal(err)
+			}
+			t.Cleanup(func() {
+				info, err := os.Stat(outside)
+				if err != nil {
+					t.Fatalf("the link's target is gone: %v", err)
+				}
+				if perm := info.Mode().Perm(); perm != 0o500 {
+					t.Errorf("the link's target is %v, want it untouched at 0500", perm)
+				}
+				if _, err := os.Stat(filepath.Join(outside, "mkfs.ext4")); err != nil {
+					t.Errorf("the link's target lost its file: %v", err)
+				}
+			})
+		}},
+		{"already gone", func(t *testing.T, dir string) {
+			t.Helper()
+			if err := os.Remove(dir); err != nil {
+				t.Fatal(err)
+			}
+		}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			dir := filepath.Join(t.TempDir(), "m")
+			if err := os.Mkdir(dir, 0o700); err != nil {
+				t.Fatal(err)
+			}
+			tc.make(t, dir)
+			if err := removeAll(dir); err != nil {
+				t.Fatalf("removing %s: %v", dir, err)
+			}
+			if _, err := os.Lstat(dir); err == nil {
+				t.Errorf("%s outlived its removal", dir)
+			}
+		})
+	}
+}
+
+// readOnlyDir makes dir a 0500 directory holding two 0400 files.
+func readOnlyDir(t *testing.T, dir string) {
+	t.Helper()
+	if err := os.MkdirAll(dir, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	for _, name := range []string{"mkfs.ext4", "spin-supervisor.qcow2"} {
+		if err := os.WriteFile(filepath.Join(dir, name), []byte(name), 0o400); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := os.Chmod(dir, 0o500); err != nil { //nolint:gosec // a directory, read-only as the test that left it made it
+		t.Fatal(err)
+	}
+}
+
 // 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 {

site/src/content/docs/mutate.mdx

@@ -172,7 +172,9 @@ can't: one edit's tests would read the files another's wrote, and the verdict wo
 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. Each process
-also gets an empty temporary directory of its own as `TMPDIR`, removed after the run. Without it,
+also gets an empty temporary directory of its own as `TMPDIR`, removed after the run, read-only
+directories and all: a test killed at its timeout never ran the cleanup that would have made them
+writable again. Links in it are removed, never followed. 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 directory is made directly under your own `TMPDIR`, not inside