Skip to content

[test-improver] Improve tests for logger package#589

Merged
lpcox merged 1 commit intomainfrom
test-improver/jsonl-logger-1770042556-72c11c36605201cc
Feb 2, 2026
Merged

[test-improver] Improve tests for logger package#589
lpcox merged 1 commit intomainfrom
test-improver/jsonl-logger-1770042556-72c11c36605201cc

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Feb 2, 2026

Test Improvements: jsonl_logger_test.go

File Analyzed

  • Test File: internal/logger/jsonl_logger_test.go
  • Package: internal/logger
  • Lines of Code: 421 → 514 (+93 lines, 22% increase)

Improvements Made

1. Better Testing Patterns

  • ✅ Added bound asserters (require and assert) to all 14 test functions
  • ✅ Converted 45+ manual error checks to testify assertions
  • ✅ Replaced all if err != nil { t.Errorf(...) } with require.NoError() or assert.NoError()
  • ✅ Converted all manual string comparisons (if got != want) to assert.Equal()
  • ✅ Converted all manual strings.Contains() checks to assert.Contains() / assert.NotContains()
  • ✅ Converted all manual emptiness checks to assert.NotEmpty()
  • ✅ Improved error messages with better context

2. Increased Coverage

  • New test: TestInitJSONLLoggerWithInvalidPath - Tests error handling when directory creation fails
  • New test: TestLogRPCMessageJSONLDirectionTypes - Table-driven test for all direction/type combinations (IN/OUT × REQUEST/RESPONSE)
  • New test: TestLogRPCMessageJSONLEmptyPayload - Edge case for empty JSON payloads
  • New test: TestLogRPCMessageJSONLWithNilError - Normal case verification when no error occurs
  • Coverage Areas Added:
    • Error path validation (invalid directory permissions)
    • All message direction/type combinations
    • Edge case handling (empty payloads, nil errors)

3. Cleaner & More Stable Tests

  • ✅ Consistent use of bound asserters eliminates repetitive t parameter passing
  • ✅ Better separation of concerns: require for critical checks, assert for non-critical validations
  • ✅ More descriptive assertion messages with context
  • ✅ Cleaner code structure with fewer conditional branches
  • ✅ Achieved 100% testify usage (90+ testify assertions, 0 manual checks)

Test Execution

Statistics

  • Before: 9 test functions, 18 testify assertions, 45 manual checks
  • After: 14 test functions (+5), 90+ testify assertions, 0 manual checks
  • Testify Usage: 28.6% → 100% (+71.4%)
  • Lines Changed: +229 insertions, -135 deletions

Changes Summary

 internal/logger/jsonl_logger_test.go | 364 ++++++++++++++++++--------
 1 file changed, 229 insertions(+), 135 deletions(-)

Why These Changes?

This test file was selected because:

  1. High improvement potential: Mixed manual error checking alongside limited testify usage
  2. Consistency: The codebase already uses testify extensively - this brings this file up to standard
  3. Coverage gaps: Missing tests for error conditions, edge cases, and message type combinations
  4. Maintainability: Manual error checking is more verbose and less maintainable than testify assertions

The improvements make the tests:

  • More readable: Bound asserters and semantic assertion methods
  • More maintainable: Consistent patterns across all test functions
  • More comprehensive: 5 new test cases covering previously untested scenarios
  • More reliable: Better error messages help debug failures faster

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

AI generated by Test Improver

- Replace 45+ manual error checks with testify assertions
- Add bound asserters to all 14 test functions
- Add 5 new test cases for better coverage
- Achieve 100% testify usage (90+ assertions)
- Improve error messages and test clarity
@lpcox lpcox marked this pull request as ready for review February 2, 2026 14:46
@lpcox lpcox merged commit f3a4d2c into main Feb 2, 2026
@lpcox lpcox deleted the test-improver/jsonl-logger-1770042556-72c11c36605201cc branch February 2, 2026 14:46
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.

1 participant