Skip to content

Modernize console render tests with testify assertions#13722

Merged
pelikhan merged 2 commits intomainfrom
codex/improve-test-quality-render-test
Feb 4, 2026
Merged

Modernize console render tests with testify assertions#13722
pelikhan merged 2 commits intomainfrom
codex/improve-test-quality-render-test

Conversation

@Codex
Copy link
Contributor

@Codex Codex AI commented Feb 4, 2026

The console rendering tests relied on manual strings.Contains/t.Errorf checks, leading to noisy failures and inconsistent style.

  • Assertion overhaul: Replaced ad hoc checks in pkg/console/render_test.go with testify/assert for clearer failures across struct, map, slice, formatting, and tag parsing cases.
  • Import update: Added the github.com/stretchr/testify/assert import and removed unused strings.
  • Consistency: Added descriptive assertion messages to align with repo testing standards.

Example:

output := RenderStruct(data)
assert.Contains(t, output, "Run ID", "output should contain Run ID header from tag")
assert.NotContains(t, output, "should not appear", "output should not contain skipped field")
Original prompt

This section details on the original issue you should resolve

<issue_title>[testify-expert] Improve Test Quality: pkg/console/render_test.go</issue_title>
<issue_description>### 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 des...

@Codex Codex AI changed the title [WIP] Improve test quality by adding testify assertions Modernize console render tests with testify assertions Feb 4, 2026
@Codex Codex AI requested a review from pelikhan February 4, 2026 13:29
@pelikhan pelikhan marked this pull request as ready for review February 4, 2026 13:34
Copilot AI review requested due to automatic review settings February 4, 2026 13:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes the console render tests by replacing manual error checking with testify assertions, improving test readability and failure diagnostics.

Changes:

  • Replaced ~60 manual strings.Contains checks and t.Error/t.Errorf calls with testify's assert.Contains, assert.NotContains, assert.Equal, and assert.Len
  • Removed unused strings import (no longer needed after assertion migration)
  • Added descriptive assertion messages following repository conventions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pelikhan pelikhan merged commit f250395 into main Feb 4, 2026
130 of 131 checks passed
@pelikhan pelikhan deleted the codex/improve-test-quality-render-test branch February 4, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants