go: all: ensure that tests do not write to the current directory

In #27957, @hyangah noticed that the tests for compress/bzip2 fail when GOROOT is not writable, and those tests are run whenever we run go test all in module mode (which is intended to be a useful default).

As noted in #28386, tests should not assume that they can write to the current directory. We should ensure that none of the tests in the standard library make that mistake.

About this issue

Original URL
State: open
Created 6 years ago
Reactions: 4
Comments: 49 (10 by maintainers)

Commits related to this issue

os: ensure tests pass even if GOROOT is read-only We achieve this by always running all tests that create files in a fresh temporary directory, rather than just on darwin/{arm,arm64}. As a bonus, thi... — committed to golang/go by dpinela 6 years ago
go/internal/gcimporter: ensure tests pass even if GOROOT is read-only This mainly entails writing compiler output files to a temporary directory, as well as the corrupted files in TestVersionHandling... — committed to golang/go by dpinela 6 years ago
unix: do not invoke Mkfifo with a relative path in a read-only directory Updates golang/go#28387 Change-Id: Ibcdc3f9cb3dc43b86b7e7d3539ed592219e54aba Reviewed-on: https://go-review.googlesource.com/... — committed to golang/sys by bcmills 5 years ago
cmd/vet: do not write test vet binary to GOROOT Updates #28387 Change-Id: Ie5a5f1f798eb5900f9c7bdef165abcca02dd0dde Reviewed-on: https://go-review.googlesource.com/c/163037 Run-TryBot: Bryan C. Mill... — committed to golang/go by bcmills 5 years ago
cmd/vet: do not write test vet binary to GOROOT Updates #28387 Change-Id: Ie5a5f1f798eb5900f9c7bdef165abcca02dd0dde Reviewed-on: https://go-review.googlesource.com/c/163037 Run-TryBot: Bryan C. Mill... — committed to nebulabox/go by bcmills 5 years ago
cmd/vet: do not write test vet binary to GOROOT Updates #28387 Change-Id: Ie5a5f1f798eb5900f9c7bdef165abcca02dd0dde Reviewed-on: https://go-review.googlesource.com/c/163037 Run-TryBot: Bryan C. Mill... — committed to nebulabox/go by bcmills 5 years ago
misc/cgo/testplugin: convert test.bash to Go and fix in module mode Updates #30228 Updates #28387 Change-Id: Iad7d960b70221f90ccc2372bb1d4d41cec3926e4 Reviewed-on: https://go-review.googlesource.com... — committed to golang/go by bcmills 5 years ago
misc/cgo/testso{,var}: fix tests in module mode Add _test.go files in the individal directories to invoke 'go build' with appropriate arguments. Move the test driver out of cmd/dist so that it's eas... — committed to golang/go by bcmills 5 years ago
cmd/go: factor the I/O-retry logic out of renameio Factor the try-on-failure variants are now in the package cmd/go/internal/robustio. Add to them a RemoveAll variant using the same retry loop, and ... — committed to golang/go by bcmills 5 years ago
all: do not write to testdata directories I got tired of spurious 'git' diffs while a 'go test' was running, so I fixed the test that produced the diffs. (We need to do that anyway in order to run th... — committed to golang/tools by bcmills 5 years ago
cmd/go: in 'go build -o', allow the destination file to exist if it is empty This allows the target of 'go build' to be a filename constructed using ioutil.TempFile or similar, without racily deletin... — committed to golang/go by bcmills 5 years ago
cmd/dist: write binaries to to GOTMPDIR instead of GOROOT in runHostTest Updates #32407 Updates #28387 Change-Id: I2ab933896940787b67ab5464c8213670e6e108c2 Reviewed-on: https://go-review.googlesourc... — committed to golang/go by bcmills 5 years ago
cmd/go: convert TestCacheVet to a script This test was failing when GOROOT was read-only. (I'm not sure why that was the case, but it's simpler to convert to to a script than to try to debug the non... — committed to golang/go by bcmills 5 years ago
misc/cgo/testgodefs: convert test from bash to Go The bash version of the test wrote intermediate files to its testdata directory. Updates #28387 Updates #30316 Fixes #35536 Change-Id: Ib81b547d3c4... — committed to golang/go by bcmills 5 years ago
doc: convert remaining bash tests to Go Updates #28387 Updates #30316 Fixes #35574 Change-Id: I21c9e18573909e092ed8dcec91b8542bb97e9f5a Reviewed-on: https://go-review.googlesource.com/c/go/+/207263 ... — committed to golang/go by bcmills 5 years ago
cmd/dist: when -compile-only is set, write binaries to os.DevNull Previously, we were writing binaries to GOROOT without ever reading them. Updates #28387 Change-Id: Ic84999f46a367fcbf2ae1eac8bfdcb... — committed to golang/go by bcmills 5 years ago
test: avoid writing temporary files to GOROOT Updates #28387 Fixes #35619 Change-Id: I162f3427b7901c117e3f3e403df7edec7c529bd1 Reviewed-on: https://go-review.googlesource.com/c/go/+/207352 Run-TryBo... — committed to golang/go by bcmills 5 years ago
cmd/dist: fix GOROOT permissions on failure While running various tests for #28387, I keep ending up with an unwritable GOROOT after a failure. While the unwritable GOROOT is a fairly exotic conditi... — committed to golang/go by bcmills 5 years ago
test: avoid writing temporary files to GOROOT This reverts CL 207477, restoring CL 207352 with a fix for the regression observed in the Windows builders. cmd/compile evidently does not fully support... — committed to golang/go by bcmills 5 years ago
cmd/go: convert Test{Local,Bad}Imports* to script tests These tests were all attempting to write to their working directory, which may fail if GOROOT is read-only. Updates #28387 Updates #30316 Cha... — committed to golang/go by bcmills 5 years ago

Most upvoted comments

@tkivisik, yes, but in most cases you shouldn’t even need to bother with Getwd and Chdir.

I’ve found this pattern pretty helpful:

	dir, err := ioutil.TempDir("", path.Base(t.Name()))
	if err != nil {
		t.Fatal(err)
	}
	defer os.RemoveAll(dir)

I wonder whether we should just make that a method on *testing.T: the os.RemoveAll doesn’t strictly need to be deferred right there at the call site (we can run it at any point after the test function returns), and it might be useful to have a standard flag (along the lines of -work) to leave the temporary directory in place.

bcmills on Oct 29, 2018