[testify-expert] Improve Test Quality: pkg/stringutil/stringutil_test.go

[github-actions]

Overview

The test file pkg/stringutil/stringutil_test.go has been selected for quality improvement by the Testify Uber Super Expert. This file covers all 6 exported functions in the source but relies entirely on plain Go t.Errorf/t.Error patterns — no testify assertions are used anywhere despite the entire codebase standardizing on testify.

Current State

• Test File: pkg/stringutil/stringutil_test.go
• Source File: pkg/stringutil/stringutil.go
• Test Functions: 13 test functions
• Lines of Code: 579 lines
• Testify assertions: 0 (none)
• Plain t.Errorf/t.Error calls: 15

Test Quality Analysis

Strengths ✅

• All 6 exported functions have corresponding tests (Truncate, NormalizeWhitespace, ParseVersionValue, FormatList, NormalizeLeadingWhitespace, IsPositiveInteger)
• Table-driven tests with t.Run() are used for most tests
• Good edge case coverage (zero, boundary, unicode, empty strings)
• Benchmark tests are included for performance regression detection

🎯 Areas for Improvement

Areas for Improvement 🎯

1. Replace All Plain Error Checks with Testify Assertions

This is the primary issue. The file imports only "strings" and "testing" — no testify. Every assertion is a manual if result != tt.expected { t.Errorf(...) } block.

Current pattern (15 occurrences):

// ❌ CURRENT (anti-pattern — used throughout the file)
result := Truncate(tt.s, tt.maxLen)
if result != tt.expected {
    t.Errorf("Truncate(%q, %d) = %q; want %q", tt.s, tt.maxLen, result, tt.expected)
}

got := IsPositiveInteger(tt.s)
if got != tt.want {
    t.Errorf("IsPositiveInteger(%q) = %v, want %v", tt.s, got, tt.want)
}

Recommended replacement:

// ✅ IMPROVED (testify)
import (
    "testing"
    "github.com/stretchr/testify/assert"
)

result := Truncate(tt.s, tt.maxLen)
assert.Equal(t, tt.expected, result, "Truncate(%q, %d) should return expected value", tt.s, tt.maxLen)

got := IsPositiveInteger(tt.s)
assert.Equal(t, tt.want, got, "IsPositiveInteger(%q) should return %v", tt.s, tt.want)

Why this matters: Testify produces clearer failure output with expected vs actual diffs, is the standard used in every other pkg/ test file, and allows assertions to continue after a failure (reporting multiple issues per test run).

2. Consolidate Standalone Tests into Table-Driven Tests

Three standalone test functions should be merged into the main TestTruncate table:

• TestTruncate_Zero — tests maxLen=0, should be a table row
• TestTruncate_ExactlyThreeChars — tests exact-length case, should be a table row
• TestTruncate_FourChars — tests two cases inline (not using t.Run), should be table rows

Recommended:

// ✅ IMPROVED — consolidated into main table
{
    name:     "maxLen zero returns empty string",
    s:        "hello",
    maxLen:   0,
    expected: "",
},
{
    name:     "string length exactly equals maxLen",
    s:        "abc",
    maxLen:   3,
    expected: "abc",
},
{
    name:     "string one char longer than maxLen adds ellipsis",
    s:        "abcde",
    maxLen:   4,
    expected: "a...",
},

3. Fix TestNormalizeWhitespace_ManyLines — Show Diffs on Failure

Current:

// ❌ CURRENT — no diff shown on failure
if result != expected.String() {
    t.Error("NormalizeWhitespace did not properly normalize many lines")
}

Recommended:

// ✅ IMPROVED — testify shows the actual diff
assert.Equal(t, expected.String(), result, "NormalizeWhitespace should normalize all 100 lines")

4. Fix TestNormalizeWhitespace_PreservesContent — Use Testify Contains

Current:

// ❌ CURRENT
if !strings.Contains(result, "middle  spaces") {
    t.Error("NormalizeWhitespace should preserve non-trailing spaces")
}

Recommended:

// ✅ IMPROVED — semantic assertion, no manual negation
assert.Contains(t, result, "middle  spaces", "NormalizeWhitespace should preserve non-trailing spaces")
assert.Contains(t, result, "middle\t\ttabs", "NormalizeWhitespace should preserve non-trailing tabs")

With this change the "strings" import can be removed entirely.

📋 Implementation Guidelines

Priority Order

1. High: Add testify import and replace all 15 t.Errorf/t.Error calls with assert.Equal
2. Medium: Consolidate TestTruncate_Zero, TestTruncate_ExactlyThreeChars, TestTruncate_FourChars into the main table
3. Low: Use assert.Contains in TestNormalizeWhitespace_PreservesContent and drop the strings import

Best Practices from scratchpad/testing.md

• ✅ Use require.* for critical setup (stops test on failure)
• ✅ Use assert.* for test validations (continues checking)
• ✅ Write table-driven tests with t.Run() and descriptive names
• ✅ Always include helpful assertion messages

Testing Commands

go test -v ./pkg/stringutil/
make test-unit

Acceptance Criteria

• testify/assert imported, all t.Errorf/t.Error replaced with assert.Equal / assert.Contains
• TestTruncate_Zero, TestTruncate_ExactlyThreeChars, TestTruncate_FourChars merged into TestTruncate table
• TestNormalizeWhitespace_PreservesContent uses assert.Contains instead of strings.Contains + t.Error
• All assertions include helpful messages
• Tests pass: make test-unit
• Code follows patterns in scratchpad/testing.md

Additional Context

• Repository Testing Guidelines: See scratchpad/testing.md for comprehensive testing patterns
• Example of correct testify usage: pkg/agentdrain/anomaly_test.go
• Testify Documentation: https://github.com/stretchr/testify

---

Priority: Medium
Effort: Small (mechanical replacement of assertion style)
Expected Impact: Consistent test style with rest of codebase, better failure diffs

Files Involved:

• Test file: pkg/stringutil/stringutil_test.go
• Source file: pkg/stringutil/stringutil.go

References:

• §25934230604

Generated by 🧪 Daily Testify Uber Super Expert · ● 13.3M · ◷

• expires on May 17, 2026, 6:28 PM UTC

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · ● 101K · ◷