Improve test quality in format_preservation_test.go with testify assertions and table-driven tests

[Copilot]

The test file used manual error checking (if err != nil { t.Fatalf() }) instead of testify assertions, lacked edge case coverage, and tested only a single scenario.

Changes

• Converted to testify assertions: Replaced manual error checks with require.NoError() and assert.Contains() with descriptive messages
• Table-driven test structure: Implemented 11 test cases covering success paths, error conditions, and edge cases (malformed YAML, missing delimiters, unicode characters, 2-space vs 4-space indentation, tab rejection)
• Organized subtests: Added TestFormatPreservationSubtests grouping assertions by concern (comments, whitespace, indentation, source field, content structure)
• Removed debug output: Eliminated fmt.Printf statements for clean test execution

Example

Before:

result, err := addSourceToWorkflow(content, "test/repo/workflow.md@v1.0.0")
if err != nil {
    t.Fatalf("Error adding source: %v", err)
}
if !strings.Contains(result, "# Daily run at 9 AM UTC on weekdays") {
    t.Error("Comments were not preserved")
}

After:

result, err := addSourceToWorkflow(tt.content, tt.source)
if tt.shouldErr {
    require.Error(t, err, tt.description)
    return
}
require.NoError(t, err, tt.description)
for _, expected := range tt.mustContain {
    assert.Contains(t, result, expected,
        "%s - expected substring not found: %q", tt.description, expected)
}

Impact: 1 scenario → 16 test cases with comprehensive edge case coverage and clear failure messages.

Original prompt

---

This section details on the original issue you should resolve

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

The test file pkg/cli/format_preservation_test.go has been selected for quality improvement by the Testify Uber Super Expert. This issue provides specific, actionable recommendations to enhance test quality, coverage, and maintainability using testify best practices. The file currently uses manual error checking instead of testify assertions and would benefit from table-driven tests and better organization.

Current State

• Test File: pkg/cli/format_preservation_test.go
• Source File: pkg/cli/add_command.go (961 lines)
• Test Functions: 1 test function (TestFormatPreservationManual)
• Lines of Code: 69 lines
• Function Tested: addSourceToWorkflow(content, source string) (string, error)

Test Quality Analysis

Strengths ✅

1. Good test coverage of formatting concerns - Tests multiple aspects of format preservation (comments, blank lines, indentation)
2. Clear test data - Uses realistic YAML content with various formatting elements
3. Descriptive error messages - Each assertion explains what formatting aspect was not preserved

Areas for Improvement 🎯

1. Testify Assertions

Current Issues:

• Using if err != nil { t.Fatalf() } instead of require.NoError(t, err)
• Using if !strings.Contains() { t.Error() } instead of assert.Contains(t, result, expected)
• Manual boolean checks instead of testify assertion methods

Recommended Changes:

// ❌ CURRENT (anti-pattern)
result, err := addSourceToWorkflow(content, "test/repo/workflow.md@v1.0.0")
if err != nil {
    t.Fatalf("Error adding source: %v", err)
}

if !strings.Contains(result, "# Daily run at 9 AM UTC on weekdays") {
    t.Error("Comments were not preserved")
}

// ✅ IMPROVED (testify)
result, err := addSourceToWorkflow(content, "test/repo/workflow.md@v1.0.0")
require.NoError(t, err, "adding source should succeed")

assert.Contains(t, result, "# Daily run at 9 AM UTC on weekdays", 
    "YAML comments should be preserved in output")
assert.Contains(t, result, "\n\n", 
    "blank lines should be preserved in output")
assert.Contains(t, result, "stop-after: +2h", 
    "inline comments should be preserved in output")
assert.Contains(t, result, "    workflow_dispatch:", 
    "4-space indentation should be preserved in output")

Why this matters: Testify provides clearer error messages, better test output, and is the standard used throughout this codebase (see specs/testing.md). The assert.Contains method will show the actual vs expected content on failure.

Example from codebase: See pkg/cli/actions_build_command_test.go:16 for good testify usage:

require.NoError(t, err, "Failed to get current directory")

2. Table-Driven Tests

Current Issues:

• Single test case that could be extended to cover multiple scenarios
• Hardcoded test data makes it difficult to add more test cases
• Each formatting concern could be tested separately with specific inputs

Recommended Changes:

func TestFormatPreservation(t *testing.T) {
    tests := []struct {
        name        string
        content     string
        source      string
        shouldErr   bool
        mustContain []string
        description string
    }{
        {
            name: "preserves comments and indentation",
            content: `---
on:
    workflow_dispatch:

    schedule:
        # Daily run at 9 AM UTC on weekdays  
        - cron: "0 9 * * 1-5"

    # Auto-stop workflow after 2 hours
    stop-after: +2h

timeout_minutes: 45
---

# Test Formatting Preservation`,
            source: "test/repo/workflow.md@v1.0.0",
            shouldErr: false,
            mustContain: []string{
                "# Daily run at 9 AM UTC on weekdays",
                "\n\n",
                "stop-after: +2h",
                "    workflow_dispatch:",
            },
            description: "should preserve YAML comments, blank lines, and indentation",
        },
        {
            name: "preserves inline comments",
            content: `---
tools:
    # Enable web search functionality
    web-search: null
    
    # Memory caching for better performance  
    cache-memory: true
---

# Test`,
            source: "test/repo/workflow.md@v1.0.0",
            shouldErr: false,
            mustContain: []string{
                "# Enable web search functionality",
                "# Memory caching for better performance",
            },
            description: "should preserve inline comments in tools section",
        },
        {
            name: "preserves complex nested indentation",
            content: `---
on:
    schedule:
        - cron: "0 9 * * 1-5"
          # Comment on nested item
          enabled: true
---

# Test`,
            source: "test/repo/workflow.md@v1.0.0",
      ...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes githubnext/gh-aw#10548

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/githubnext/gh-aw/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

[pelikhan]

@copilot merge main