Merge pull request #18686 from mmorel-35/testifylint/bool-compare

fix: enable bool-compare rule from testifylint
etcd-io · Oct 6, 2024 · 17fb752 · 17fb752
2 parents 448fb7e + b23947b
commit 17fb752
Show file tree

Hide file tree

Showing 11 changed files with 64 additions and 56 deletions.
diff --git a/client/pkg/testutil/assert.go b/client/pkg/testutil/assert.go
@@ -34,10 +34,10 @@ func AssertNotNil(t *testing.T, v any) {
 
 func AssertTrue(t *testing.T, v bool, msg ...string) {
 	t.Helper()
-	assert.Equal(t, true, v, msg)
+	assert.True(t, v, msg)
 }
 
 func AssertFalse(t *testing.T, v bool, msg ...string) {
 	t.Helper()
-	assert.Equal(t, false, v, msg)
+	assert.False(t, v, msg)
 }
diff --git a/client/v3/client_test.go b/client/v3/client_test.go
@@ -199,61 +199,51 @@ func TestBackoffJitterFraction(t *testing.T) {
 }
 
 func TestIsHaltErr(t *testing.T) {
-	assert.Equal(t,
+	assert.True(t,
 		isHaltErr(context.TODO(), errors.New("etcdserver: some etcdserver error")),
-		true,
 		"error created by errors.New should be unavailable error",
 	)
-	assert.Equal(t,
+	assert.False(t,
 		isHaltErr(context.TODO(), rpctypes.ErrGRPCStopped),
-		false,
 		fmt.Sprintf(`error "%v" should not be halt error`, rpctypes.ErrGRPCStopped),
 	)
-	assert.Equal(t,
+	assert.False(t,
 		isHaltErr(context.TODO(), rpctypes.ErrGRPCNoLeader),
-		false,
 		fmt.Sprintf(`error "%v" should not be halt error`, rpctypes.ErrGRPCNoLeader),
 	)
 	ctx, cancel := context.WithCancel(context.TODO())
-	assert.Equal(t,
+	assert.False(t,
 		isHaltErr(ctx, nil),
-		false,
 		"no error and active context should be halt error",
 	)
 	cancel()
-	assert.Equal(t,
+	assert.True(t,
 		isHaltErr(ctx, nil),
-		true,
 		"cancel on context should be halte error",
 	)
 }
 
 func TestIsUnavailableErr(t *testing.T) {
-	assert.Equal(t,
+	assert.False(t,
 		isUnavailableErr(context.TODO(), errors.New("etcdserver: some etcdserver error")),
-		false,
 		"error created by errors.New should not be unavailable error",
 	)
-	assert.Equal(t,
+	assert.True(t,
 		isUnavailableErr(context.TODO(), rpctypes.ErrGRPCStopped),
-		true,
 		fmt.Sprintf(`error "%v" should be unavailable error`, rpctypes.ErrGRPCStopped),
 	)
-	assert.Equal(t,
+	assert.False(t,
 		isUnavailableErr(context.TODO(), rpctypes.ErrGRPCNotCapable),
-		false,
 		fmt.Sprintf("error %v should not be unavailable error", rpctypes.ErrGRPCNotCapable),
 	)
 	ctx, cancel := context.WithCancel(context.TODO())
-	assert.Equal(t,
+	assert.False(t,
 		isUnavailableErr(ctx, nil),
-		false,
 		"no error and active context should not be unavailable error",
 	)
 	cancel()
-	assert.Equal(t,
+	assert.False(t,
 		isUnavailableErr(ctx, nil),
-		false,
 		"cancel on context should not be unavailable error",
 	)
 }

diff --git a/server/embed/config_test.go b/server/embed/config_test.go
@@ -85,11 +85,11 @@ func TestConfigFileOtherFields(t *testing.T) {
 		t.Errorf("PeerTLS = %v, want %v", cfg.PeerTLSInfo, ptls)
 	}
 
-	assert.Equal(t, true, cfg.ForceNewCluster, "ForceNewCluster does not match")
+	assert.True(t, cfg.ForceNewCluster, "ForceNewCluster does not match")
 
-	assert.Equal(t, true, cfg.SocketOpts.ReusePort, "ReusePort does not match")
+	assert.True(t, cfg.SocketOpts.ReusePort, "ReusePort does not match")
 
-	assert.Equal(t, false, cfg.SocketOpts.ReuseAddress, "ReuseAddress does not match")
+	assert.False(t, cfg.SocketOpts.ReuseAddress, "ReuseAddress does not match")
 }
 
 func TestConfigFileFeatureGates(t *testing.T) {

diff --git a/server/etcdserver/api/membership/membership_test.go b/server/etcdserver/api/membership/membership_test.go
@@ -46,9 +46,9 @@ func TestAddRemoveMember(t *testing.T) {
 	c2.Recover(func(*zap.Logger, *semver.Version) {})
 	assert.Equal(t, []*Member{{ID: types.ID(19),
 		Attributes: Attributes{Name: "node19"}}}, c2.Members())
-	assert.Equal(t, true, c2.IsIDRemoved(17))
-	assert.Equal(t, true, c2.IsIDRemoved(18))
-	assert.Equal(t, false, c2.IsIDRemoved(19))
+	assert.True(t, c2.IsIDRemoved(17))
+	assert.True(t, c2.IsIDRemoved(18))
+	assert.False(t, c2.IsIDRemoved(19))
 }
 
 type backendMock struct {

diff --git a/server/etcdserver/api/v2store/store_ttl_test.go b/server/etcdserver/api/v2store/store_ttl_test.go
@@ -31,7 +31,7 @@ func TestMinExpireTime(t *testing.T) {
 	fc := clockwork.NewFakeClockAt(time.Date(1984, time.April, 4, 0, 0, 0, 0, time.UTC))
 	s.clock = fc
 	// FakeClock starts at 0, so minExpireTime should be far in the future.. but just in case
-	testutil.AssertTrue(t, minExpireTime.After(fc.Now()), "minExpireTime should be ahead of FakeClock!")
+	assert.True(t, minExpireTime.After(fc.Now()), "minExpireTime should be ahead of FakeClock!")
 	s.Create("/foo", false, "Y", false, TTLOptionSet{ExpireTime: fc.Now().Add(3 * time.Second)})
 	fc.Advance(5 * time.Second)
 	// Ensure it hasn't expired
@@ -70,9 +70,9 @@ func TestStoreGetDirectory(t *testing.T) {
 		switch node.Key {
 		case "/foo/bar":
 			assert.Equal(t, *node.Value, "X")
-			assert.Equal(t, node.Dir, false)
+			assert.False(t, node.Dir)
 		case "/foo/baz":
-			assert.Equal(t, node.Dir, true)
+			assert.True(t, node.Dir)
 			assert.Equal(t, len(node.Nodes), 2)
 			bazNodes = node.Nodes
 		default:
@@ -83,10 +83,10 @@ func TestStoreGetDirectory(t *testing.T) {
 		switch node.Key {
 		case "/foo/baz/bat":
 			assert.Equal(t, *node.Value, "Y")
-			assert.Equal(t, node.Dir, false)
+			assert.False(t, node.Dir)
 		case "/foo/baz/ttl":
 			assert.Equal(t, *node.Value, "Y")
-			assert.Equal(t, node.Dir, false)
+			assert.False(t, node.Dir)
 			assert.Equal(t, node.TTL, int64(3))
 		default:
 			t.Errorf("key = %s, not matched", node.Key)
@@ -127,7 +127,7 @@ func TestStoreUpdateDirTTL(t *testing.T) {
 	testutil.AssertNil(t, err)
 	e, err := s.Update("/foo/bar", "", TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond)})
 	testutil.AssertNil(t, err)
-	assert.Equal(t, e.Node.Dir, false)
+	assert.False(t, e.Node.Dir)
 	assert.Equal(t, e.EtcdIndex, eidx)
 	e, _ = s.Get("/foo/bar", false, false)
 	assert.Equal(t, *e.Node.Value, "")
@@ -175,7 +175,7 @@ func TestStoreWatchExpire(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "expire")
 	assert.Equal(t, e.Node.Key, "/foodir")
-	assert.Equal(t, e.Node.Dir, true)
+	assert.True(t, e.Node.Dir)
 }
 
 // TestStoreWatchExpireRefresh ensures that the store can watch for key expiration when refreshing.

diff --git a/server/storage/mvcc/store_test.go b/server/storage/mvcc/store_test.go
@@ -61,7 +61,7 @@ func TestScheduledCompact(t *testing.T) {
 			b := backend.NewDefaultBackend(lg, tmpPath)
 			defer b.Close()
 			v, found := UnsafeReadScheduledCompact(b.BatchTx())
-			assert.Equal(t, true, found)
+			assert.True(t, found)
 			assert.Equal(t, tc.value, v)
 		})
 	}
@@ -100,7 +100,7 @@ func TestFinishedCompact(t *testing.T) {
 			b := backend.NewDefaultBackend(lg, tmpPath)
 			defer b.Close()
 			v, found := UnsafeReadFinishedCompact(b.BatchTx())
-			assert.Equal(t, true, found)
+			assert.True(t, found)
 			assert.Equal(t, tc.value, v)
 		})
 	}

diff --git a/tests/e2e/corrupt_test.go b/tests/e2e/corrupt_test.go
@@ -220,7 +220,7 @@ func TestPeriodicCheckDetectsCorruption(t *testing.T) {
 
 	memberID, found, err := getMemberIDByName(ctx, cc, epc.Procs[0].Config().Name)
 	assert.NoError(t, err, "error on member list")
-	assert.Equal(t, found, true, "member not found")
+	assert.True(t, found, "member not found")
 
 	epc.Procs[0].Stop()
 	err = testutil.CorruptBBolt(datadir.ToBackendFileName(epc.Procs[0].Config().DataDirPath))
@@ -260,7 +260,7 @@ func TestCompactHashCheckDetectCorruption(t *testing.T) {
 	}
 	memberID, found, err := getMemberIDByName(ctx, cc, epc.Procs[0].Config().Name)
 	assert.NoError(t, err, "error on member list")
-	assert.Equal(t, found, true, "member not found")
+	assert.True(t, found, "member not found")
 
 	epc.Procs[0].Stop()
 	err = testutil.CorruptBBolt(datadir.ToBackendFileName(epc.Procs[0].Config().DataDirPath))

diff --git a/tests/e2e/ctl_v3_member_no_proxy_test.go b/tests/e2e/ctl_v3_member_no_proxy_test.go
@@ -52,7 +52,7 @@ func TestMemberReplace(t *testing.T) {
 
 	memberID, found, err := getMemberIDByName(ctx, cc, memberName)
 	require.NoError(t, err)
-	require.Equal(t, found, true, "Member not found")
+	require.True(t, found, "Member not found")
 
 	// Need to wait health interval for cluster to accept member changes
 	time.Sleep(etcdserver.HealthInterval)
@@ -62,7 +62,7 @@ func TestMemberReplace(t *testing.T) {
 	require.NoError(t, err)
 	_, found, err = getMemberIDByName(ctx, cc, memberName)
 	require.NoError(t, err)
-	require.Equal(t, found, false, "Expected member to be removed")
+	require.False(t, found, "Expected member to be removed")
 	for member.IsRunning() {
 		err = member.Wait(ctx)
 		if err != nil && !strings.Contains(err.Error(), "unexpected exit code") {
@@ -119,7 +119,7 @@ func TestMemberReplaceWithLearner(t *testing.T) {
 
 	memberID, found, err := getMemberIDByName(ctx, cc, memberName)
 	require.NoError(t, err)
-	require.Equal(t, true, found, "Member not found")
+	require.True(t, found, "Member not found")
 
 	// Need to wait health interval for cluster to accept member changes
 	time.Sleep(etcdserver.HealthInterval)
@@ -129,7 +129,7 @@ func TestMemberReplaceWithLearner(t *testing.T) {
 	require.NoError(t, err)
 	_, found, err = getMemberIDByName(ctx, cc, memberName)
 	require.NoError(t, err)
-	require.Equal(t, false, found, "Expected member to be removed")
+	require.False(t, found, "Expected member to be removed")
 	for member.IsRunning() {
 		err = member.Wait(ctx)
 		if err != nil && !strings.Contains(err.Error(), "unexpected exit code") {
@@ -169,7 +169,7 @@ func TestMemberReplaceWithLearner(t *testing.T) {
 
 	learnMemberID, found, err = getMemberIDByName(ctx, cc, memberName)
 	require.NoError(t, err)
-	require.Equal(t, true, found, "Member not found")
+	require.True(t, found, "Member not found")
 
 	_, err = cc.MemberPromote(ctx, learnMemberID)
 	require.NoError(t, err)

diff --git a/tests/integration/v2store/store_test.go b/tests/integration/v2store/store_test.go
@@ -100,7 +100,7 @@ func TestSet(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "set")
 	assert.Equal(t, e.Node.Key, "/foo")
-	testutil.AssertFalse(t, e.Node.Dir)
+	assert.False(t, e.Node.Dir)
 	assert.Equal(t, *e.Node.Value, "")
 	testutil.AssertNil(t, e.Node.Nodes)
 	testutil.AssertNil(t, e.Node.Expiration)
@@ -114,7 +114,7 @@ func TestSet(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "set")
 	assert.Equal(t, e.Node.Key, "/foo")
-	testutil.AssertFalse(t, e.Node.Dir)
+	assert.False(t, e.Node.Dir)
 	assert.Equal(t, *e.Node.Value, "bar")
 	testutil.AssertNil(t, e.Node.Nodes)
 	testutil.AssertNil(t, e.Node.Expiration)
@@ -132,7 +132,7 @@ func TestSet(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "set")
 	assert.Equal(t, e.Node.Key, "/foo")
-	testutil.AssertFalse(t, e.Node.Dir)
+	assert.False(t, e.Node.Dir)
 	assert.Equal(t, *e.Node.Value, "baz")
 	testutil.AssertNil(t, e.Node.Nodes)
 	testutil.AssertNil(t, e.Node.Expiration)
@@ -150,7 +150,7 @@ func TestSet(t *testing.T) {
 	testutil.AssertNil(t, err)
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Node.Key, "/a/b/c/d")
-	testutil.AssertFalse(t, e.Node.Dir)
+	assert.False(t, e.Node.Dir)
 	assert.Equal(t, *e.Node.Value, "efg")
 	testutil.AssertNil(t, e.Node.Nodes)
 	testutil.AssertNil(t, e.Node.Expiration)
@@ -164,7 +164,7 @@ func TestSet(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "set")
 	assert.Equal(t, e.Node.Key, "/dir")
-	testutil.AssertTrue(t, e.Node.Dir)
+	assert.True(t, e.Node.Dir)
 	testutil.AssertNil(t, e.Node.Value)
 	testutil.AssertNil(t, e.Node.Nodes)
 	testutil.AssertNil(t, e.Node.Expiration)
@@ -183,7 +183,7 @@ func TestStoreCreateValue(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "create")
 	assert.Equal(t, e.Node.Key, "/foo")
-	testutil.AssertFalse(t, e.Node.Dir)
+	assert.False(t, e.Node.Dir)
 	assert.Equal(t, *e.Node.Value, "bar")
 	testutil.AssertNil(t, e.Node.Nodes)
 	testutil.AssertNil(t, e.Node.Expiration)
@@ -197,7 +197,7 @@ func TestStoreCreateValue(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "create")
 	assert.Equal(t, e.Node.Key, "/empty")
-	testutil.AssertFalse(t, e.Node.Dir)
+	assert.False(t, e.Node.Dir)
 	assert.Equal(t, *e.Node.Value, "")
 	testutil.AssertNil(t, e.Node.Nodes)
 	testutil.AssertNil(t, e.Node.Expiration)
@@ -216,7 +216,7 @@ func TestStoreCreateDirectory(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "create")
 	assert.Equal(t, e.Node.Key, "/foo")
-	testutil.AssertTrue(t, e.Node.Dir)
+	assert.True(t, e.Node.Dir)
 }
 
 // TestStoreCreateFailsIfExists ensure that the store fails to create a key if it already exists.
@@ -249,7 +249,7 @@ func TestStoreUpdateValue(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "update")
 	assert.Equal(t, e.Node.Key, "/foo")
-	testutil.AssertFalse(t, e.Node.Dir)
+	assert.False(t, e.Node.Dir)
 	assert.Equal(t, *e.Node.Value, "baz")
 	assert.Equal(t, e.Node.TTL, int64(0))
 	assert.Equal(t, e.Node.ModifiedIndex, uint64(2))
@@ -270,7 +270,7 @@ func TestStoreUpdateValue(t *testing.T) {
 	assert.Equal(t, e.EtcdIndex, eidx)
 	assert.Equal(t, e.Action, "update")
 	assert.Equal(t, e.Node.Key, "/foo")
-	testutil.AssertFalse(t, e.Node.Dir)
+	assert.False(t, e.Node.Dir)
 	assert.Equal(t, *e.Node.Value, "")
 	assert.Equal(t, e.Node.TTL, int64(0))
 	assert.Equal(t, e.Node.ModifiedIndex, uint64(3))
@@ -330,7 +330,7 @@ func TestStoreDeleteDirectory(t *testing.T) {
 	// check prevNode
 	testutil.AssertNotNil(t, e.PrevNode)
 	assert.Equal(t, e.PrevNode.Key, "/foo")
-	assert.Equal(t, e.PrevNode.Dir, true)
+	assert.True(t, e.PrevNode.Dir)
 
 	// create directory /foo and directory /foo/bar
 	_, err = s.Create("/foo/bar", true, "", false, v2store.TTLOptionSet{ExpireTime: v2store.Permanent})

diff --git a/tests/integration/v3_auth_test.go b/tests/integration/v3_auth_test.go
@@ -21,6 +21,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	"go.etcd.io/etcd/api/v3/authpb"
 	pb "go.etcd.io/etcd/api/v3/etcdserverpb"
 	"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
@@ -454,7 +456,7 @@ func TestV3AuthWatchErrorAndWatchId0(t *testing.T) {
 		watchStartCh <- struct{}{}
 		watchResponse := <-wChan
 		t.Logf("watch response from k1: %v", watchResponse)
-		testutil.AssertTrue(t, len(watchResponse.Events) != 0)
+		assert.True(t, len(watchResponse.Events) != 0)
 		watchEndCh <- struct{}{}
 	}()
 

diff --git a/tools/.golangci.yaml b/tools/.golangci.yaml
@@ -24,6 +24,7 @@ linters:
     - revive
     - staticcheck
     - stylecheck
+    - testifylint
     - unconvert # Remove unnecessary type conversions
     - unparam
     - unused
@@ -106,3 +107,18 @@ linters-settings: # please keep this alphabetized
   stylecheck:
     checks:
       - ST1019 # Importing the same package multiple times.
+  testifylint:
+    disable:
+      - compares
+      - empty
+      - error-is-as
+      - error-nil
+      - expected-actual
+      - float-compare
+      - formatter
+      - go-require
+      - len
+      - negative-positive
+      - nil-compare
+      - require-error
+    enable-all: true