Skip to content

Commit

Permalink
*: fix staticcheck errors
Browse files Browse the repository at this point in the history
Fix following errors from staticcheck:
- remove redundant return statements
- use for range instead of for select
- use time.Since instead of time.Now().Sub
- omit comparison to bool constant
- replace T.Fatal and T.Fatalf in tests with T.Error and T.Fatalf respectively because the goroutine calls T.Fatal must be called in the same goroutine as the test
- fix error strings that should not be capitalized
- use sort.Strings(...) instead of sort.Sort(sort.StringSlice(...))
- use he status code of Canceled instead of grpc.ErrClientConnClosing which is deprecated
- use use status.Errorf instead of grpc.Errorf which is deprecated
  • Loading branch information
spzala committed Apr 17, 2019
1 parent fc16b98 commit 5548bc4
Show file tree
Hide file tree
Showing 53 changed files with 146 additions and 151 deletions.
4 changes: 2 additions & 2 deletions auth/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,10 +751,10 @@ func TestHammerSimpleAuthenticate(t *testing.T) {
token := fmt.Sprintf("%s(%d)", user, i)
ctx := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(1)), AuthenticateParamSimpleTokenPrefix{}, token)
if _, err := as.Authenticate(ctx, user, "123"); err != nil {
t.Fatal(err)
t.Error(err)
}
if _, err := as.AuthInfoFromCtx(ctx); err != nil {
t.Fatal(err)
t.Error(err)
}
}(u)
}
Expand Down
4 changes: 2 additions & 2 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,11 +640,11 @@ func (r *redirectFollowingHTTPClient) Do(ctx context.Context, act httpAction) (*
if resp.StatusCode/100 == 3 {
hdr := resp.Header.Get("Location")
if hdr == "" {
return nil, nil, fmt.Errorf("Location header not set")
return nil, nil, fmt.Errorf("location header not set")
}
loc, err := url.Parse(hdr)
if err != nil {
return nil, nil, fmt.Errorf("Location header not valid URL: %s", hdr)
return nil, nil, fmt.Errorf("location header not valid URL: %s", hdr)
}
next = &redirectedHTTPAction{
action: act,
Expand Down
6 changes: 3 additions & 3 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ func TestRedirectFollowingHTTPClient(t *testing.T) {
},
},
},
wantErr: errors.New("Location header not set"),
wantErr: errors.New("location header not set"),
},

// fail if Location header is invalid
Expand All @@ -707,7 +707,7 @@ func TestRedirectFollowingHTTPClient(t *testing.T) {
},
},
},
wantErr: errors.New("Location header not valid URL: :"),
wantErr: errors.New("location header not valid URL: :"),
},

// fail if redirects checked way too many times
Expand Down Expand Up @@ -795,7 +795,7 @@ func TestHTTPClusterClientSync(t *testing.T) {

want = []string{"http://127.0.0.1:2379", "http://127.0.0.1:4001", "http://127.0.0.1:4002", "http://127.0.0.1:4003"}
got = hc.Endpoints()
sort.Sort(sort.StringSlice(got))
sort.Strings(got)
if !reflect.DeepEqual(want, got) {
t.Fatalf("incorrect endpoints post-Sync: want=%#v got=%#v", want, got)
}
Expand Down
2 changes: 1 addition & 1 deletion client/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (e Error) Error() string {
}

var (
ErrInvalidJSON = errors.New("client: response is invalid json. The endpoint is probably not valid etcd cluster endpoint.")
ErrInvalidJSON = errors.New("client: response is invalid json. The endpoint is probably not valid etcd cluster endpoint")
ErrEmptyBody = errors.New("client: response body is empty")
)

Expand Down
1 change: 0 additions & 1 deletion clientv3/balancer/balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ func (bb *baseBalancer) HandleSubConnStateChange(sc balancer.SubConn, s connecti
}

bb.currentConn.UpdateBalancerState(bb.currentState, bb.Picker)
return
}

func (bb *baseBalancer) regeneratePicker() {
Expand Down
6 changes: 3 additions & 3 deletions clientv3/balancer/grpc1.7-health.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ func (b *GRPC17Health) Get(ctx context.Context, opts grpc.BalancerGetOptions) (g
addr = b.pinAddr
b.mu.RUnlock()
if closed {
return grpc.Address{Addr: ""}, nil, grpc.ErrClientConnClosing
return grpc.Address{Addr: ""}, nil, context.Canceled
}
if addr == "" {
return grpc.Address{Addr: ""}, nil, ErrNoAddrAvilable
Expand All @@ -522,7 +522,7 @@ func (b *GRPC17Health) Get(ctx context.Context, opts grpc.BalancerGetOptions) (g
select {
case <-ch:
case <-b.donec:
return grpc.Address{Addr: ""}, nil, grpc.ErrClientConnClosing
return grpc.Address{Addr: ""}, nil, context.Canceled
case <-ctx.Done():
return grpc.Address{Addr: ""}, nil, ctx.Err()
}
Expand All @@ -532,7 +532,7 @@ func (b *GRPC17Health) Get(ctx context.Context, opts grpc.BalancerGetOptions) (g
b.mu.RUnlock()
// Close() which sets b.closed = true can be called before Get(), Get() must exit if balancer is closed.
if closed {
return grpc.Address{Addr: ""}, nil, grpc.ErrClientConnClosing
return grpc.Address{Addr: ""}, nil, context.Canceled
}
if addr != "" {
break
Expand Down
2 changes: 1 addition & 1 deletion clientv3/balancer/grpc1.7-health_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func (kcl *killConnListener) listen(l net.Listener) {
default:
}
if err != nil {
kcl.t.Fatal(err)
kcl.t.Error(err)
}
time.Sleep(1 * time.Millisecond)
conn.Close()
Expand Down
4 changes: 3 additions & 1 deletion clientv3/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,9 @@ func toErr(ctx context.Context, err error) error {
}
case codes.Unavailable:
case codes.FailedPrecondition:
err = grpc.ErrClientConnClosing
if ctx.Err() != nil {
err = ctx.Err()
}
}
}
return err
Expand Down
18 changes: 6 additions & 12 deletions clientv3/concurrency/election_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,13 @@ func TestResumeElection(t *testing.T) {
go func() {
o := e.Observe(ctx)
respChan <- nil
for {
select {
case resp, ok := <-o:
if !ok {
t.Fatal("Observe() channel closed prematurely")
}
// Ignore any observations that candidate1 was elected
if string(resp.Kvs[0].Value) == "candidate1" {
continue
}
respChan <- &resp
return
for resp := range o {
// Ignore any observations that candidate1 was elected
if string(resp.Kvs[0].Value) == "candidate1" {
continue
}
respChan <- &resp
return
}
}()

Expand Down
2 changes: 1 addition & 1 deletion clientv3/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
// // with etcd clientv3 <= v3.3
// if err == context.Canceled {
// // grpc balancer calls 'Get' with an inflight client.Close
// } else if err == grpc.ErrClientConnClosing {
// } else if err == codes.Canceled {
// // grpc balancer calls 'Get' after client.Close.
// }
// // with etcd clientv3 >= v3.4
Expand Down
18 changes: 9 additions & 9 deletions clientv3/integration/kv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import (
"go.etcd.io/etcd/mvcc/mvccpb"
"go.etcd.io/etcd/pkg/testutil"

"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

func TestKVPutError(t *testing.T) {
Expand Down Expand Up @@ -448,7 +448,7 @@ func TestKVGetErrConnClosed(t *testing.T) {
defer close(donec)
_, err := cli.Get(context.TODO(), "foo")
if !clientv3.IsConnCanceled(err) {
t.Fatalf("expected %v or %v, got %v", context.Canceled, grpc.ErrClientConnClosing, err)
t.Errorf("expected %v or %v, got %v", context.Canceled, codes.Canceled, err)
}
}()

Expand All @@ -475,7 +475,7 @@ func TestKVNewAfterClose(t *testing.T) {
go func() {
_, err := cli.Get(context.TODO(), "foo")
if !clientv3.IsConnCanceled(err) {
t.Fatalf("expected %v or %v, got %v", context.Canceled, grpc.ErrClientConnClosing, err)
t.Errorf("expected %v or %v, got %v", context.Canceled, codes.Canceled, err)
}
close(donec)
}()
Expand Down Expand Up @@ -689,7 +689,7 @@ func TestKVGetRetry(t *testing.T) {
// Get will fail, but reconnect will trigger
gresp, gerr := kv.Get(ctx, "foo")
if gerr != nil {
t.Fatal(gerr)
t.Error(gerr)
}
wkvs := []*mvccpb.KeyValue{
{
Expand All @@ -701,7 +701,7 @@ func TestKVGetRetry(t *testing.T) {
},
}
if !reflect.DeepEqual(gresp.Kvs, wkvs) {
t.Fatalf("bad get: got %v, want %v", gresp.Kvs, wkvs)
t.Errorf("bad get: got %v, want %v", gresp.Kvs, wkvs)
}
donec <- struct{}{}
}()
Expand Down Expand Up @@ -739,10 +739,10 @@ func TestKVPutFailGetRetry(t *testing.T) {
// Get will fail, but reconnect will trigger
gresp, gerr := kv.Get(context.TODO(), "foo")
if gerr != nil {
t.Fatal(gerr)
t.Error(gerr)
}
if len(gresp.Kvs) != 0 {
t.Fatalf("bad get kvs: got %+v, want empty", gresp.Kvs)
t.Errorf("bad get kvs: got %+v, want empty", gresp.Kvs)
}
donec <- struct{}{}
}()
Expand Down Expand Up @@ -908,7 +908,7 @@ func TestKVLargeRequests(t *testing.T) {
maxCallSendBytesClient: 10 * 1024 * 1024,
maxCallRecvBytesClient: 0,
valueSize: 10 * 1024 * 1024,
expectError: grpc.Errorf(codes.ResourceExhausted, "trying to send message larger than max "),
expectError: status.Errorf(codes.ResourceExhausted, "trying to send message larger than max "),
},
{
maxRequestBytesServer: 10 * 1024 * 1024,
Expand All @@ -922,7 +922,7 @@ func TestKVLargeRequests(t *testing.T) {
maxCallSendBytesClient: 10 * 1024 * 1024,
maxCallRecvBytesClient: 0,
valueSize: 10*1024*1024 + 5,
expectError: grpc.Errorf(codes.ResourceExhausted, "trying to send message larger than max "),
expectError: status.Errorf(codes.ResourceExhausted, "trying to send message larger than max "),
},
}
for i, test := range tests {
Expand Down
12 changes: 6 additions & 6 deletions clientv3/integration/lease_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"go.etcd.io/etcd/integration"
"go.etcd.io/etcd/pkg/testutil"

"google.golang.org/grpc"
"google.golang.org/grpc/codes"
)

func TestLeaseNotFoundError(t *testing.T) {
Expand Down Expand Up @@ -300,9 +300,9 @@ func TestLeaseGrantErrConnClosed(t *testing.T) {
defer close(donec)
_, err := cli.Grant(context.TODO(), 5)
if !clientv3.IsConnCanceled(err) {
// grpc.ErrClientConnClosing if grpc-go balancer calls 'Get' after client.Close.
// codes.Canceled if grpc-go balancer calls 'Get' after client.Close.
// context.Canceled if grpc-go balancer calls 'Get' with an inflight client.Close.
t.Fatalf("expected %v, %v or server unavailable, got %v", err != context.Canceled, grpc.ErrClientConnClosing, err)
t.Errorf("expected %v, %v or server unavailable, got %v", err != context.Canceled, codes.Canceled, err)
}
}()

Expand Down Expand Up @@ -372,7 +372,7 @@ func TestLeaseGrantNewAfterClose(t *testing.T) {
go func() {
_, err := cli.Grant(context.TODO(), 5)
if !clientv3.IsConnCanceled(err) {
t.Fatalf("expected %v, %v or server unavailable, got %v", err != context.Canceled, grpc.ErrClientConnClosing, err)
t.Errorf("expected %v, %v or server unavailable, got %v", err != context.Canceled, codes.Canceled, err)
}
close(donec)
}()
Expand Down Expand Up @@ -405,7 +405,7 @@ func TestLeaseRevokeNewAfterClose(t *testing.T) {
go func() {
_, err := cli.Revoke(context.TODO(), leaseID)
if !clientv3.IsConnCanceled(err) {
t.Fatalf("expected %v, %v or server unavailable, got %v", err != context.Canceled, grpc.ErrClientConnClosing, err)
t.Fatalf("expected %v, %v or server unavailable, got %v", err != context.Canceled, codes.Canceled, err)
}
close(donec)
}()
Expand Down Expand Up @@ -767,7 +767,7 @@ func TestV3LeaseFailureOverlap(t *testing.T) {
if err == nil || err == rpctypes.ErrTimeoutDueToConnectionLost {
return
}
t.Fatal(err)
t.Error(err)
}()
}
}
Expand Down
22 changes: 11 additions & 11 deletions clientv3/integration/leasing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestLeasingInterval(t *testing.T) {
}

// load into cache
if resp, err = lkv.Get(context.TODO(), "abc/a"); err != nil {
if _, err = lkv.Get(context.TODO(), "abc/a"); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -389,7 +389,7 @@ func TestLeasingConcurrentPut(t *testing.T) {
go func() {
resp, perr := lkv.Put(context.TODO(), "k", "abc")
if perr != nil {
t.Fatal(perr)
t.Error(perr)
}
putc <- resp
}()
Expand Down Expand Up @@ -559,7 +559,7 @@ func TestLeasingOwnerPutResponse(t *testing.T) {
if _, err = clus.Client(0).Put(context.TODO(), "k", "abc"); err != nil {
t.Fatal(err)
}
gresp, gerr := lkv.Get(context.TODO(), "k")
_, gerr := lkv.Get(context.TODO(), "k")
if gerr != nil {
t.Fatal(gerr)
}
Expand All @@ -573,7 +573,7 @@ func TestLeasingOwnerPutResponse(t *testing.T) {

clus.Members[0].Stop(t)

gresp, gerr = lkv.Get(context.TODO(), "k")
gresp, gerr := lkv.Get(context.TODO(), "k")
if gerr != nil {
t.Fatal(gerr)
}
Expand Down Expand Up @@ -992,7 +992,7 @@ func TestLeasingTxnRandIfThenOrElse(t *testing.T) {
for i := 0; i < keyCount/2; i++ {
k := fmt.Sprintf("k-%d", rand.Intn(keyCount))
if _, err := kv.Get(context.TODO(), k); err != nil {
t.Fatal(err)
t.Error(err)
}
getc <- struct{}{}
}
Expand Down Expand Up @@ -1399,10 +1399,10 @@ func TestLeasingReconnectOwnerRevoke(t *testing.T) {
// blocks until lkv1 connection comes back
resp, err := lkv1.Get(cctx, "k")
if err != nil {
t.Fatal(err)
t.Error(err)
}
if string(resp.Kvs[0].Value) != "v" {
t.Fatalf(`expected "v" value, got %+v`, resp)
t.Errorf(`expected "v" value, got %+v`, resp)
}
}()
select {
Expand Down Expand Up @@ -1440,11 +1440,11 @@ func TestLeasingReconnectOwnerRevokeCompact(t *testing.T) {
clus.WaitLeader(t)

// put some more revisions for compaction
presp, err := clus.Client(1).Put(context.TODO(), "a", "123")
_, err := clus.Client(1).Put(context.TODO(), "a", "123")
if err != nil {
t.Fatal(err)
}
presp, err = clus.Client(1).Put(context.TODO(), "a", "123")
presp, err := clus.Client(1).Put(context.TODO(), "a", "123")
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -1595,7 +1595,7 @@ func TestLeasingTxnAtomicCache(t *testing.T) {
}
tresp, err := lkv.Txn(context.TODO()).Then(gets...).Commit()
if err != nil {
t.Fatal(err)
t.Error(err)
}
revs := make([]int64, len(gets))
for i, resp := range tresp.Responses {
Expand All @@ -1604,7 +1604,7 @@ func TestLeasingTxnAtomicCache(t *testing.T) {
}
for i := 1; i < len(revs); i++ {
if revs[i] != revs[i-1] {
t.Fatalf("expected matching revisions, got %+v", revs)
t.Errorf("expected matching revisions, got %+v", revs)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions clientv3/integration/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ func TestV3ClientMetrics(t *testing.T) {

ln, err = transport.NewUnixListener(addr)
if err != nil {
t.Fatalf("Error: %v occurred while listening on addr: %v", err, addr)
t.Errorf("Error: %v occurred while listening on addr: %v", err, addr)
}

err = srv.Serve(ln)
if err != nil && !transport.IsClosedConnError(err) {
t.Fatalf("Err serving http requests: %v", err)
t.Errorf("Err serving http requests: %v", err)
}
}()

Expand Down
2 changes: 1 addition & 1 deletion clientv3/integration/mirror_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestMirrorSyncBase(t *testing.T) {

for key := range keyCh {
if _, err := cli.Put(ctx, key, "test"); err != nil {
t.Fatal(err)
t.Error(err)
}
}
}()
Expand Down
Loading

0 comments on commit 5548bc4

Please sign in to comment.