Conversation
There was a problem hiding this comment.
Pull request overview
This PR applies linting fixes to improve code quality and consistency across test files. The changes address linter warnings from golangci-lint, specifically focusing on test code improvements.
Changes:
- Extracts repeated error message strings into constants to satisfy the
goconstlinter - Converts
t.Fatalf()calls without format arguments tot.Fatal()for cleaner test code - Adds
//nolintdirectives for complex test functions that legitimately exceed linter thresholds - Removes redundant type conversion and improves variable naming in fuzz tests
- Updates golangci configuration to ignore "alice" test fixture string
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/tlsconfig/config_test.go | Extracts error message constant and converts t.Fatalf to t.Fatal where appropriate |
| pkg/secrets/secrets_test.go | Converts t.Fatalf to t.Fatal and adds nolint directives for complex test functions |
| pkg/sanitize/sql_test.go | Uses errMsgUnexpected constant (defined elsewhere) and converts t.Fatalf to t.Fatal |
| pkg/sanitize/markdown_test.go | Uses errMsgUnexpected constant (defined elsewhere) and converts t.Fatalf to t.Fatal |
| pkg/sanitize/html_test.go | Uses errMsgUnexpected constant (defined elsewhere) and converts t.Fatalf to t.Fatal |
| pkg/sanitize/filename_test.go | Defines errMsgUnexpected constant for reuse across package test files |
| pkg/password/compare_test.go | Converts t.Fatalf to t.Fatal where no format arguments needed |
| pkg/mfa/totp_test.go | Converts t.Fatalf to t.Fatal where no format arguments needed |
| pkg/mfa/rate_limit_test.go | Converts t.Fatalf to t.Fatal where no format arguments needed |
| pkg/mfa/hotp_test.go | Converts t.Fatalf to t.Fatal where no format arguments needed |
| pkg/mfa/backup_test.go | Converts t.Fatalf to t.Fatal where no format arguments needed |
| pkg/memory/reader_test.go | Converts t.Fatalf to t.Fatal where no format arguments needed |
| pkg/converters/convert_test.go | Removes redundant type conversion for cleaner code |
| pkg/auth/paseto_test.go | Converts t.Fatalf to t.Fatal where no format arguments needed |
| internal/io/write_fuzz_test.go | Improves variable naming (r → ch) and adds nolint directive for fuzz function |
| .golangci.yaml | Adds "alice" to goconst ignore list to prevent unnecessary constant extraction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sanitizer, err := NewHTMLSanitizer() | ||
| if err != nil { | ||
| t.Fatalf("expected sanitizer, got %v", err) | ||
| t.Fatalf(errMsgUnexpected, err) |
There was a problem hiding this comment.
The constant errMsgUnexpected is referenced here but is only defined in filename_test.go within the same package. While this is technically valid in Go (test files in the same package share namespace), it creates a maintainability issue where the constant definition is not co-located with all its usages. Consider defining this constant in each test file that uses it, or creating a common test helper file to hold shared test constants.
| sanitizer, err := NewSQLSanitizer() | ||
| if err != nil { | ||
| t.Fatalf("expected sanitizer, got %v", err) | ||
| t.Fatalf(errMsgUnexpected, err) |
There was a problem hiding this comment.
The constant errMsgUnexpected is referenced here but is only defined in filename_test.go within the same package. While this is technically valid in Go (test files in the same package share namespace), it creates a maintainability issue where the constant definition is not co-located with all its usages. Consider defining this constant in each test file that uses it, or creating a common test helper file to hold shared test constants.
| sanitizer, err := NewMarkdownSanitizer() | ||
| if err != nil { | ||
| t.Fatalf("expected sanitizer, got %v", err) | ||
| t.Fatalf(errMsgUnexpected, err) |
There was a problem hiding this comment.
The constant errMsgUnexpected is referenced here but is only defined in filename_test.go within the same package. While this is technically valid in Go (test files in the same package share namespace), it creates a maintainability issue where the constant definition is not co-located with all its usages. Consider defining this constant in each test file that uses it, or creating a common test helper file to hold shared test constants.
No description provided.