Skip to content

[test] Add tests for logger.writeJSONToFile and initAndSetGlobalNoFileLogger#9276

Open
github-actions[bot] wants to merge 4 commits into
mainfrom
test/coverage-write-json-to-file-44f2c34b680fe1be
Open

[test] Add tests for logger.writeJSONToFile and initAndSetGlobalNoFileLogger#9276
github-actions[bot] wants to merge 4 commits into
mainfrom
test/coverage-write-json-to-file-44f2c34b680fe1be

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Test Coverage Improvement: logger.writeJSONToFile

Function Analyzed

  • Package: internal/logger
  • Function: writeJSONToFile (primary), initAndSetGlobalNoFileLogger (secondary)
  • Previous Coverage: writeJSONToFile 75.0%, initAndSetGlobalNoFileLogger 81.8%
  • New Coverage: writeJSONToFile 100%, initAndSetGlobalNoFileLogger 90.9%
  • Complexity: Medium

Why These Functions?

writeJSONToFile had zero dedicated tests despite being a shared helper used by multiple logger types (ToolsLogger, ObservedURLDomainsLogger) for persisting state to disk. Its uncovered branch — the json.MarshalIndent failure path — is only reachable with unmarshalable types like channels.

initAndSetGlobalNoFileLogger was missing coverage for the factory.setup() error path, which required constructing a custom factory that returns an error from setup.

Tests Added

  • Happy path: simple map, nil value, struct, slice, empty struct
  • Atomic overwrite: second call overwrites first
  • Marshal error: json.MarshalIndent fails for a channel (unmarshalable type)
  • Write error: atomicWriteFile fails when destination directory does not exist
  • initAndSetGlobalNoFileLogger: error returned when factory.setup() fails

Coverage Report

writeJSONToFile:              75.0% -> 100%  (+25.0%)
initAndSetGlobalNoFileLogger: 81.8% ->  90.9% (+9.1%)
logger package overall:       98.6% ->  98.9% (+0.3%)

Test Execution

All tests pass:

ok  github.com/github/gh-aw-mcpg/internal/logger  0.362s

Generated by Test Coverage Improver

Warning

Firewall blocked 7 domains

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

  • awmgmcpg
  • 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
    - "awmgmcpg"
    - "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 · 232.6 AIC · ⊞ 6.9K ·

Adds comprehensive table-driven tests for writeJSONToFile (0→100% coverage)
and an additional test for the uncovered error path in initAndSetGlobalNoFileLogger
(81.8%→90.9% coverage).

Tests cover:
- Happy path: simple map, nil value, struct, slice, empty struct
- Overwrite existing file atomically
- json.MarshalIndent failure for unmarshalable type (channel)
- atomicWriteFile failure when destination directory does not exist
- factory.setup() error path in initAndSetGlobalNoFileLogger

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review July 13, 2026 21:59
Copilot AI review requested due to automatic review settings July 13, 2026 21:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds dedicated logger tests for JSON persistence and no-file logger initialization error handling.

Changes:

  • Covers JSON write success, overwrite, marshal failure, and write failure paths.
  • Covers factory setup errors during global logger initialization.
Show a summary per file
File Description
internal/logger/write_json_to_file_test.go Adds unit tests for logger persistence and initialization helpers.

Review details

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment thread internal/logger/write_json_to_file_test.go Outdated
Comment thread internal/logger/write_json_to_file_test.go Outdated
lpcox and others added 2 commits July 13, 2026 16:26
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

lpcox commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Copilot AI commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

@copilot fix the failing lint tests https://github.com/github/gh-aw-mcpg/actions/runs/29293061387/job/86960644208?pr=9276 https://github.com/github/gh-aw-mcpg/actions/runs/29293063538/job/86960650527...

Fixed in 1f65b66.

I pulled both failing lint job logs and applied the minimal fix: gofmt on internal/logger/write_json_to_file_test.go (indentation only). Local make lint, targeted logger tests, and make agent-finished now pass.

Copilot finished work on behalf of lpcox July 13, 2026 23:36
Copilot AI requested a review from lpcox July 13, 2026 23:36
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