Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: fix some errors detected by testifylint #7580

Merged
merged 2 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
8 changes: 2 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 ..."
Expand All @@ -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 ####

Expand Down
2 changes: 1 addition & 1 deletion pkg/autoscaling/calculation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/autoscaling/prometheus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
6 changes: 3 additions & 3 deletions pkg/balancer/balancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}

Expand Down
32 changes: 16 additions & 16 deletions pkg/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand All @@ -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 {
Expand All @@ -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())

Expand All @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/core/storelimit/limit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
12 changes: 7 additions & 5 deletions pkg/dashboard/adapter/redirector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/election/leadership_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions pkg/encryption/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
10 changes: 5 additions & 5 deletions pkg/encryption/crypter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/encryption/key_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading
Loading