[testify-expert] Improve Test Quality: pkg/workflow/strings_test.go

[agentic-workflows-dev]

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 258-261 with assert.Equal(t, tt.expected, result, message)
   • 8 test cases with clear expected values

5. TestSanitizeName (lines 266-446)

   • Most complex test with 21 test cases and struct options
   • Replace line 439-442 with assert.Equal(t, tt.expected, result, message)
   • Benefits most from testify's struct diff capabilities

Implementation Guidelines

Step-by-Step Refactoring

Step 1: Add testify import

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

Step 2: Refactor each test function

For each test function, replace the assertion pattern:

Before:

if result != tt.expected {
    t.Errorf("FunctionName(%q) = %q, want %q", tt.input, result, tt.expected)
}

After:

assert.Equal(t, tt.expected, result, "FunctionName should process input correctly")

For slice comparisons, before:

if len(result) != len(tt.expected) {
    t.Errorf("FunctionName() length = %d, want %d", len(result), len(tt.expected))
    return
}

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

After:

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

Testing Commands

# Run tests for this file
go test -v ./pkg/workflow -run TestSortStrings
go test -v ./pkg/workflow -run TestSanitize

# Run all tests in the package
make test-unit

# Check that changes don't break anything
make test

Expected Benefits

Before (manual assertions):

--- FAIL: TestSortStrings (0.00s)
    --- FAIL: TestSortStrings/reverse_order (0.00s)
        strings_test.go:58: SortStrings() at index 0 = "a", want "c"

After (testify):

--- FAIL: TestSortStrings (0.00s)
    --- FAIL: TestSortStrings/reverse_order (0.00s)
        strings_test.go:50: 
            	Error:      	Not equal: 
            	            	expected: []string{"c", "b", "a"}
            	            	actual  : []string{"a", "b", "c"}
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1,4 +1,4 @@
            	            	 ([]string) (len=3) {
            	            	- (string) (len=1) "c",
            	            	+ (string) (len=1) "a",
            	            	  (string) (len=1) "b",
            	            	- (string) (len=1) "a"
            	            	+ (string) (len=1) "c"
            	            	 }
            	Test:       	TestSortStrings/reverse_order
            	Messages:   	SortStrings should return correctly sorted slice

Acceptance Criteria

• Import github.com/stretchr/testify/assert added
• TestSortStrings refactored to use assert.Equal
• TestSortPermissionScopes refactored to use assert.Equal
• TestSanitizeWorkflowName refactored to use assert.Equal
• TestShortenCommand refactored to use assert.Equal
• TestSanitizeName refactored to use assert.Equal
• All assertions include helpful messages
• Tests pass: make test-unit
• No functionality changes - only assertion improvements

Additional Context

• Repository Testing Guidelines: See specs/testing.md for comprehensive testing patterns
• Example Tests: Look at pkg/workflow/compiler_test.go or other recent tests for testify usage examples
• Testify Documentation: https://github.com/stretchr/testify
• Current Test Pattern: This file uses excellent table-driven tests - we're just modernizing the assertions

---

Priority: Medium
Effort: Small (~30 minutes - simple find/replace pattern across 5 functions)
Expected Impact: Improved debugging experience, better error messages, consistency with codebase standards

Estimated Changes:

• Lines to add: ~1 import line
• Lines to modify: ~5 assertion blocks (one per test function)
• Lines to remove: ~20 lines (manual comparison loops)
• Net change: -15 lines, +improved error output

Files Involved:

• Test file: pkg/workflow/strings_test.go
• Source file: pkg/workflow/strings.go (no changes needed)

AI generated by Daily Testify Uber Super Expert

[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