Skip to content

Test: Add edge case coverage and improve diagnostics for strings_test.go#11252

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

Test: Add edge case coverage and improve diagnostics for strings_test.go#11252
pelikhan merged 2 commits intomainfrom
copilot/improve-test-quality-strings

Conversation

Copy link
Contributor

Copilot AI commented Jan 22, 2026

The test file pkg/workflow/strings_test.go lacked coverage for nil inputs and unicode edge cases, and had generic assertion messages that made failures harder to debug.

Changes

  • Nil slice handling: Added TestSortStrings_NilSlice() and TestSortPermissionScopes_NilSlice() to verify Go's sort functions handle nil slices without panics

  • Nil options coverage: Added TestSanitizeName_NilOptions() with 7 test cases covering the nil codepath (lines 155-157 in source). Key insight: when opts is nil, special characters are removed completely, not replaced with hyphens:

    SanitizeName("workflow.test.name", nil)  // Returns: "workflowtestname"
    SanitizeName("test_workflow_name", nil) // Returns: "test-workflow-name"
  • Unicode truncation: Added 2 test cases to TestShortenCommand() documenting that truncation operates on bytes (20 bytes), not characters:

    ShortenCommand("echo 你好世界 αβγ test")  // Returns: "echo 你好世界 α..." (20 bytes)
  • Assertion diagnostics: Updated all assertion messages across 5 test functions to include test case names:

    assert.Equal(t, tt.expected, result, "SortStrings failed for test case: %s", tt.name)

All 464 lines of test code passing. No functional changes to source code.

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. This file demonstrates excellent test structure with comprehensive table-driven tests, but has opportunities for improvement in test coverage and edge case testing.

Current State

  • Test File: pkg/workflow/strings_test.go
  • Source File: pkg/workflow/strings.go
  • Test Functions: 5 test functions
  • Lines of Code: 425 lines
  • Last Modified: 2026-01-22
  • Testify Usage: ✅ Already using testify assertions correctly

Test Quality Analysis

Strengths ✅

  1. Excellent table-driven test structure - All tests use proper table-driven patterns with t.Run() and descriptive test case names
  2. Good testify assertion usage - Consistently uses assert.Equal() with helpful messages
  3. Comprehensive test coverage for main functions - SortStrings, SortPermissionScopes, SanitizeWorkflowName, ShortenCommand, and SanitizeName all have thorough test cases
  4. Edge case coverage - Tests include empty strings, single elements, unicode characters, and boundary conditions

Areas for Improvement 🎯

1. Missing Test Coverage for Source Functions

Current State:
The source file strings.go exports 5 functions, but the test file only covers 5 of them. However, there's a critical gap: While SanitizeName is tested extensively, the tests could benefit from testing the function behavior when opts parameter is nil more explicitly.

Coverage Analysis:

  • SortStrings - Well tested (6 test cases)
  • SortPermissionScopes - Well tested (5 test cases)
  • SanitizeWorkflowName - Indirectly tested via SanitizeName tests (15 test cases)
  • ShortenCommand - Well tested (8 test cases)
  • ⚠️ SanitizeName - Well tested BUT missing explicit nil options test cases

2. Enhance SanitizeName Testing with Nil Options

Current Issue:
The test file has only 2 test cases that explicitly test nil options behavior (lines 256-267). Given that SanitizeName is the core function that SanitizeWorkflowName depends on, we should ensure nil options are thoroughly tested.

Recommended Test Cases:

func TestSanitizeName_NilOptions(t *testing.T) {
    tests := []struct {
        name     string
        input    string
        expected string
    }{
        {
            name:     "nil options - empty string",
            input:    "",
            expected: "",
        },
        {
            name:     "nil options - only hyphens",
            input:    "---",
            expected: "---", // Hyphens NOT trimmed when opts is nil
        },
        {
            name:     "nil options - leading/trailing hyphens",
            input:    "-workflow-",
            expected: "-workflow-", // Preserved with nil opts
        },
        {
            name:     "nil options - underscores replaced",
            input:    "test_workflow_name",
            expected: "test-workflow-name", // Underscores replaced
        },
        {
            name:     "nil options - dots replaced",
            input:    "workflow.test.name",
            expected: "workflow-test-name", // Dots NOT preserved with nil opts
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            result := SanitizeName(tt.input, nil)
            assert.Equal(t, tt.expected, result, "SanitizeName with nil options should handle edge cases correctly")
        })
    }
}

Why this matters: The function explicitly checks for nil options (line 155 in source), so we should have comprehensive tests for this code path.

3. Add Tests for Empty/Nil Slice Edge Cases

Current Issue:
While both SortStrings and SortPermissionScopes test empty slices, they don't explicitly test nil slices, which is a common edge case in Go.

Recommended Test Cases:

func TestSortStrings_NilSlice(t *testing.T) {
    var nilSlice []string
    
    // Should not panic with nil slice
    SortStrings(nilSlice)
    
    assert.Nil(t, nilSlice, "SortStrings should handle nil slice without panic")
}

func TestSortPermissionScopes_NilSlice(t *testing.T) {
    var nilSlice []PermissionScope
    
    // Should not panic with nil slice
    SortPermissionScopes(nilSlice)
    
    assert.Nil(t, nilSlice, "SortPermissionScopes should handle nil slice without panic")
}

Why this matters: Go's sort.Strings() and sort.Slice() handle nil slices gracefully, but we should explicitly test this behavior to document it and prevent regressions.

4. Test ShortenCommand with Unicode Characters

Current Issue:
The ShortenCommand tests cover most edge cases but don't test behavior with unicode characters, e...


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Added nil slice tests for SortStrings and SortPermissionScopes
- Added comprehensive nil options tests for SanitizeName
- Added unicode character tests for ShortenCommand
- Improved all assertion messages to include test case names for better debugging
- All tests passing with proper edge case coverage

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve test coverage and edge case handling in strings_test.go Test: Add edge case coverage and improve diagnostics for strings_test.go Jan 22, 2026
Copilot AI requested a review from pelikhan January 22, 2026 12:14
@pelikhan pelikhan marked this pull request as ready for review January 22, 2026 12:15
@pelikhan pelikhan merged commit 0e46c71 into main Jan 22, 2026
151 checks passed
@pelikhan pelikhan deleted the copilot/improve-test-quality-strings branch January 22, 2026 12:20
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

2 participants