Skip to content

fix: create errors from non-constant strings#1054

Merged
eriknordmark merged 1 commit intolf-edge:masterfrom
europaul:fix-non-constant-error-strings
Mar 14, 2025
Merged

fix: create errors from non-constant strings#1054
eriknordmark merged 1 commit intolf-edge:masterfrom
europaul:fix-non-constant-error-strings

Conversation

@europaul
Copy link
Copy Markdown
Contributor

@europaul europaul commented Mar 11, 2025

Since go 1.24 vet reports the usage of non-constant strings in Printf as errors (see https://tip.golang.org/doc/go1.24#vet). This change fixes the found occurrences in our tests.

Example:

./nw_test.go:91:21: non-constant format string in call to fmt.Errorf

This fix is mostly relevant for local builds as we are not yet using go 1.24 in the CI, but the nature of the issue itself is independent of the go version used.

Since go 1.24 vet reports the usage of non-constant strings in Printf as
errors (see https://tip.golang.org/doc/go1.24#vet). This change fixes
the found occurrences in our tests.

Signed-off-by: Paul Gaiduk <paulg@zededa.com>
@europaul europaul requested a review from uncleDecart as a code owner March 11, 2025 09:03
@europaul europaul requested a review from Copilot March 11, 2025 12:09
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR addresses Go 1.24 vet issues by replacing fmt.Errorf calls with errors.New so that error messages are built from constant strings. It also includes a change in logging in tests/vcom/vcom_test.go to call Log instead of Logf.

  • Updated tests/vcom/vcom_test.go to use logT.Log.
  • Replaced fmt.Errorf with errors.New in tests across network, app, volume, docker, and lim packages.
  • Ensured that error message construction complies with constant string requirements per Go 1.24 vet.

Reviewed Changes

File Description
tests/vcom/vcom_test.go Changed logT.Logf to logT.Log to avoid non-constant formatting usage.
tests/network/nw_test.go Replaced fmt.Errorf with errors.New for building error messages.
tests/app/app_test.go Replaced fmt.Errorf with errors.New in error returns.
tests/volume/vol_test.go Replaced fmt.Errorf with errors.New in error returns.
tests/docker/docker_test.go Replaced fmt.Errorf with errors.New in error handling of HTTP responses.
tests/lim/lim_test.go Replaced fmt.Errorf with errors.New in test assertions.

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

tests/vcom/vcom_test.go:41

  • Verify that switching from logT.Logf to logT.Log preserves the intended output formatting, especially if additional formatting was expected.
logT.Log(out)

tests/network/nw_test.go:88

  • [nitpick] The error message string is initialized with a leading newline, which may result in unexpected formatting of error messages. Consider removing the initial newline unless it is intentionally required.
out := "\n"

@europaul
Copy link
Copy Markdown
Contributor Author

@uncleDecart could you please restart the failed tests? I think they are just being flaky

Copy link
Copy Markdown
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eriknordmark eriknordmark merged commit 6f1e071 into lf-edge:master Mar 14, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants