Merge pull request #113 from bojand/golint

Improve makefile and add golangci-lint

.circleci/config.yml

@@ -5,6 +5,7 @@ jobs:
       - image: circleci/golang:1.11
     steps:
       - checkout
+      - run: make setup
       - run: make
 workflows:
   version: 2

.golangci.yml

@@ -0,0 +1,16 @@
+# TODO Look into enabling all with some exception where practical
+# linters:
+#   enable-all: true
+#   disable:
+#     - gochecknoglobals
+#     - dupl
+#     - lll
+issues:
+  exclude-rules:
+    # Exclude lostcancel govet rule specifically for requester.go
+    # Since we purpocefully do that. See comments in code.
+    - path: runner/requester.go
+      text: "lostcancel"
+    # TODO Look into fixing time.Tick() usage SA1015 in worker.go
+    - path: runner/worker.go
+      text: "SA1015"

Makefile

@@ -61,6 +61,8 @@ unexport GOROOT
 export GO111MODULE := on
 # Set the location where we install Golang binaries via go install to TMP_BIN.
 export GOBIN := $(abspath $(TMP_BIN))
+# Set BINDIR location needed by golangci-lint install script, otherwise it goes to ./bin
+export BINDIR := $(GOBIN)
 # Add GOBIN to to the front of the PATH. This allows us to invoke binaries we install.
 export PATH := :$(GOBIN):$(PATH)
 
@@ -70,6 +72,12 @@ GO_MODULE := $(shell grep '^module ' go.mod | cut -f 2 -d ' ')
 # Run all by default when "make" is invoked.
 .DEFAULT_GOAL := all
 
+# Install all the build and lint dependencies
+setup:
+	curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh
+	go mod download
+.PHONY: setup
+
 # All runs the default lint, test, and code coverage targets.
 .PHONY: all
 all: lint cover
@@ -79,34 +87,21 @@ all: lint cover
 clean:
 	rm -rf $(TMP_BASE)
 
-# Golint runs the golint linter.
-.PHONY: golint
-golint:
-	$(AT) go install golang.org/x/lint/golint
-	golint -set_exit_status $(GO_PKGS)
-
-# Errcheck runs the errcheck linter.
-.PHONY: errcheck
-errcheck:
-	$(AT) go install github.com/kisielk/errcheck
-	errcheck -ignoretests $(GO_PKGS)
-
-# Staticcheck runs the staticcheck linter.
-.PHONY: staticcheck
-staticcheck:
-	$(AT) go install honnef.co/go/tools/cmd/staticcheck
-	staticcheck --tests=false $(GO_PKGS)
-
 # Lint runs all linters. This is the main lint target to run.
-# TODO: add errcheck and staticcheck when the code is updated to pass them
 .PHONY: lint
-lint: golint errcheck
+lint: 
+	golangci-lint run ./...
 
 # Test runs go test on GO_PKGS. This does not produce code coverage.
 .PHONY: test
 test:
 	go test $(GO_TEST_FLAGS) $(GO_PKGS)
 
+# gofmt and goimports all go files
+fmt:
+	find . -name '*.go' -not -wholename './vendor/*' | while read -r file; do gofmt -w -s "$$file"; goimports -w "$$file"; done
+.PHONY: fmt
+
 # Cover runs go_test on GO_PKGS and produces code coverage in multiple formats.
 # A coverage.html file for human viewing will be at $(TMP_COVERAGE)/coverage.html
 # This target will echo "open $(TMP_COVERAGE)/coverage.html" with TMP_COVERAGE

internal/helloworld/greeter_server.go

