Skip to content

[test] Add tests for logger error paths: MarkdownLogger.Close, FileLogger.Log, Logger.Print#6647

Merged
lpcox merged 4 commits into
mainfrom
add-logger-error-path-coverage-tests-9d3233f68dcc120f
May 29, 2026
Merged

[test] Add tests for logger error paths: MarkdownLogger.Close, FileLogger.Log, Logger.Print#6647
lpcox merged 4 commits into
mainfrom
add-logger-error-path-coverage-tests-9d3233f68dcc120f

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Test Coverage Improvement: logger error-handling paths

Functions Analyzed

Function Package Previous Coverage New Coverage
MarkdownLogger.Close internal/logger 85.7% 100%
MarkdownLogger.initializeFile internal/logger 87.5% 100%
FileLogger.Log internal/logger 85.7% 100%
Logger.Print internal/logger 91.7% 100%
MarkdownLogger.Log internal/logger 92.3% 96.2%

Overall logger package: 97.6% → 98.6%

Why These Functions?

Five related functions in internal/logger had the lowest coverage in the buildable package set, all with untested error-handling branches:

  • MarkdownLogger.Close (85.7%) — the footer WriteString error path was never exercised; the implementation correctly falls through to closeLogFile even when the footer write fails
  • MarkdownLogger.initializeFile (87.5%) — the header WriteString error path; the function should return the error and leave ml.initialized = false
  • FileLogger.Log (85.7%) — the Sync() error path; a WARNING: Failed to sync log file message should be emitted to stderr
  • Logger.Print (91.7%) — the disabled-logger early-return branch; calling Print on a logger with no DEBUG match was never tested

Tests Added

  • TestMarkdownLogger_Close_FooterWriteError — triggers footer write failure by pre-closing the underlying *os.File
  • TestMarkdownLogger_Close_NilLogFile — exercises the logFile == nil branch (fallback/no-file mode)
  • TestMarkdownLogger_InitializeFile_WriteError — triggers header write failure via closed file
  • TestMarkdownLogger_InitializeFile_AlreadyInitialized — fast-path no-op when already initialized
  • TestMarkdownLogger_Log_WriteError — body write failure after successful header init
  • TestMarkdownLogger_Log_NilLogFile — silent drop when logFile is nil
  • TestFileLogger_Log_SyncError — sync warning path via closed file
  • TestLogger_Print_Disabled — early return when logger is disabled

Technique

All error paths are triggered by closing the underlying *os.File before calling the function, causing subsequent OS operations to return os.ErrClosed. Tests are in package logger (white-box) to allow direct struct construction and field access without requiring additional exported helpers.

Coverage Report

Before: 97.6% logger package coverage
After:  98.6% logger package coverage
Improvement: +1.0%

MarkdownLogger.Close:       85.7% → 100%  (+14.3%)
MarkdownLogger.initializeFile: 87.5% → 100%  (+12.5%)
FileLogger.Log:             85.7% → 100%  (+14.3%)
Logger.Print:               91.7% → 100%  ( +8.3%)
MarkdownLogger.Log:         92.3% → 96.2% ( +3.9%)

Test Execution

All 8 new tests pass:

=== RUN   TestMarkdownLogger_Close_FooterWriteError
--- PASS: TestMarkdownLogger_Close_FooterWriteError (0.00s)
=== RUN   TestMarkdownLogger_Close_NilLogFile
--- PASS: TestMarkdownLogger_Close_NilLogFile (0.00s)
=== RUN   TestMarkdownLogger_InitializeFile_WriteError
--- PASS: TestMarkdownLogger_InitializeFile_WriteError (0.00s)
=== RUN   TestMarkdownLogger_InitializeFile_AlreadyInitialized
--- PASS: TestMarkdownLogger_InitializeFile_AlreadyInitialized (0.00s)
=== RUN   TestMarkdownLogger_Log_WriteError
--- PASS: TestMarkdownLogger_Log_WriteError (0.00s)
=== RUN   TestMarkdownLogger_Log_NilLogFile
--- PASS: TestMarkdownLogger_Log_NilLogFile (0.00s)
=== RUN   TestFileLogger_Log_SyncError
--- PASS: TestFileLogger_Log_SyncError (0.00s)
=== RUN   TestLogger_Print_Disabled
--- PASS: TestLogger_Print_Disabled (0.00s)
PASS
ok      github.com/github/gh-aw-mcpg/internal/logger   0.008s

Generated by Test Coverage Improver
Next run will target the next most complex under-tested function

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • go.opentelemetry.io
  • go.yaml.in
  • golang.org
  • google.golang.org
  • gopkg.in
  • proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "go.opentelemetry.io"
    - "go.yaml.in"
    - "golang.org"
    - "google.golang.org"
    - "gopkg.in"
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Test Coverage Improver · sonnet46 9.9M ·

Cover previously untested error-handling branches in the logger package:

- MarkdownLogger.Close: footer WriteString error path (was 85.7%, now 100%)
- MarkdownLogger.initializeFile: header WriteString error path (was 87.5%, now 100%)
- MarkdownLogger.Log: body WriteString error path, nil logFile path (was 92.3%, now 96.2%)
- FileLogger.Log: Sync error warning path (was 85.7%, now 100%)
- Logger.Print: disabled logger early-return path (was 91.7%, now 100%)

All error paths are triggered by closing the underlying *os.File before
calling the respective function, which causes file operations to return
os.ErrClosed. This approach avoids complex mocking while testing real
error-handling code paths.

Overall logger package coverage: 97.6% → 98.6%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review May 29, 2026 11:48
Copilot AI review requested due to automatic review settings May 29, 2026 11:48
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

This PR improves internal/logger test coverage by adding white-box tests for logger error-handling and disabled-output paths without changing production code.

Changes:

  • Adds tests for MarkdownLogger.Close, initializeFile, and Log fallback/error branches.
  • Adds coverage for FileLogger.Log sync-error handling.
  • Adds coverage for Logger.Print when disabled by an empty DEBUG setting.
Show a summary per file
File Description
internal/logger/markdown_logger_close_coverage_test.go Adds focused logger coverage tests for markdown, file, and debug logger error/disabled paths.

Copilot's findings

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 2

Comment thread internal/logger/markdown_logger_close_coverage_test.go Outdated
Comment thread internal/logger/markdown_logger_close_coverage_test.go Outdated
lpcox and others added 2 commits May 29, 2026 04:54
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 29, 2026

@copilot fix the failing lint ci check

Copy link
Copy Markdown
Contributor

Copilot AI commented May 29, 2026

@copilot fix the failing lint ci check

Fixed in fe37fcfgofmt was applied to internal/logger/markdown_logger_close_coverage_test.go. make lint now passes cleanly.

Copilot AI requested a review from lpcox May 29, 2026 12:01
Copilot finished work on behalf of lpcox May 29, 2026 12:03
@lpcox lpcox merged commit 984adaa into main May 29, 2026
16 checks passed
@lpcox lpcox deleted the add-logger-error-path-coverage-tests-9d3233f68dcc120f branch May 29, 2026 12:09
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.

3 participants