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

*: fix revive linter #16634

Merged
merged 1 commit into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 0 additions & 5 deletions scripts/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -433,11 +433,6 @@ function lint_fix_pass {
run_for_modules generic_checker run golangci-lint run --config "${ETCD_ROOT_DIR}/tools/.golangci.yaml" --fix
}

function revive_pass {
# TODO: etcdserverpb/raft_internal_stringer.go:15:1: should have a package comment
run_for_modules generic_checker run_go_tool "github.com/mgechev/revive" -config "${ETCD_ROOT_DIR}/tests/revive.toml" -exclude "vendor/..." -exclude "out/..."
}

function unconvert_pass {
# TODO: pb package should be filtered out.
run_for_modules generic_checker run_go_tool "github.com/mdempsky/unconvert" unconvert -v
Expand Down
2 changes: 1 addition & 1 deletion server/etcdserver/api/membership/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ func (c *RaftCluster) Store(store v2store.Store) {
zap.Bool("is-learner", m.IsLearner),
)
}
for id, _ := range c.removed {
for id := range c.removed {
//We do not need to delete the member since the store is empty.
mustAddToRemovedMembersInStore(c.lg, store, id)
}
Expand Down
6 changes: 1 addition & 5 deletions tests/common/auth_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,7 @@ func setupAuth(c interfaces.Client, roles []authRole, users []authUser) error {
}

// enable auth
if err := c.AuthEnable(context.TODO()); err != nil {
return err
}

return nil
return c.AuthEnable(context.TODO())
}

func requireRolePermissionEqual(t *testing.T, expectRole authRole, actual []*authpb.Permission) {
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/cmux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,14 @@ func TestConnectionMultiplexing(t *testing.T) {
name = "ClientTLS"
}
t.Run(name, func(t *testing.T) {
testConnectionMultiplexing(t, ctx, clus.Procs[0], clientTLS)
testConnectionMultiplexing(ctx, t, clus.Procs[0], clientTLS)
})
}
})
}
}

func testConnectionMultiplexing(t *testing.T, ctx context.Context, member e2e.EtcdProcess, connType e2e.ClientConnType) {
func testConnectionMultiplexing(ctx context.Context, t *testing.T, member e2e.EtcdProcess, connType e2e.ClientConnType) {
httpEndpoint := member.EndpointsHTTP()[0]
grpcEndpoint := member.EndpointsGRPC()[0]
switch connType {
Expand Down
3 changes: 1 addition & 2 deletions tests/e2e/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,8 @@ func tlsInfo(t testing.TB, cfg e2e.ClientConfig) (*transport.TLSInfo, error) {
return nil, fmt.Errorf("failed to generate cert: %s", err)
}
return &tls, nil
} else {
return &integration.TestTLSInfo, nil
}
return &integration.TestTLSInfo, nil
default:
return nil, fmt.Errorf("config %v not supported", cfg)
}
Expand Down
9 changes: 3 additions & 6 deletions tests/e2e/watch_delay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,8 @@ func TestWatchDelayForManualProgressNotification(t *testing.T) {
if err != nil {
if strings.Contains(err.Error(), "context deadline exceeded") {
return nil
} else {
return err
}
return err
}
time.Sleep(watchResponsePeriod)
}
Expand Down Expand Up @@ -155,9 +154,8 @@ func TestWatchDelayForEvent(t *testing.T) {
if err != nil {
if strings.Contains(err.Error(), "context deadline exceeded") {
return nil
} else {
return err
}
return err
}
time.Sleep(watchResponsePeriod)
}
Expand Down Expand Up @@ -204,9 +202,8 @@ func continuouslyExecuteGetAll(ctx context.Context, t *testing.T, g *errgroup.Gr
if err != nil {
if strings.Contains(err.Error(), "context deadline exceeded") {
return nil
} else {
return err
}
return err
}
respSize := 0
for _, kv := range resp.Kvs {
Expand Down
4 changes: 2 additions & 2 deletions tests/framework/e2e/etcd_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ func (ep *EtcdServerProcess) EndpointsHTTP() []string {
}
func (ep *EtcdServerProcess) EndpointsMetrics() []string { return []string{ep.cfg.MetricsURL} }

func (epc *EtcdServerProcess) Etcdctl(opts ...config.ClientOption) *EtcdctlV3 {
etcdctl, err := NewEtcdctl(epc.Config().Client, epc.EndpointsGRPC(), opts...)
func (ep *EtcdServerProcess) Etcdctl(opts ...config.ClientOption) *EtcdctlV3 {
etcdctl, err := NewEtcdctl(ep.Config().Client, ep.EndpointsGRPC(), opts...)
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/clientv3/concurrency/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestSessionTTLOptions(t *testing.T) {
}
defer cli.Close()

var setTTL int = 90
var setTTL = 90
s, err := concurrency.NewSession(cli, concurrency.WithTTL(setTTL))
if err != nil {
t.Fatal(err)
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/revision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,15 @@ func testRevisionMonotonicWithFailures(t *testing.T, testDuration time.Duration,
wg.Add(1)
go func() {
defer wg.Done()
putWorker(t, ctx, clus)
putWorker(ctx, t, clus)
}()
}

for i := 0; i < 10; i++ {
wg.Add(1)
go func() {
defer wg.Done()
getWorker(t, ctx, clus)
getWorker(ctx, t, clus)
}()
}

Expand All @@ -109,7 +109,7 @@ func testRevisionMonotonicWithFailures(t *testing.T, testDuration time.Duration,
t.Logf("Revision %d", resp.Header.Revision)
}

func putWorker(t *testing.T, ctx context.Context, clus *integration.Cluster) {
func putWorker(ctx context.Context, t *testing.T, clus *integration.Cluster) {
for i := 0; ; i++ {
kv := clus.Client(i % 3)
_, err := kv.Put(ctx, "foo", fmt.Sprintf("%d", i))
Expand All @@ -122,7 +122,7 @@ func putWorker(t *testing.T, ctx context.Context, clus *integration.Cluster) {
}
}

func getWorker(t *testing.T, ctx context.Context, clus *integration.Cluster) {
func getWorker(ctx context.Context, t *testing.T, clus *integration.Cluster) {
var prevRev int64
for i := 0; ; i++ {
kv := clus.Client(i % 3)
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/v2store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ func TestStoreCompareAndSwapPrevIndexFailsIfNotMatch(t *testing.T) {
// TestStoreWatchCreate ensures that the store can watch for key creation.
func TestStoreWatchCreate(t *testing.T) {
s := v2store.New()
var eidx uint64 = 0
var eidx uint64
w, _ := s.Watch("/foo", false, false, 0)
c := w.EventChan()
assert.Equal(t, w.StartIndex(), eidx)
Expand All @@ -564,7 +564,7 @@ func TestStoreWatchCreate(t *testing.T) {
// can watch for recursive key creation.
func TestStoreWatchRecursiveCreate(t *testing.T) {
s := v2store.New()
var eidx uint64 = 0
var eidx uint64
w, err := s.Watch("/foo", true, false, 0)
testutil.AssertNil(t, err)
assert.Equal(t, w.StartIndex(), eidx)
Expand Down
38 changes: 0 additions & 38 deletions tests/revive.toml

This file was deleted.

21 changes: 10 additions & 11 deletions tests/robustness/failpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ type goPanicFailpoint struct {
}

type trigger interface {
Trigger(t *testing.T, ctx context.Context, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster) error
Trigger(ctx context.Context, t *testing.T, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster) error
AvailabilityChecker
}

Expand Down Expand Up @@ -270,7 +270,7 @@ func (f goPanicFailpoint) Inject(ctx context.Context, t *testing.T, lg *zap.Logg
}
if f.trigger != nil {
lg.Info("Triggering gofailpoint", zap.String("failpoint", f.Name()))
err = f.trigger.Trigger(t, ctx, member, clus)
err = f.trigger.Trigger(ctx, t, member, clus)
if err != nil {
lg.Info("gofailpoint trigger failed", zap.String("failpoint", f.Name()), zap.Error(err))
}
Expand Down Expand Up @@ -330,7 +330,7 @@ func (f goPanicFailpoint) Name() string {

type triggerDefrag struct{}

func (t triggerDefrag) Trigger(_ *testing.T, ctx context.Context, member e2e.EtcdProcess, _ *e2e.EtcdProcessCluster) error {
func (t triggerDefrag) Trigger(ctx context.Context, _ *testing.T, member e2e.EtcdProcess, _ *e2e.EtcdProcessCluster) error {
cc, err := clientv3.New(clientv3.Config{
Endpoints: member.EndpointsGRPC(),
Logger: zap.NewNop(),
Expand All @@ -356,7 +356,7 @@ type triggerCompact struct {
multiBatchCompaction bool
}

func (t triggerCompact) Trigger(_ *testing.T, ctx context.Context, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster) error {
func (t triggerCompact) Trigger(ctx context.Context, _ *testing.T, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster) error {
cc, err := clientv3.New(clientv3.Config{
Endpoints: member.EndpointsGRPC(),
Logger: zap.NewNop(),
Expand Down Expand Up @@ -399,7 +399,7 @@ type blackholePeerNetworkFailpoint struct {

func (f blackholePeerNetworkFailpoint) Inject(ctx context.Context, t *testing.T, lg *zap.Logger, clus *e2e.EtcdProcessCluster) error {
member := clus.Procs[rand.Int()%len(clus.Procs)]
return f.Trigger(t, ctx, member, clus)
return f.Trigger(ctx, t, member, clus)
}

func (f blackholePeerNetworkFailpoint) Name() string {
Expand All @@ -410,8 +410,8 @@ type triggerBlackhole struct {
waitTillSnapshot bool
}

func (tb triggerBlackhole) Trigger(t *testing.T, ctx context.Context, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster) error {
return blackhole(t, ctx, member, clus, tb.waitTillSnapshot)
func (tb triggerBlackhole) Trigger(ctx context.Context, t *testing.T, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster) error {
return blackhole(ctx, t, member, clus, tb.waitTillSnapshot)
}

func (tb triggerBlackhole) Available(config e2e.EtcdProcessClusterConfig, process e2e.EtcdProcess) bool {
Expand All @@ -421,7 +421,7 @@ func (tb triggerBlackhole) Available(config e2e.EtcdProcessClusterConfig, proces
return config.ClusterSize > 1 && process.PeerProxy() != nil
}

func blackhole(t *testing.T, ctx context.Context, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster, shouldWaitTillSnapshot bool) error {
func blackhole(ctx context.Context, t *testing.T, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster, shouldWaitTillSnapshot bool) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did revive linter raise a warning on this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

failpoints.go:424:30: context-as-argument: context.Context should be the first parameter of a function (revive)
func blackhole(t *testing.T, ctx context.Context, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster, shouldWaitTillSnapshot bool) error {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx

proxy := member.PeerProxy()

// Blackholing will cause peers to not be able to use streamWriters registered with member
Expand All @@ -437,10 +437,9 @@ func blackhole(t *testing.T, ctx context.Context, member e2e.EtcdProcess, clus *
}()
if shouldWaitTillSnapshot {
return waitTillSnapshot(ctx, t, clus, member)
} else {
time.Sleep(time.Second)
return nil
}
time.Sleep(time.Second)
return nil
}

func waitTillSnapshot(ctx context.Context, t *testing.T, clus *e2e.EtcdProcessCluster, blackholedMember e2e.EtcdProcess) error {
Expand Down
14 changes: 6 additions & 8 deletions tests/robustness/model/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,8 @@ func describeTxnResponse(request *TxnRequest, response *TxnResponse) string {
}
if response.Failure {
return fmt.Sprintf("failure(%s)", description)
} else {
return fmt.Sprintf("success(%s)", description)
}
return fmt.Sprintf("success(%s)", description)
}

func describeEtcdOperation(op EtcdOperation) string {
Expand Down Expand Up @@ -162,13 +161,12 @@ func describeRangeResponse(request RangeOptions, response RangeResponse) string
kvs[i] = describeValueOrHash(kv.Value)
}
return fmt.Sprintf("[%s], count: %d", strings.Join(kvs, ","), response.Count)
} else {
if len(response.KVs) == 0 {
return "nil"
} else {
return describeValueOrHash(response.KVs[0].Value)
}
}

if len(response.KVs) == 0 {
return "nil"
}
return describeValueOrHash(response.KVs[0].Value)
}

func describeValueOrHash(value ValueOrHash) string {
Expand Down
17 changes: 8 additions & 9 deletions tests/robustness/model/deterministic.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,11 @@ func (s EtcdState) Step(request EtcdRequest) (EtcdState, MaybeEtcdResponse) {
if request.Range.Revision == 0 || request.Range.Revision == s.Revision {
resp := s.getRange(request.Range.RangeOptions)
return s, MaybeEtcdResponse{EtcdResponse: EtcdResponse{Range: &resp, Revision: s.Revision}}
} else {
if request.Range.Revision > s.Revision {
return s, MaybeEtcdResponse{Error: EtcdFutureRevErr.Error()}
}
return s, MaybeEtcdResponse{PartialResponse: true, EtcdResponse: EtcdResponse{Revision: s.Revision}}
}
if request.Range.Revision > s.Revision {
return s, MaybeEtcdResponse{Error: ErrEtcdFutureRev.Error()}
}
return s, MaybeEtcdResponse{PartialResponse: true, EtcdResponse: EtcdResponse{Revision: s.Revision}}
case Txn:
failure := false
for _, cond := range request.Txn.Conditions {
Expand Down Expand Up @@ -148,7 +147,7 @@ func (s EtcdState) Step(request EtcdRequest) (EtcdState, MaybeEtcdResponse) {
}
}
if increaseRevision {
s.Revision += 1
s.Revision++
}
return s, MaybeEtcdResponse{EtcdResponse: EtcdResponse{Txn: &TxnResponse{Failure: failure, Results: opResp}, Revision: s.Revision}}
case LeaseGrant:
Expand All @@ -174,7 +173,7 @@ func (s EtcdState) Step(request EtcdRequest) (EtcdState, MaybeEtcdResponse) {
//delete the lease
delete(s.Leases, request.LeaseRevoke.LeaseID)
if keyDeleted {
s.Revision += 1
s.Revision++
}
return s, MaybeEtcdResponse{EtcdResponse: EtcdResponse{Revision: s.Revision, LeaseRevoke: &LeaseRevokeResponse{}}}
case Defragment:
Expand All @@ -193,7 +192,7 @@ func (s EtcdState) getRange(options RangeOptions) RangeResponse {
for k, v := range s.KeyValues {
if k >= options.Start && k < options.End {
response.KVs = append(response.KVs, KeyValue{Key: k, ValueRevision: v})
count += 1
count++
}
}
sort.Slice(response.KVs, func(j, k int) bool {
Expand Down Expand Up @@ -315,7 +314,7 @@ type MaybeEtcdResponse struct {
Error string
}

var EtcdFutureRevErr = errors.New("future rev")
var ErrEtcdFutureRev = errors.New("future rev")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did revive linter raise a warning on this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.

The error is here

deterministic.go:318:5: error-naming: error var EtcdFutureRevErr should have name of the form ErrFoo (revive)
var EtcdFutureRevErr = errors.New("future rev")
    ^

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx for the clarification.


type EtcdResponse struct {
Txn *TxnResponse
Expand Down
3 changes: 1 addition & 2 deletions tests/robustness/model/replay.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,8 @@ type Event struct {
func (e Event) Match(request WatchRequest) bool {
if request.WithPrefix {
return strings.HasPrefix(e.Key, request.Key)
} else {
return e.Key == request.Key
}
return e.Key == request.Key
}

type WatchRequest struct {
Expand Down
2 changes: 1 addition & 1 deletion tests/robustness/traffic/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (c etcdTrafficClient) Request(ctx context.Context, request etcdRequestType,
opCtx, cancel := context.WithTimeout(ctx, RequestTimeout)
defer cancel()

var limit int64 = 0
var limit int64
switch request {
case StaleGet:
_, rev, err = c.client.Get(opCtx, c.randomKey(), lastRev)
Expand Down
2 changes: 1 addition & 1 deletion tests/robustness/traffic/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (t kubernetesTraffic) Read(ctx context.Context, kc *kubernetesClient, s *st
hasMore := true
rangeStart := keyPrefix
var kvs []*mvccpb.KeyValue
var revision int64 = 0
var revision int64

for hasMore {
readCtx, cancel := context.WithTimeout(ctx, RequestTimeout)
Expand Down
Loading
Loading