Skip to content

[test-improver] Improve tests for logger/markdown_logger package#3058

Merged
lpcox merged 1 commit intomainfrom
test-improver/markdown-logger-20260402-f1066b65e9376c9b
Apr 3, 2026
Merged

[test-improver] Improve tests for logger/markdown_logger package#3058
lpcox merged 1 commit intomainfrom
test-improver/markdown-logger-20260402-f1066b65e9376c9b

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Apr 2, 2026

Test Improvements: markdown_logger_test.go

File Analyzed

  • Test File: internal/logger/markdown_logger_test.go
  • Package: internal/logger
  • Lines Changed: 387 → 379 (net −8 lines; replaced verbose manual checks with concise testify calls)

Improvements Made

1. Better Testing Patterns

The file previously mixed testify assertions with 14 manual if/t.Errorf/t.Fatalf checks. All have been replaced with idiomatic testify equivalents for consistency and better error messages:

  • if _, err := os.Stat(path); os.IsNotExist(err) { t.Errorf(...) }assert.DirExists / assert.FileExists
  • assert.True(t, strings.Contains(content, x), msg)assert.Contains(t, content, x, msg)
  • assert.False(t, count < 2, msg)assert.GreaterOrEqual(t, count, 2, msg) (clearer intent)
  • if !strings.Contains(content, x) { t.Errorf(...) } loops → assert.Contains in loop
  • if strings.Contains(content, secret) { t.Errorf(...) } loops → assert.NotContains in loop
  • if err != nil { t.Fatalf(...) }require.NoError(t, err, ...) (stops test on failure as intended)

2. Increased Coverage

  • ✅ Added TestGetEmojiForLevel — a table-driven test covering all five branches of getEmojiForLevel, including the previously untested default "•" bullet case (reached only by an unknown LogLevel value)
Branch Before After
LogLevelInfo"✓" Covered indirectly ✅ Explicit
LogLevelWarn"⚠️" Covered indirectly ✅ Explicit
LogLevelError"✗" Covered indirectly ✅ Explicit
LogLevelDebug"🔍" Covered indirectly ✅ Explicit
default"•" ❌ Not covered ✅ Covered

3. Cleaner & More Stable Tests

  • ✅ Eliminated strings.Contains in assertion position — testify's assert.Contains provides better failure messages showing both the haystack and the needle
  • ✅ Reduced cognitive overhead: each assertion now clearly expresses its intent without an if/negate/message pattern

Why This File?

markdown_logger_test.go used testify for some assertions but fell back to manual if+t.Errorf/Fatalf for others — an inconsistency that made the tests harder to read and produced weaker failure messages. The file also left getEmojiForLevel's default branch completely untested despite being in the same package.


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

Generated by Test Improver ·

Replace manual error checking with idiomatic testify assertions and add
a new table-driven test covering the previously untested default branch
in getEmojiForLevel.

Changes:
- assert.True(t, strings.Contains(...)) -> assert.Contains
- assert.False(t, codeBlockCount < 2) -> assert.GreaterOrEqual
- Manual if+t.Errorf loops -> assert.Contains / assert.NotContains
- os.Stat existence checks -> assert.DirExists / assert.FileExists
- if err != nil { t.Fatalf } -> require.NoError
- Add TestGetEmojiForLevel covering all levels including default bullet

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review April 3, 2026 04:54
Copilot AI review requested due to automatic review settings April 3, 2026 04:54
@lpcox lpcox merged commit 2e3c045 into main Apr 3, 2026
3 checks passed
@lpcox lpcox deleted the test-improver/markdown-logger-20260402-f1066b65e9376c9b branch April 3, 2026 04:54
Copy link
Copy Markdown
Contributor

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.

Pull request overview

Improves the internal/logger markdown logger tests by standardizing assertions on testify (better consistency and failure output) and adding explicit branch coverage for getEmojiForLevel, including the default/unknown-level case.

Changes:

  • Replaced manual if + t.Errorf/t.Fatalf checks with assert.* / require.* equivalents throughout markdown_logger_test.go.
  • Improved intent clarity in a few assertions (e.g., GreaterOrEqual for code fence counts).
  • Added TestGetEmojiForLevel to cover all switch branches, including the default "•" case.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants