[test-improver] Improve tests for logger/slog_adapter#1672
Draft
github-actions[bot] wants to merge 1 commit intomainfrom
Draft
[test-improver] Improve tests for logger/slog_adapter#1672github-actions[bot] wants to merge 1 commit intomainfrom
github-actions[bot] wants to merge 1 commit intomainfrom
Conversation
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>
This was referenced Mar 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test Improvements:
slog_adapter_test.goFile Analyzed
internal/logger/slog_adapter_test.gointernal/loggerProblem Identified
Five major test functions in
slog_adapter_test.gowere always skipped in normal CI environments because they guarded execution withos.Getenv("DEBUG") == ""checks and calledt.Skip(). Since CI does not setDEBUG, these tests never ran — meaning the coreHandle()method onSlogHandlerwas completely untested in practice.The skipped tests were:
TestSlogAdapterTestNewSlogLoggerWithHandlerTestSlogHandler_Handle_LevelsTestSlogHandler_Handle_AttributesTestSlogHandler_Handle_EdgeCasesRoot Cause
computeEnabled()inlogger.goexplicitly re-readsos.Getenv("DEBUG")each timeNew()is called, with a code comment stating:// Read DEBUG from environment each time to support t.Setenv() in testsThis 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):
After:
This change:
t.Setenvwhich auto-cleans up viat.Cleanup— no env leakage between testst.Setenv)osimport dependency in tests that no longer need it2. Converted
TestSlogAdapterDisabledto Uset.SetenvBefore: Implicitly relied on
DEBUGnot being set in the environmentAfter: Explicitly sets
t.Setenv("DEBUG", "")to ensure a disabled logger state regardless of the environment — making the test deterministic even whenDEBUG=*is set locally.3. Added
TestFormatSlogValue— New Coverage for Unexported HelperAdded 6 test cases for the
formatSlogValuehelper function which had no direct tests:This covers both code paths in
formatSlogValue:slog.Valueslog.AnyValue())Test Execution
All tests run in CI. Example coverage now includes:
SlogHandler.Handle()— all log levels (DEBUG/INFO/WARN/ERROR)formatSlogValue()— both code pathsWhy These Changes?
The
slog_adapter.goprovides the bridge between Go's standardlog/slogpackage and this project's custom logger. Despite being a non-trivial adapter (~109 lines), its coreHandle()method was effectively untested in CI because the test file relied on a manual environment variable check instead of using the test framework'st.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