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

Configured golangci-lint #51

Merged
merged 6 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ jobs:
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v2
with:
version: v1.37
version: latest
119 changes: 118 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,127 @@
# Configuration reference: https://golangci-lint.run/usage/configuration/
# Linters reference: https://golangci-lint.run/usage/linters/
run:
build-tags:
- examples
skip-dirs:
- apiv6/localhost_apiserver/cloudexport
- apiv6/localhost_apiserver/synthetics

issues:
max-issues-per-linter: 0
max-same-issues: 0
# TODO(dfurman): enable more linters
exclude:
# EXC0002 golint: Annoying issue about not having a comment. The rare codebase has such comments
- (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)
- (struct (of|with)|could be)
palczyn marked this conversation as resolved.
Show resolved Hide resolved
exclude-use-default: false
exclude-rules: # exclude linters impossible to exclude via //nolint
- path: ^kentikapi/models/enum_ # these files are generated and shouldn't be edited with //nolint
linters:
- gochecknoglobals
- lll
- gomnd

# Disabled linters:
# - cyclop - duplicates functionality of gocyclo
# - exhaustivestruct - breaks "Make the zero value useful" proverb, meant to be used only for special cases
# - funlen - only test functions exceeds the line limit of 60 - too strict, these functions are easily readable
palczyn marked this conversation as resolved.
Show resolved Hide resolved
# - godox - requires all TODOs to be removed - too strict
# - gomoddirectives - does not allow "replace" directives - too strict
# - goerr113 - following check is too strict: "do not define dynamic errors, use wrapped static errors instead",
# the check cannot be disabled
# - interfacer - deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner
# - maligned - deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner
# - nlreturn - leads to using too many line breaks
# - prealloc - from docs:
palczyn marked this conversation as resolved.
Show resolved Hide resolved
# XXX: we don't recommend using this linter before doing performance profiling.
# For most programs usage of prealloc will be a premature optimization.
# - scopelint - deprecated (since v1.39.0) due to: The repository of the linter has been deprecated by the owner
# - thelper - enforcing t.Helper() everywhere is too strict
# - wrapcheck - valuable linter which I think needs higher level knowledge on the project to be fixed
palczyn marked this conversation as resolved.
Show resolved Hide resolved
# - wsl - leads to using too many line breaks
linters:
enable:
- asciicheck
- bodyclose
- deadcode
- depguard
- dogsled
- dupl
- durationcheck
- errcheck
- errorlint
- exhaustive
- exportloopref
- forbidigo
- forcetypeassert
- gci
- gochecknoglobals
- gochecknoinits
- gocognit
- goconst
- gocritic
- gocyclo
- godot
- gofmt
- gofumpt
- goheader
- goimports
- revive
palczyn marked this conversation as resolved.
Show resolved Hide resolved
- gomnd
- gomodguard
- goprintffuncname
- gosec
- gosimple
- govet
- ifshort
- importas
- ineffassign
- lll
- makezero
- misspell
- nakedret
- nestif
- nilerr
- noctx
- nolintlint
- paralleltest
- predeclared
- revive
- rowserrcheck
- sqlclosecheck
- staticcheck
- structcheck
- stylecheck
- testpackage
- tparallel
- typecheck
- unconvert
- unparam
- unused
- varcheck
- wastedassign
- whitespace

linters-settings:
dupl:
tests: false
palczyn marked this conversation as resolved.
Show resolved Hide resolved
errcheck:
check-type-assertions: true
check-blank: true
errorlint:
# Check whether fmt.Errorf uses the %w verb for formatting errors - too strict
errorf: false
gocyclo:
min-complexity: 11
palczyn marked this conversation as resolved.
Show resolved Hide resolved
golint:
palczyn marked this conversation as resolved.
Show resolved Hide resolved
min-confidence: 0
gosec:
tests: false
palczyn marked this conversation as resolved.
Show resolved Hide resolved
govet:
enable-all: true
lll:
# lines longer than 127 will be reported, 120 is default
palczyn marked this conversation as resolved.
Show resolved Hide resolved
line-length: 127
nakedret:
max-func-lines: 5
2 changes: 2 additions & 0 deletions apiv6/kentikapi/client.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//nolint:revive,stylecheck // Changing Api to API forces changes in generated files
palczyn marked this conversation as resolved.
Show resolved Hide resolved
package kentikapi

