Skip to content

Commit

Permalink
Merge pull request #18 from rleungx/fix-code-smells
Browse files Browse the repository at this point in the history 
*: fix some code smells found by golangci-lint
  • Loading branch information
@YangruiEmma
YangruiEmma authored Jul 16, 2021
2 parents 97f16ff + 32a27ac commit 65b943a
Show file tree
Hide file tree
Showing 17 changed files with 76 additions and 66 deletions.
4 changes: 4 additions & 0 deletions pkg/circuitbreak/cbsuite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,11 +363,13 @@ func TestCBSuite_Dump(t *testing.T) {
test.Assert(t, len(cbCfg) == 2, len(cbCfg), cbCfg)

instCfg, ok := cbCfg[instanceCBKey].(CBConfig)
test.Assert(t, ok)
test.Assert(t, instCfg.Enable)
test.Assert(t, instCfg.MinSample == GetDefaultCBConfig().MinSample)
test.Assert(t, instCfg.ErrRate == GetDefaultCBConfig().ErrRate)

svcCfg, ok := cbCfg[serviceCBKey].(map[string]interface{})
test.Assert(t, ok)
cfg, ok := svcCfg[cfgKey].(CBConfig)
test.Assert(t, ok)
test.Assert(t, cfg.Enable)
Expand All @@ -389,11 +391,13 @@ func TestCBSuite_Dump(t *testing.T) {
test.Assert(t, len(cbCfg) == 2, len(cbCfg), cbCfg)

instCfg, ok = cbCfg[instanceCBKey].(CBConfig)
test.Assert(t, ok)
test.Assert(t, instCfg.Enable)
test.Assert(t, instCfg.MinSample == newCfg.MinSample)
test.Assert(t, instCfg.ErrRate == newCfg.ErrRate)

svcCfg, ok = cbCfg[serviceCBKey].(map[string]interface{})
test.Assert(t, ok)
cfg, ok = svcCfg[cfgKey].(CBConfig)
test.Assert(t, ok)
test.Assert(t, cfg.Enable)
Expand Down
4 changes: 4 additions & 0 deletions pkg/generic/binary_test/generic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func TestRawThriftBinaryMockReq(t *testing.T) {
// encode
rc := utils.NewThriftMessageCodec()
buf, err := rc.Encode("Test", thrift.CALL, 100, args)
test.Assert(t, err == nil, err)

resp, err := cli.GenericCall(context.Background(), "Test", buf)
test.Assert(t, err == nil, err)
Expand All @@ -96,6 +97,7 @@ func TestRawThriftBinaryMockReq(t *testing.T) {
buf = resp.([]byte)
var result kt.MockTestResult
method, seqID, err := rc.Decode(buf, &result)
test.Assert(t, err == nil, err)
test.Assert(t, method == "Test", method)
test.Assert(t, seqID != 100, seqID)
test.Assert(t, *result.Success == respMsg)
Expand All @@ -121,6 +123,7 @@ func TestRawThriftBinary2NormalServer(t *testing.T) {
// encode
rc := utils.NewThriftMessageCodec()
buf, err := rc.Encode("Test", thrift.CALL, 100, args)
test.Assert(t, err == nil, err)

resp, err := cli.GenericCall(context.Background(), "Test", buf, callopt.WithRPCTimeout(100*time.Second))
test.Assert(t, err == nil, err)
Expand All @@ -129,6 +132,7 @@ func TestRawThriftBinary2NormalServer(t *testing.T) {
buf = resp.([]byte)
var result kt.MockTestResult
method, seqID, err := rc.Decode(buf, &result)
test.Assert(t, err == nil, err)
test.Assert(t, method == "Test", method)
// seqID会在kitex中覆盖，避免TTHeader和Payload codec 不一致问题
test.Assert(t, seqID != 100, seqID)
Expand Down
4 changes: 2 additions & 2 deletions pkg/remote/codec/perrors/protocol_error_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ func TestNewProtocolErrorWithType(t *testing.T) {
// check success
test.Assert(t, err2.Error() == msg)
test.Assert(t, IsProtocolError(err2))
test.Assert(t, err2.(ProtocolError).TypeId() == typeID)
test.Assert(t, err2.TypeId() == typeID)

err3 := NewProtocolErrorWithType(typeID, err2.Error())
test.Assert(t, err3.Error() == msg)
test.Assert(t, IsProtocolError(err3))
test.Assert(t, err3.(ProtocolError).TypeId() == typeID)
test.Assert(t, err3.TypeId() == typeID)
}

func TestNewProtocolErrorWithMsg(t *testing.T) {
Expand Down
8 changes: 8 additions & 0 deletions pkg/remote/codec/thrift/binary_protocol_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,20 +235,28 @@ func TestConst(t *testing.T) {
test.Assert(t, err == nil, err)

_bool, err := prot.ReadBool()
test.Assert(t, err == nil, err)
test.Assert(t, _bool == false)
_byte, err := prot.ReadByte()
test.Assert(t, err == nil, err)
test.Assert(t, _byte == 0x1)
_i16, err := prot.ReadI16()
test.Assert(t, err == nil, err)
test.Assert(t, _i16 == 0x2)
_i32, err := prot.ReadI32()
test.Assert(t, err == nil, err)
test.Assert(t, _i32 == 0x3)
_i64, err := prot.ReadI64()
test.Assert(t, err == nil, err)
test.Assert(t, _i64 == 0x4)
_double, err := prot.ReadDouble()
test.Assert(t, err == nil, err)
test.Assert(t, _double == 5.0)
_string, err := prot.ReadString()
test.Assert(t, err == nil, err)
test.Assert(t, _string == "6")
_binary, err := prot.ReadBinary()
test.Assert(t, err == nil, err)
test.Assert(t, len(_binary) == 1)
test.Assert(t, _binary[0] == 0x7)
}
1 change: 1 addition & 0 deletions pkg/remote/connpool/long_pool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ func (lp *LongPool) getPeer(addr netAddr) *peer {
}

// Get pick or generate a net.Conn and return
// The context is not used but leave it for now.
func (lp *LongPool) Get(ctx context.Context, network, address string, opt *remote.ConnOption) (net.Conn, error) {
addr := netAddr{network, address}
p := lp.getPeer(addr)
Expand Down
60 changes: 31 additions & 29 deletions pkg/remote/connpool/long_pool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package connpool

import (
"context"
"errors"
"fmt"
"math/rand"
Expand Down Expand Up @@ -70,10 +71,10 @@ func TestLongConnPoolGetTimeout(t *testing.T) {
}
var err error

_, err = p.Get(nil, "tcp", mockAddr0, &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Second})
_, err = p.Get(context.TODO(), "tcp", mockAddr0, &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Second})
test.Assert(t, err == nil)

_, err = p.Get(nil, "tcp", mockAddr0, &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Millisecond})
_, err = p.Get(context.TODO(), "tcp", mockAddr0, &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Millisecond})
test.Assert(t, err != nil)
}

Expand All @@ -94,15 +95,15 @@ func TestLongConnPoolReuse(t *testing.T) {

count := make(map[net.Conn]int)
for i := 0; i < 10; i++ {
c, err := p.Get(nil, "tcp", addr1, opt)
c, err := p.Get(context.TODO(), "tcp", addr1, opt)
test.Assert(t, err == nil)
count[c]++
}
test.Assert(t, len(count) == 10)

count = make(map[net.Conn]int)
for i := 0; i < 10; i++ {
c, err := p.Get(nil, "tcp", addr1, opt)
c, err := p.Get(context.TODO(), "tcp", addr1, opt)
test.Assert(t, err == nil)
err = p.Put(c)
test.Assert(t, err == nil)
Expand All @@ -112,13 +113,13 @@ func TestLongConnPoolReuse(t *testing.T) {

count = make(map[net.Conn]int)
for i := 0; i < 10; i++ {
c, err := p.Get(nil, "tcp", addr1, opt)
c, err := p.Get(context.TODO(), "tcp", addr1, opt)
test.Assert(t, err == nil)
err = p.Put(c)
test.Assert(t, err == nil)
count[c]++

c, err = p.Get(nil, "tcp", addr2, opt)
c, err = p.Get(context.TODO(), "tcp", addr2, opt)
test.Assert(t, err == nil)
err = p.Put(c)
test.Assert(t, err == nil)
Expand All @@ -129,7 +130,7 @@ func TestLongConnPoolReuse(t *testing.T) {
// test exceed idleTime
count = make(map[net.Conn]int)
for i := 0; i < 3; i++ {
c, err := p.Get(nil, "tcp", addr1, opt)
c, err := p.Get(context.TODO(), "tcp", addr1, opt)
test.Assert(t, err == nil)
err = p.Put(c)
test.Assert(t, err == nil)
Expand All @@ -156,7 +157,7 @@ func TestLongConnPoolMaxIdle(t *testing.T) {
var conns []net.Conn
count := make(map[net.Conn]int)
for i := 0; i < 10; i++ {
c, err := p.Get(nil, "tcp", addr, opt)
c, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
count[c]++
conns = append(conns, c)
Expand All @@ -169,7 +170,7 @@ func TestLongConnPoolMaxIdle(t *testing.T) {
}

for i := 0; i < 10; i++ {
c, err := p.Get(nil, "tcp", addr, opt)
c, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
count[c]++
}
Expand All @@ -193,12 +194,12 @@ func TestLongConnPoolGlobalMaxIdle(t *testing.T) {
var conns []net.Conn
count := make(map[net.Conn]int)
for i := 0; i < 10; i++ {
c, err := p.Get(nil, "tcp", addr1, opt)
c, err := p.Get(context.TODO(), "tcp", addr1, opt)
test.Assert(t, err == nil)
count[c]++
conns = append(conns, c)

c, err = p.Get(nil, "tcp", addr2, opt)
c, err = p.Get(context.TODO(), "tcp", addr2, opt)
test.Assert(t, err == nil)
count[c]++
conns = append(conns, c)
Expand All @@ -211,11 +212,11 @@ func TestLongConnPoolGlobalMaxIdle(t *testing.T) {
}

for i := 0; i < 10; i++ {
c, err := p.Get(nil, "tcp", addr1, opt)
c, err := p.Get(context.TODO(), "tcp", addr1, opt)
test.Assert(t, err == nil)
count[c]++

c, err = p.Get(nil, "tcp", addr2, opt)
c, err = p.Get(context.TODO(), "tcp", addr2, opt)
test.Assert(t, err == nil)
count[c]++
}
Expand Down Expand Up @@ -247,7 +248,7 @@ func TestLongConnPoolCloseOnDiscard(t *testing.T) {
addr := mockAddr1
opt := &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Second}

c, err := p.Get(nil, "tcp", addr, opt)
c, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
test.Assert(t, !closed)

Expand All @@ -259,7 +260,7 @@ func TestLongConnPoolCloseOnDiscard(t *testing.T) {
test.Assert(t, err == nil)
test.Assert(t, !closed)

c2, err := p.Get(nil, "tcp", addr, opt)
c2, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
test.Assert(t, c == c2)
test.Assert(t, !closed)
Expand All @@ -268,7 +269,7 @@ func TestLongConnPoolCloseOnDiscard(t *testing.T) {
test.Assert(t, err == nil)
test.Assert(t, closed)

c3, err := p.Get(nil, "tcp", addr, opt)
c3, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
test.Assert(t, c3 != c2)
test.Assert(t, closed)
Expand Down Expand Up @@ -307,7 +308,7 @@ func TestLongConnPoolCloseOnError(t *testing.T) {
addr := mockAddr1
opt := &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Second}

c, err := p.Get(nil, "tcp", addr, opt)
c, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
test.Assert(t, !closed)

Expand All @@ -320,7 +321,7 @@ func TestLongConnPoolCloseOnError(t *testing.T) {
test.Assert(t, err == nil)
test.Assert(t, !closed)

c2, err := p.Get(nil, "tcp", addr, opt)
c2, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
test.Assert(t, c == c2)
test.Assert(t, !closed)
Expand All @@ -329,7 +330,7 @@ func TestLongConnPoolCloseOnError(t *testing.T) {
test.Assert(t, err != nil)
test.Assert(t, !closed)

c3, err := p.Get(nil, "tcp", addr, opt)
c3, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
test.Assert(t, c2 != c3)
}
Expand All @@ -356,7 +357,7 @@ func TestLongConnPoolCloseOnIdleTimeout(t *testing.T) {
addr := mockAddr1
opt := &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Second}

c, err := p.Get(nil, "tcp", addr, opt)
c, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
test.Assert(t, !closed)

Expand All @@ -366,7 +367,7 @@ func TestLongConnPoolCloseOnIdleTimeout(t *testing.T) {

time.Sleep(time.Millisecond * 2)

c2, err := p.Get(nil, "tcp", addr, opt)
c2, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
test.Assert(t, c != c2)
test.Assert(t, closed)
Expand Down Expand Up @@ -400,7 +401,7 @@ func TestLongConnPoolCloseOnClean(t *testing.T) {
addr := mockAddr1
opt := &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Second}

c, err := p.Get(nil, "tcp", addr, opt)
c, err := p.Get(context.TODO(), "tcp", addr, opt)
test.Assert(t, err == nil)
mu.RLock()
test.Assert(t, !closed)
Expand Down Expand Up @@ -442,7 +443,7 @@ func TestLongConnPoolDiscardUnknownConnection(t *testing.T) {
}

opt := &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Second}
c, err := p.Get(nil, network, address, opt)
c, err := p.Get(context.TODO(), network, address, opt)
test.Assert(t, err == nil)
test.Assert(t, !closed)

Expand Down Expand Up @@ -477,12 +478,13 @@ func TestConnPoolClose(t *testing.T) {
}

network, address := "tcp", mockAddr0
conn, err := p.Get(nil, network, address, &dialer.ConnOption{Dialer: d})
conn, err := p.Get(context.TODO(), network, address, &dialer.ConnOption{Dialer: d})
test.Assert(t, err == nil)
p.Put(conn)

network, address = "tcp", mockAddr1
_, err = p.Get(nil, network, address, &dialer.ConnOption{Dialer: d})
_, err = p.Get(context.TODO(), network, address, &dialer.ConnOption{Dialer: d})
test.Assert(t, err == nil)
p.Put(conn)

connCount := 0
Expand Down Expand Up @@ -526,7 +528,7 @@ func TestLongConnPoolPutUnknownConnection(t *testing.T) {
}

opt := &dialer.ConnOption{Dialer: d, ConnectTimeout: time.Second}
c, err := p.Get(nil, network, address, opt)
c, err := p.Get(context.TODO(), network, address, opt)
test.Assert(t, err == nil)
test.Assert(t, !closed)

Expand Down Expand Up @@ -559,7 +561,7 @@ func BenchmarkLongPoolGetOne(b *testing.B) {
b.ReportAllocs()
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
p.Get(nil, "tcp", mockAddr1, opt)
p.Get(context.TODO(), "tcp", mockAddr1, opt)
}
})
}
Expand All @@ -584,7 +586,7 @@ func BenchmarkLongPoolGetRand2000(b *testing.B) {
b.ReportAllocs()
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
p.Get(nil, "tcp", addrs[rand.Intn(2000)], opt)
p.Get(context.TODO(), "tcp", addrs[rand.Intn(2000)], opt)
}
})
}
Expand All @@ -609,7 +611,7 @@ func BenchmarkLongPoolGetRand2000Mesh(b *testing.B) {
b.ReportAllocs()
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
p.Get(nil, "tcp", addrs[rand.Intn(2000)], opt)
p.Get(context.TODO(), "tcp", addrs[rand.Intn(2000)], opt)
}
})
}
Loading

0 comments on commit 65b943a

Please sign in to comment.