[test] Add tests for logger.JSONLLogger.logEntry error paths

[github-actions]

Test Coverage Improvement: logger.JSONLLogger.logEntry

Function Analyzed

• Package: internal/logger
• Function: logEntry (private method on *JSONLLogger)
• Previous Coverage: 66.7%
• New Coverage: 100.0%
• Complexity: Medium — 3 distinct error-handling branches plus mutex protection and sync operation

Why This Function?

logEntry is the core write method for all JSONL audit-log entries (RPC messages and DIFC filter events). Its three error branches — uninitialized logger, JSON encode failure, and fsync failure — were completely untested despite being critical for robust log-write error handling. It was the lowest-coverage non-trivial function (66.7%) in the packages that can currently be compiled in the offline CI environment.

LogDifcFilteredItem was also improved from 87.5% by covering the nil-entry guard and additional execution paths.

Tests Added

logEntry (66.7% → 100%)

• ✅ Nil logFile: direct struct construction with uninitialized logger returns "JSONL logger not initialized" error
• ✅ Encode error: passing chan int (un-marshallable) returns "failed to encode JSON: ..." wrapping a *json.UnsupportedTypeError
• ✅ Sync error: using an os.Pipe write-end as the log file — Encode succeeds into the pipe buffer but Sync returns EINVAL (pipes don't support fsync)
• ✅ Happy path: valid file + encodable value writes a correct JSONL line
• ✅ Concurrent access: 20 goroutines call logEntry concurrently; mutex must prevent data races and all writes must succeed
• ✅ Error wrapping: errors.As unwraps to *json.UnsupportedTypeError from the encode-error path

LogDifcFilteredItem (87.5% — nil-logger branch is unreachable dead code)

• ✅ Nil entry guard: calling with nil must not panic — even with no global logger
• ✅ Sets Timestamp and Type: entry written to disk has Type == "DIFC_FILTERED" and a non-empty RFC 3339 timestamp
• ✅ No logger initialised: must not panic when called before InitJSONLLogger

Coverage Report

Before:
  logger.JSONLLogger.logEntry          66.7%
  logger.LogDifcFilteredItem           87.5%
  internal/logger package (overall)    97.0%

After:
  logger.JSONLLogger.logEntry         100.0%  (+33.3%)
  logger.LogDifcFilteredItem           87.5%  (unchanged — remaining branch is dead code)
  internal/logger package (overall)    97.6%  (+0.6%)

The logger == nil branch inside the withGlobalLogger callback of LogDifcFilteredItem is unreachable: withGlobalLogger already guards the callback invocation with if *logger != nil, so the inner nil check can never be triggered.

Test Execution

=== RUN   TestLogEntry_NilLogFile
--- PASS: TestLogEntry_NilLogFile (0.00s)
=== RUN   TestLogEntry_EncodeError
--- PASS: TestLogEntry_EncodeError (0.00s)
=== RUN   TestLogEntry_SyncError
--- PASS: TestLogEntry_SyncError (0.00s)
=== RUN   TestLogEntry_HappyPath
--- PASS: TestLogEntry_HappyPath (0.00s)
=== RUN   TestLogEntry_ConcurrentAccess
--- PASS: TestLogEntry_ConcurrentAccess (0.01s)
=== RUN   TestLogEntry_EncodeErrorMessage
--- PASS: TestLogEntry_EncodeErrorMessage (0.00s)
=== RUN   TestLogDifcFilteredItem_NilEntry
--- PASS: TestLogDifcFilteredItem_NilEntry (0.00s)
=== RUN   TestLogDifcFilteredItem_SetsTimestampAndType
--- PASS: TestLogDifcFilteredItem_SetsTimestampAndType (0.00s)
=== RUN   TestLogDifcFilteredItem_NoLogger
--- PASS: TestLogDifcFilteredItem_NoLogger (0.00s)
PASS
ok  github.com/github/gh-aw-mcpg/internal/logger  0.019s

All pre-existing tests continue to pass. No production code was modified.

---

Generated by Test Coverage Improver
Next run should target internal/sys.DetectContainerID (59.1%) or internal/version.BuildVersionString (68.2%)

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 · ● 7.2M · ◷

[Copilot]

Pull request overview

This PR improves test coverage in internal/logger by adding focused unit tests for (*JSONLLogger).logEntry error paths and for LogDifcFilteredItem’s nil-entry guard and field population behavior.

Changes:

• Added tests covering logEntry’s nil logFile, JSON encode failure, sync failure, happy path, and concurrent access behavior.
• Added tests ensuring LogDifcFilteredItem(nil) does not panic and that non-nil entries get Timestamp and Type populated before being written.

Show a summary per file

File	Description
internal/logger/jsonl_logger_test.go	Adds new unit tests for JSONLLogger.logEntry error branches and LogDifcFilteredItem behavior.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

internal/logger/jsonl_logger_test.go:917

• TestLogEntry_ConcurrentAccess closes the file via require.NoError(t, f.Close()) and also schedules t.Cleanup(func() { f.Close() }). This double-close is harmless but can mask close errors and makes the test harder to reason about. Prefer closing exactly once (either cleanup/defer OR the explicit Close with an assertion).

	f, err := os.Create(logPath)
	require.NoError(t, err, "failed to create log file")
	t.Cleanup(func() { f.Close() })

	jl := &JSONLLogger{
		logFile: f,
		encoder: json.NewEncoder(f),
	}

	const numGoroutines = 20
	done := make(chan error, numGoroutines)
	for i := 0; i < numGoroutines; i++ {
		i := i
		go func() {
			done <- jl.logEntry(map[string]int{"n": i})
		}()
	}

	for i := 0; i < numGoroutines; i++ {
		assert.NoError(t, <-done, "concurrent logEntry call should not error")
	}

	// Count lines written: each successful logEntry writes exactly one JSONL line.
	require.NoError(t, f.Close())
	data, err := os.ReadFile(logPath)

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