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

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 <[email protected]>
uber-go · Aug 15, 2022 · bdd673d · bdd673d
1 parent 4b03bc5
commit bdd673d
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 42 deletions.
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()