From eea9d4a10193a7fce3a1138a9114d798e0e1ff51 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Tue, 3 Jan 2023 19:48:18 +0800 Subject: [PATCH 1/2] test: use `T.TempDir` to create temporary test directory This commit replaces `os.MkdirTemp` with `t.TempDir` in tests. The directory created by `t.TempDir` is automatically removed when the test and all its subtests complete. Prior to this commit, temporary directory created using `os.MkdirTemp` needs to be removed manually by calling `os.RemoveAll`, which is omitted in some tests. The error handling boilerplate e.g. defer func() { if err := os.RemoveAll(dir); err != nil { t.Fatal(err) } } is also tedious, but `t.TempDir` handles this for us nicely. Reference: https://pkg.go.dev/testing#T.TempDir Signed-off-by: Eng Zer Jun --- internal/raft/log/compaction_test.go | 21 ++----------- internal/raft/log/log_test.go | 21 ++----------- internal/raft/log/snapshot_storage_test.go | 21 ++----------- .../zone/segmentedfile/segmented_file_test.go | 8 +---- internal/store/meta/async_test.go | 9 +----- internal/store/meta/sync_test.go | 9 +----- internal/store/segment/recovery_test.go | 9 ++---- internal/store/wal/recovery_test.go | 9 +----- internal/store/wal/wal_bench_test.go | 19 ++---------- internal/store/wal/wal_test.go | 30 ++++--------------- 10 files changed, 23 insertions(+), 133 deletions(-) diff --git a/internal/raft/log/compaction_test.go b/internal/raft/log/compaction_test.go index aac802468..6f53419c5 100644 --- a/internal/raft/log/compaction_test.go +++ b/internal/raft/log/compaction_test.go @@ -19,7 +19,6 @@ import ( // standard libraries. "encoding/binary" "fmt" - "os" "testing" // third-party libraries. @@ -37,23 +36,9 @@ func TestLog_Compact(t *testing.T) { data := make([]byte, blockSize) copy(data, []byte("hello world!")) - metaDir, err := os.MkdirTemp("", "meta-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(metaDir) - - offsetDir, err := os.MkdirTemp("", "offset-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(offsetDir) - - walDir, err := os.MkdirTemp("", "wal-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(walDir) + metaDir := t.TempDir() + offsetDir := t.TempDir() + walDir := t.TempDir() Convey("raft log compaction", t, func() { metaStore, err := meta.RecoverSyncStore(stdCtx.Background(), metaCfg, metaDir) diff --git a/internal/raft/log/log_test.go b/internal/raft/log/log_test.go index f2971af2e..df64e3c03 100644 --- a/internal/raft/log/log_test.go +++ b/internal/raft/log/log_test.go @@ -18,7 +18,6 @@ import ( // standard libraries. "context" "math" - "os" "testing" // third-party libraries. @@ -61,23 +60,9 @@ var ( func TestLog(t *testing.T) { ctx := context.Background() - metaDir, err := os.MkdirTemp("", "meta-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(metaDir) - - offsetDir, err := os.MkdirTemp("", "offset-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(offsetDir) - - walDir, err := os.MkdirTemp("", "wal-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(walDir) + metaDir := t.TempDir() + offsetDir := t.TempDir() + walDir := t.TempDir() cc1 := raftpb.ConfChange{ Type: raftpb.ConfChangeAddNode, NodeID: nodeID1.Uint64(), diff --git a/internal/raft/log/snapshot_storage_test.go b/internal/raft/log/snapshot_storage_test.go index 4f93b7203..68739aaf5 100644 --- a/internal/raft/log/snapshot_storage_test.go +++ b/internal/raft/log/snapshot_storage_test.go @@ -17,7 +17,6 @@ package log import ( // standard libraries. "context" - "os" "testing" // third-party libraries. @@ -35,23 +34,9 @@ import ( func TestLog_SnapshotStorage(t *testing.T) { ctx := context.Background() - metaDir, err := os.MkdirTemp("", "meta-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(metaDir) - - offsetDir, err := os.MkdirTemp("", "offset-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(offsetDir) - - walDir, err := os.MkdirTemp("", "wal-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(walDir) + metaDir := t.TempDir() + offsetDir := t.TempDir() + walDir := t.TempDir() cc1 := raftpb.ConfChange{ Type: raftpb.ConfChangeAddNode, NodeID: nodeID1.Uint64(), diff --git a/internal/store/io/zone/segmentedfile/segmented_file_test.go b/internal/store/io/zone/segmentedfile/segmented_file_test.go index 6f7470bfa..217e170d1 100644 --- a/internal/store/io/zone/segmentedfile/segmented_file_test.go +++ b/internal/store/io/zone/segmentedfile/segmented_file_test.go @@ -15,8 +15,6 @@ package segmentedfile import ( - // standard libraries. - "os" "testing" // third-party libraries. @@ -27,8 +25,7 @@ const fileSize = 32 * 1024 func TestSegmentedFile_SelectSegment(t *testing.T) { Convey("segmented file", t, func() { - dir, err := os.MkdirTemp("", "sf-*") - So(err, ShouldBeNil) + dir := t.TempDir() sf, err := Open(dir, WithSegmentSize(fileSize)) So(err, ShouldBeNil) @@ -79,9 +76,6 @@ func TestSegmentedFile_SelectSegment(t *testing.T) { Reset(func() { sf.Close() - - err = os.RemoveAll(dir) - So(err, ShouldBeNil) }) }) } diff --git a/internal/store/meta/async_test.go b/internal/store/meta/async_test.go index 686db7c14..38b5745d1 100644 --- a/internal/store/meta/async_test.go +++ b/internal/store/meta/async_test.go @@ -17,7 +17,6 @@ package meta import ( // standard libraries. "context" - "os" "testing" // third-party libraries. @@ -31,8 +30,7 @@ func TestAsyncStore(t *testing.T) { ctx := context.Background() Convey("AsyncStore", t, func() { - walDir, err := os.MkdirTemp("", "async-*") - So(err, ShouldBeNil) + walDir := t.TempDir() Convey("new empty AsyncStore by recovery", func() { ss, err := RecoverAsyncStore(ctx, config.AsyncStore{}, walDir) @@ -97,10 +95,5 @@ func TestAsyncStore(t *testing.T) { }) }) }) - - Reset(func() { - err := os.RemoveAll(walDir) - So(err, ShouldBeNil) - }) }) } diff --git a/internal/store/meta/sync_test.go b/internal/store/meta/sync_test.go index 13155f768..3cc3bca88 100644 --- a/internal/store/meta/sync_test.go +++ b/internal/store/meta/sync_test.go @@ -17,7 +17,6 @@ package meta import ( // standard libraries. "context" - "os" "testing" // third-party libraries. @@ -35,8 +34,7 @@ var ( func TestSyncStore(t *testing.T) { Convey("SyncStore", t, func() { - walDir, err := os.MkdirTemp("", "sync-*") - So(err, ShouldBeNil) + walDir := t.TempDir() Convey("new empty SyncStore by recovery", func() { ss, err := RecoverSyncStore(context.Background(), config.SyncStore{}, walDir) @@ -101,10 +99,5 @@ func TestSyncStore(t *testing.T) { }) }) }) - - Reset(func() { - err := os.RemoveAll(walDir) - So(err, ShouldBeNil) - }) }) } diff --git a/internal/store/segment/recovery_test.go b/internal/store/segment/recovery_test.go index 74bdfc2f4..17a086f0d 100644 --- a/internal/store/segment/recovery_test.go +++ b/internal/store/segment/recovery_test.go @@ -17,7 +17,6 @@ package segment import ( // standard libraries. "context" - "os" "testing" // third-party libraries. @@ -33,11 +32,7 @@ func TestServer_recover(t *testing.T) { defer ctrl.Finish() Convey("recover", t, func() { - dir, err := os.MkdirTemp("", "volume-*") - So(err, ShouldBeNil) - defer func() { - So(os.RemoveAll(dir), ShouldBeNil) - }() + dir := t.TempDir() srv := &server{ cfg: store.Config{ @@ -46,7 +41,7 @@ func TestServer_recover(t *testing.T) { }, }, } - err = srv.loadVSBEngine(context.Background(), srv.cfg.VSB) + err := srv.loadVSBEngine(context.Background(), srv.cfg.VSB) So(err, ShouldBeNil) err = srv.recover(context.Background()) diff --git a/internal/store/wal/recovery_test.go b/internal/store/wal/recovery_test.go index 0ba59825a..f707ef27f 100644 --- a/internal/store/wal/recovery_test.go +++ b/internal/store/wal/recovery_test.go @@ -17,7 +17,6 @@ package wal import ( // standard libraries. "context" - "os" "testing" // third-party libraries. @@ -28,8 +27,7 @@ func TestOpen(t *testing.T) { ctx := context.Background() Convey("open wal in recover mode", t, func() { - walDir, err := os.MkdirTemp("", "wal-*") - So(err, ShouldBeNil) + walDir := t.TempDir() Convey("create empty wal", func() { var entryNum int @@ -107,10 +105,5 @@ func TestOpen(t *testing.T) { wal.Close() wal.Wait() }) - - Reset(func() { - err := os.RemoveAll(walDir) - So(err, ShouldBeNil) - }) }) } diff --git a/internal/store/wal/wal_bench_test.go b/internal/store/wal/wal_bench_test.go index 133136911..850b49b85 100644 --- a/internal/store/wal/wal_bench_test.go +++ b/internal/store/wal/wal_bench_test.go @@ -19,7 +19,6 @@ import ( "context" "fmt" "log" - "os" "sync" "testing" @@ -30,11 +29,7 @@ import ( const walTempDir = "" func BenchmarkWAL_AppendOneWithBatching(b *testing.B) { - walDir, err := os.MkdirTemp(walTempDir, "wal-*") - if err != nil { - b.Fatal(err) - } - defer os.RemoveAll(walDir) + walDir := b.TempDir() wal, err := Open(context.Background(), walDir) if err != nil { @@ -63,11 +58,7 @@ func BenchmarkWAL_AppendOneWithBatching(b *testing.B) { } func BenchmarkWAL_AppendOneWithoutBatching(b *testing.B) { - walDir, err := os.MkdirTemp(walTempDir, "wal-*") - if err != nil { - b.Fatal(err) - } - defer os.RemoveAll(walDir) + walDir := b.TempDir() wal, err := Open(context.Background(), walDir) if err != nil { @@ -97,11 +88,7 @@ func BenchmarkWAL_AppendOneWithoutBatching(b *testing.B) { func BenchmarkWAL_AppendOneWithCallback(b *testing.B) { // walDir := filepath.Join(walTempDir, "wal-test") - walDir, err := os.MkdirTemp(walTempDir, "wal-*") - if err != nil { - b.Fatal(err) - } - defer os.RemoveAll(walDir) + walDir := b.TempDir() wal, err := Open(context.Background(), walDir) if err != nil { diff --git a/internal/store/wal/wal_test.go b/internal/store/wal/wal_test.go index 2c7666429..313990553 100644 --- a/internal/store/wal/wal_test.go +++ b/internal/store/wal/wal_test.go @@ -41,8 +41,7 @@ func TestWAL_AppendOne(t *testing.T) { ctx := context.Background() Convey("wal append one", t, func() { - walDir, err := os.MkdirTemp("", "wal-*") - So(err, ShouldBeNil) + walDir := t.TempDir() wal, err := Open(ctx, walDir, WithFileSize(fileSize)) So(err, ShouldBeNil) @@ -86,15 +85,11 @@ func TestWAL_AppendOne(t *testing.T) { Reset(func() { wal.Close() wal.Wait() - - err := os.RemoveAll(walDir) - So(err, ShouldBeNil) }) }) Convey("flush wal when timeout", t, func() { - walDir, err := os.MkdirTemp("", "wal-*") - So(err, ShouldBeNil) + walDir := t.TempDir() flushTimeout := time.Second wal, err := Open(ctx, walDir, WithFileSize(fileSize), WithFlushTimeout(flushTimeout)) @@ -120,14 +115,10 @@ func TestWAL_AppendOne(t *testing.T) { wal.Close() wal.Wait() - - err = os.RemoveAll(walDir) - So(err, ShouldBeNil) }) Convey("wal append one after close", t, func() { - walDir, err := os.MkdirTemp("", "wal-*") - So(err, ShouldBeNil) + walDir := t.TempDir() flushTimeout := 100 * time.Millisecond wal, err := Open(ctx, walDir, WithFileSize(fileSize), WithFlushTimeout(flushTimeout)) @@ -153,9 +144,6 @@ func TestWAL_AppendOne(t *testing.T) { // NOTE: There is no guarantee that data0 will be successfully written. // So(wal.wb.Size(), ShouldEqual, 10) // So(wal.wb.Committed(), ShouldEqual, 10) - - err = os.RemoveAll(walDir) - So(err, ShouldBeNil) }) } @@ -163,8 +151,7 @@ func TestWAL_Append(t *testing.T) { ctx := context.Background() Convey("wal append", t, func() { - walDir, err := os.MkdirTemp("", "wal-*") - So(err, ShouldBeNil) + walDir := t.TempDir() wal, err := Open(ctx, walDir, WithFileSize(fileSize)) So(err, ShouldBeNil) @@ -194,9 +181,6 @@ func TestWAL_Append(t *testing.T) { Reset(func() { wal.Close() wal.Wait() - - err := os.RemoveAll(walDir) - So(err, ShouldBeNil) }) }) } @@ -207,8 +191,7 @@ func TestWAL_Compact(t *testing.T) { copy(data, []byte("hello world!")) Convey("wal compaction", t, func() { - walDir, err := os.MkdirTemp("", "wal-*") - So(err, ShouldBeNil) + walDir := t.TempDir() wal, err := Open(ctx, walDir, WithFileSize(fileSize)) So(err, ShouldBeNil) @@ -244,8 +227,5 @@ func TestWAL_Compact(t *testing.T) { wal.Close() wal.Wait() - - err = os.RemoveAll(walDir) - So(err, ShouldBeNil) }) } From 72f5c9b207620ed29247f37d984ada3a69813334 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Fri, 6 Jan 2023 18:39:47 +0800 Subject: [PATCH 2/2] test: import comments Signed-off-by: Eng Zer Jun --- internal/store/io/zone/segmentedfile/segmented_file_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/store/io/zone/segmentedfile/segmented_file_test.go b/internal/store/io/zone/segmentedfile/segmented_file_test.go index 217e170d1..e2f656aed 100644 --- a/internal/store/io/zone/segmentedfile/segmented_file_test.go +++ b/internal/store/io/zone/segmentedfile/segmented_file_test.go @@ -15,6 +15,7 @@ package segmentedfile import ( + // standard libraries. "testing" // third-party libraries.