Skip to content

Commit

Permalink
Merge pull request #18800 from mmorel-35/testifylint/require-error
Browse files Browse the repository at this point in the history
fix: enable require-error rule from testifylint in client, pkg and server packages
  • Loading branch information
ahrtr authored Oct 30, 2024
2 parents c2bfbe8 + 3a31151 commit 6883308
Show file tree
Hide file tree
Showing 13 changed files with 60 additions and 53 deletions.
5 changes: 3 additions & 2 deletions client/pkg/testutil/before.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.etcd.io/etcd/client/pkg/v3/verify"
)
Expand All @@ -31,9 +32,9 @@ func BeforeTest(tb testing.TB) {
revertVerifyFunc := verify.EnableAllVerifications()

path, err := os.Getwd()
assert.NoError(tb, err)
require.NoError(tb, err)
tempDir := tb.TempDir()
assert.NoError(tb, os.Chdir(tempDir))
require.NoError(tb, os.Chdir(tempDir))
tb.Logf("Changing working directory to: %s", tempDir)

tb.Cleanup(func() {
Expand Down
6 changes: 3 additions & 3 deletions pkg/proxy/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"go.uber.org/zap/zaptest"

Expand Down Expand Up @@ -559,7 +559,7 @@ func testServerHTTP(t *testing.T, secure, delayTx bool) {
now := time.Now()
if secure {
tp, terr := transport.NewTransport(tlsInfo, 3*time.Second)
assert.NoError(t, terr)
require.NoError(t, terr)
cli := &http.Client{Transport: tp}
resp, err = cli.Post("https://"+srcAddr+"/hello", "", strings.NewReader(data))
defer cli.CloseIdleConnections()
Expand All @@ -568,7 +568,7 @@ func testServerHTTP(t *testing.T, secure, delayTx bool) {
resp, err = http.Post("http://"+srcAddr+"/hello", "", strings.NewReader(data))
defer http.DefaultClient.CloseIdleConnections()
}
assert.NoError(t, err)
require.NoError(t, err)
d, err := io.ReadAll(resp.Body)
if err != nil {
t.Fatal(err)
Expand Down
11 changes: 6 additions & 5 deletions server/etcdserver/api/membership/storev2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/coreos/go-semver/semver"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zaptest"

"go.etcd.io/etcd/server/v3/etcdserver/api/v2store"
Expand All @@ -29,23 +30,23 @@ func TestIsMetaStoreOnly(t *testing.T) {
s := v2store.New("/0", "/1")

metaOnly, err := IsMetaStoreOnly(s)
assert.NoError(t, err)
require.NoError(t, err)
assert.Truef(t, metaOnly, "Just created v2store should be meta-only")

mustSaveClusterVersionToStore(lg, s, semver.New("3.5.17"))
metaOnly, err = IsMetaStoreOnly(s)
assert.NoError(t, err)
require.NoError(t, err)
assert.Truef(t, metaOnly, "Just created v2store should be meta-only")

mustSaveMemberToStore(lg, s, &Member{ID: 0x00abcd})
metaOnly, err = IsMetaStoreOnly(s)
assert.NoError(t, err)
require.NoError(t, err)
assert.Truef(t, metaOnly, "Just created v2store should be meta-only")

_, err = s.Create("/1/foo", false, "v1", false, v2store.TTLOptionSet{ExpireTime: v2store.Permanent})
assert.NoError(t, err)
require.NoError(t, err)
metaOnly, err = IsMetaStoreOnly(s)
assert.NoError(t, err)
require.NoError(t, err)
assert.Falsef(t, metaOnly, "Just created v2store should be meta-only")

_, err = s.Delete("/1/foo", false, false)
Expand Down
22 changes: 11 additions & 11 deletions server/etcdserver/api/v2store/store_ttl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestMinExpireTime(t *testing.T) {
s.DeleteExpiredKeys(fc.Now())
var eidx uint64 = 1
e, err := s.Get("/foo", true, false)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, eidx, e.EtcdIndex)
assert.Equal(t, "get", e.Action)
assert.Equal(t, "/foo", e.Node.Key)
Expand All @@ -60,7 +60,7 @@ func TestStoreGetDirectory(t *testing.T) {
s.Create("/foo/baz/ttl", false, "Y", false, TTLOptionSet{ExpireTime: fc.Now().Add(time.Second * 3)})
var eidx uint64 = 7
e, err := s.Get("/foo", true, false)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, eidx, e.EtcdIndex)
assert.Equal(t, "get", e.Action)
assert.Equal(t, "/foo", e.Node.Key)
Expand Down Expand Up @@ -103,7 +103,7 @@ func TestStoreUpdateValueTTL(t *testing.T) {
var eidx uint64 = 2
s.Create("/foo", false, "bar", false, TTLOptionSet{ExpireTime: Permanent})
_, err := s.Update("/foo", "baz", TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond)})
assert.NoError(t, err)
require.NoError(t, err)
e, _ := s.Get("/foo", false, false)
assert.Equal(t, "baz", *e.Node.Value)
assert.Equal(t, eidx, e.EtcdIndex)
Expand All @@ -124,11 +124,11 @@ func TestStoreUpdateDirTTL(t *testing.T) {

var eidx uint64 = 3
_, err := s.Create("/foo", true, "", false, TTLOptionSet{ExpireTime: Permanent})
assert.NoError(t, err)
require.NoError(t, err)
_, err = s.Create("/foo/bar", false, "baz", false, TTLOptionSet{ExpireTime: Permanent})
assert.NoError(t, err)
require.NoError(t, err)
e, err := s.Update("/foo/bar", "", TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond)})
assert.NoError(t, err)
require.NoError(t, err)
assert.False(t, e.Node.Dir)
assert.Equal(t, eidx, e.EtcdIndex)
e, _ = s.Get("/foo/bar", false, false)
Expand Down Expand Up @@ -279,13 +279,13 @@ func TestStoreRefresh(t *testing.T) {
s.Create("/bar", true, "bar", false, TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond)})
s.Create("/bar/z", false, "bar", false, TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond)})
_, err := s.Update("/foo", "", TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond), Refresh: true})
assert.NoError(t, err)
require.NoError(t, err)

