Skip to content

[test-improver] Improve tests for logger common package#3114

Draft
github-actions[bot] wants to merge 1 commit intomainfrom
test-improver/improve-logger-common-tests-c041b781dbaf0324
Draft

[test-improver] Improve tests for logger common package#3114
github-actions[bot] wants to merge 1 commit intomainfrom
test-improver/improve-logger-common-tests-c041b781dbaf0324

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot commented Apr 3, 2026

Test Improvements: internal/logger/common_test.go

File Analyzed

  • Test File: internal/logger/common_test.go
  • Package: internal/logger
  • Lines of Code: ~696 → ~770 (74 lines added)

Improvements Made

1. Better Testify Patterns — Replace t.Errorf/t.Fatal with Idiomatic Assertions

Problem: Several tests used t.Errorf("should not be called") inside callback closures and t.Fatal inside if err == nil guards instead of proper testify patterns.

  • TestInitLogFile_InvalidDirectory — replaced if err == nil { file.Close(); t.Fatal(...) } with require.Error(err, ...) after closing any file handle
  • TestInitLogFile_EmptyFileName — same pattern; added require := require.New(t) so the test uses testify require.Error consistently
  • TestInitLogger_FileLogger — replaced t.Errorf("Error handler should not be called") inside callback with a errorHandlerCalled boolean flag + assert.False(errorHandlerCalled, ...)
  • TestInitLogger_FileLoggerFallback — replaced t.Errorf in setup callback with setupHandlerCalled flag + assert.False
  • TestInitLogger_JSONLLogger — same improvement as FileLogger
  • TestInitLogger_JSONLLoggerError — same improvement as FileLoggerFallback
  • TestInitLogger_MarkdownLogger — same improvement as FileLogger
  • TestInitLogger_MarkdownLoggerFallback — same improvement as FileLoggerFallback
  • TestInitLogger_SetupError — replaced t.Errorf in error-handler callback with flag + assert.False

The boolean flag pattern makes the intent explicit alongside the other post-call assertions and avoids mixing raw t.Errorf with testify assertions in the same test.

2. Increased Coverage — Direct Tests for formatLogLine

Problem: The formatLogLine function in common.go was exercised only indirectly through FileLogger.Log() and ServerFileLogger, but had no direct unit tests. The function is responsible for the canonical log line format used across all file-based loggers.

  • ✅ Added TestFormatLogLine with subtests covering:
    • All four log levels (INFO, WARN, ERROR, DEBUG) appear in brackets
    • Category appears in its own bracket
    • Format-string interpolation with args (count=%d name=%s)
    • Full bracket structure regex: ^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z\] \[INFO\] \[auth\] event occurred$
    • Timestamp is RFC3339 UTC and falls within the test's time window
    • Format string used as-is when no args provided
    • Empty category produces [] bracket pair

3. Cleaner & More Stable Tests

  • ✅ Added "strings" and "time" imports for the new TestFormatLogLine test
  • ✅ Consistent testify usage — no more mixing of t.Errorf with assert/require in the same tests
  • ✅ Better resource cleanup — files are closed before require.Error stops the test

Why These Changes?

internal/logger/common_test.go tests the core initialization primitives (closeLogFile, initLogFile, initLogger) used by all logger types in the package. The formatLogLine function — which defines the log line format for all operational logs — was entirely untested directly. The t.Errorf callback pattern, while valid Go, was inconsistent with the testify style used throughout the rest of the file.


Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Generated by Test Improver ·

…LogLine coverage

- Replace t.Errorf/t.Fatal patterns with testify assertions:
  - TestInitLogFile_InvalidDirectory: use require.Error instead of if/t.Fatal
  - TestInitLogFile_EmptyFileName: use require.Error instead of if/t.Fatal
  - TestInitLogger_FileLogger/JSONLLogger/MarkdownLogger: replace t.Errorf
    in error-handler callbacks with boolean flag + assert.False
  - TestInitLogger_FileLoggerFallback/JSONLLoggerError/MarkdownLoggerFallback:
    replace t.Errorf in setup-handler callbacks with boolean flag + assert.False
  - TestInitLogger_SetupError: same pattern for error-handler callback

- Add TestFormatLogLine to cover the previously untested formatLogLine function:
  - Tests all four log levels (INFO, WARN, ERROR, DEBUG)
  - Verifies bracket structure [timestamp] [level] [category] message
  - Validates RFC3339 UTC timestamp format and time window
  - Tests format string interpolation with args
  - Tests empty category edge case

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to 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.

0 participants