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

Add more linters #382

Merged
merged 1 commit into from
Sep 24, 2024
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
11 changes: 11 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,33 @@ linters-settings:
ignore-generated-header: true
rules:
- name: blank-imports
- name: constant-logical-expr
- name: context-as-argument
- name: context-keys-type
- name: defer
- name: dot-imports
- name: duplicated-imports
- name: empty-block
- name: error-naming
- name: error-return
- name: error-strings
- name: errorf
- name: exported
- name: import-shadowing
- name: increment-decrement
- name: indent-error-flow
- name: package-comments
- name: range
- name: range-val-address
- name: range-val-in-closure
- name: receiver-naming
- name: redefines-builtin-id
- name: string-of-int
- name: superfluous-else
- name: time-naming
- name: unchecked-type-assertion
- name: unexported-return
- name: unnecessary-stmt
- name: unreachable-code
- name: unused-parameter
- name: var-declaration
Expand Down Expand Up @@ -60,6 +69,7 @@ linters:
- ineffassign
- intrange
- makezero
- mirror
- misspell
- musttag
- nilerr
Expand All @@ -68,6 +78,7 @@ linters:
- perfsprint
- prealloc
- predeclared
- paralleltest
- reassign
- revive
- staticcheck
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ GOLANGCI_LINT_VERSION = v1.61.0
test: unit-test test-integration test-integration-no-stream-block clean

lint:
docker run --pull always --rm -v $(shell pwd):/nginx-plus-go-client -w /nginx-plus-go-client -v $(shell go env GOCACHE):/cache/go -e GOCACHE=/cache/go -e GOLANGCI_LINT_CACHE=/cache/go -v $(shell go env GOPATH)/pkg:/go/pkg golangci/golangci-lint:$(GOLANGCI_LINT_VERSION) golangci-lint --color always run
go run github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) run --fix

unit-test:
go test -v -shuffle=on -race client/*.go
Expand Down
82 changes: 61 additions & 21 deletions client/nginx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func TestDetermineUpdates(t *testing.T) {
t.Parallel()
maxConns := 1
tests := []struct {
name string
updated []UpstreamServer
nginx []UpstreamServer
expectedToAdd []UpstreamServer
Expand Down Expand Up @@ -57,6 +58,7 @@ func TestDetermineUpdates(t *testing.T) {
Server: "10.0.0.2:80",
},
},
name: "replace all",
},
{
updated: []UpstreamServer{
Expand Down Expand Up @@ -95,6 +97,7 @@ func TestDetermineUpdates(t *testing.T) {
Server: "10.0.0.1:80",
},
},
name: "add and delete",
},
{
updated: []UpstreamServer{
Expand All @@ -119,6 +122,7 @@ func TestDetermineUpdates(t *testing.T) {
Server: "10.0.0.3:80",
},
},
name: "same",
},
{
// empty values
Expand Down Expand Up @@ -153,21 +157,26 @@ func TestDetermineUpdates(t *testing.T) {
MaxConns: &maxConns,
},
},
name: "update field and delete",
},
}

for _, test := range tests {
toAdd, toDelete, toUpdate := determineUpdates(test.updated, test.nginx)
if !reflect.DeepEqual(toAdd, test.expectedToAdd) || !reflect.DeepEqual(toDelete, test.expectedToDelete) || !reflect.DeepEqual(toUpdate, test.expectedToUpdate) {
t.Errorf("determineUpdates(%v, %v) = (%v, %v, %v)", test.updated, test.nginx, toAdd, toDelete, toUpdate)
}
t.Run(test.name, func(t *testing.T) {
t.Parallel()
toAdd, toDelete, toUpdate := determineUpdates(test.updated, test.nginx)
if !reflect.DeepEqual(toAdd, test.expectedToAdd) || !reflect.DeepEqual(toDelete, test.expectedToDelete) || !reflect.DeepEqual(toUpdate, test.expectedToUpdate) {
t.Errorf("determineUpdates(%v, %v) = (%v, %v, %v)", test.updated, test.nginx, toAdd, toDelete, toUpdate)
}
})
}
}

func TestStreamDetermineUpdates(t *testing.T) {
t.Parallel()
maxConns := 1
tests := []struct {
name string
updated []StreamUpstreamServer
nginx []StreamUpstreamServer
expectedToAdd []StreamUpstreamServer
Expand Down Expand Up @@ -211,6 +220,7 @@ func TestStreamDetermineUpdates(t *testing.T) {
Server: "10.0.0.2:80",
},
},
name: "replace all",
},
{
updated: []StreamUpstreamServer{
Expand Down Expand Up @@ -249,6 +259,7 @@ func TestStreamDetermineUpdates(t *testing.T) {
Server: "10.0.0.1:80",
},
},
name: "add and delete",
},
{
updated: []StreamUpstreamServer{
Expand Down Expand Up @@ -276,6 +287,7 @@ func TestStreamDetermineUpdates(t *testing.T) {
Server: "10.0.0.3:80",
},
},
name: "same",
},
{
// empty values
Expand Down Expand Up @@ -310,14 +322,18 @@ func TestStreamDetermineUpdates(t *testing.T) {
MaxConns: &maxConns,
},
},
name: "update field and delete",
},
}

for _, test := range tests {
toAdd, toDelete, toUpdate := determineStreamUpdates(test.updated, test.nginx)
if !reflect.DeepEqual(toAdd, test.expectedToAdd) || !reflect.DeepEqual(toDelete, test.expectedToDelete) || !reflect.DeepEqual(toUpdate, test.expectedToUpdate) {
t.Errorf("determiteUpdates(%v, %v) = (%v, %v, %v)", test.updated, test.nginx, toAdd, toDelete, toUpdate)
}
t.Run(test.name, func(t *testing.T) {
t.Parallel()
toAdd, toDelete, toUpdate := determineStreamUpdates(test.updated, test.nginx)
if !reflect.DeepEqual(toAdd, test.expectedToAdd) || !reflect.DeepEqual(toDelete, test.expectedToDelete) || !reflect.DeepEqual(toUpdate, test.expectedToUpdate) {
t.Errorf("determiteUpdates(%v, %v) = (%v, %v, %v)", test.updated, test.nginx, toAdd, toDelete, toUpdate)
}
})
}
}

Expand Down Expand Up @@ -367,16 +383,20 @@ func TestAddPortToServer(t *testing.T) {
}

for _, test := range tests {
result := addPortToServer(test.address)
if result != test.expected {
t.Errorf("addPortToServer(%v) returned %v but expected %v for %v", test.address, result, test.expected, test.msg)
}
t.Run(test.msg, func(t *testing.T) {
t.Parallel()
result := addPortToServer(test.address)
if result != test.expected {
t.Errorf("addPortToServer(%v) returned %v but expected %v for %v", test.address, result, test.expected, test.msg)
}
})
}
}

func TestHaveSameParameters(t *testing.T) {
t.Parallel()
tests := []struct {
msg string
server UpstreamServer
serverNGX UpstreamServer
expected bool
Expand All @@ -385,11 +405,13 @@ func TestHaveSameParameters(t *testing.T) {
server: UpstreamServer{},
serverNGX: UpstreamServer{},
expected: true,
msg: "empty",
},
{
server: UpstreamServer{ID: 2},
serverNGX: UpstreamServer{ID: 3},
expected: true,
msg: "different ID",
},
{
server: UpstreamServer{},
Expand All @@ -403,6 +425,7 @@ func TestHaveSameParameters(t *testing.T) {
Down: &defaultDown,
},
expected: true,
msg: "default values",
},
{
server: UpstreamServer{
Expand All @@ -428,35 +451,43 @@ func TestHaveSameParameters(t *testing.T) {
Down: &defaultDown,
},
expected: true,
msg: "same values",
},
{
server: UpstreamServer{SlowStart: "10s"},
serverNGX: UpstreamServer{},
expected: false,
msg: "different SlowStart",
},
{
server: UpstreamServer{},
serverNGX: UpstreamServer{SlowStart: "10s"},
expected: false,
msg: "different SlowStart 2",
},
{
server: UpstreamServer{SlowStart: "20s"},
serverNGX: UpstreamServer{SlowStart: "10s"},
expected: false,
msg: "different SlowStart 3",
},
}

for _, test := range tests {
result := haveSameParameters(test.server, test.serverNGX)
if result != test.expected {
t.Errorf("haveSameParameters(%v, %v) returned %v but expected %v", test.server, test.serverNGX, result, test.expected)
}
t.Run(test.msg, func(t *testing.T) {
t.Parallel()
result := haveSameParameters(test.server, test.serverNGX)
if result != test.expected {
t.Errorf("haveSameParameters(%v, %v) returned %v but expected %v", test.server, test.serverNGX, result, test.expected)
}
})
}
}

func TestHaveSameParametersForStream(t *testing.T) {
t.Parallel()
tests := []struct {
msg string
server StreamUpstreamServer
serverNGX StreamUpstreamServer
expected bool
Expand All @@ -465,11 +496,13 @@ func TestHaveSameParametersForStream(t *testing.T) {
server: StreamUpstreamServer{},
serverNGX: StreamUpstreamServer{},
expected: true,
msg: "empty",
},
{
server: StreamUpstreamServer{ID: 2},
serverNGX: StreamUpstreamServer{ID: 3},
expected: true,
msg: "different ID",
},
{
server: StreamUpstreamServer{},
Expand All @@ -483,6 +516,7 @@ func TestHaveSameParametersForStream(t *testing.T) {
Down: &defaultDown,
},
expected: true,
msg: "default values",
},
{
server: StreamUpstreamServer{
Expand All @@ -508,24 +542,30 @@ func TestHaveSameParametersForStream(t *testing.T) {
Down: &defaultDown,
},
expected: true,
msg: "same values",
},
{
server: StreamUpstreamServer{},
serverNGX: StreamUpstreamServer{SlowStart: "10s"},
expected: false,
msg: "different SlowStart",
},
{
server: StreamUpstreamServer{SlowStart: "20s"},
serverNGX: StreamUpstreamServer{SlowStart: "10s"},
expected: false,
msg: "different SlowStart 2",
},
}

for _, test := range tests {
result := haveSameParametersForStream(test.server, test.serverNGX)
if result != test.expected {
t.Errorf("haveSameParametersForStream(%v, %v) returned %v but expected %v", test.server, test.serverNGX, result, test.expected)
}
t.Run(test.msg, func(t *testing.T) {
t.Parallel()
result := haveSameParametersForStream(test.server, test.serverNGX)
if result != test.expected {
t.Errorf("haveSameParametersForStream(%v, %v) returned %v but expected %v", test.server, test.serverNGX, result, test.expected)
}
})
}
}

Expand Down Expand Up @@ -665,9 +705,9 @@ func TestClientWithMaxAPI(t *testing.T) {
}

func TestGetStats_NoStreamEndpoint(t *testing.T) {
t.Parallel()
var writeLock sync.Mutex

t.Parallel()
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
writeLock.Lock()
defer writeLock.Unlock()
Expand Down
1 change: 1 addition & 0 deletions tests/client_no_stream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
// The API returns a special error code that we can use to determine if the API
// is misconfigured or of the stream block is missing.
func TestStatsNoStream(t *testing.T) {
t.Parallel()
c, err := client.NewNginxClient(helpers.GetAPIEndpoint())
if err != nil {
t.Fatalf("Error connecting to nginx: %v", err)
Expand Down
Loading