nit:

Also considering if we should try to make this more of a "drop-in" replacement for t.TempDir() and have the utility do a t.Fatal() if it fails to create the directory; similar to t.TempDir();

• https://github.com/golang/go/blob/go1.20.7/src/testing/testing.go#L1175
• https://github.com/golang/go/blob/go1.20.7/src/testing/testing.go#L1224

Looking at the referenced code, would something like:

func TempDir(t *testing.T) string {
	t.Helper()
	dir := t.TempDir()

	parent := filepath.Dir(dir)
	if parent != "" {
		if err := os.Chmod(parent, 0o777); err != nil {
			t.Fatalf("Failed to chmod parent of temp directory %q: %v", parent, err)
		}
	}

	return dir
}

be cleaner?

This should mean that we reuse the pattern based on the name for the temp dir. Note, we could also do a check on the parent to see whether the chmod is actually required.

Yes, I was considering if we should re-use t.TempDir(). The only thing I was considering there is that (I think) t.TempDir() was conditionally creating the parent directory; would there be situations where it already existed, and we'd be changing permissions of it?

If that's not a concern, then sticking closer to t.TempDir() sounds good to me 👍

From the perspective of the client, the c.tempDir member that represents the parent directory is only set in TempDir(). It is conditionally created to ensure that a single directory is created -- or reused -- for a given instance of t.

You're right that if a test calls t.TempDir() before calling this implementation, then we would be updating the permissions of the parent after it has been created. I don't have a good feeling of whether this is a concern, but my first instinct would be that the risk is low enough.

One option here would be to:

1. Update all test code to call the new function
2. Update the implementation to check the permissions and not update them unconditionally (although this is probably not required).

Note that the returned parent is still created using os.MkdirTemp("", pattern) (where pattern is a sanitized representation of t.Name()) meaning that is should be rooted at /tmp by default.

Thanks! Yes, at a first glance, I think it should be fairly safe to make that change (safe, as in: should not disrupt out tests).

I'll have a quick chat with others to see of someone sees any issues with that, just to be sure, and will comment 👍