[testify-expert] Improve Test Quality: pkg/console/render_test.go

[github-actions]

Overview

The test file pkg/console/render_test.go has been selected for quality improvement by the Testify Uber Super Expert. This file contains comprehensive tests for the console rendering system but uses zero testify assertions, relying entirely on plain Go error handling. This issue provides specific, actionable recommendations to modernize these tests using testify best practices.

Current State

• Test File: pkg/console/render_test.go
• Source File: pkg/console/render.go
• Test Functions: 15 test functions
• Lines of Code: 519 lines
• Coverage: Good - Most exported functions are tested
• Testify Usage: ❌ ZERO - No testify assertions used

Test Quality Analysis

Strengths ✅

1. Comprehensive coverage - Tests cover struct rendering, slice/table rendering, maps, tag parsing, and formatting
2. Some table-driven tests exist - TestParseConsoleTag, TestIsZeroValue, TestFormatFieldValue, and TestFormatNumber already use table-driven patterns
3. Good edge case coverage - Tests include nil pointers, empty values, omitempty behavior, and nested structs

Areas for Improvement 🎯

1. Testify Assertions (CRITICAL - Entire File)

Current Issues:

• ZERO testify assertions in the entire 519-line test file
• All tests use plain Go error handling: t.Error(), t.Errorf()
• Many manual string checks using strings.Contains()
• No structured assertions for better error messages

Impact:

• Poor error messages when tests fail
• Harder to debug failures
• Inconsistent with repository standard (see scratchpad/testing.md)
• More verbose code

Recommended Changes:

// ❌ CURRENT (anti-pattern) - From TestRenderStruct_SimpleStruct
if !strings.Contains(output, "Run ID") {
    t.Errorf("Output should contain Run ID field, got:\n%s", output)
}
if !strings.Contains(output, "12345") {
    t.Error("Output should contain RunID value")
}
if strings.Contains(output, "should not appear") {
    t.Error("Output should not contain skipped field")
}

// ✅ IMPROVED (testify)
assert.Contains(t, output, "Run ID", "output should contain Run ID header")
assert.Contains(t, output, "12345", "output should contain RunID value")
assert.NotContains(t, output, "should not appear", "output should not contain skipped field")

// ❌ CURRENT - From TestRenderMap
if !strings.Contains(output, "key1") {
    t.Error("Output should contain key1")
}
if !strings.Contains(output, "value1") {
    t.Error("Output should contain value1")
}

// ✅ IMPROVED
assert.Contains(t, output, "key1", "map output should contain key1")
assert.Contains(t, output, "value1", "map output should contain value1")

// ❌ CURRENT - From TestParseConsoleTag (table-driven but no testify)
if result.skip != tt.expected.skip {
    t.Errorf("skip: got %v, want %v", result.skip, tt.expected.skip)
}
if result.omitempty != tt.expected.omitempty {
    t.Errorf("omitempty: got %v, want %v", result.omitempty, tt.expected.omitempty)
}

// ✅ IMPROVED
assert.Equal(t, tt.expected.skip, result.skip, "skip flag should match")
assert.Equal(t, tt.expected.omitempty, result.omitempty, "omitempty flag should match")
assert.Equal(t, tt.expected.header, result.header, "header value should match")
assert.Equal(t, tt.expected.title, result.title, "title value should match")

Why this matters: Testify provides:

• Clearer error messages with actual vs expected values
• Better test output formatting
• Consistent patterns across the codebase
• Less verbose assertion code

2. Import Statement Required

Action Needed:
Add testify import at the top of the test file:

import (
    "reflect"
    "strings"
    "testing"
    
    "github.com/stretchr/testify/assert"  // ADD THIS
)

3. Specific Test Conversions

Priority High - Most Impactful Conversions:

1. TestRenderStruct_SimpleStruct (lines 32-58)

   • Replace 6 manual string checks with assert.Contains / assert.NotContains

2. TestRenderStruct_OmitEmpty (lines 60-89)

   • Replace 4 manual string checks with assert.Contains

3. TestRenderSlice_AsTable (lines 91-115)

   • Replace 8 manual string checks with assert.Contains

4. TestRenderMap (lines 117-138)

   • Replace 8 manual string checks with assert.Contains

5. TestParseConsoleTag (lines 140-210)

   • Replace 4 manual equality checks per test case with assert.Equal
   • This table-driven test is perfect for testify

6. TestIsZeroValue (lines 212-238)

   • Replace manual equality check with assert.Equal

7. TestFormatFieldValue (lines 240-262)

   • Replace manual equality check with assert.Equal

8. TestFormatNumber (lines 396-449)

   • Replace 41 manual equality checks with assert.Equal
   • Add descriptive failure messages

9. TestBuildTableConfig (lines 307-328)

   • Replace manual equality checks with assert.Equal and assert.Len

10. All "Complex Example" tests (lines 264-518)

    • Convert all manual string checks to testify assertions

4. Example Full Test Conversion

Before:

