[test-improver] Improve tests for logger package#1974
Merged
Conversation
- Replace 29 manual t.Errorf/t.Error/t.Fatal calls with testify assertions (assert.Equal, assert.Contains, assert.NotContains, assert.Empty, assert.NotEmpty, assert.True, assert.False, assert.Regexp) - Remove unused 'strings' import now that assert.Contains replaces strings.Contains checks - Fix import ordering: move 'time' into the stdlib block - Add TestLogger_Printf_WithColors and TestLogger_Print_WithColors to cover the color-output branch of Printf/Print (previously untested code path) - Use t.Cleanup for restoring package-level state in color tests instead of defer with manual variable restore - Use assert.Regexp for time-unit check in TestLogger_TimeDiff for a more precise and readable assertion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the internal/logger package tests by refactoring assertions to idiomatic testify usage and adding coverage for the ANSI color-output branches in Logger.Printf and Logger.Print.
Changes:
- Replaced many manual
t.Error*checks withtestify/assert(and cleaned up imports accordingly). - Added new tests to cover the colored stderr output paths for
PrintfandPrint. - Made time-diff validation more structured via
assert.Regexp.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
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>
This was referenced Mar 16, 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.
File Analyzed
internal/logger/logger_test.gointernal/loggerImprovements Made
1. Better Testify Usage
t.Errorf/t.Error/t.Fatalcalls with idiomatic testify assertions (assert.Equal,assert.Contains,assert.NotContains,assert.Empty,assert.NotEmpty,assert.True,assert.False,assert.Regexp)"strings"import (was only needed forstrings.Containschecks replaced byassert.Contains)"time"into the stdlib block (was mixed with third-party imports)if !strings.Contains(output, x) { t.Errorf(...) }patterns withassert.Contains(t, output, x, ...)if strings.Contains(logContent, "\033[") { t.Errorf(...) }withassert.NotContainsassert.RegexpinTestLogger_TimeDifffor a more precise time-unit check (replaces a fragile two-condition string check)2. Increased Coverage
TestLogger_Printf_WithColors— tests the previously untested color-output branch ofPrintf(theif l.color != ""branch that writes ANSI escape codes to stderr)TestLogger_Print_WithColors— tests the previously untested color-output branch ofPrint\033[), and presence of the color reset code3. Cleaner & More Stable Tests
t.Cleanup()(instead ofdeferwith manual variable restore) for resetting package-leveldebugColorsandisTTYstate in the new color tests — ensures cleanup even if the test panicsWhy This File?
logger_test.gowas the highest-priority target: it importstestify/assertandtestify/requirebut then proceeds to use rawt.Errorfin 29 places — a clear inconsistency that makes the tests harder to read and produces less helpful failure output. The color-output branches ofPrintf/Printwere completely untested despite being real code paths exercised in production.Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests