X Tutup
Skip to content

[test-improver] Improve tests for logger/slog_adapter#1672

Draft
github-actions[bot] wants to merge 1 commit intomainfrom
test-improver/slog-adapter-coverage-275fe8ea1ed6fa51
Draft

[test-improver] Improve tests for logger/slog_adapter#1672
github-actions[bot] wants to merge 1 commit intomainfrom
test-improver/slog-adapter-coverage-275fe8ea1ed6fa51

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Mar 8, 2026

Test Improvements: slog_adapter_test.go

File Analyzed

  • Test File: internal/logger/slog_adapter_test.go
  • Package: internal/logger
  • Lines of Code: ~396 → ~434 (70 insertions, 32 deletions — net increase from new test function)

Problem Identified

Five major test functions in slog_adapter_test.go were always skipped in normal CI environments because they guarded execution with os.Getenv("DEBUG") == "" checks and called t.Skip(). Since CI does not set DEBUG, these tests never ran — meaning the core Handle() method on SlogHandler was completely untested in practice.

The skipped tests were:

  • TestSlogAdapter
  • TestNewSlogLoggerWithHandler
  • TestSlogHandler_Handle_Levels
  • TestSlogHandler_Handle_Attributes
  • TestSlogHandler_Handle_EdgeCases

Root Cause

computeEnabled() in logger.go explicitly re-reads os.Getenv("DEBUG") each time New() is called, with a code comment stating:

// Read DEBUG from environment each time to support t.Setenv() in tests

This means calling t.Setenv("DEBUG", "*") before creating a logger will produce an enabled logger, and cleanup is automatic. The original tests did not exploit this.

Improvements Made

1. Converted 5 Always-Skipped Tests to Always Run

Before (5 tests × same pattern):

func TestSlogAdapter(t *testing.T) {
    if os.Getenv("DEBUG") == "" {
        t.Skip("Skipping: DEBUG not set")
    }
    slogLogger := NewSlogLogger("test:slog")
    ...
}

After:

func TestSlogAdapter(t *testing.T) {
    // Enable debug logging for this test
    t.Setenv("DEBUG", "*")

    // Create slog logger using our adapter (must be created AFTER t.Setenv)
    slogLogger := NewSlogLogger("test:slog")
    ...
}

This change:

  • Uses t.Setenv which auto-cleans up via t.Cleanup — no env leakage between tests
  • Adds a comment explaining the ordering requirement (logger must be created after t.Setenv)
  • Removes the os import dependency in tests that no longer need it

2. Converted TestSlogAdapterDisabled to Use t.Setenv

Before: Implicitly relied on DEBUG not being set in the environment

After: Explicitly sets t.Setenv("DEBUG", "") to ensure a disabled logger state regardless of the environment — making the test deterministic even when DEBUG=* is set locally.

3. Added TestFormatSlogValue — New Coverage for Unexported Helper

Added 6 test cases for the formatSlogValue helper function which had no direct tests:

func TestFormatSlogValue(t *testing.T) {
    tests := []struct {
        name  string
        input any
        want  string
    }{
        {"slog.Value string",   slog.StringValue("hello"), "hello"},
        {"slog.Value int",      slog.IntValue(42),         "42"},
        {"slog.Value bool",     slog.BoolValue(true),      "true"},
        {"slog.Value float64",  slog.Float64Value(3.14),   "3.14"},
        {"raw string (non-slog-Value)", "hello",           "hello"},
        {"raw int (non-slog-Value)",    42,                "42"},
    }
    ...
}

This covers both code paths in formatSlogValue:

  1. The normal path: input is already a slog.Value
  2. The defensive fallback path: input is a raw Go value (wrapped via slog.AnyValue())

Test Execution

All tests run in CI. Example coverage now includes:

  • SlogHandler.Handle() — all log levels (DEBUG/INFO/WARN/ERROR)
  • Attribute formatting in log output
  • Edge cases: nil attrs, empty messages, zero values
  • formatSlogValue() — both code paths
  • Disabled logger behavior (deterministic, not environment-dependent)

Why These Changes?

The slog_adapter.go provides the bridge between Go's standard log/slog package and this project's custom logger. Despite being a non-trivial adapter (~109 lines), its core Handle() method was effectively untested in CI because the test file relied on a manual environment variable check instead of using the test framework's t.Setenv() capability that the logger was specifically designed to support.


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

Generated by Test Improver

Convert skip-based tests to always run using t.Setenv instead of t.Skip.

Changes:
- TestSlogAdapter: use t.Setenv instead of t.Skip
- TestSlogAdapterDisabled: use t.Setenv to ensure disabled state
- TestNewSlogLoggerWithHandler: use t.Setenv instead of t.Skip
- TestSlogHandler_Handle_Levels: use t.Setenv instead of t.Skip
- TestSlogHandler_Handle_Attributes: use t.Setenv instead of t.Skip
- TestSlogHandler_Handle_EdgeCases: use t.Setenv instead of t.Skip
- Add TestFormatSlogValue to cover both code paths in formatSlogValue

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

0 participants

X Tutup