Skip to content

Refactor strings_test.go to use testify assertions#9894

Merged
pelikhan merged 2 commits intomainfrom
copilot/improve-strings-test-quality
Jan 14, 2026
Merged

Refactor strings_test.go to use testify assertions#9894
pelikhan merged 2 commits intomainfrom
copilot/improve-strings-test-quality

Conversation

Copy link
Contributor

Copilot AI commented Jan 14, 2026

Replace manual assertion loops with testify/assert.Equal for clearer test failures and improved diff output.

Changes

  • Import: Added github.com/stretchr/testify/assert
  • Assertion Pattern: Replaced manual length checks and element-by-element loops with single-line assert.Equal calls across all 5 test functions
  • Messages: Added descriptive assertion messages for debugging context
  • Net Impact: -20 lines (445 → 425)

Example

Before:

if len(result) != len(tt.expected) {
    t.Errorf("SortStrings() length = %d, want %d", len(result), len(tt.expected))
    return
}
for i := range result {
    if result[i] != tt.expected[i] {
        t.Errorf("SortStrings() at index %d = %q, want %q", i, result[i], tt.expected[i])
    }
}

After:

assert.Equal(t, tt.expected, result, "SortStrings should return correctly sorted slice")

Testify automatically handles slice comparison and provides structured diff output on failure, showing visual side-by-side comparison instead of individual element mismatches.

Functions Updated

  • TestSortStrings (6 cases)
  • TestSortPermissionScopes (5 cases)
  • TestSanitizeWorkflowName (14 cases)
  • TestShortenCommand (8 cases)
  • TestSanitizeName (21 cases)

All 58 subtests pass with identical behavior.

Original prompt

This section details on the original issue you should resolve

<issue_title>[testify-expert] Improve Test Quality: pkg/workflow/strings_test.go</issue_title>
<issue_description>## Overview

The test file pkg/workflow/strings_test.go has been selected for quality improvement by the Testify Uber Super Expert. While this test file demonstrates excellent use of table-driven tests and comprehensive coverage, there are opportunities to enhance it by adopting testify assertions for clearer error messages and better debugging experience.

Current State

  • Test File: pkg/workflow/strings_test.go
  • Source File: pkg/workflow/strings.go
  • Test Functions: 5 test functions (all exported functions tested ✅)
  • Lines of Code: 445 lines
  • Last Modified: 2026-01-14
  • Test Coverage: 100% function coverage (all 5 exported functions have tests)

Test Quality Analysis

Strengths ✅

  1. Excellent table-driven test patterns - All tests use table-driven approach with descriptive test case names
  2. Comprehensive test coverage - Every exported function in the source file has corresponding tests
  3. Edge case coverage - Tests include empty strings, unicode, special characters, and boundary conditions
  4. Good test organization - Tests use t.Run() for subtests with clear naming

Areas for Improvement 🎯

1. Testify Assertions

Current Issues:

  • All assertions use manual comparison with t.Errorf() instead of testify assertions
  • Error messages are manually formatted, making test failures less readable
  • Example: Lines 51-60 use manual length comparison and loop-based element comparison
  • Example: Lines 106-115, 199-203, 257-261, 438-442 all use t.Errorf() pattern

Recommended Changes:

// ❌ CURRENT (lines 51-60)
if len(result) != len(tt.expected) {
    t.Errorf("SortStrings() length = %d, want %d", len(result), len(tt.expected))
    return
}

for i := range result {
    if result[i] != tt.expected[i] {
        t.Errorf("SortStrings() at index %d = %q, want %q", i, result[i], tt.expected[i])
    }
}

// ✅ IMPROVED (testify)
assert.Equal(t, tt.expected, result, "SortStrings should return correctly sorted slice")
// ❌ CURRENT (line 202)
if result != tt.expected {
    t.Errorf("SanitizeWorkflowName(%q) = %q, want %q", tt.input, result, tt.expected)
}

// ✅ IMPROVED (testify)
assert.Equal(t, tt.expected, result, "SanitizeWorkflowName should sanitize input correctly")

Why this matters:

  • Testify provides diff output for complex comparisons (arrays, structs)
  • Clearer failure messages with better formatting
  • Standard pattern used throughout the codebase (see specs/testing.md)
  • Easier debugging with structured output

2. Imports and Package Setup

Current State:

import "testing"

Recommended:

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

3. Consistent Assertion Pattern

Recommended Pattern for String Comparisons:

func TestSanitizeWorkflowName(t *testing.T) {
    tests := []struct {
        name     string
        input    string
        expected string
    }{
        // ... test cases
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            result := SanitizeWorkflowName(tt.input)
            assert.Equal(t, tt.expected, result, 
                "SanitizeWorkflowName(%q) should return expected value", tt.input)
        })
    }
}

Recommended Pattern for Slice Comparisons:

func TestSortStrings(t *testing.T) {
    tests := []struct {
        name     string
        input    []string
        expected []string
    }{
        // ... test cases
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            // Make a copy to avoid modifying the test case
            result := make([]string, len(tt.input))
            copy(result, tt.input)
            
            SortStrings(result)
            
            assert.Equal(t, tt.expected, result,
                "SortStrings should correctly sort the slice")
        })
    }
}

Benefits:

  • assert.Equal automatically shows detailed diffs for slices
  • No need for manual length checks - testify does it
  • No need for element-by-element comparison loops
  • Better error output showing expected vs actual

4. Specific Functions to Update

Priority Order (High to Low):

  1. TestSortStrings (lines 5-63)

    • Replace manual length check and loop with assert.Equal(t, tt.expected, result)
    • Simplifies 10 lines of comparison logic to 1 line
  2. TestSortPermissionScopes (lines 65-118)

    • Same pattern as TestSortStrings
    • Replace lines 106-115 with single assert.Equal
  3. TestSanitizeWorkflowName (lines 120-206)

    • Replace line 200-203 with assert.Equal(t, tt.expected, result, message)
    • 14 test cases benefit from better error messages
  4. TestShortenCommand (lines 208-264)

    • Replace line ...

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Add testify/assert import
- Replace manual length checks and loops with assert.Equal
- Add helpful assertion messages for all tests
- Reduce test code from 445 to 425 lines (-20 lines)
- All 5 test functions refactored: TestSortStrings, TestSortPermissionScopes,
  TestSanitizeWorkflowName, TestShortenCommand, TestSanitizeName

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve test quality in strings_test.go Refactor strings_test.go to use testify assertions Jan 14, 2026
Copilot AI requested a review from mnkiefer January 14, 2026 02:51
@pelikhan pelikhan marked this pull request as ready for review January 14, 2026 06:06
@pelikhan pelikhan merged commit 99c636f into main Jan 14, 2026
149 checks passed
@pelikhan pelikhan deleted the copilot/improve-strings-test-quality branch January 14, 2026 06:07
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/workflow/strings_test.go

3 participants