Replace os.TempDir usage with t.TempDir (#1146)

prashantv · abhinav · web-flow · commit bdd673d714c1 · 2022-08-15T08:58:30.000-07:00
Current tests can leave files around if tests fail, and even though they
use rand, it's not seeded, different runs can share the same output file
which causes other failures (e.g., file exists when it's not expected
to).  Avoid this by using  t.TempDir which automatically removed after
tests have run.

Co-authored-by: Abhinav Gupta &lt;mail@abhinavg.net&gt;
diff --git a/config_test.go b/config_test.go
@@ -21,8 +21,8 @@
 package zap
 
 import (
-	"io"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -58,11 +58,9 @@ func TestConfig(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			temp, err := os.CreateTemp("", "zap-prod-config-test")
-			require.NoError(t, err, "Failed to create temp file.")
-			defer os.Remove(temp.Name())
+			logOut := filepath.Join(t.TempDir(), "test.log")
 
-			tt.cfg.OutputPaths = []string{temp.Name()}
+			tt.cfg.OutputPaths = []string{logOut}
 			tt.cfg.EncoderConfig.TimeKey = "" // no timestamps in tests
 			tt.cfg.InitialFields = map[string]interface{}{"z": "zz", "k": "v"}
 
@@ -74,7 +72,7 @@ func TestConfig(t *testing.T) {
 			logger.Info("info")
 			logger.Warn("warn")
 
-			byteContents, err := io.ReadAll(temp)
+			byteContents, err := os.ReadFile(logOut)
 			require.NoError(t, err, "Couldn't read log contents from temp file.")
 			logs := string(byteContents)
 			assert.Regexp(t, tt.expectRe, logs, "Unexpected log output.")
@@ -180,16 +178,8 @@ func TestConfigWithSamplingHook(t *testing.T) {
 	expectDropped := 99  // 200 - 100 initial - 1 thereafter
 	expectSampled := 103 // 2 from initial + 100 + 1 thereafter
 
-	temp, err := os.CreateTemp("", "zap-prod-config-test")
-	require.NoError(t, err, "Failed to create temp file.")
-	defer func() {
-		err := os.Remove(temp.Name())
-		if err != nil {
-			return
-		}
-	}()
-
-	cfg.OutputPaths = []string{temp.Name()}
+	logOut := filepath.Join(t.TempDir(), "test.log")
+	cfg.OutputPaths = []string{logOut}
 	cfg.EncoderConfig.TimeKey = "" // no timestamps in tests
 	cfg.InitialFields = map[string]interface{}{"z": "zz", "k": "v"}
 
@@ -200,7 +190,7 @@ func TestConfigWithSamplingHook(t *testing.T) {
 	logger.Info("info")
 	logger.Warn("warn")
 
-	byteContents, err := io.ReadAll(temp)
+	byteContents, err := os.ReadFile(logOut)
 	require.NoError(t, err, "Couldn't read log contents from temp file.")
 	logs := string(byteContents)
 	assert.Regexp(t, expectRe, logs, "Unexpected log output.")
diff --git a/stacktrace_ext_test.go b/stacktrace_ext_test.go
@@ -160,12 +160,8 @@ func verifyNoZap(t *testing.T, logs string) {
 }
 
 func withGoPath(t *testing.T, f func(goPath string)) {
-	goPath, err := os.MkdirTemp("", "gopath")
-	require.NoError(t, err, "Failed to create temporary directory for GOPATH")
-	//defer os.RemoveAll(goPath)
-
-	os.Setenv("GOPATH", goPath)
-	defer os.Setenv("GOPATH", os.Getenv("GOPATH"))
+	goPath := filepath.Join(t.TempDir(), "gopath")
+	t.Setenv("GOPATH", goPath)
 
 	f(goPath)
 }
diff --git a/writer_test.go b/writer_test.go
@@ -21,10 +21,8 @@
 package zap
 
 import (
-	"encoding/hex"
 	"errors"
 	"io"
-	"math/rand"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -50,7 +48,7 @@ func TestOpenNoPaths(t *testing.T) {
 }
 
 func TestOpen(t *testing.T) {
-	tempName := tempFileName("", "zap-open-test")
+	tempName := filepath.Join(t.TempDir(), "test.log")
 	assert.False(t, fileExists(tempName))
 	require.True(t, strings.HasPrefix(tempName, "/"), "Expected absolute temp file path.")
 
@@ -171,12 +169,6 @@ func TestCombineWriteSyncers(t *testing.T) {
 	w.Write([]byte("test"))
 }
 
-func tempFileName(prefix, suffix string) string {
-	randBytes := make([]byte, 16)
-	rand.Read(randBytes)
-	return filepath.Join(os.TempDir(), prefix+hex.EncodeToString(randBytes)+suffix)
-}
-
 func fileExists(name string) bool {
 	if _, err := os.Stat(name); os.IsNotExist(err) {
 		return false
diff --git a/zapcore/buffered_write_syncer_bench_test.go b/zapcore/buffered_write_syncer_bench_test.go
@@ -30,12 +30,11 @@ import (
 
 func BenchmarkBufferedWriteSyncer(b *testing.B) {
 	b.Run("write file with buffer", func(b *testing.B) {
-		file, err := os.CreateTemp("", "log")
+		file, err := os.CreateTemp(b.TempDir(), "test.log")
 		require.NoError(b, err)
 
 		defer func() {
 			assert.NoError(b, file.Close())
-			assert.NoError(b, os.Remove(file.Name()))
 		}()
 
 		w := &BufferedWriteSyncer{
diff --git a/zapcore/core_test.go b/zapcore/core_test.go
@@ -67,9 +67,8 @@ func TestNopCore(t *testing.T) {
 }
 
 func TestIOCore(t *testing.T) {
-	temp, err := os.CreateTemp("", "zapcore-test-iocore")
-	require.NoError(t, err, "Failed to create temp file.")
-	defer os.Remove(temp.Name())
+	temp, err := os.CreateTemp(t.TempDir(), "test.log")
+	require.NoError(t, err)
 
 	// Drop timestamps for simpler assertions (timestamp encoding is tested
 	// elsewhere).
diff --git a/zapcore/write_syncer_bench_test.go b/zapcore/write_syncer_bench_test.go
@@ -24,7 +24,7 @@ import (
 	"os"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.uber.org/zap/internal/ztest"
 )
 
@@ -76,10 +76,8 @@ func BenchmarkMultiWriteSyncer(b *testing.B) {
 
 func BenchmarkWriteSyncer(b *testing.B) {
 	b.Run("write file with no buffer", func(b *testing.B) {
-		file, err := os.CreateTemp("", "log")
-		assert.NoError(b, err)
-		defer file.Close()
-		defer os.Remove(file.Name())
+		file, err := os.CreateTemp(b.TempDir(), "test.log")
+		require.NoError(b, err)
 
 		w := AddSync(file)
 		b.ResetTimer()

Original file line number	Diff line number	Diff line change
`@@ -160,12 +160,8 @@ func verifyNoZap(t *testing.T, logs string) {`
`160`	`160`	`}`
`161`	`161`
`162`	`162`	`func withGoPath(t *testing.T, f func(goPath string)) {`
`163`		`- goPath, err := os.MkdirTemp("", "gopath")`
`164`		`- require.NoError(t, err, "Failed to create temporary directory for GOPATH")`
`165`		`- //defer os.RemoveAll(goPath)`
`166`		`-`
`167`		`- os.Setenv("GOPATH", goPath)`
`168`		`- defer os.Setenv("GOPATH", os.Getenv("GOPATH"))`
	`163`	`+ goPath := filepath.Join(t.TempDir(), "gopath")`
	`164`	`+ t.Setenv("GOPATH", goPath)`
`169`	`165`
`170`	`166`	`f(goPath)`
`171`	`167`	`}`