mirror of
https://github.com/hanzoai/tasks.git
synced 2026-11-03 03:56:04 +00:00
Tim Deeb-SwihartandGitHub
1b08ce8984
## What changed?
I updated our linter rules to forbid the usage of time.Sleep in new
tests.
Note that our linters can be ignored but _please don't_ :(
## Why?
This has been a source of race conditions and flaky tests historically.
If you've a good reason to use `time.Sleep` in your test (we have some)
then you can override and/or ignore this, but you should otherwise
default to using `require.Eventually` or `assert.Eventually`.
## How did you test it?
I applied the following diff:
```diff
diff --git a/service/fx.go b/service/fx.go
index 3b024069c..52cf3e4bf 100644
--- a/service/fx.go
+++ b/service/fx.go
@@ -26,6 +26,7 @@ package service
import (
"sync/atomic"
+ "time"
"go.uber.org/fx"
"google.golang.org/grpc"
@@ -168,6 +169,8 @@ func getUnaryInterceptors(params GrpcServerOptionsParams) []grpc.UnaryServerInte
interceptors = append(interceptors, params.AdditionalInterceptors...)
+ time.Sleep(12 * time.Second)
+
return append(
interceptors,
params.RateLimitInterceptor.Intercept,
diff --git a/tests/dlq_test.go b/tests/dlq_test.go
index fd0546120..5c264b28f 100644
--- a/tests/dlq_test.go
+++ b/tests/dlq_test.go
@@ -27,6 +27,7 @@ package tests
import (
"flag"
"testing"
+ "time"
"github.com/stretchr/testify/suite"
)
@@ -34,4 +35,5 @@ import (
func TestDLQSuite(t *testing.T) {
flag.Parse()
suite.Run(t, new(DLQSuite))
+ time.Sleep(1)
}
```
Then ran our linters as follows to make sure it only complained about
the time.Sleep I added in test code:
```
$ make GOLANGCI_LINT_FIX=false GOLANGCI_LINT_BASE_REV=HEAD~ lint-code
Linting code...
...
INFO [runner] linters took 730.646834ms with stages: goanalysis_metalinter: 338.380792ms
tests/dlq_test.go:38:2: use of `time.Sleep` forbidden because "Please use require.Eventually or assert.Eventually instead unless you've no other option" (forbidigo)
time.Sleep(1)
^
INFO File cache stats: 1 entries of total size 1.4KiB
INFO Memory: 23 samples, avg is 52.3MB, max is 89.1MB
INFO Execution took 2.124820459s
make: *** [Makefile:333: lint-code] Error 1
```
126 lines
3.2 KiB
YAML
126 lines
3.2 KiB
YAML
# https://golangci-lint.run/usage/configuration/#config-file
|
|
linters:
|
|
disable-all: true
|
|
enable:
|
|
- goerr113
|
|
- errcheck
|
|
- goimports
|
|
# - paralleltest # missing the call to method parallel, but testify does not seem to work well with parallel test: https://github.com/stretchr/testify/issues/187
|
|
- revive # revive supersedes golint, which is now archived
|
|
- staticcheck
|
|
- vet
|
|
- forbidigo
|
|
- exhaustive
|
|
- godox
|
|
linters-settings:
|
|
godox:
|
|
keywords:
|
|
- FIXME # marks TODOs that must be fixed before merging
|
|
govet:
|
|
fieldalignment: 0
|
|
forbidigo:
|
|
forbid:
|
|
- p: time.Sleep
|
|
msg: "Please use require.Eventually or assert.Eventually instead unless you've no other option"
|
|
- p: ^time\.After$
|
|
msg: "time.After may leak resources. Use time.NewTimer instead."
|
|
revive:
|
|
severity: error
|
|
confidence: 0.8
|
|
enable-all-rules: true
|
|
rules:
|
|
# Disabled rules
|
|
- name: add-constant
|
|
disabled: true
|
|
- name: argument-limit
|
|
disabled: true
|
|
- name: bare-return
|
|
disabled: true
|
|
- name: banned-characters
|
|
disabled: true
|
|
- name: bool-literal-in-expr
|
|
disabled: true
|
|
- name: confusing-naming
|
|
disabled: true
|
|
- name: empty-lines
|
|
disabled: true
|
|
- name: error-naming
|
|
disabled: true
|
|
- name: errorf
|
|
disabled: true
|
|
- name: exported
|
|
disabled: true
|
|
- name: file-header
|
|
disabled: true
|
|
- name: function-length
|
|
disabled: true
|
|
- name: imports-blacklist
|
|
disabled: true
|
|
- name: increment-decrement
|
|
disabled: true
|
|
- name: line-length-limit
|
|
disabled: true
|
|
- name: max-public-structs
|
|
disabled: true
|
|
- name: nested-structs
|
|
disabled: true
|
|
- name: package-comments
|
|
disabled: true
|
|
- name: string-format
|
|
disabled: true
|
|
- name: unexported-naming
|
|
disabled: true
|
|
- name: unexported-return
|
|
disabled: true
|
|
- name: unused-parameter
|
|
disabled: true
|
|
- name: unused-receiver
|
|
disabled: true
|
|
- name: use-any
|
|
disabled: true
|
|
- name: var-naming
|
|
disabled: true
|
|
- name: empty-block
|
|
disabled: true
|
|
- name: flag-parameter
|
|
disabled: true
|
|
- name: unnecessary-stmt
|
|
disabled: true
|
|
|
|
# Rule tuning
|
|
- name: cognitive-complexity
|
|
arguments:
|
|
- 25
|
|
- name: cyclomatic
|
|
arguments:
|
|
- 25
|
|
- name: function-result-limit
|
|
arguments:
|
|
- 5
|
|
- name: unhandled-error
|
|
arguments:
|
|
- "fmt.*"
|
|
- "bytes.Buffer.*"
|
|
- "strings.Builder.*"
|
|
issues:
|
|
exclude-dirs:
|
|
- ^api
|
|
- ^proto
|
|
- ^.git
|
|
exclude-rules:
|
|
- path-except: _test\.go|tests/.+\.go
|
|
text: "time.Sleep"
|
|
linters:
|
|
- forbidigo
|
|
- path: _test\.go|tests/.+\.go
|
|
text: "(cyclomatic|cognitive)" # false positives when using subtests
|
|
linters:
|
|
- revive
|
|
- path: _test\.go|tests/.+\.go
|
|
linters:
|
|
- goerr113 # like err = errors.New("test error")
|
|
- path: ^tools\/.+\.go
|
|
linters:
|
|
- goerr113
|
|
- revive
|