@@ -35,7 +35,7 @@ type Greeter struct {
 
 	mutex      *sync.RWMutex
 	callCounts map[CallType]int
-	calls map[CallType][][]*HelloRequest
+	calls      map[CallType][][]*HelloRequest
 }
 
 func randomSleep() {
@@ -188,10 +188,10 @@ func (s *Greeter) GetConnectionCount() int {
 // NewGreeter creates new greeter server
 func NewGreeter() *Greeter {
 	streamData := []*HelloReply{
-		&HelloReply{Message: "Hello Bob"},
-		&HelloReply{Message: "Hello Kate"},
-		&HelloReply{Message: "Hello Jim"},
-		&HelloReply{Message: "Hello Sara"},
+		{Message: "Hello Bob"},
+		{Message: "Hello Kate"},
+		{Message: "Hello Jim"},
+		{Message: "Hello Sara"},
 	}
 
 	greeter := &Greeter{streamData: streamData, mutex: &sync.RWMutex{}}
@@ -245,4 +245,4 @@ func (c *HWStatsHandler) HandleRPC(ctx context.Context, rs stats.RPCStats) {
 // TagRPC implements per-RPC context management.
 func (c *HWStatsHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) context.Context {
 	return ctx
-}
+}

printer/printer.go

@@ -269,7 +269,7 @@ func jsonify(v interface{}, pretty bool) string {
 		return string(d)
 	}
 
-	return string(out.Bytes())
+	return out.String()
 }
 
 func formatNanoUnit(d time.Duration) string {

printer/printer_test.go

@@ -57,59 +57,59 @@ func TestPrinter_getInfluxLine(t *testing.T) {
 					},
 				},
 				LatencyDistribution: []runner.LatencyDistribution{
-					runner.LatencyDistribution{
+					{
 						Percentage: 25,
 						Latency:    time.Duration(1 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 50,
 						Latency:    time.Duration(5 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 75,
 						Latency:    time.Duration(10 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 90,
 						Latency:    time.Duration(15 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 95,
 						Latency:    time.Duration(20 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 99,
 						Latency:    time.Duration(25 * time.Millisecond),
 					}},
 				Histogram: []runner.Bucket{
-					runner.Bucket{
+					{
 						Mark:      0.01,
 						Count:     1,
 						Frequency: 0.005,
 					},
-					runner.Bucket{
+					{
 						Mark:      0.02,
 						Count:     10,
 						Frequency: 0.01,
 					},
-					runner.Bucket{
+					{
 						Mark:      0.03,
 						Count:     50,
 						Frequency: 0.1,
 					},
-					runner.Bucket{
+					{
 						Mark:      0.05,
 						Count:     60,
 						Frequency: 0.15,
 					},
-					runner.Bucket{
+					{
 						Mark:      0.1,
 						Count:     15,
 						Frequency: 0.07,
 					},
 				},
 				Details: []runner.ResultDetail{
-					runner.ResultDetail{
+					{
 						Timestamp: date,
 						Latency:   time.Duration(1 * time.Millisecond),
 						Status:    "OK",
@@ -176,59 +176,59 @@ func TestPrinter_printInfluxDetails(t *testing.T) {
 					},
 				},
 				LatencyDistribution: []runner.LatencyDistribution{
-					runner.LatencyDistribution{
+					{
 						Percentage: 25,
 						Latency:    time.Duration(1 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 50,
 						Latency:    time.Duration(5 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 75,
 						Latency:    time.Duration(10 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 90,
 						Latency:    time.Duration(15 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 95,
 						Latency:    time.Duration(20 * time.Millisecond),
 					},
-					runner.LatencyDistribution{
+					{
 						Percentage: 99,
 						Latency:    time.Duration(25 * time.Millisecond),
 					}},
 				Histogram: []runner.Bucket{
-					runner.Bucket{
+					{
 						Mark:      0.01,
 						Count:     1,
 						Frequency: 0.005,
 					},
-					runner.Bucket{
+					{
 						Mark:      0.02,
 						Count:     10,
 						Frequency: 0.01,
 					},
-					runner.Bucket{
+					{
 						Mark:      0.03,
 						Count:     50,
 						Frequency: 0.1,
 					},
-					runner.Bucket{
+					{
 						Mark:      0.05,
 						Count:     60,
 						Frequency: 0.15,
 					},
-					runner.Bucket{
+					{
 						Mark:      0.1,
 						Count:     15,
 						Frequency: 0.07,
 					},
 				},
 				Details: []runner.ResultDetail{
-					runner.ResultDetail{
+					{
 						Timestamp: date,
 						Latency:   time.Duration(1 * time.Millisecond),
 						Status:    "OK",
@@ -244,7 +244,8 @@ func TestPrinter_printInfluxDetails(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			buf := bytes.NewBufferString("")
 			p := ReportPrinter{Report: &tt.report, Out: buf}
-			p.printInfluxDetails()
+			err := p.printInfluxDetails()
+			assert.NoError(t, err)
 			actual := buf.String()
 			assert.Equal(t, tt.expected, actual)
 		})

runner/data_test.go

@@ -2,10 +2,11 @@ package runner
 
 import (
 	"encoding/json"
-	"github.com/bojand/ghz/testdata"
-	"github.com/golang/protobuf/proto"
 	"testing"
 
+	"github.com/golang/protobuf/proto"
+
+	"github.com/bojand/ghz/internal/helloworld"
 	"github.com/bojand/ghz/protodesc"
 
 	"github.com/stretchr/testify/assert"
@@ -208,6 +209,7 @@ func TestData_createPayloads(t *testing.T) {
 		msg1.Name = "bob"
 
 		binData, err := proto.Marshal(msg1)
+		assert.NoError(t, err)
 
 		inputs, err := createPayloadsFromBin(binData, mtdUnary)
 

runner/options.go

@@ -517,7 +517,7 @@ func createClientTransportCredentials(skipVerify bool, cacertFile, clientCertFil
 		tlsConf.Certificates = []tls.Certificate{certificate}
 	}
 
-	if skipVerify == true {
+	if skipVerify {
 		tlsConf.InsecureSkipVerify = true
 	} else if cacertFile != "" {
 		// Create a certificate pool from the certificate authority

runner/reporter.go

@@ -144,7 +144,7 @@ func (r *Reporter) Run() {
 			errStr = res.err.Error()
 			r.errorDist[errStr]++
 		}
-		
+
 		if len(r.details) < maxResult {
 			r.details = append(r.details, ResultDetail{
 				Latency:   res.duration,

runner/requester.go

@@ -200,7 +200,7 @@ func (b *Requester) Stop(reason StopReason) {
 // Finish finishes the test run
 func (b *Requester) Finish() *Report {
 	close(b.results)
-	total := time.Now().Sub(b.start)
+	total := time.Since(b.start)
 
 	// Wait until the reporter is done.
 	<-b.reporter.done