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.

Sign up for GitHub

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

Jump to bottom

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
FrozenDueToAge Proposal
Projects
Proposals (old)
Milestone
Proposal

Comments

@perillo
Copy link
Contributor

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 (old) 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

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 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 (old) Apr 21, 2021
@rsc rsc mentioned this issue Apr 21, 2021
Open
@rsc
Copy link
Contributor

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 (old) Apr 28, 2021
@rsc rsc moved this from Likely Decline to Declined in Proposals (old) May 5, 2021
@rsc
Copy link
Contributor

rsc commented May 5, 2021

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

@rsc rsc closed this as completed May 5, 2021
@golang golang locked and limited conversation to collaborators May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
No open projects
Proposals (old)
Declined
Milestone
Proposal
Development

No branches or pull requests
4 participants
@rsc @ianlancetaylor @perillo @gopherbot