Open alexandear opened 3 months ago
Hello
Thanks for the feedback,
The main reason why I am searching for os.TempDir()
is: most of the time, people create files using os.TempDir()
instead t.TempDir()
, specialty on creating temporary files.
dir := os.TempDir()
file, err := os.CreateTemp(dir, "test-xxx-*")
// handle error
// defer / t.Cleanup to remove file
versus
dir := t.TempDir()
file, err := os.CreateTemp(dir, "test-xxx-*")
// handle error
// no need for defer / t.Cleanup to remove file, tempdir will be removed after test
since I can't easily check if os.CreateTemp was called with the os.TempDir or t.TempDir, I simplify the linter by searching all calls to os.TempDir()
and it is very aggressive.
I can imagine two possible solutions:
//nolint
or I can explicit skip some file / package / function -- this should be easier to do inside golangci-lint, I think the go analysis tools does not support such annotations )I think the option 1 seems a quick win on this case. What do you think @alexandear ?
Now, about this test in specific: there is a workaround:
on https://github.com/gohugoio/hugo/blob/master/helpers/path_test.go#L511C13-L511C64 we can see that we verify if the helper.GetTempDir
returns the expected path
this helper uses afero.GetTempDir
https://github.com/spf13/afero/blob/v1.11.0/util.go#L104
since it calls os.TempDir and os.TempDir check for a env var TMPDIR
based on https://pkg.go.dev/os#TempDir
$ git diff
diff --git a/helpers/path_test.go b/helpers/path_test.go
index 6f3699589..2b8eea934 100644
--- a/helpers/path_test.go
+++ b/helpers/path_test.go
@@ -487,7 +487,9 @@ func TestWriteToDisk(t *testing.T) {
}
func TestGetTempDir(t *testing.T) {
- dir := os.TempDir()
+ dir := t.TempDir()
+ t.Setenv("TMPDIR", dir)
+
if helpers.FilePathSeparator != dir[len(dir)-1:] {
dir = dir + helpers.FilePathSeparator
}
$ go test ./helpers/...
ok github.com/gohugoio/hugo/helpers 1.531s
It is, indeed, possible on this example change os.TempDir
by t.TempDir
however this is not very portable. And I agree that on this example it adds no value.
Hi, thank you for the quick and meaningful reply.
I believe this would be a good solution:
disable the check of os.TempDir by default, this can be enabled via configuration if needed
Thank you for providing the workaround for TestGetTempDir
.
Describe the bug
There are instances where replacing
os.TempDir
witht.TempDir()
results in test failures.To Reproduce
Steps to reproduce the behavior:
git checkout 7ee36b371
.ttempdir ./helpers/...
Expected behavior
The
ttempdir
should not report any issues.Actual behavior
The following issue is reported:
Desktop:
Additional context
os.TempDir
on the line 490 cannot be replaced byt.TempDir
. Attempting to do so results in the following error:Details
``` === RUN TestGetTempDir /Users/Oleksandr_Redko/src/github.com/gohugoio/hugo/helpers/path_test.go:513: Expected "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/TestGetTempDir592286977/001/", got "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/" /Users/Oleksandr_Redko/src/github.com/gohugoio/hugo/helpers/path_test.go:513: Expected "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/TestGetTempDir592286977/001/hugoTestFolder/ Foo bar /", got "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/hugoTestFolder/ Foo bar /" /Users/Oleksandr_Redko/src/github.com/gohugoio/hugo/helpers/path_test.go:513: Expected "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/TestGetTempDir592286977/001/hugoTestFolder/Foo.Bar/foo_Bar-Foo/", got "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/hugoTestFolder/Foo.Bar/foo_Bar-Foo/" /Users/Oleksandr_Redko/src/github.com/gohugoio/hugo/helpers/path_test.go:513: Expected "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/TestGetTempDir592286977/001/hugoTestFolder/fOObarfoo%bAR/", got "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/hugoTestFolder/fOObarfoo%bAR/" /Users/Oleksandr_Redko/src/github.com/gohugoio/hugo/helpers/path_test.go:513: Expected "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/TestGetTempDir592286977/001/hugoTestFolder/fOObarfoobAR/", got "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/hugoTestFolder/fOObarfoobAR/" /Users/Oleksandr_Redko/src/github.com/gohugoio/hugo/helpers/path_test.go:513: Expected "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/TestGetTempDir592286977/001/hugoTestFolder/FOo/BaR.html/", got "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/hugoTestFolder/FOo/BaR.html/" /Users/Oleksandr_Redko/src/github.com/gohugoio/hugo/helpers/path_test.go:513: Expected "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/TestGetTempDir592286977/001/hugoTestFolder/трям/трям/", got "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/hugoTestFolder/трям/трям/" /Users/Oleksandr_Redko/src/github.com/gohugoio/hugo/helpers/path_test.go:513: Expected "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/TestGetTempDir592286977/001/hugoTestFolder/은행/", got "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/hugoTestFolder/은행/" /Users/Oleksandr_Redko/src/github.com/gohugoio/hugo/helpers/path_test.go:513: Expected "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/TestGetTempDir592286977/001/hugoTestFolder/Банковский кассир/", got "/var/folders/pk/5dzf3qsj6l18s2b3zfw194840000gn/T/hugoTestFolder/Банковский кассир/" --- FAIL: TestGetTempDir (0.00s) FAIL ```Please note that the descriptions of
os.TempDir
andt.TempDir
are slightly different.https://pkg.go.dev/os#TempDir
https://pkg.go.dev/testing#T.TempDir