import (
Expand All @@ -6,6 +7,7 @@ import (
"github.com/kentik/community_sdk_golang/apiv6/kentikapi/synthetics"
)

//nolint:gosec
const (
authAPITokenKey = "X-CH-Auth-API-Token"
authEmailKey = "X-CH-Auth-Email"
Expand Down
62 changes: 39 additions & 23 deletions apiv6/kentikapi/client_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/stretchr/testify/assert"
)

//nolint:gosec
const (
authEmailKey = "X-CH-Auth-Email"
authAPITokenKey = "X-CH-Auth-API-Token"
Expand All @@ -24,7 +25,10 @@ const (
testAgentID = "968"
)

//nolint:errcheck // Conflict with defer Close() (additional info: https://github.com/kisielk/errcheck/issues/55)
palczyn marked this conversation as resolved.
Show resolved Hide resolved
func TestClient_PatchAgent(t *testing.T) {
t.Parallel()

tests := []struct {
name string
retryMax *int
Expand Down Expand Up @@ -160,7 +164,10 @@ func TestClient_PatchAgent(t *testing.T) {
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
palczyn marked this conversation as resolved.
Show resolved Hide resolved

// arrange
h := newSpyHTTPHandler(t, tt.responses)
s := httptest.NewServer(h)
Expand All @@ -185,6 +192,7 @@ func TestClient_PatchAgent(t *testing.T) {
AgentPatch(context.Background(), testAgentID).
Body(tt.request).
Execute()
defer httpResp.Body.Close()

// assert
t.Logf("Got result: %v, httpResp: %v, err: %v", result, httpResp, err)
Expand All @@ -197,7 +205,7 @@ func TestClient_PatchAgent(t *testing.T) {
assert.Equal(t, len(h.responses), len(h.requests), "invalid number of requests")
for _, r := range h.requests {
assert.Equal(t, http.MethodPatch, r.method)
assert.Equal(t, fmt.Sprintf("/synthetics/v202101beta1/agents/%v", testAgentID), r.url_.Path)
assert.Equal(t, fmt.Sprintf("/synthetics/v202101beta1/agents/%v", testAgentID), r.url.Path)
assert.Equal(t, dummyAuthEmail, r.header.Get(authEmailKey))
assert.Equal(t, dummyAuthToken, r.header.Get(authAPITokenKey))
assert.Equal(t, tt.expectedRequestBody, unmarshalJSONToIf(t, r.body))
Expand Down Expand Up @@ -225,26 +233,34 @@ func newDummyAgent() *synthetics.V202101beta1Agent {
status := synthetics.V202101BETA1AGENTSTATUS_WAIT
family := synthetics.V202101BETA1IPFAMILY_DUAL
agent := &synthetics.V202101beta1Agent{
Id: stringPtr(testAgentID),
Name: stringPtr("dummy-agent"),
Status: &status,
Alias: stringPtr("probe-4-ams-1"),
Type: stringPtr("global"),
Os: stringPtr("I use Manjaro BTW"),
Ip: stringPtr("95.179.136.58"),
Lat: float64Ptr(52.374031),
Long: float64Ptr(4.88969),
LastAuthed: timePtr(time.Date(2020, time.July, 9, 21, 37, 00, 826*1000000, time.UTC)),
Family: &family,
Asn: int64Ptr(20473),
SiteId: stringPtr("2137"),
Version: stringPtr("0.0.2"),
Challenge: stringPtr("dummy-challenge"),
City: stringPtr("Amsterdam"),
Region: stringPtr("Noord-Holland"),
Country: stringPtr("Netherlands"),
TestIds: &[]string{"13", "133", "1337"},
LocalIp: stringPtr("10.10.10.10"),
Id: stringPtr(testAgentID),
Name: stringPtr("dummy-agent"),
Status: &status,
Alias: stringPtr("probe-4-ams-1"),
Type: stringPtr("global"),
Os: stringPtr("I use Manjaro BTW"),
Ip: stringPtr("95.179.136.58"),
Lat: float64Ptr(52.374031),
Long: float64Ptr(4.88969),
LastAuthed: timePtr(time.Date(2020,
time.July,
9,
21,
37,
0,
826*1000000,
time.UTC,
)),
Family: &family,
Asn: int64Ptr(20473),
SiteId: stringPtr("2137"),
Version: stringPtr("0.0.2"),
Challenge: stringPtr("dummy-challenge"),
City: stringPtr("Amsterdam"),
Region: stringPtr("Noord-Holland"),
Country: stringPtr("Netherlands"),
TestIds: &[]string{"13", "133", "1337"},
LocalIp: stringPtr("10.10.10.10"),
}

return agent
Expand Down Expand Up @@ -336,7 +352,7 @@ func (h *spyHTTPHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {

h.requests = append(h.requests, httpRequest{
method: r.Method,
url_: r.URL,
url: r.URL,
header: r.Header,
body: string(body),
})
Expand Down Expand Up @@ -364,7 +380,7 @@ func (h *spyHTTPHandler) response() httpResponse {

type httpRequest struct {
method string
url_ *url.URL
url *url.URL
header http.Header
body string
}
Expand Down
1 change: 1 addition & 0 deletions apiv6/kentikapi/httputil/httputil.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func (cfg *ClientConfig) FillDefaults() {
}
}

//nolint:gomnd // This is the only place for these numbers to turn up.
func defaultHTTPClient() *http.Client {
return &http.Client{
Transport: &http.Transport{
Expand Down
30 changes: 24 additions & 6 deletions apiv6/kentikapi/httputil/httputil_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package httputil
package httputil_test

import (
"context"
Expand All @@ -10,7 +10,7 @@ import (
"time"

"github.com/hashicorp/go-retryablehttp"

"github.com/kentik/community_sdk_golang/apiv6/kentikapi/httputil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand All @@ -22,15 +22,23 @@ import (
// 4. retryingClient.retryableRoundTripper.retryableClient.httpClient.Do()
// 5. retryingClient.retryableRoundTripper.retryableClient.httpClient.httpTransport.RoundTrip()

//nolint:errcheck // https://github.com/kisielk/errcheck/issues/55
// Closing a response you only read from cannot yield a meaningful error.
palczyn marked this conversation as resolved.
Show resolved Hide resolved
func TestRetryingClient_Do_ReturnsHTTPTransportError(t *testing.T) {
t.Parallel()

// arrange
c := NewRetryingClient(ClientConfig{})
c := httputil.NewRetryingClient(httputil.ClientConfig{})

req, err := retryablehttp.NewRequest(http.MethodGet, "https://invalid.url", nil)
require.NoError(t, err)

// act
resp, err := c.Do(req.WithContext(context.Background()))
if err != nil {
return
palczyn marked this conversation as resolved.
Show resolved Hide resolved
}
defer resp.Body.Close()

// assert
t.Logf("Got response: %v, err: %v", resp, err)
Expand All @@ -40,7 +48,11 @@ func TestRetryingClient_Do_ReturnsHTTPTransportError(t *testing.T) {
assert.Equal(t, "no such host", dnsErr.Err)
}

//nolint:errcheck // https://github.com/kisielk/errcheck/issues/55
palczyn marked this conversation as resolved.
Show resolved Hide resolved
// Closing a response you only read from cannot yield a meaningful error.
func TestRetryingClientWithSpyHTTPTransport_Do(t *testing.T) {
t.Parallel()

const retryMax = 5

tests := []struct {
Expand Down Expand Up @@ -73,15 +85,17 @@ func TestRetryingClientWithSpyHTTPTransport_Do(t *testing.T) {
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
// arrange
t.Parallel()

// arrange
st := spyTransport{transportError: tt.transportError}
c := NewRetryingClient(ClientConfig{
c := httputil.NewRetryingClient(httputil.ClientConfig{
HTTPClient: &http.Client{
Transport: &st,
},
RetryCfg: RetryConfig{
RetryCfg: httputil.RetryConfig{
MaxAttempts: intPtr(retryMax),
MinDelay: durationPtr(1 * time.Microsecond),
MaxDelay: durationPtr(10 * time.Microsecond),
Expand All @@ -93,6 +107,10 @@ func TestRetryingClientWithSpyHTTPTransport_Do(t *testing.T) {

// act
resp, err := c.Do(req.WithContext(context.Background()))
if err != nil {
palczyn marked this conversation as resolved.
Show resolved Hide resolved
return
}
defer resp.Body.Close()

// assert
t.Logf("Got response: %v, err: %v", resp, err)
Expand Down
Loading