Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
perillo opened this issue Apr 14, 2021 · 4 comments
Closed

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

perillo opened this issue Apr 14, 2021 · 4 comments
Labels
Projects
Milestone

Comments

@perillo
Copy link
Contributor

@perillo perillo commented Apr 14, 2021

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.

@gopherbot gopherbot added this to the Proposal milestone Apr 14, 2021
@perillo perillo changed the title proposal: add TB.WriteFile(name string, data string) proposal: add TB.WriteFile(name string, data string) string Apr 14, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Apr 14, 2021
@ianlancetaylor ianlancetaylor changed the title proposal: add TB.WriteFile(name string, data string) string proposal: testing: add TB.WriteFile(name string, data string) string Apr 14, 2021
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 14, 2021

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
Copy link
Contributor

@rsc rsc commented Apr 21, 2021

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 rsc moved this from Incoming to Active in Proposals Apr 21, 2021
@rsc
Copy link
Contributor

@rsc rsc commented Apr 28, 2021

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

@rsc rsc moved this from Active to Likely Decline in Proposals Apr 28, 2021
@rsc rsc moved this from Likely Decline to Declined in Proposals May 5, 2021
@rsc
Copy link
Contributor

@rsc rsc commented May 5, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Proposals
Declined
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants