Skip to content

Commit

Permalink
code health: fix all places highlighted by linter
Browse files Browse the repository at this point in the history
Changed error's suppression in check.yaml. The suppression of rule
`errcheck` may be removed after fixing errors check in all methods
with calling encodeXxx inside. See details below.
The rules `structcheck` and `unused` highlight the 'hack' with using
_msgpack struct{} `msgpack:",asArray"`. The workaround with `//nolint`
is not the best way to suppress it, cause this comment gets rendered
by godoc and there is no way to make it invisible. For details see
golang/go#20925.

Suppressed the highlighting of error check in all methods,
having encodeXxx inside. For now these methods are not able to
return any error cause of internal implementation of writer
interface (see smallbuf.go). For future, if the implementation of writer
will be changed, and there will be a need to check errors, we must think
about how to say to compiler that the error check is 'unlikely'.

Fixed the use of time package API, all places with calls of
time.Now().Sub(). Now time package propose the explicit time.Until().

Replaced all calls of Errorf() with Fatalf() in tests. That change
prevents nil dereferences below in the code and stops test function
execution, where it is expected in tests.

Suppressed the highlighting of all unused constants and functions
(Rules structcheck,unused in golangci-lint).

Fixed calling of Fatalf from non-testing goroutine in queue tests.
It is not a valid way to stop test from another goroutine.

Closes #142
Closes #150
  • Loading branch information
vr009 committed Apr 26, 2022
1 parent e9b9ba1 commit 80d8efb
Show file tree
Hide file tree
Showing 13 changed files with 113 additions and 89 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ jobs:
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
with:
args: --issues-exit-code=0 -E gofmt
args: --disable errcheck,unused,structcheck -E gofmt
10 changes: 5 additions & 5 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ func (conn *Connection) createConnection(reconnect bool) (err error) {
conn.notify(ReconnectFailed)
reconnects++
conn.mutex.Unlock()
time.Sleep(now.Add(conn.opts.Reconnect).Sub(time.Now()))
time.Sleep(time.Until(now.Add(conn.opts.Reconnect)))
conn.mutex.Lock()
}
if conn.state == connClosed {
Expand Down Expand Up @@ -688,7 +688,7 @@ func (conn *Connection) newFuture(requestCode int32) (fut *Future) {
*pair.last = fut
pair.last = &fut.next
if conn.opts.Timeout > 0 {
fut.timeout = time.Now().Sub(epoch) + conn.opts.Timeout
fut.timeout = time.Until(epoch) + conn.opts.Timeout
}
shard.rmut.Unlock()
if conn.rlimit != nil && conn.opts.RLimitAction == RLimitWait {
Expand Down Expand Up @@ -796,9 +796,9 @@ func (conn *Connection) timeouts() {
return
case <-t.C:
}
minNext := time.Now().Sub(epoch) + timeout
minNext := time.Until(epoch) + timeout
for i := range conn.shard {
nowepoch = time.Now().Sub(epoch)
nowepoch = time.Until(epoch)
shard := &conn.shard[i]
for pos := range shard.requests {
shard.rmut.Lock()
Expand All @@ -825,7 +825,7 @@ func (conn *Connection) timeouts() {
shard.rmut.Unlock()
}
}
nowepoch = time.Now().Sub(epoch)
nowepoch = time.Until(epoch)
if nowepoch+time.Microsecond < minNext {
t.Reset(minNext - nowepoch)
} else {
Expand Down
10 changes: 5 additions & 5 deletions connection_pool/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
type Tuple struct {
// Instruct msgpack to pack this struct as array, so no custom packer
// is needed.
_msgpack struct{} `msgpack:",asArray"` //nolint: structcheck,unused
_msgpack struct{} `msgpack:",asArray"`
Key string
Value string
}
Expand Down Expand Up @@ -53,7 +53,7 @@ func ExampleConnectionPool_Select() {
return
}
// Insert a new tuple {"key2", "value2"}.
_, err = conn.Insert(spaceName, &Tuple{Key: "key2", Value: "value2"})
_, err = conn.Insert(spaceName, &Tuple{Key: "key2", Value: "value2"})
if err != nil {
fmt.Printf("Failed to insert: %s", err.Error())
return
Expand Down Expand Up @@ -114,7 +114,7 @@ func ExampleConnectionPool_SelectTyped() {
return
}
// Insert a new tuple {"key2", "value2"}.
_, err = conn.Insert(spaceName, &Tuple{Key: "key2", Value: "value2"})
_, err = conn.Insert(spaceName, &Tuple{Key: "key2", Value: "value2"})
if err != nil {
fmt.Printf("Failed to insert: %s", err.Error())
return
Expand Down Expand Up @@ -176,7 +176,7 @@ func ExampleConnectionPool_SelectAsync() {
return
}
// Insert a new tuple {"key2", "value2"}.
_, err = conn.Insert(spaceName, &Tuple{Key: "key2", Value: "value2"})
_, err = conn.Insert(spaceName, &Tuple{Key: "key2", Value: "value2"})
if err != nil {
fmt.Printf("Failed to insert: %s", err.Error())
return
Expand All @@ -196,7 +196,7 @@ func ExampleConnectionPool_SelectAsync() {
spaceName, indexName, 0, 1, tarantool.IterEq,
[]interface{}{"key2"}, connection_pool.RW)
futs[2] = pool.SelectAsync(
spaceName, indexName, 0, 1,tarantool.IterEq,
spaceName, indexName, 0, 1, tarantool.IterEq,
[]interface{}{"key3"}, connection_pool.RW)
var t []Tuple
err = futs[0].GetTyped(&t)
Expand Down
1 change: 1 addition & 0 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func ExampleConnection_SelectTyped() {
// response is [{{} 1111 hello world}]
}

//nolint
func ExampleConnection_SelectAsync() {
conn := example_connect()
defer conn.Close()
Expand Down
8 changes: 4 additions & 4 deletions multi/multi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@ var connOptsMulti = OptsMulti{
func TestConnError_IncorrectParams(t *testing.T) {
multiConn, err := Connect([]string{}, tarantool.Opts{})
if err == nil {
t.Errorf("err is nil with incorrect params")
t.Fatalf("err is nil with incorrect params")
}
if multiConn != nil {
t.Errorf("conn is not nill with incorrect params")
t.Fatalf("conn is not nill with incorrect params")
}
if err.Error() != "addrs should not be empty" {
t.Errorf("incorrect error: %s", err.Error())
}

multiConn, err = ConnectWithOpts([]string{server1}, tarantool.Opts{}, OptsMulti{})
if err == nil {
t.Errorf("err is nil with incorrect params")
t.Fatal("err is nil with incorrect params")
}
if multiConn != nil {
t.Errorf("conn is not nill with incorrect params")
t.Fatal("conn is not nill with incorrect params")
}
if err.Error() != "wrong check timeout, must be greater than 0" {
t.Errorf("incorrect error: %s", err.Error())
Expand Down
1 change: 1 addition & 0 deletions queue/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/tarantool/go-tarantool/queue"
)

//nolint
// Example demonstrates an operations like Put and Take with queue.
func Example_simpleQueue() {
cfg := queue.Cfg{
Expand Down
1 change: 1 addition & 0 deletions queue/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ type queueData struct {
result interface{}
}

//nolint
func (qd *queueData) DecodeMsgpack(d *msgpack.Decoder) error {
var err error
var l int
Expand Down
23 changes: 16 additions & 7 deletions queue/queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ func TestFifoQueue_Release(t *testing.T) {
func TestTtlQueue(t *testing.T) {
conn, err := Connect(server, opts)
if err != nil {
t.Errorf("Failed to connect: %s", err.Error())
t.Fatalf("Failed to connect: %s", err.Error())
return
}
defer conn.Close()
Expand Down Expand Up @@ -683,11 +683,11 @@ func TestTtlQueue(t *testing.T) {
func TestTtlQueue_Put(t *testing.T) {
conn, err := Connect(server, opts)
if err != nil {
t.Errorf("Failed to connect: %s", err.Error())
t.Fatalf("Failed to connect: %s", err.Error())
return
}
if conn == nil {
t.Errorf("conn is nil after Connect")
t.Fatalf("conn is nil after Connect")
return
}
defer conn.Close()
Expand Down Expand Up @@ -755,11 +755,11 @@ func TestTtlQueue_Put(t *testing.T) {
func TestUtube_Put(t *testing.T) {
conn, err := Connect(server, opts)
if err != nil {
t.Errorf("Failed to connect: %s", err.Error())
t.Fatalf("Failed to connect: %s", err.Error())
return
}
if conn == nil {
t.Errorf("conn is nil after Connect")
t.Fatalf("conn is nil after Connect")
return
}
defer conn.Close()
Expand Down Expand Up @@ -794,16 +794,22 @@ func TestUtube_Put(t *testing.T) {
t.Fatalf("Failed put task to queue: %s", err.Error())
}

errChan := make(chan struct{})
go func() {
t1, err := q.TakeTimeout(2 * time.Second)
if err != nil {
t.Fatalf("Failed to take task from utube: %s", err.Error())
t.Errorf("Failed to take task from utube: %s", err.Error())
errChan <- struct{}{}
return
}

time.Sleep(2 * time.Second)
if err := t1.Ack(); err != nil {
t.Fatalf("Failed to ack task: %s", err.Error())
t.Errorf("Failed to ack task: %s", err.Error())
errChan <- struct{}{}
return
}
close(errChan)
}()

time.Sleep(100 * time.Millisecond)
Expand All @@ -817,6 +823,9 @@ func TestUtube_Put(t *testing.T) {
t.Fatalf("Failed to ack task: %s", err.Error())
}
end := time.Now()
if _, ok := <-errChan; ok {
t.Fatalf("One of tasks failed")
}
if math.Abs(float64(end.Sub(start)-2*time.Second)) > float64(200*time.Millisecond) {
t.Fatalf("Blocking time is less than expected: actual = %.2fs, expected = 1s", end.Sub(start).Seconds())
}
Expand Down
1 change: 1 addition & 0 deletions response.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type Response struct {
buf smallBuf
}

//nolint: unused
func (resp *Response) fill(b []byte) {
resp.buf.b = b
}
Expand Down
1 change: 1 addition & 0 deletions schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type IndexField struct {
Type string
}

//nolint: varcheck,deadcode
const (
maxSchemas = 10000
spaceSpId = 280
Expand Down
Loading

0 comments on commit 80d8efb

Please sign in to comment.