Improve test coverage and remove redundant tests in add_command_test.go

[Copilot]

The test file had incomplete coverage (67% of exported functions) with redundant and placeholder tests that provided false confidence.

Changes

Removed (2 tests)

• TestAddCommandStructure - duplicated nil check already in TestNewAddCommand
• TestAddWorkflows_InvalidNumber - placeholder that never invoked the function under test

Enhanced (1 test)

• TestAddWorkflows - consolidated from TestAddWorkflows_EmptyWorkflows, added table-driven scenarios for empty list and repo-only spec handling

Added (3 tests)

• TestAddResolvedWorkflows - covers previously untested exported function with number validation error cases
• TestAddWorkflowsResult - struct initialization and field access validation across 5 scenarios
• TestAddCommandFlagInteractions - edge cases for flag combinations (--no-stop-after with --stop-after, --pr alias, --force with --number)

Example

Before:

func TestAddWorkflows_InvalidNumber(t *testing.T) {
    // ... table setup ...
    if tt.expectError && tt.number <= 0 {
        assert.LessOrEqual(t, tt.number, 0, "Invalid number should be non-positive")
        // Note: Never calls AddWorkflows()
    }
}

After:

func TestAddResolvedWorkflows(t *testing.T) {
    tests := []struct {
        name          string
        number        int
        errorContains string
    }{
        {"invalid number - zero", 0, "number of copies must be a positive integer"},
        {"invalid number - negative", -1, "number of copies must be a positive integer"},
    }
    // ... actually calls AddResolvedWorkflows() and validates behavior
}

Result

• Test functions: 7 → 8 (net +1 after removing 2, adding 3)
• Exported function coverage: 67% → 100%
• All assertions follow testify best practices (require.* for setup, assert.* for validation)

Original prompt

---

This section details on the original issue you should resolve

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

The test file pkg/cli/add_command_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.

Current State

• Test File: pkg/cli/add_command_test.go
• Source File: pkg/cli/add_command.go
• Test Functions: 7 test functions
• Lines of Code: 208 lines
• Assertions: 38 testify assertions
• Last Modified: 2026-01-29

Test Quality Analysis

Strengths ✅

1. Excellent testify adoption - The file consistently uses assert.* and require.* throughout
2. Good use of table-driven tests - Tests like TestAddWorkflows_InvalidNumber and TestAddCommandFlagDefaults follow table-driven patterns
3. Comprehensive flag testing - Thorough validation of command flags, shorthands, and defaults

Areas for Improvement 🎯

1. Missing Test Coverage for Core Functions

Current Gap: The source file contains 3 exported functions, but only 2 are tested:

• ✅ NewAddCommand - TESTED
• ✅ AddWorkflows - PARTIALLY TESTED (only error cases)
• ❌ AddResolvedWorkflows - NOT TESTED

Impact: AddResolvedWorkflows is a critical function that handles the actual workflow addition logic after resolution. This represents a significant coverage gap.

Recommended Test Cases:

View Proposed Test Implementation

func TestAddResolvedWorkflows(t *testing.T) {
    tests := []struct {
        name           string
        workflowStrings []string
        resolved       *ResolvedWorkflows
        number         int
        shouldErr      bool
        errorContains  string
    }{
        {
            name:           "nil resolved workflows",
            workflowStrings: []string{"test-workflow"},
            resolved:       nil,
            number:         1,
            shouldErr:      true,
            errorContains:  "resolved",
        },
        {
            name:           "empty resolved workflows",
            workflowStrings: []string{"test-workflow"},
            resolved:       &ResolvedWorkflows{Workflows: []*ResolvedWorkflow{}},
            number:         1,
            shouldErr:      true,
            errorContains:  "no workflows",
        },
        {
            name:           "invalid number zero",
            workflowStrings: []string{"test-workflow"},
            resolved: &ResolvedWorkflows{
                Workflows: []*ResolvedWorkflow{
                    {Spec: &WorkflowSpec{WorkflowName: "test"}},
                },
            },
            number:         0,
            shouldErr:      true,
            errorContains:  "number",
        },
        {
            name:           "invalid number negative",
            workflowStrings: []string{"test-workflow"},
            resolved: &ResolvedWorkflows{
                Workflows: []*ResolvedWorkflow{
                    {Spec: &WorkflowSpec{WorkflowName: "test"}},
                },
            },
            number:         -1,
            shouldErr:      true,
            errorContains:  "number",
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            result, err := AddResolvedWorkflows(
                tt.workflowStrings,
                tt.resolved,
                tt.number,
                false, // verbose
                false, // quiet
                "",    // engineOverride
                "",    // name
                false, // force
                "",    // appendText
                false, // createPR
                false, // push
                false, // noGitattributes
                "",    // workflowDir
                false, // noStopAfter
                "",    // stopAfter
            )

            if tt.shouldErr {
                require.Error(t, err, "Should return error for: %s", tt.name)
                if tt.errorContains != "" {
                    assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected text")
                }
                assert.Nil(t, result, "Result should be nil on error")
            } else {
                require.NoError(t, err, "Should not return error for: %s", tt.name)
                assert.NotNil(t, result, "Result should not be nil on success")
            }
        })
    }
}

Why this matters: Testing the full function chain ensures that workflow addition logic works correctly end-to-end, not just in error cases.

2. Incomplete Test for AddWorkflows Success Cases

Current Issue: TestAddWorkflows_EmptyWorkflows only tests the error case. The success path of AddWorkflows is not tested.

**Recommended Ch...

• Fixes [testify-expert] Improve Test Quality: pkg/cli/add_command_test.go #12508

---

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