tests: fix some errors detected by testifylint (#7580)

ref #4813 Fix some errors detected by `testifylint`. Signed-off-by: JmPotato <[email protected]>
tikv · Dec 20, 2023 · 59c9d04 · 59c9d04
1 parent 83da4c2
commit 59c9d04
Show file tree

Hide file tree

Showing 62 changed files with 541 additions and 534 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -11,15 +11,12 @@ linters:
     - makezero
     - gosec
     - bodyclose
+    # TODO: enable when all existing errors are fixed
+    # - testifylint
   disable:
     - errcheck
 linters-settings:
   gocritic:
-    # Which checks should be enabled; can't be combined with 'disabled-checks';
-    # See https://go-critic.github.io/overview#checks-overview
-    # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
-    # By default list of stable checks is used.
-    enabled-checks:
     # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
     disabled-checks:
       - regexpMust
@@ -33,3 +30,15 @@ linters-settings:
       - G402
       - G404
       - G601
+  testifylint:
+    enable:
+      - bool-compare
+      - compares
+      - empty
+      - error-is-as
+      - error-nil
+      - expected-actual
+      - len
+      - require-error
+      - suite-dont-use-pkg
+      - suite-extra-assert-call
diff --git a/Makefile b/Makefile
@@ -172,7 +172,7 @@ install-tools:
 
 #### Static checks ####
 
-check: install-tools tidy static generate-errdoc check-test
+check: install-tools tidy static generate-errdoc
 
 static: install-tools
 	@ echo "gofmt ..."
@@ -199,11 +199,7 @@ check-plugin:
 	@echo "checking plugin..."
 	cd ./plugin/scheduler_example && $(MAKE) evictLeaderPlugin.so && rm evictLeaderPlugin.so
 
-check-test:
-	@echo "checking test..."
-	./scripts/check-test.sh
-
-.PHONY: check static tidy generate-errdoc check-plugin check-test
+.PHONY: check static tidy generate-errdoc check-plugin
 
 #### Test utils ####
 

diff --git a/pkg/autoscaling/calculation_test.go b/pkg/autoscaling/calculation_test.go
@@ -233,7 +233,7 @@ func TestGetTotalCPUUseTime(t *testing.T) {
 	}
 	totalCPUUseTime, _ := getTotalCPUUseTime(querier, TiDB, instances, time.Now(), 0)
 	expected := mockResultValue * float64(len(instances))
-	re.True(math.Abs(expected-totalCPUUseTime) < 1e-6)
+	re.Less(math.Abs(expected-totalCPUUseTime), 1e-6)
 }
 
 func TestGetTotalCPUQuota(t *testing.T) {

diff --git a/pkg/autoscaling/prometheus_test.go b/pkg/autoscaling/prometheus_test.go
@@ -196,7 +196,7 @@ func TestRetrieveCPUMetrics(t *testing.T) {
 			for i := 0; i < len(addresses)-1; i++ {
 				value, ok := result[addresses[i]]
 				re.True(ok)
-				re.True(math.Abs(value-mockResultValue) < 1e-6)
+				re.Less(math.Abs(value-mockResultValue), 1e-6)
 			}
 
 			_, ok := result[addresses[len(addresses)-1]]

diff --git a/pkg/balancer/balancer_test.go b/pkg/balancer/balancer_test.go
@@ -62,17 +62,17 @@ func TestBalancerDuplicate(t *testing.T) {
 		NewRoundRobin[uint32](),
 	}
 	for _, balancer := range balancers {
-		re.Len(balancer.GetAll(), 0)
+		re.Empty(balancer.GetAll())
 		// test duplicate put
 		balancer.Put(1)
 		re.Len(balancer.GetAll(), 1)
 		balancer.Put(1)
 		re.Len(balancer.GetAll(), 1)
 		// test duplicate delete
 		balancer.Delete(1)
-		re.Len(balancer.GetAll(), 0)
+		re.Empty(balancer.GetAll())
 		balancer.Delete(1)
-		re.Len(balancer.GetAll(), 0)
+		re.Empty(balancer.GetAll())
 	}
 }
 

diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go
@@ -77,7 +77,7 @@ func TestExpireRegionCache(t *testing.T) {
 
 	re.Equal(3, cache.Len())
 
-	re.Equal(sortIDs(cache.GetAllID()), []uint64{1, 2, 3})
+	re.Equal([]uint64{1, 2, 3}, sortIDs(cache.GetAllID()))
 
 	// after 20ms, the key 1 will be expired
 	time.Sleep(20 * time.Millisecond)
@@ -98,7 +98,7 @@ func TestExpireRegionCache(t *testing.T) {
 		// we can't ensure whether gc is executed, so we check the length of cache in a loop.
 		return cache.Len() == 2
 	}, testutil.WithWaitFor(50*time.Millisecond), testutil.WithTickInterval(time.Millisecond))
-	re.Equal(sortIDs(cache.GetAllID()), []uint64{2, 3})
+	re.Equal([]uint64{2, 3}, sortIDs(cache.GetAllID()))
 
 	cache.Remove(2)
 
@@ -111,7 +111,7 @@ func TestExpireRegionCache(t *testing.T) {
 	re.Equal(3.0, value)
 
 	re.Equal(1, cache.Len())
-	re.Equal(sortIDs(cache.GetAllID()), []uint64{3})
+	re.Equal([]uint64{3}, sortIDs(cache.GetAllID()))
 }
 
 func sortIDs(ids []uint64) []uint64 {
@@ -131,15 +131,15 @@ func TestLRUCache(t *testing.T) {
 
 	val, ok := cache.Get(3)
 	re.True(ok)
-	re.Equal(val, "3")
+	re.Equal("3", val)
 
 	val, ok = cache.Get(2)
 	re.True(ok)
-	re.Equal(val, "2")
+	re.Equal("2", val)
 
 	val, ok = cache.Get(1)
 	re.True(ok)
-	re.Equal(val, "1")
+	re.Equal("1", val)
 
 	re.Equal(3, cache.Len())
 
@@ -153,27 +153,27 @@ func TestLRUCache(t *testing.T) {
 
 	val, ok = cache.Get(1)
 	re.True(ok)
-	re.Equal(val, "1")
+	re.Equal("1", val)
 
 	val, ok = cache.Get(2)
 	re.True(ok)
-	re.Equal(val, "2")
+	re.Equal("2", val)
 
 	val, ok = cache.Get(4)
 	re.True(ok)
-	re.Equal(val, "4")
+	re.Equal("4", val)
 
 	re.Equal(3, cache.Len())
 
 	val, ok = cache.Peek(1)
 	re.True(ok)
-	re.Equal(val, "1")
+	re.Equal("1", val)
 
 	elems := cache.Elems()
 	re.Len(elems, 3)
-	re.Equal(elems[0].Value, "4")
-	re.Equal(elems[1].Value, "2")
-	re.Equal(elems[2].Value, "1")
+	re.Equal("4", elems[0].Value)
+	re.Equal("2", elems[1].Value)
+	re.Equal("1", elems[2].Value)
 
 	cache.Remove(1)
 	cache.Remove(2)
@@ -247,15 +247,15 @@ func TestFifoFromLastSameElems(t *testing.T) {
 			})
 	}
 	items := fun()
-	re.Equal(1, len(items))
+	re.Len(items, 1)
 	cache.Put(1, &testStruct{value: "3"})
 	cache.Put(2, &testStruct{value: "3"})
 	items = fun()
-	re.Equal(3, len(items))
+	re.Len(items, 3)
 	re.Equal("3", items[0].Value.(*testStruct).value)
 	cache.Put(1, &testStruct{value: "2"})
 	items = fun()
-	re.Equal(1, len(items))
+	re.Len(items, 1)
 	re.Equal("2", items[0].Value.(*testStruct).value)
 }
 

diff --git a/pkg/core/storelimit/limit_test.go b/pkg/core/storelimit/limit_test.go
@@ -30,7 +30,7 @@ func TestStoreLimit(t *testing.T) {
 	re := require.New(t)
 	rate := int64(15)
 	limit := NewStoreRateLimit(float64(rate)).(*StoreRateLimit)
-	re.Equal(limit.Rate(AddPeer), float64(15))
+	re.Equal(float64(15), limit.Rate(AddPeer))
 	re.True(limit.Available(influence*rate, AddPeer, constant.Low))
 	re.True(limit.Take(influence*rate, AddPeer, constant.Low))
 	re.False(limit.Take(influence, AddPeer, constant.Low))

diff --git a/pkg/dashboard/adapter/redirector_test.go b/pkg/dashboard/adapter/redirector_test.go
@@ -65,37 +65,39 @@ func (suite *redirectorTestSuite) TearDownSuite() {
 }
 
 func (suite *redirectorTestSuite) TestReverseProxy() {
+	re := suite.Require()
 	redirectorServer := httptest.NewServer(http.HandlerFunc(suite.redirector.ReverseProxy))
 	defer redirectorServer.Close()
 
 	suite.redirector.SetAddress(suite.tempServer.URL)
 	// Test normal forwarding
 	req, err := http.NewRequest(http.MethodGet, redirectorServer.URL, http.NoBody)
-	suite.NoError(err)
+	re.NoError(err)
 	checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusOK, suite.tempText)
 	// Test the requests that are forwarded by others
 	req, err = http.NewRequest(http.MethodGet, redirectorServer.URL, http.NoBody)
-	suite.NoError(err)
+	re.NoError(err)
 	req.Header.Set(proxyHeader, "other")
 	checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusOK, suite.tempText)
 	// Test LoopDetected
 	suite.redirector.SetAddress(redirectorServer.URL)
 	req, err = http.NewRequest(http.MethodGet, redirectorServer.URL, http.NoBody)
-	suite.NoError(err)
+	re.NoError(err)
 	checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusLoopDetected, "")
 }
 
 func (suite *redirectorTestSuite) TestTemporaryRedirect() {
+	re := suite.Require()
 	redirectorServer := httptest.NewServer(http.HandlerFunc(suite.redirector.TemporaryRedirect))
 	defer redirectorServer.Close()
 	suite.redirector.SetAddress(suite.tempServer.URL)
 	// Test TemporaryRedirect
 	req, err := http.NewRequest(http.MethodGet, redirectorServer.URL, http.NoBody)
-	suite.NoError(err)
+	re.NoError(err)
 	checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusTemporaryRedirect, "")
 	// Test Response
 	req, err = http.NewRequest(http.MethodGet, redirectorServer.URL, http.NoBody)
