Skip to content

Dockerfile: update golangci-lint to v1.64.5, replace deprecated tenv linter in favor of usetesting - #5835

Merged
thaJeztah merged 4 commits into
docker:masterfrom
thaJeztah:bump_golangci_lint
Feb 18, 2025
Merged

Dockerfile: update golangci-lint to v1.64.5, replace deprecated tenv linter in favor of usetesting#5835
thaJeztah merged 4 commits into
docker:masterfrom
thaJeztah:bump_golangci_lint

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

Dockerfile: update golangci-lint to v1.64.5

adds go1.24 support

full diff: golangci/golangci-lint@v1.63.4...v1.64.5
changelog: https://golangci-lint.run/product/changelog/#v1645

cli/command/trust: fix "usetesting" linting errors

cli/command/trust/signer_add_test.go:71:18: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestSignerAddCommandNoTargetsKey (usetesting)
tmpfile, err := os.CreateTemp("", "pemfile")
^
cli/command/trust/signer_add_test.go:133:18: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestIngestPublicKeys (usetesting)
tmpfile, err := os.CreateTemp("", "pemfile")
^

cli/command/trust: fix "usetesting" linting errors

Also fix some unhandled errors

cli/debug/debug_test.go:12:3: os.Setenv() could be replaced by t.Setenv() in TestEnable (usetesting)
os.Setenv("DEBUG", "")
^

golangci-lint: replace deprecated tenv linter in favor of usetesting

WARN The linter 'tenv' is deprecated (since v1.64.0) due to: Duplicate feature another linter. Replaced by usetesting.

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

adds go1.24 support
full diff: golangci/golangci-lint@v1.63.4...v1.64.5
changelog: https://golangci-lint.run/product/changelog/#v1645
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
 cli/command/trust/signer_add_test.go:71:18: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestSignerAddCommandNoTargetsKey (usetesting)
tmpfile, err := os.CreateTemp("", "pemfile")
^
cli/command/trust/signer_add_test.go:133:18: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestIngestPublicKeys (usetesting)
tmpfile, err := os.CreateTemp("", "pemfile")
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also fix some unhandled errors
cli/debug/debug_test.go:12:3: os.Setenv() could be replaced by t.Setenv() in TestEnable (usetesting)
os.Setenv("DEBUG", "")
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter

codecov-commenter commented Feb 17, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.18%. Comparing base (88a019a) to head (a8affef).
Report is 42 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #5835 +/- ##
=======================================
Coverage 59.18% 59.18% =======================================
Files 352 352 Lines 29548 29548 =======================================
Hits 17487 17487 Misses 11080 11080 Partials 981 981 

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

Ah, derp, I thought I checked locally;

#15 [lint 4/4] RUN --mount=type=bind,target=. --mount=type=cache,target=/root/.cache golangci-lint run
#15 0.214 level=error msg="[formatter] gci: reading go.mod: read .: is a directory"
#15 68.48 cli/command/cli_test.go:85:26: context.Background() could be replaced by t.Context() in TestNewAPIClientFromFlagsWithCustomHeaders (usetesting)
#15 68.48 _, err = apiClient.Ping(context.Background())
#15 68.48 ^
#15 68.48 cli/command/cli_test.go:120:26: context.Background() could be replaced by t.Context() in TestNewAPIClientFromFlagsWithCustomHeadersFromEnv (usetesting)
#15 68.48 _, err = apiClient.Ping(context.Background())
#15 68.48 ^
#15 68.48 cli/command/cli_test.go:216:44: context.Background() could be replaced by t.Context() in TestInitializeFromClientHangs (usetesting)
#15 68.48 timeoutCtx, cancel := context.WithTimeout(context.Background(), time.Second)
#15 68.48 ^
#15 68.48 cli/command/utils_test.go:86:37: context.Background() could be replaced by t.Context() in TestPromptForInput (usetesting)
#15 68.48 ctx, cancel := context.WithCancel(context.Background())
#15 68.48 ^

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

Ah, I see what's happening; golangci-lint is assuming go1.24, but t.Context() was not yet available in go1.23; https://pkg.go.dev/testing#T.Context.

ISTR I had a patch to make sure it picks the right version of Go; let me check

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

Interesting; so that's already there in this repo, so why does it come with suggestions for go1.24?

cli/.golangci.yml

Lines 43 to 47 in 88a019a

run:
# prevent golangci-lint from deducting the go version to lint for through go.mod,
# which causes it to fallback to go1.17 semantics.
go: "1.23.6"
timeout: 5m

 WARN The linter 'tenv' is deprecated (since v1.64.0) due to: Duplicate feature another linter. Replaced by usetesting.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

Looks like flaky test;

--- FAIL: TestConnectAndWait (0.00s)
--- FAIL: TestConnectAndWait/connect_goroutine_exits_after_EOF (0.00s)
socket_test.go:190: assertion failed: 6 (int) != 7 (numGoroutines + 1 int)

@thaJeztah
thaJeztah merged commit 5e32cce into docker:masterFeb 18, 2025
@thaJeztah
thaJeztah deleted the bump_golangci_lint branch February 18, 2025 12:15
@thaJeztahthaJeztah added this to the 28.0.0 milestone Feb 18, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@thaJeztah@codecov-commenter@vvoland