golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.37k stars 17.71k forks source link

proposal: testing: add TB.WriteFile(name string, data string) string #45562

Closed perillo closed 3 years ago

perillo commented 3 years ago

While working on some CL on the tests I noted a common pattern: calling os.WriteFile to create a temporary file using a string as input.

Here is the list, obtained using grep -F "os.WriteFile(" **/*_test.go | grep -F '[]byte':

cmd/cover/cover_test.go:    if err := os.WriteFile(coverInput, bytes.Join(lines, []byte("\n")), 0666); err != nil {
cmd/cover/cover_test.go:    if err := os.WriteFile(filepath.Join(htmlUDir, "go.mod"), []byte("module htmlunformatted\n"), 0666); err != nil {
cmd/cover/cover_test.go:    if err := os.WriteFile(htmlU, []byte(htmlUContents), 0444); err != nil {
cmd/cover/cover_test.go:    if err := os.WriteFile(htmlUTest, []byte(htmlUTestContents), 0444); err != nil {
cmd/cover/cover_test.go:    if err := os.WriteFile(filepath.Join(lineDupDir, "go.mod"), []byte("module linedup\n"), 0666); err != nil {
cmd/cover/cover_test.go:    if err := os.WriteFile(lineDupGo, []byte(lineDupContents), 0444); err != nil {
cmd/cover/cover_test.go:    if err := os.WriteFile(lineDupTestGo, []byte(lineDupTestContents), 0444); err != nil {
cmd/go/go_windows_test.go:  err = os.WriteFile(file, []byte{}, 0644)
cmd/go/internal/lockedfile/lockedfile_test.go:  if err := os.WriteFile(path, []byte("ok"), 0777); err != nil {
cmd/go/internal/lockedfile/lockedfile_test.go:      if err := os.WriteFile(filepath.Join(dir, "locked"), []byte("ok"), 0666); err != nil {
cmd/go/script_test.go:      if err := os.WriteFile(fcap, []byte{}, 0644); err != nil {
cmd/go/script_test.go:      ts.check(os.WriteFile(file, bytes.ReplaceAll(data, []byte("\n"), []byte("\r\n")), 0666))
cmd/nm/nm_test.go:      err = os.WriteFile(filepath.Join(libpath, "go.mod"), []byte("module mylib\n"), 0666)
cmd/pack/pack_test.go:  err := os.WriteFile(hello, []byte(prog), 0666)
cmd/pack/pack_test.go:  err = os.WriteFile(main, []byte(prog), 0666)
cmd/pack/pack_test.go:  err := os.WriteFile(filepath.Join(dir, "a.go"), []byte(aSrc), 0666)
cmd/pack/pack_test.go:  err = os.WriteFile(filepath.Join(dir, "b.go"), []byte(bSrc), 0666)
cmd/pack/pack_test.go:  err := os.WriteFile(src, []byte(prog), 0666)
cmd/pack/pack_test.go:  err := os.WriteFile(src, []byte(prog), 0666)
crypto/tls/link_test.go:            if err := os.WriteFile(goFile, []byte(tt.program), 0644); err != nil {
crypto/x509/root_unix_test.go:      if err := os.WriteFile(certOutFile, []byte(certPEM), 0655); err != nil {
debug/pe/file_test.go:  err = os.WriteFile(srcpath, []byte(src), 0644)
debug/pe/file_test.go:  if err := os.WriteFile(src, []byte(`package main; func main() {}`), 0644); err != nil {
go/build/build_test.go: if err := os.WriteFile(filepath.Join(gopath, "src/example.com/p/p.go"), []byte("package p"), 0666); err != nil {
go/build/build_test.go: if err := os.WriteFile(filepath.Join(tmp, "go.mod"), []byte("module m"), 0666); err != nil {
internal/execabs/execabs_test.go:       if err := os.WriteFile(filepath.Join(tmpDir, executable), []byte{1, 2, 3}, 0111); err != nil {
internal/execabs/execabs_test.go:   if err := os.WriteFile(filepath.Join(tmpDir, executable), []byte{1, 2, 3}, 0111); err != nil {
net/http/filetransport_test.go: err := os.WriteFile(fname, []byte("Bar"), 0644)
net/http/fs_test.go:    if err := os.WriteFile(filepath.Join(tempDir, "foo.txt"), []byte("Hello world"), 0644); err != nil {
net/http/fs_test.go:    if err := os.WriteFile(filepath, bytes.Repeat([]byte{'a'}, 1<<10), 0755); err != nil {
net/unixsock_test.go:       if err := os.WriteFile(name, []byte("hello world"), 0666); err != nil {
os/example_test.go: if err := os.WriteFile(file, []byte("content"), 0666); err != nil {
os/example_test.go: err := os.WriteFile("testdata/hello", []byte("Hello, Gophers!"), 0666)
os/exec/lp_windows_test.go: err := os.WriteFile(filepath.Join(dir, srcname), []byte(printpathSrc), 0644)
os/os_test.go:  if err := os.WriteFile(filepath.Join(d, "control.txt"), []byte(string("Hello, world!")), 0644); err != nil {
os/os_test.go:  if err := os.WriteFile(filepath.Join(d, `e:xperi\ment.txt`), []byte(string("Hello, colon and backslash!")), 0644); err != nil {
os/os_unix_test.go: if err := os.WriteFile(filepath.Join(dir, "some-file"), []byte("hello"), 0644); err != nil {
os/os_windows_test.go:  err = os.WriteFile(filepath.Join(dir, "abc"), []byte("abc"), 0644)
os/os_windows_test.go:  if err := os.WriteFile(src, []byte(prog), 0666); err != nil {
os/os_windows_test.go:  if err := os.WriteFile(dummyFile, []byte(""), 0644); err != nil {
os/os_windows_test.go:  err = os.WriteFile(file, []byte(""), 0666)
os/stat_test.go:    if err := os.WriteFile(file, []byte(""), 0644); err != nil {
path/filepath/path_windows_test.go: err = os.WriteFile(file, []byte(""), 0666)
runtime/crash_unix_test.go: if err := os.WriteFile(filepath.Join(dir, "main.go"), []byte(crashDumpsAllThreadsSource), 0666); err != nil {
runtime/runtime-gdb_test.go:    err := os.WriteFile(src, []byte(backtraceSource), 0644)
runtime/runtime-gdb_test.go:    err := os.WriteFile(src, []byte(autotmpTypeSource), 0644)
runtime/runtime-gdb_test.go:    err := os.WriteFile(src, []byte(constsSource), 0644)
runtime/runtime-gdb_test.go:    err := os.WriteFile(src, []byte(panicSource), 0644)
runtime/runtime-gdb_test.go:    err := os.WriteFile(src, []byte(InfCallstackSource), 0644)
runtime/runtime-lldb_test.go:   err := os.WriteFile(src, []byte(lldbHelloSource), 0644)
runtime/runtime-lldb_test.go:   err = os.WriteFile(mod, []byte("module lldbtest"), 0644)
runtime/runtime-lldb_test.go:   err = os.WriteFile(src, []byte(lldbScriptSource), 0755)
runtime/syscall_windows_test.go:    err := os.WriteFile(filepath.Join(tmpdir, srcname), []byte(src), 0)
runtime/syscall_windows_test.go:    err := os.WriteFile(filepath.Join(tmpdir, srcname), []byte(src), 0)
runtime/syscall_windows_test.go:    err := os.WriteFile(filepath.Join(tmpdir, srcname), []byte(src), 0)
runtime/syscall_windows_test.go:    err := os.WriteFile(filepath.Join(tmpdir, srcname), []byte(src), 0)
runtime/syscall_windows_test.go:    err = os.WriteFile(filepath.Join(tmpdir, srcname), []byte(src), 0)
runtime/syscall_windows_test.go:    err := os.WriteFile(src, []byte(benchmarkRunningGoProgram), 0666)
syscall/dirent_test.go:     err := os.WriteFile(filepath.Join(d, file), []byte("contents"), 0644)
syscall/exec_windows_test.go:       err := os.WriteFile(dumpPath, []byte(fmt.Sprintf("%d", os.Getppid())), 0644)
syscall/getdirentries_test.go:      err := os.WriteFile(filepath.Join(d, name), []byte("data"), 0)
testing/fstest/testfs_test.go:  if err := os.WriteFile(filepath.Join(tmp, "hello"), []byte("hello, world\n"), 0644); err != nil {
text/template/link_test.go: if err := os.WriteFile(filepath.Join(td, "x.go"), []byte(prog), 0644); err != nil {

The list is quite long. In contrast, while working on #45448 I noted that the new T.Setenv could be used only in few cases.

There is an interesting fact in the list: the perm argument is not always 0644; the interesting cases are 0 (in one cases it is for windows so it make senses) and 0755 used on non executable files.

Using the proposed TB.WriteFile should reduce a bit of noise in the tests.

I'm not sure if TB.WriteTempFile(name, data string) string (where the returned value is the file name) is a better API (assuming that each call of TB.WriteTempFile reuses the same temporary directory).

Thanks.

ianlancetaylor commented 3 years ago

The T.Setenv function is useful because it sets global state and then undoes that global state when the test is complete. That is code that it is easy for a test to get wrong, so it makes some sense to provide as part of the testing package.

That argument doesn't apply here. There is nothing specific to the testing package about providing a function that writes a string to a file with a default mode. We could do that in the os package.

rsc commented 3 years ago

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group

rsc commented 3 years ago

Based on the discussion above, this proposal seems like a likely decline. — rsc for the proposal review group

rsc commented 3 years ago

No change in consensus, so declined. — rsc for the proposal review group