From 1f4f70723f26affba8fb3d76c9055f031c52f435 Mon Sep 17 00:00:00 2001 From: "jianfei.zhang" Date: Wed, 9 Nov 2022 21:22:03 +0800 Subject: [PATCH 1/2] feat: use testify packages in tests Signed-off-by: jianfei.zhang --- raft/storage_test.go | 102 ++++++++++++------------------------------- raft/util_test.go | 27 +++--------- 2 files changed, 34 insertions(+), 95 deletions(-) diff --git a/raft/storage_test.go b/raft/storage_test.go index 04eb85bdf15..f317d276d94 100644 --- a/raft/storage_test.go +++ b/raft/storage_test.go @@ -16,9 +16,9 @@ package raft import ( "math" - "reflect" "testing" + "github.com/stretchr/testify/require" pb "go.etcd.io/etcd/raft/v3/raftpb" ) @@ -51,12 +51,8 @@ func TestStorageTerm(t *testing.T) { }() term, err := s.Term(tt.i) - if err != tt.werr { - t.Errorf("#%d: err = %v, want %v", i, err, tt.werr) - } - if term != tt.wterm { - t.Errorf("#%d: term = %d, want %d", i, term, tt.wterm) - } + require.Equal(t, tt.werr, err, "#%d", i) + require.Equal(t, tt.wterm, term, "#%d", i) }() } } @@ -85,15 +81,11 @@ func TestStorageEntries(t *testing.T) { {4, 7, uint64(ents[1].Size() + ents[2].Size() + ents[3].Size()), nil, []pb.Entry{{Index: 4, Term: 4}, {Index: 5, Term: 5}, {Index: 6, Term: 6}}}, } - for i, tt := range tests { + for _, tt := range tests { s := &MemoryStorage{ents: ents} entries, err := s.Entries(tt.lo, tt.hi, tt.maxsize) - if err != tt.werr { - t.Errorf("#%d: err = %v, want %v", i, err, tt.werr) - } - if !reflect.DeepEqual(entries, tt.wentries) { - t.Errorf("#%d: entries = %v, want %v", i, entries, tt.wentries) - } + require.Equal(t, tt.werr, err) + require.Equal(t, tt.wentries, entries) } } @@ -102,21 +94,13 @@ func TestStorageLastIndex(t *testing.T) { s := &MemoryStorage{ents: ents} last, err := s.LastIndex() - if err != nil { - t.Errorf("err = %v, want nil", err) - } - if last != 5 { - t.Errorf("last = %d, want %d", last, 5) - } + require.NoError(t, err) + require.Equal(t, uint64(5), last) s.Append([]pb.Entry{{Index: 6, Term: 5}}) last, err = s.LastIndex() - if err != nil { - t.Errorf("err = %v, want nil", err) - } - if last != 6 { - t.Errorf("last = %d, want %d", last, 6) - } + require.NoError(t, err) + require.Equal(t, uint64(6), last) } func TestStorageFirstIndex(t *testing.T) { @@ -124,21 +108,13 @@ func TestStorageFirstIndex(t *testing.T) { s := &MemoryStorage{ents: ents} first, err := s.FirstIndex() - if err != nil { - t.Errorf("err = %v, want nil", err) - } - if first != 4 { - t.Errorf("first = %d, want %d", first, 4) - } + require.NoError(t, err) + require.Equal(t, uint64(4), first) s.Compact(4) first, err = s.FirstIndex() - if err != nil { - t.Errorf("err = %v, want nil", err) - } - if first != 5 { - t.Errorf("first = %d, want %d", first, 5) - } + require.NoError(t, err) + require.Equal(t, uint64(5), first) } func TestStorageCompact(t *testing.T) { @@ -157,21 +133,12 @@ func TestStorageCompact(t *testing.T) { {5, nil, 5, 5, 1}, } - for i, tt := range tests { + for _, tt := range tests { s := &MemoryStorage{ents: ents} - err := s.Compact(tt.i) - if err != tt.werr { - t.Errorf("#%d: err = %v, want %v", i, err, tt.werr) - } - if s.ents[0].Index != tt.windex { - t.Errorf("#%d: index = %d, want %d", i, s.ents[0].Index, tt.windex) - } - if s.ents[0].Term != tt.wterm { - t.Errorf("#%d: term = %d, want %d", i, s.ents[0].Term, tt.wterm) - } - if len(s.ents) != tt.wlen { - t.Errorf("#%d: len = %d, want %d", i, len(s.ents), tt.wlen) - } + require.Equal(t, tt.werr, s.Compact(tt.i)) + require.Equal(t, tt.windex, s.ents[0].Index) + require.Equal(t, tt.wterm, s.ents[0].Term) + require.Equal(t, tt.wlen, len(s.ents)) } } @@ -190,15 +157,11 @@ func TestStorageCreateSnapshot(t *testing.T) { {5, nil, pb.Snapshot{Data: data, Metadata: pb.SnapshotMetadata{Index: 5, Term: 5, ConfState: *cs}}}, } - for i, tt := range tests { + for _, tt := range tests { s := &MemoryStorage{ents: ents} snap, err := s.CreateSnapshot(tt.i, cs, data) - if err != tt.werr { - t.Errorf("#%d: err = %v, want %v", i, err, tt.werr) - } - if !reflect.DeepEqual(snap, tt.wsnap) { - t.Errorf("#%d: snap = %+v, want %+v", i, snap, tt.wsnap) - } + require.Equal(t, tt.werr, err) + require.Equal(t, tt.wsnap, snap) } } @@ -250,15 +213,10 @@ func TestStorageAppend(t *testing.T) { }, } - for i, tt := range tests { + for _, tt := range tests { s := &MemoryStorage{ents: ents} - err := s.Append(tt.entries) - if err != tt.werr { - t.Errorf("#%d: err = %v, want %v", i, err, tt.werr) - } - if !reflect.DeepEqual(s.ents, tt.wentries) { - t.Errorf("#%d: entries = %v, want %v", i, s.ents, tt.wentries) - } + require.Equal(t, tt.werr, s.Append(tt.entries)) + require.Equal(t, tt.wentries, s.ents) } } @@ -275,16 +233,10 @@ func TestStorageApplySnapshot(t *testing.T) { //Apply Snapshot successful i := 0 tt := tests[i] - err := s.ApplySnapshot(tt) - if err != nil { - t.Errorf("#%d: err = %v, want %v", i, err, nil) - } + require.NoError(t, s.ApplySnapshot(tt)) //Apply Snapshot fails due to ErrSnapOutOfDate i = 1 tt = tests[i] - err = s.ApplySnapshot(tt) - if err != ErrSnapOutOfDate { - t.Errorf("#%d: err = %v, want %v", i, err, ErrSnapOutOfDate) - } + require.Equal(t, ErrSnapOutOfDate, s.ApplySnapshot(tt)) } diff --git a/raft/util_test.go b/raft/util_test.go index 65bc95501bf..b16dfeda70e 100644 --- a/raft/util_test.go +++ b/raft/util_test.go @@ -16,10 +16,10 @@ package raft import ( "math" - "reflect" "strings" "testing" + "github.com/stretchr/testify/require" pb "go.etcd.io/etcd/raft/v3/raftpb" ) @@ -34,16 +34,8 @@ func TestDescribeEntry(t *testing.T) { Type: pb.EntryNormal, Data: []byte("hello\x00world"), } - - defaultFormatted := DescribeEntry(entry, nil) - if defaultFormatted != "1/2 EntryNormal \"hello\\x00world\"" { - t.Errorf("unexpected default output: %s", defaultFormatted) - } - - customFormatted := DescribeEntry(entry, testFormatter) - if customFormatted != "1/2 EntryNormal HELLO\x00WORLD" { - t.Errorf("unexpected custom output: %s", customFormatted) - } + require.Equal(t, "1/2 EntryNormal \"hello\\x00world\"", DescribeEntry(entry, nil)) + require.Equal(t, "1/2 EntryNormal HELLO\x00WORLD", DescribeEntry(entry, testFormatter)) } func TestLimitSize(t *testing.T) { @@ -64,10 +56,8 @@ func TestLimitSize(t *testing.T) { {uint64(ents[0].Size() + ents[1].Size() + ents[2].Size()), []pb.Entry{{Index: 4, Term: 4}, {Index: 5, Term: 5}, {Index: 6, Term: 6}}}, } - for i, tt := range tests { - if !reflect.DeepEqual(limitSize(ents, tt.maxsize), tt.wentries) { - t.Errorf("#%d: entries = %v, want %v", i, limitSize(ents, tt.maxsize), tt.wentries) - } + for _, tt := range tests { + require.Equal(t, tt.wentries, limitSize(ents, tt.maxsize)) } } @@ -97,10 +87,7 @@ func TestIsLocalMsg(t *testing.T) { {pb.MsgPreVoteResp, false}, } - for i, tt := range tests { - got := IsLocalMsg(tt.msgt) - if got != tt.isLocal { - t.Errorf("#%d: got %v, want %v", i, got, tt.isLocal) - } + for _, tt := range tests { + require.Equal(t, tt.isLocal, IsLocalMsg(tt.msgt)) } } From de97f6aa3d245e0345365fc20fced4f8ab2d8d49 Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Sun, 13 Nov 2022 22:34:09 +0100 Subject: [PATCH 2/2] raft: tidy up the unit tests some more Use `t.Run` for each test case, and make some tests more idiomatic. Signed-off-by: Tobias Grieger --- raft/storage_test.go | 80 +++++++++++++++++++++++--------------------- raft/util_test.go | 19 +++++++---- 2 files changed, 54 insertions(+), 45 deletions(-) diff --git a/raft/storage_test.go b/raft/storage_test.go index f317d276d94..b7b8ed4e0ea 100644 --- a/raft/storage_test.go +++ b/raft/storage_test.go @@ -38,22 +38,19 @@ func TestStorageTerm(t *testing.T) { {6, ErrUnavailable, 0, false}, } - for i, tt := range tests { - s := &MemoryStorage{ents: ents} - - func() { - defer func() { - if r := recover(); r != nil { - if !tt.wpanic { - t.Errorf("%d: panic = %v, want %v", i, true, tt.wpanic) - } - } - }() - + for _, tt := range tests { + t.Run("", func(t *testing.T) { + s := &MemoryStorage{ents: ents} + + if tt.wpanic { + require.Panics(t, func() { + _, _ = s.Term(tt.i) + }) + } term, err := s.Term(tt.i) - require.Equal(t, tt.werr, err, "#%d", i) - require.Equal(t, tt.wterm, term, "#%d", i) - }() + require.Equal(t, tt.werr, err) + require.Equal(t, tt.wterm, term) + }) } } @@ -82,10 +79,12 @@ func TestStorageEntries(t *testing.T) { } for _, tt := range tests { - s := &MemoryStorage{ents: ents} - entries, err := s.Entries(tt.lo, tt.hi, tt.maxsize) - require.Equal(t, tt.werr, err) - require.Equal(t, tt.wentries, entries) + t.Run("", func(t *testing.T) { + s := &MemoryStorage{ents: ents} + entries, err := s.Entries(tt.lo, tt.hi, tt.maxsize) + require.Equal(t, tt.werr, err) + require.Equal(t, tt.wentries, entries) + }) } } @@ -97,7 +96,7 @@ func TestStorageLastIndex(t *testing.T) { require.NoError(t, err) require.Equal(t, uint64(5), last) - s.Append([]pb.Entry{{Index: 6, Term: 5}}) + require.NoError(t, s.Append([]pb.Entry{{Index: 6, Term: 5}})) last, err = s.LastIndex() require.NoError(t, err) require.Equal(t, uint64(6), last) @@ -111,7 +110,7 @@ func TestStorageFirstIndex(t *testing.T) { require.NoError(t, err) require.Equal(t, uint64(4), first) - s.Compact(4) + require.NoError(t, s.Compact(4)) first, err = s.FirstIndex() require.NoError(t, err) require.Equal(t, uint64(5), first) @@ -134,11 +133,13 @@ func TestStorageCompact(t *testing.T) { } for _, tt := range tests { - s := &MemoryStorage{ents: ents} - require.Equal(t, tt.werr, s.Compact(tt.i)) - require.Equal(t, tt.windex, s.ents[0].Index) - require.Equal(t, tt.wterm, s.ents[0].Term) - require.Equal(t, tt.wlen, len(s.ents)) + t.Run("", func(t *testing.T) { + s := &MemoryStorage{ents: ents} + require.Equal(t, tt.werr, s.Compact(tt.i)) + require.Equal(t, tt.windex, s.ents[0].Index) + require.Equal(t, tt.wterm, s.ents[0].Term) + require.Equal(t, tt.wlen, len(s.ents)) + }) } } @@ -158,10 +159,12 @@ func TestStorageCreateSnapshot(t *testing.T) { } for _, tt := range tests { - s := &MemoryStorage{ents: ents} - snap, err := s.CreateSnapshot(tt.i, cs, data) - require.Equal(t, tt.werr, err) - require.Equal(t, tt.wsnap, snap) + t.Run("", func(t *testing.T) { + s := &MemoryStorage{ents: ents} + snap, err := s.CreateSnapshot(tt.i, cs, data) + require.Equal(t, tt.werr, err) + require.Equal(t, tt.wsnap, snap) + }) } } @@ -193,19 +196,19 @@ func TestStorageAppend(t *testing.T) { nil, []pb.Entry{{Index: 3, Term: 3}, {Index: 4, Term: 4}, {Index: 5, Term: 5}, {Index: 6, Term: 5}}, }, - // truncate incoming entries, truncate the existing entries and append + // Truncate incoming entries, truncate the existing entries and append. { []pb.Entry{{Index: 2, Term: 3}, {Index: 3, Term: 3}, {Index: 4, Term: 5}}, nil, []pb.Entry{{Index: 3, Term: 3}, {Index: 4, Term: 5}}, }, - // truncate the existing entries and append + // Truncate the existing entries and append. { []pb.Entry{{Index: 4, Term: 5}}, nil, []pb.Entry{{Index: 3, Term: 3}, {Index: 4, Term: 5}}, }, - // direct append + // Direct append. { []pb.Entry{{Index: 6, Term: 5}}, nil, @@ -214,9 +217,11 @@ func TestStorageAppend(t *testing.T) { } for _, tt := range tests { - s := &MemoryStorage{ents: ents} - require.Equal(t, tt.werr, s.Append(tt.entries)) - require.Equal(t, tt.wentries, s.ents) + t.Run("", func(t *testing.T) { + s := &MemoryStorage{ents: ents} + require.Equal(t, tt.werr, s.Append(tt.entries)) + require.Equal(t, tt.wentries, s.ents) + }) } } @@ -230,12 +235,11 @@ func TestStorageApplySnapshot(t *testing.T) { s := NewMemoryStorage() - //Apply Snapshot successful i := 0 tt := tests[i] require.NoError(t, s.ApplySnapshot(tt)) - //Apply Snapshot fails due to ErrSnapOutOfDate + // ApplySnapshot fails due to ErrSnapOutOfDate. i = 1 tt = tests[i] require.Equal(t, ErrSnapOutOfDate, s.ApplySnapshot(tt)) diff --git a/raft/util_test.go b/raft/util_test.go index b16dfeda70e..eeedebd416b 100644 --- a/raft/util_test.go +++ b/raft/util_test.go @@ -15,6 +15,7 @@ package raft import ( + "fmt" "math" "strings" "testing" @@ -34,7 +35,7 @@ func TestDescribeEntry(t *testing.T) { Type: pb.EntryNormal, Data: []byte("hello\x00world"), } - require.Equal(t, "1/2 EntryNormal \"hello\\x00world\"", DescribeEntry(entry, nil)) + require.Equal(t, `1/2 EntryNormal "hello\x00world"`, DescribeEntry(entry, nil)) require.Equal(t, "1/2 EntryNormal HELLO\x00WORLD", DescribeEntry(entry, testFormatter)) } @@ -45,19 +46,21 @@ func TestLimitSize(t *testing.T) { wentries []pb.Entry }{ {math.MaxUint64, []pb.Entry{{Index: 4, Term: 4}, {Index: 5, Term: 5}, {Index: 6, Term: 6}}}, - // even if maxsize is zero, the first entry should be returned + // Even if maxsize is zero, the first entry should be returned. {0, []pb.Entry{{Index: 4, Term: 4}}}, - // limit to 2 + // Limit to 2. {uint64(ents[0].Size() + ents[1].Size()), []pb.Entry{{Index: 4, Term: 4}, {Index: 5, Term: 5}}}, - // limit to 2 + // Limit to 2. {uint64(ents[0].Size() + ents[1].Size() + ents[2].Size()/2), []pb.Entry{{Index: 4, Term: 4}, {Index: 5, Term: 5}}}, {uint64(ents[0].Size() + ents[1].Size() + ents[2].Size() - 1), []pb.Entry{{Index: 4, Term: 4}, {Index: 5, Term: 5}}}, - // all + // All. {uint64(ents[0].Size() + ents[1].Size() + ents[2].Size()), []pb.Entry{{Index: 4, Term: 4}, {Index: 5, Term: 5}, {Index: 6, Term: 6}}}, } for _, tt := range tests { - require.Equal(t, tt.wentries, limitSize(ents, tt.maxsize)) + t.Run("", func(t *testing.T) { + require.Equal(t, tt.wentries, limitSize(ents, tt.maxsize)) + }) } } @@ -88,6 +91,8 @@ func TestIsLocalMsg(t *testing.T) { } for _, tt := range tests { - require.Equal(t, tt.isLocal, IsLocalMsg(tt.msgt)) + t.Run(fmt.Sprint(tt.msgt), func(t *testing.T) { + require.Equal(t, tt.isLocal, IsLocalMsg(tt.msgt)) + }) } }