From bdd673d714c1278fd09836d7cf57ed60fa072bf4 Mon Sep 17 00:00:00 2001 From: Prashant Varanasi Date: Mon, 15 Aug 2022 08:58:30 -0700 Subject: [PATCH] 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 --- config_test.go | 24 ++++++--------------- stacktrace_ext_test.go | 8 ++----- writer_test.go | 10 +-------- zapcore/buffered_write_syncer_bench_test.go | 3 +-- zapcore/core_test.go | 5 ++--- zapcore/write_syncer_bench_test.go | 8 +++---- 6 files changed, 16 insertions(+), 42 deletions(-) diff --git a/config_test.go b/config_test.go index f5bc98a1a..081ecbc15 100644 --- 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 index 2546843a0..71f098333 100644 --- 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 index 9129af70a..77da8d85b 100644 --- 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 index 42ed881b7..1e3db59f1 100644 --- 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 index 06bc82c5d..0542270ca 100644 --- 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 index ff9a5bcaf..db6ec4523 100644 --- 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()