_, err = s.Set("/foo", false, "", TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond), Refresh: true})
assert.NoError(t, err)
require.NoError(t, err)

_, err = s.Update("/bar/z", "", TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond), Refresh: true})
assert.NoError(t, err)
require.NoError(t, err)

_, err = s.CompareAndSwap("/foo", "bar", 0, "", TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond), Refresh: true})
assert.NoError(t, err)
Expand All @@ -303,7 +303,7 @@ func TestStoreRecoverWithExpiration(t *testing.T) {
s.Create("/foo/x", false, "bar", false, TTLOptionSet{ExpireTime: Permanent})
s.Create("/foo/y", false, "baz", false, TTLOptionSet{ExpireTime: fc.Now().Add(5 * time.Millisecond)})
b, err := s.Save()
assert.NoError(t, err)
require.NoError(t, err)

time.Sleep(10 * time.Millisecond)

Expand All @@ -316,7 +316,7 @@ func TestStoreRecoverWithExpiration(t *testing.T) {
s.DeleteExpiredKeys(fc.Now())

e, err := s.Get("/foo/x", false, false)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, eidx, e.EtcdIndex)
assert.Equal(t, "bar", *e.Node.Value)

Expand Down
20 changes: 10 additions & 10 deletions server/etcdserver/apply/apply_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -782,28 +782,28 @@ func TestAuthApplierV3_RoleGet(t *testing.T) {

func TestCheckLeasePutsKeys(t *testing.T) {
aa := defaultAuthApplierV3(t)
assert.NoErrorf(t, aa.checkLeasePutsKeys(lease.NewLease(lease.LeaseID(1), 3600)), "auth is disabled, should allow puts")
require.NoErrorf(t, aa.checkLeasePutsKeys(lease.NewLease(lease.LeaseID(1), 3600)), "auth is disabled, should allow puts")
mustCreateRolesAndEnableAuth(t, aa)
aa.authInfo = auth.AuthInfo{Username: "root"}
assert.NoErrorf(t, aa.checkLeasePutsKeys(lease.NewLease(lease.LeaseID(1), 3600)), "auth is enabled, should allow puts for root")
require.NoErrorf(t, aa.checkLeasePutsKeys(lease.NewLease(lease.LeaseID(1), 3600)), "auth is enabled, should allow puts for root")

l := lease.NewLease(lease.LeaseID(1), 3600)
l.SetLeaseItem(lease.LeaseItem{Key: "a"})
aa.authInfo = auth.AuthInfo{Username: "bob", Revision: 0}
assert.ErrorIsf(t, aa.checkLeasePutsKeys(l), auth.ErrUserEmpty, "auth is enabled, should not allow bob, non existing at rev 0")
require.ErrorIsf(t, aa.checkLeasePutsKeys(l), auth.ErrUserEmpty, "auth is enabled, should not allow bob, non existing at rev 0")
aa.authInfo = auth.AuthInfo{Username: "bob", Revision: 1}
assert.ErrorIsf(t, aa.checkLeasePutsKeys(l), auth.ErrAuthOldRevision, "auth is enabled, old revision")
require.ErrorIsf(t, aa.checkLeasePutsKeys(l), auth.ErrAuthOldRevision, "auth is enabled, old revision")

aa.authInfo = auth.AuthInfo{Username: "bob", Revision: aa.as.Revision()}
assert.ErrorIsf(t, aa.checkLeasePutsKeys(l), auth.ErrPermissionDenied, "auth is enabled, bob does not have permissions, bob does not exist")
require.ErrorIsf(t, aa.checkLeasePutsKeys(l), auth.ErrPermissionDenied, "auth is enabled, bob does not have permissions, bob does not exist")
_, err := aa.as.UserAdd(&pb.AuthUserAddRequest{Name: "bob", Options: &authpb.UserAddOptions{NoPassword: true}})
assert.NoErrorf(t, err, "bob should be added without error")
require.NoErrorf(t, err, "bob should be added without error")
aa.authInfo = auth.AuthInfo{Username: "bob", Revision: aa.as.Revision()}
assert.ErrorIsf(t, aa.checkLeasePutsKeys(l), auth.ErrPermissionDenied, "auth is enabled, bob exists yet does not have permissions")
require.ErrorIsf(t, aa.checkLeasePutsKeys(l), auth.ErrPermissionDenied, "auth is enabled, bob exists yet does not have permissions")

// allow bob to access "a"
_, err = aa.as.RoleAdd(&pb.AuthRoleAddRequest{Name: "bobsrole"})
assert.NoErrorf(t, err, "bobsrole should be added without error")
require.NoErrorf(t, err, "bobsrole should be added without error")
_, err = aa.as.RoleGrantPermission(&pb.AuthRoleGrantPermissionRequest{
Name: "bobsrole",
Perm: &authpb.Permission{
Expand All @@ -812,12 +812,12 @@ func TestCheckLeasePutsKeys(t *testing.T) {
RangeEnd: nil,
},
})
assert.NoErrorf(t, err, "bobsrole should be granted permissions without error")
require.NoErrorf(t, err, "bobsrole should be granted permissions without error")
_, err = aa.as.UserGrantRole(&pb.AuthUserGrantRoleRequest{
User: "bob",
Role: "bobsrole",
})
assert.NoErrorf(t, err, "bob should be granted bobsrole without error")
require.NoErrorf(t, err, "bob should be granted bobsrole without error")

aa.authInfo = auth.AuthInfo{Username: "bob", Revision: aa.as.Revision()}
assert.NoErrorf(t, aa.checkLeasePutsKeys(l), "bob should be able to access key 'a'")
Expand Down
7 changes: 4 additions & 3 deletions server/etcdserver/version/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/coreos/go-semver/semver"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"go.uber.org/zap/zaptest"

Expand Down Expand Up @@ -64,7 +65,7 @@ func TestDowngradeSingleNode(t *testing.T) {
c.StepMonitors()
assert.Equal(t, newCluster(lg, 1, version.V3_6), c)

assert.NoError(t, c.Version().DowngradeEnable(context.Background(), &version.V3_5))
require.NoError(t, c.Version().DowngradeEnable(context.Background(), &version.V3_5))
c.StepMonitors()
assert.Equal(t, version.V3_5, c.clusterVersion)

Expand All @@ -80,7 +81,7 @@ func TestDowngradeThreeNode(t *testing.T) {
c.StepMonitors()
assert.Equal(t, newCluster(lg, 3, version.V3_6), c)

assert.NoError(t, c.Version().DowngradeEnable(context.Background(), &version.V3_5))
require.NoError(t, c.Version().DowngradeEnable(context.Background(), &version.V3_5))
c.StepMonitors()
assert.Equal(t, version.V3_5, c.clusterVersion)

Expand All @@ -100,7 +101,7 @@ func TestNewerMemberCanReconnectDuringDowngrade(t *testing.T) {
c.StepMonitors()
assert.Equal(t, newCluster(lg, 3, version.V3_6), c)

assert.NoError(t, c.Version().DowngradeEnable(context.Background(), &version.V3_5))
require.NoError(t, c.Version().DowngradeEnable(context.Background(), &version.V3_5))
c.StepMonitors()
assert.Equal(t, version.V3_5, c.clusterVersion)

Expand Down
6 changes: 3 additions & 3 deletions server/storage/backend/backend_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

betesting "go.etcd.io/etcd/server/v3/storage/backend/testing"
"go.etcd.io/etcd/server/v3/storage/schema"
Expand All @@ -34,11 +34,11 @@ func BenchmarkBackendPut(b *testing.B) {
for i := 0; i < b.N; i++ {
keys[i] = make([]byte, 64)
_, err := rand.Read(keys[i])
assert.NoError(b, err)
require.NoError(b, err)
}
value := make([]byte, 128)
_, err := rand.Read(value)
assert.NoError(b, err)
require.NoError(b, err)

batchTx := backend.BatchTx()

Expand Down
7 changes: 4 additions & 3 deletions server/storage/backend/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zaptest"

bolt "go.etcd.io/bbolt"
Expand Down Expand Up @@ -70,7 +71,7 @@ func TestBackendSnapshot(t *testing.T) {
if _, err := snap.WriteTo(f); err != nil {
t.Fatal(err)
}
assert.NoError(t, f.Close())
require.NoError(t, f.Close())

// bootstrap new backend from the snapshot
bcfg := backend.DefaultBackendConfig(zaptest.NewLogger(t))
Expand Down Expand Up @@ -332,7 +333,7 @@ func TestBackendWritebackForEach(t *testing.T) {
}
rtx := b.ReadTx()
rtx.RLock()
assert.NoError(t, rtx.UnsafeForEach(schema.Key, getSeq))
require.NoError(t, rtx.UnsafeForEach(schema.Key, getSeq))
rtx.RUnlock()

partialSeq := seq
Expand All @@ -341,7 +342,7 @@ func TestBackendWritebackForEach(t *testing.T) {
b.ForceCommit()

tx.Lock()
assert.NoError(t, tx.UnsafeForEach(schema.Key, getSeq))
require.NoError(t, tx.UnsafeForEach(schema.Key, getSeq))
tx.Unlock()

if seq != partialSeq {
Expand Down
3 changes: 2 additions & 1 deletion server/storage/mvcc/hash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zaptest"

"go.etcd.io/etcd/pkg/v3/traceutil"
Expand Down Expand Up @@ -124,7 +125,7 @@ func testHashByRev(t *testing.T, s *store, rev int64) KeyValueHash {
rev = s.Rev()
}
hash, _, err := s.hashByRev(rev)
assert.NoErrorf(t, err, "error on rev %v", rev)
require.NoErrorf(t, err, "error on rev %v", rev)
_, err = s.Compact(traceutil.TODO(), rev)
assert.NoErrorf(t, err, "error on compact %v", rev)
return hash
Expand Down
4 changes: 2 additions & 2 deletions server/storage/mvcc/kvstore_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zaptest"

"go.etcd.io/etcd/pkg/v3/traceutil"
Expand Down Expand Up @@ -152,7 +152,7 @@ func benchmarkStoreRestore(revsPerKey int, b *testing.B) {
txn.End()
}
}
assert.NoError(b, s.Close())
require.NoError(b, s.Close())

b.ReportAllocs()
b.ResetTimer()
Expand Down
13 changes: 7 additions & 6 deletions server/storage/mvcc/testutil/hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.etcd.io/bbolt"
"go.etcd.io/etcd/api/v3/mvccpb"
Expand Down Expand Up @@ -61,23 +62,23 @@ func testCompactionHash(ctx context.Context, t *testing.T, h CompactionHashTestC
for i := start; i <= stop; i++ {
if i%67 == 0 {
err := h.Delete(ctx, PickKey(i+83))
assert.NoErrorf(t, err, "error on delete")
require.NoErrorf(t, err, "error on delete")
} else {
err := h.Put(ctx, PickKey(i), fmt.Sprint(i))
assert.NoErrorf(t, err, "error on put")
require.NoErrorf(t, err, "error on put")
}
}
hash1, err := h.HashByRev(ctx, stop)
assert.NoErrorf(t, err, "error on hash (rev %v)", stop)
require.NoErrorf(t, err, "error on hash (rev %v)", stop)

err = h.Compact(ctx, stop)
assert.NoErrorf(t, err, "error on compact (rev %v)", stop)
require.NoErrorf(t, err, "error on compact (rev %v)", stop)

err = h.Defrag(ctx)
assert.NoErrorf(t, err, "error on defrag")
require.NoErrorf(t, err, "error on defrag")

hash2, err := h.HashByRev(ctx, stop)
assert.NoErrorf(t, err, "error on hash (rev %v)", stop)
require.NoErrorf(t, err, "error on hash (rev %v)", stop)
assert.Equalf(t, hash1, hash2, "hashes do not match on rev %v", stop)
}

Expand Down
Loading

0 comments on commit 6883308

Please sign in to comment.