-	suite.NoError(err)
+	re.NoError(err)
 	checkHTTPRequest(suite.Require(), http.DefaultClient, req, http.StatusOK, suite.tempText)
 }
 

diff --git a/pkg/election/leadership_test.go b/pkg/election/leadership_test.go
@@ -175,7 +175,7 @@ func TestExitWatch(t *testing.T) {
 
 		resp2, err := client.MemberList(context.Background())
 		re.NoError(err)
-		re.Equal(3, len(resp2.Members))
+		re.Len(resp2.Members, 3)
 
 		etcd2.Server.HardStop()
 		etcd3.Server.HardStop()

diff --git a/pkg/encryption/config_test.go b/pkg/encryption/config_test.go
@@ -38,19 +38,19 @@ func TestAdjustInvalidDataEncryptionMethod(t *testing.T) {
 	t.Parallel()
 	re := require.New(t)
 	config := &Config{DataEncryptionMethod: "unknown"}
-	re.NotNil(config.Adjust())
+	re.Error(config.Adjust())
 }
 
 func TestAdjustNegativeRotationDuration(t *testing.T) {
 	t.Parallel()
 	re := require.New(t)
 	config := &Config{DataKeyRotationPeriod: typeutil.NewDuration(time.Duration(int64(-1)))}
-	re.NotNil(config.Adjust())
+	re.Error(config.Adjust())
 }
 
 func TestAdjustInvalidMasterKeyType(t *testing.T) {
 	t.Parallel()
 	re := require.New(t)
 	config := &Config{MasterKey: MasterKeyConfig{Type: "unknown"}}
-	re.NotNil(config.Adjust())
+	re.Error(config.Adjust())
 }
diff --git a/pkg/encryption/crypter_test.go b/pkg/encryption/crypter_test.go
@@ -26,11 +26,11 @@ import (
 func TestEncryptionMethodSupported(t *testing.T) {
 	t.Parallel()
 	re := require.New(t)
-	re.NotNil(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_PLAINTEXT))
-	re.NotNil(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_UNKNOWN))
-	re.Nil(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_AES128_CTR))
-	re.Nil(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_AES192_CTR))
-	re.Nil(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_AES256_CTR))
+	re.Error(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_PLAINTEXT))
+	re.Error(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_UNKNOWN))
+	re.NoError(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_AES128_CTR))
+	re.NoError(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_AES192_CTR))
+	re.NoError(CheckEncryptionMethodSupported(encryptionpb.EncryptionMethod_AES256_CTR))
 }
 
 func TestKeyLength(t *testing.T) {

diff --git a/pkg/encryption/key_manager_test.go b/pkg/encryption/key_manager_test.go
@@ -313,7 +313,7 @@ func TestLoadKeyEmpty(t *testing.T) {
 	// Simulate keys get deleted.
 	_, err = client.Delete(context.Background(), EncryptionKeysPath)
 	re.NoError(err)
-	re.NotNil(m.loadKeys())
+	re.Error(m.loadKeys())
 }
 
 func TestWatcher(t *testing.T) {