func TestRenderStruct_SimpleStruct(t *testing.T) {
    data := TestOverview{
        RunID:      12345,
        Workflow:   "test-workflow",
        Status:     "completed",
        EmptyField: "",
        SkipField:  "should not appear",
    }

    output := RenderStruct(data)

    // Check that basic fields are present
    if !strings.Contains(output, "Run ID") {
        t.Errorf("Output should contain Run ID field, got:\n%s", output)
    }
    if !strings.Contains(output, "12345") {
        t.Error("Output should contain RunID value")
    }
    if !strings.Contains(output, "test-workflow") {
        t.Error("Output should contain Workflow value")
    }

    // Check that skip field is not present
    if strings.Contains(output, "should not appear") {
        t.Error("Output should not contain skipped field")
    }
}

After (with testify):

func TestRenderStruct_SimpleStruct(t *testing.T) {
    data := TestOverview{
        RunID:      12345,
        Workflow:   "test-workflow",
        Status:     "completed",
        EmptyField: "",
        SkipField:  "should not appear",
    }

    output := RenderStruct(data)

    // Check that basic fields are present (using header names from tags)
    assert.Contains(t, output, "Run ID", "output should contain Run ID header from tag")
    assert.Contains(t, output, "12345", "output should contain RunID value")
    assert.Contains(t, output, "test-workflow", "output should contain Workflow value")

    // Check that skip field is not present
    assert.NotContains(t, output, "should not appear", "output should not contain skipped field (console:\"-\" tag)")
}

Benefits:

• 4 fewer lines of code
• Better error messages on failure
• Consistent with repository patterns
• More readable assertions

Implementation Guidelines

Priority Order

1. CRITICAL: Add testify import statement
2. HIGH: Convert all manual string checks to assert.Contains / assert.NotContains (80% of the file)
3. HIGH: Convert all equality checks to assert.Equal in table-driven tests
4. MEDIUM: Add descriptive assertion messages to all assertions
5. LOW: Consider using require.* for critical setup (though this file doesn't have much setup)

Conversion Pattern

For every test function, follow this pattern:

1. Add testify import if not present
2. Replace if !strings.Contains(output, X) with assert.Contains(t, output, X, "message")
3. Replace if strings.Contains(output, X) with assert.NotContains(t, output, X, "message")
4. Replace manual equality checks with assert.Equal(t, expected, actual, "message")
5. Replace length checks with assert.Len(t, slice, expectedLen, "message")
6. Add descriptive messages explaining what's being tested

Best Practices from scratchpad/testing.md

• ✅ Use assert.* for test validations (this file doesn't need require.*)
• ✅ Keep existing table-driven tests (they're good!)
• ✅ Always include helpful assertion messages
• ✅ Use assert.Contains for substring checks (not manual strings.Contains)
• ✅ Use assert.Equal for exact equality (not manual == checks)

Testing Commands

# Run tests for this file
go test -v ./pkg/console/ -run TestRender

# Run specific test
go test -v ./pkg/console/ -run TestRenderStruct_SimpleStruct

# Run with coverage
go test -cover ./pkg/console/

# Run all tests before committing
make test-unit

Acceptance Criteria

• Testify import added: "github.com/stretchr/testify/assert"
• All manual strings.Contains checks replaced with assert.Contains / assert.NotContains
• All manual equality checks replaced with assert.Equal
• All manual length checks replaced with assert.Len
• All assertions include descriptive messages
• Tests pass: go test -v ./pkg/console/
• Full test suite passes: make test-unit
• Code follows patterns in scratchpad/testing.md

Expected Impact

Before:

• ❌ 0 testify assertions
• ❌ ~60+ manual string checks
• ❌ ~15+ manual equality checks
• ❌ Poor error messages
• ❌ Inconsistent with repository standard

After:

• ✅ 75+ testify assertions
• ✅ Clear, structured error messages
• ✅ Consistent with repository patterns
• ✅ Easier to maintain and extend
• ✅ Better debugging experience

Additional Context

• Repository Testing Guidelines: See scratchpad/testing.md for comprehensive testing patterns
• Example Tests: Look at pkg/workflow/*_test.go for testify examples (though most use both assert and require)
• Testify Documentation: https://github.com/stretchr/testify

This is a high-impact, low-risk refactoring. The test logic is already solid - we're just modernizing the assertions for better developer experience and consistency.

---

Priority: Medium
Effort: Medium (~2-3 hours to convert all 15 test functions)
Expected Impact: High - Improved test quality, better error messages, easier maintenance

Files Involved:

• Test file: pkg/console/render_test.go (519 lines, 15 test functions)
• Source file: pkg/console/render.go (2 exported functions, good coverage)

AI generated by Daily Testify Uber Super Expert

• expires on Feb 6, 2026, 11:26 AM UTC

[github-actions]

🍪 Issue Monster has assigned this to Copilot!

I've identified this issue as a good candidate for automated resolution and assigned it to the Copilot agent.

The Copilot agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

AI generated by Issue Monster