From 79d006244ff38f7582bf8051ee93ece23f8b84b9 Mon Sep 17 00:00:00 2001 From: Yujie Xia Date: Tue, 23 Nov 2021 11:33:05 +0800 Subject: [PATCH 1/4] br/pkg/utils: migrate tests to testify --- br/pkg/utils/backoff_test.go | 67 ++++++------ br/pkg/utils/env_test.go | 19 ++-- br/pkg/utils/json_test.go | 50 +++------ br/pkg/utils/key_test.go | 29 +++--- br/pkg/utils/main_test.go | 30 ++++++ br/pkg/utils/math_test.go | 70 +++++++------ br/pkg/utils/progress.go | 40 +++++--- br/pkg/utils/progress_test.go | 27 ++--- br/pkg/utils/retry_test.go | 71 +++++++------ br/pkg/utils/safe_point_test.go | 36 +++---- br/pkg/utils/schema_test.go | 176 ++++++++++++++++++-------------- br/pkg/utils/utils_test.go | 13 --- 12 files changed, 324 insertions(+), 304 deletions(-) create mode 100644 br/pkg/utils/main_test.go delete mode 100644 br/pkg/utils/utils_test.go diff --git a/br/pkg/utils/backoff_test.go b/br/pkg/utils/backoff_test.go index 31b1a0214d255..e8af459c78de6 100644 --- a/br/pkg/utils/backoff_test.go +++ b/br/pkg/utils/backoff_test.go @@ -4,35 +4,20 @@ package utils_test import ( "context" + "testing" "time" - . "github.com/pingcap/check" berrors "github.com/pingcap/tidb/br/pkg/errors" - "github.com/pingcap/tidb/br/pkg/mock" "github.com/pingcap/tidb/br/pkg/utils" - "github.com/pingcap/tidb/util/testleak" + "github.com/stretchr/testify/require" "go.uber.org/multierr" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) -var _ = Suite(&testBackofferSuite{}) +func TestBackoffWithSuccess(t *testing.T) { + t.Parallel() -type testBackofferSuite struct { - mock *mock.Cluster -} - -func (s *testBackofferSuite) SetUpSuite(c *C) { - var err error - s.mock, err = mock.NewCluster() - c.Assert(err, IsNil) -} - -func (s *testBackofferSuite) TearDownSuite(c *C) { - testleak.AfterTest(c)() -} - -func (s *testBackofferSuite) TestBackoffWithSuccess(c *C) { var counter int backoffer := utils.NewBackoffer(10, time.Nanosecond, time.Nanosecond) err := utils.WithRetry(context.Background(), func() error { @@ -47,11 +32,13 @@ func (s *testBackofferSuite) TestBackoffWithSuccess(c *C) { } return nil }, backoffer) - c.Assert(counter, Equals, 3) - c.Assert(err, IsNil) + require.Equal(t, 3, counter) + require.NoError(t, err) } -func (s *testBackofferSuite) TestBackoffWithFatalError(c *C) { +func TestBackoffWithFatalError(t *testing.T) { + t.Parallel() + var counter int backoffer := utils.NewBackoffer(10, time.Nanosecond, time.Nanosecond) gRPCError := status.Error(codes.Unavailable, "transport is closing") @@ -69,16 +56,18 @@ func (s *testBackofferSuite) TestBackoffWithFatalError(c *C) { } return nil }, backoffer) - c.Assert(counter, Equals, 4) - c.Assert(multierr.Errors(err), DeepEquals, []error{ + require.Equal(t, 4, counter) + require.Equal(t, []error{ gRPCError, berrors.ErrKVEpochNotMatch, berrors.ErrKVDownloadFailed, berrors.ErrKVRangeIsEmpty, - }) + }, multierr.Errors(err)) } -func (s *testBackofferSuite) TestBackoffWithFatalRawGRPCError(c *C) { +func TestBackoffWithFatalRawGRPCError(t *testing.T) { + t.Parallel() + var counter int canceledError := status.Error(codes.Canceled, "context canceled") backoffer := utils.NewBackoffer(10, time.Nanosecond, time.Nanosecond) @@ -86,21 +75,21 @@ func (s *testBackofferSuite) TestBackoffWithFatalRawGRPCError(c *C) { defer func() { counter++ }() return canceledError // nolint:wrapcheck }, backoffer) - c.Assert(counter, Equals, 1) - c.Assert(multierr.Errors(err), DeepEquals, []error{ - canceledError, - }) + require.Equal(t, 1, counter) + require.Equal(t, []error{canceledError}, multierr.Errors(err)) } -func (s *testBackofferSuite) TestBackoffWithRetryableError(c *C) { +func TestBackoffWithRetryableError(t *testing.T) { + t.Parallel() + var counter int backoffer := utils.NewBackoffer(10, time.Nanosecond, time.Nanosecond) err := utils.WithRetry(context.Background(), func() error { defer func() { counter++ }() return berrors.ErrKVEpochNotMatch }, backoffer) - c.Assert(counter, Equals, 10) - c.Assert(multierr.Errors(err), DeepEquals, []error{ + require.Equal(t, 10, counter) + require.Equal(t, []error{ berrors.ErrKVEpochNotMatch, berrors.ErrKVEpochNotMatch, berrors.ErrKVEpochNotMatch, @@ -111,10 +100,12 @@ func (s *testBackofferSuite) TestBackoffWithRetryableError(c *C) { berrors.ErrKVEpochNotMatch, berrors.ErrKVEpochNotMatch, berrors.ErrKVEpochNotMatch, - }) + }, multierr.Errors(err)) } -func (s *testBackofferSuite) TestPdBackoffWithRetryableError(c *C) { +func TestPdBackoffWithRetryableError(t *testing.T) { + t.Parallel() + var counter int backoffer := utils.NewPDReqBackoffer() gRPCError := status.Error(codes.Unavailable, "transport is closing") @@ -122,8 +113,8 @@ func (s *testBackofferSuite) TestPdBackoffWithRetryableError(c *C) { defer func() { counter++ }() return gRPCError }, backoffer) - c.Assert(counter, Equals, 16) - c.Assert(multierr.Errors(err), DeepEquals, []error{ + require.Equal(t, 16, counter) + require.Equal(t, []error{ gRPCError, gRPCError, gRPCError, @@ -140,5 +131,5 @@ func (s *testBackofferSuite) TestPdBackoffWithRetryableError(c *C) { gRPCError, gRPCError, gRPCError, - }) + }, multierr.Errors(err)) } diff --git a/br/pkg/utils/env_test.go b/br/pkg/utils/env_test.go index 89ad840ee7fc7..493f30aaa4240 100644 --- a/br/pkg/utils/env_test.go +++ b/br/pkg/utils/env_test.go @@ -4,15 +4,14 @@ package utils import ( "os" + "testing" - "github.com/pingcap/check" + "github.com/stretchr/testify/require" ) -type envSuit struct{} +func TestProxyFields(t *testing.T) { + t.Parallel() -var _ = check.Suite(&envSuit{}) - -func (s *envSuit) TestProxyFields(c *check.C) { revIndex := map[string]int{ "http_proxy": 0, "https_proxy": 1, @@ -25,20 +24,20 @@ func (s *envSuit) TestProxyFields(c *check.C) { // Each bit of the mask decided whether this index of `envs` would be set. for mask := 0; mask <= 0b111; mask++ { for _, env := range envs { - c.Assert(os.Unsetenv(env), check.IsNil) + require.NoError(t, os.Unsetenv(env)) } for i := 0; i < 3; i++ { if (1< Date: Tue, 23 Nov 2021 15:31:22 +0800 Subject: [PATCH 2/4] use more suitable method --- br/pkg/utils/key_test.go | 1 + br/pkg/utils/progress_test.go | 14 +++++++------- br/pkg/utils/safe_point_test.go | 9 +++++---- br/pkg/utils/schema_test.go | 16 ++++++++-------- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/br/pkg/utils/key_test.go b/br/pkg/utils/key_test.go index b9b05bf7405b4..6bb1c00f55bab 100644 --- a/br/pkg/utils/key_test.go +++ b/br/pkg/utils/key_test.go @@ -88,6 +88,7 @@ func TestParseKey(t *testing.T) { for _, tt := range testNotSupportKey { _, err := ParseKey("notSupport", tt.any) + require.Error(t, err) require.Regexp(t, "unknown format.*", err.Error()) } } diff --git a/br/pkg/utils/progress_test.go b/br/pkg/utils/progress_test.go index a101101166168..a8503c966db39 100644 --- a/br/pkg/utils/progress_test.go +++ b/br/pkg/utils/progress_test.go @@ -33,15 +33,15 @@ func TestProgress(t *testing.T) { progress2.Inc() time.Sleep(2 * time.Second) p = <-pCh2 - require.Regexp(t, `.*"P":"50\.00%".*`, p) + require.Contains(t, p, `"P":"50.00%"`) progress2.Inc() time.Sleep(2 * time.Second) p = <-pCh2 - require.Regexp(t, `.*"P":"100\.00%".*`, p) + require.Contains(t, p, `"P":"100.00%"`) progress2.Inc() time.Sleep(2 * time.Second) p = <-pCh2 - require.Regexp(t, `.*"P":"100\.00%".*`, p) + require.Contains(t, p, `"P":"100.00%"`) progress2.Close() pCh4 := make(chan string, 4) @@ -52,12 +52,12 @@ func TestProgress(t *testing.T) { progress4.Inc() time.Sleep(2 * time.Second) p = <-pCh4 - require.Regexp(t, `.*"P":"25\.00%".*`, p) + require.Contains(t, p, `"P":"25.00%"`) progress4.Inc() progress4.Close() time.Sleep(2 * time.Second) p = <-pCh4 - require.Regexp(t, `.*"P":"100\.00%".*`, p) + require.Contains(t, p, `"P":"100.00%"`) pCh8 := make(chan string, 8) progress8 := NewProgressPrinter("test", 8, false) @@ -68,11 +68,11 @@ func TestProgress(t *testing.T) { progress8.Inc() time.Sleep(2 * time.Second) p = <-pCh8 - require.Regexp(t, `.*"P":"25\.00%".*`, p) + require.Contains(t, p, `"P":"25.00%"`) // Cancel should stop progress at the current position. cancel() p = <-pCh8 - require.Regexp(t, `.*"P":"25\.00%".*`, p) + require.Contains(t, p, `"P":"25.00%"`) progress8.Close() } diff --git a/br/pkg/utils/safe_point_test.go b/br/pkg/utils/safe_point_test.go index cb964e5b9352f..f80f7fff4c9dc 100644 --- a/br/pkg/utils/safe_point_test.go +++ b/br/pkg/utils/safe_point_test.go @@ -23,15 +23,16 @@ func TestCheckGCSafepoint(t *testing.T) { } { err := utils.CheckGCSafePoint(ctx, pdClient, 2333) - require.NotNil(t, err) + require.Error(t, err) } { err := utils.CheckGCSafePoint(ctx, pdClient, 2333-1) - require.NotNil(t, err) + require.Error(t, err) } { err := utils.CheckGCSafePoint(ctx, pdClient, 0) - require.Regexp(t, ".*GC safepoint 2333 exceed TS 0.*", err) + require.Error(t, err) + require.Regexp(t, ".*GC safepoint 2333 exceed TS 0.*", err.Error()) } } @@ -125,7 +126,7 @@ func TestStartServiceSafePointKeeper(t *testing.T) { if cs.ok { require.NoErrorf(t, err, "case #%d, %v", i, cs) } else { - require.NotNilf(t, err, "case #%d, %v", i, cs) + require.Errorf(t, err, "case #%d, %v", i, cs) } cancel() } diff --git a/br/pkg/utils/schema_test.go b/br/pkg/utils/schema_test.go index 97ace3c6c130b..d523969c73ab9 100644 --- a/br/pkg/utils/schema_test.go +++ b/br/pkg/utils/schema_test.go @@ -103,7 +103,7 @@ func TestLoadBackupMeta(t *testing.T) { ) tbl := dbs[dbName.String()].GetTable(tblName.String()) require.NoError(t, err) - require.Equal(t, 1, len(tbl.Files)) + require.Len(t, tbl.Files, 1) require.Equal(t, "1.sst", tbl.Files[0].Name) } @@ -210,7 +210,7 @@ func TestLoadBackupMetaPartionTable(t *testing.T) { ) tbl := dbs[dbName.String()].GetTable(tblName.String()) require.NoError(t, err) - require.Equal(t, 3, len(tbl.Files)) + require.Len(t, tbl.Files, 3) contains := func(name string) bool { for i := range tbl.Files { if tbl.Files[i].Name == name { @@ -294,9 +294,9 @@ func BenchmarkLoadBackupMeta64(b *testing.B) { ), ) require.NoError(b, err) - require.Equal(b, 1, len(dbs)) + require.Len(b, dbs, 1) require.Contains(b, dbs, "bench") - require.Equal(b, 64, len(dbs["bench"].Tables)) + require.Len(b, dbs["bench"].Tables, 64) } } @@ -325,9 +325,9 @@ func BenchmarkLoadBackupMeta1024(b *testing.B) { ), ) require.NoError(b, err) - require.Equal(b, 1, len(dbs)) + require.Len(b, dbs, 1) require.Contains(b, dbs, "bench") - require.Equal(b, 1024, len(dbs["bench"].Tables)) + require.Len(b, dbs["bench"].Tables, 1024) } } @@ -356,8 +356,8 @@ func BenchmarkLoadBackupMeta10240(b *testing.B) { ), ) require.NoError(b, err) - require.Equal(b, 1, len(dbs)) + require.Len(b, dbs, 1) require.Contains(b, dbs, "bench") - require.Equal(b, 10240, len(dbs["bench"].Tables)) + require.Len(b, dbs["bench"].Tables, 10240) } } From c6cff732f718989c7c7c97dd494282c9b1150c7c Mon Sep 17 00:00:00 2001 From: Yujie Xia Date: Tue, 23 Nov 2021 16:28:32 +0800 Subject: [PATCH 3/4] address comments --- br/pkg/utils/env_test.go | 2 +- br/pkg/utils/json_test.go | 8 +------- br/pkg/utils/schema_test.go | 34 +++++++++++++--------------------- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/br/pkg/utils/env_test.go b/br/pkg/utils/env_test.go index 493f30aaa4240..6934ef3c6e870 100644 --- a/br/pkg/utils/env_test.go +++ b/br/pkg/utils/env_test.go @@ -36,7 +36,7 @@ func TestProxyFields(t *testing.T) { for _, field := range proxyFields() { idx, ok := revIndex[field.Key] require.True(t, ok) - require.NotEqual(t, 0, (1< Date: Tue, 23 Nov 2021 16:41:09 +0800 Subject: [PATCH 4/4] fix tempDir --- br/pkg/utils/schema_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/br/pkg/utils/schema_test.go b/br/pkg/utils/schema_test.go index a8cc861fcd11e..243b72932322c 100644 --- a/br/pkg/utils/schema_test.go +++ b/br/pkg/utils/schema_test.go @@ -6,7 +6,6 @@ import ( "context" "encoding/json" "fmt" - "os" "testing" "github.com/golang/protobuf/proto" @@ -108,7 +107,7 @@ func TestLoadBackupMeta(t *testing.T) { func TestLoadBackupMetaPartionTable(t *testing.T) { t.Parallel() - testDir := os.TempDir() + testDir := t.TempDir() store, err := storage.NewLocalStorage(testDir) require.NoError(t, err) @@ -259,7 +258,7 @@ func buildBenchmarkBackupmeta(b *testing.B, dbName string, tableCount, fileCount } func BenchmarkLoadBackupMeta64(b *testing.B) { - testDir := os.TempDir() + testDir := b.TempDir() store, err := storage.NewLocalStorage(testDir) require.NoError(b, err) @@ -291,7 +290,7 @@ func BenchmarkLoadBackupMeta64(b *testing.B) { } func BenchmarkLoadBackupMeta1024(b *testing.B) { - testDir := os.TempDir() + testDir := b.TempDir() store, err := storage.NewLocalStorage(testDir) require.NoError(b, err) @@ -323,7 +322,7 @@ func BenchmarkLoadBackupMeta1024(b *testing.B) { } func BenchmarkLoadBackupMeta10240(b *testing.B) { - testDir := os.TempDir() + testDir := b.TempDir() store, err := storage.NewLocalStorage(testDir) require.NoError(b, err)