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

testing: t.TempDir() doesn't remove tmp dir and files as expected on Windows OS #50510

Closed
Monokaix opened this issue Jan 8, 2022 · 5 comments
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@Monokaix
Copy link

Monokaix commented Jan 8, 2022

What version of Go are you using (go version)?

$ go version
1.17.6
OS:
Windows 10 Professional 2004 19041.1081

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\xxx\AppData\Local\go-build
set GOENV=C:\Users\xxx\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOMODCACHE=D:\go\pkg\mod
set GONOSUMDB=*
set GOOS=windows
set GOPATH=D:\go
set GOPRIVATE=
set GOROOT=C:\Program Files\Go
set GOSUMDB=off
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.17.6
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=D:\go\src\test\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\xxx\AppData\Local\Temp\go-build1098534111=/tmp/go-build -gno-record-gcc-switches

What did you do?

I ran the test func below,which using t.TempDir() to create a tmp dir, from the func's comment, it will do Cleanup to remove the tmp dir after testing ends, but I got an err:
testing.go:967: TempDir RemoveAll cleanup: remove C:\Users\xxx\AppData\Local\Temp\TestTmpDirRemove3730116776\001\foo2619329076.txt: The process cannot access the file because it is being used by another process.,
and I found the tmp dir and file was not removed correctly, after some digging I found that this seems happen only on Windows, Linux works correctly. I tried uncomment f.Close(), it worked and the file was removed as expected. There is also some issues mentioned this problem like #44919,but it seems that this problem is not solved.
And my question is: Should we callf.Close()when using t.TempDir() on Windows to make it work as expected, but this may causeTempDir()becoming meaningless, or we should fix it on Windows?

func TestTmpDirRemove(t *testing.T) {
	tmpDir := t.TempDir()

	f, err := ioutil.TempFile(tmpDir, "foo")
	if err != nil {
		t.Fatal(err)
	}

	ioutil.WriteFile(f.Name(), []byte("bar"), os.FileMode(0755))
	t.Run("", func(t *testing.T) {
		fmt.Println("This is a test")
	})
	// f.Close()
}

What did you expect to see?

The tmp was removed and no errors

What did you see instead?

The tmp dir and file was not removed

@Monokaix Monokaix changed the title affected/package: testing: t.TempDir() doesn't remove tmp dir and files as expected on Windows OS testing: t.TempDir() doesn't remove tmp dir and files as expected on Windows OS Jan 8, 2022
@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Jan 10, 2022

I don't think that the testing package should remove a temporary file created by ioutil.TempFile (or os.CreateTemp). Those functions don't have anything to do with the testing package.

I don't think there is much we can do about this. People writing tests that run on Windows should be sure to close their temporary files in tests (which is good practice on any system).

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 10, 2022
@Monokaix
Copy link
Author

Monokaix commented Jan 10, 2022

@ianlancetaylor Thank you, it makes sense.But I' still a little confused that the same code behaves differently on different OS, the tmp dir can be removed correctly on Linux even though we don't close file descriptor, can we tolerate this?And because of the Testing hook of tmp dir CleanUp, it usually makes developers forget to Close file, and On Linux it do work fine, which will mislead us that things work fine everywhere.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Jan 10, 2022

That's because Unix and Windows handle open files differently. Unix permits removing an open file, Windows does not.

You're right that the fact that it works on Linux may lead people to forget to close the file, but I don't see what we can do about that. Unix and Windows are different, and there is only so much Go can do to hide that fact.

@Monokaix
Copy link
Author

Monokaix commented Jan 10, 2022

@ianlancetaylor Thanks a lot! Maybe we can close this.

@bcmills
Copy link
Member

bcmills commented Jan 11, 2022

#50051 is somewhat related, but it (intentionally) probably won't paper over a missing Close call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants