Improve test quality in pkg/cli/actionlint_test.go: adopt testify, reduce duplication

[Copilot]

• Add testify/assert and testify/require imports
• Extract captureStderr helper to reduce code duplication
• Move captureStderr to testutil.CaptureStderr for codebase-wide reuse
• Merge TestParseAndDisplayActionlintOutputMultiFile into TestParseAndDisplayActionlintOutput
• Convert all t.Errorf/t.Error/t.Fatal to testify assertions with descriptive messages
• Fix TestGetActionlintVersion to only test the cached version case properly
• Convert TestInitActionlintStats to use testify assertions
• Convert TestGetActionlintDocsURL to use testify assertions
• All tests pass and linting is clean

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[testify-expert] Improve Test Quality: pkg/cli/actionlint_test.go</issue_title>
<issue_description>The test file pkg/cli/actionlint_test.go has been selected for quality improvement by the Testify Uber Super Expert. This issue provides specific, actionable recommendations to enhance test quality using testify best practices.

Current State

• Test File: pkg/cli/actionlint_test.go
• Source File: pkg/cli/actionlint.go
• Test Functions: 5 test functions (TestParseAndDisplayActionlintOutput, TestGetActionlintVersion, TestParseAndDisplayActionlintOutputMultiFile, TestDisplayActionlintSummary, TestInitActionlintStats, TestGetActionlintDocsURL)
• Lines of Code: 427 lines
• Critical Issue: Zero testify usage despite testify being available and used throughout the codebase

Test Quality Analysis

Strengths ✅

• Uses table-driven tests with t.Run() subtests throughout
• Good test case coverage with distinct scenarios (errors, multi-file, no errors)
• Tests include both success and failure paths

Areas for Improvement 🎯

1. No Testify Assertions — Zero testify usage

The file does not import testify at all, relying entirely on manual Go testing patterns (t.Errorf, t.Error, t.Fatal). This is inconsistent with the rest of the codebase.

Current Issues:

// ❌ CURRENT (anti-pattern) — manual error checks
if tt.expectError && err == nil {
    t.Errorf("Expected error but got none")
}
if !tt.expectError && err != nil {
    t.Errorf("Unexpected error: %v", err)
}

// ❌ CURRENT — manual equality check
if count != tt.expectedCount {
    t.Errorf("Expected count %d, got %d", tt.expectedCount, count)
}

// ❌ CURRENT — manual nil check
if actionlintStats == nil {
    t.Fatal("actionlintStats should not be nil after initialization")
}

// ❌ CURRENT — manual string comparison
if result != tt.expected {
    t.Errorf("getActionlintDocsURL(%q) = %q, want %q", tt.kind, result, tt.expected)
}

Recommended Changes:

// ✅ IMPROVED (testify)
import (
    "github.com/stretchr/testify/assert"
    "github.com/stretchr/testify/require"
)

// Error checks
if tt.expectError {
    require.Error(t, err, "should return error for invalid input")
} else {
    require.NoError(t, err, "should not return error for valid input")
}

// Equality checks
assert.Equal(t, tt.expectedCount, count, "error count should match expected")

// Nil checks
require.NotNil(t, actionlintStats, "actionlintStats should be initialized")

// String comparisons
assert.Equal(t, tt.expected, result, "docs URL should match expected for kind %q", tt.kind)

Why this matters: Testify provides clearer error messages, better test output, and is the standard used throughout this codebase (see scratchpad/testing.md).

2. Duplicated Test Body in TestParseAndDisplayActionlintOutputMultiFile

TestParseAndDisplayActionlintOutputMultiFile is a separate test function that duplicates the exact same assertion logic as TestParseAndDisplayActionlintOutput. They test the same function with different data but have identical assertion boilerplate.

Recommended Change: Merge into a single TestParseAndDisplayActionlintOutput with additional table-driven test cases:

func TestParseAndDisplayActionlintOutput(t *testing.T) {
    tests := []struct {
        name          string
        stdout        string
        verbose       bool
        expectedParts []string
        expectError   bool
        expectedCount int
        expectedKinds map[string]int
    }{
        // ... existing single-file cases ...
        {
            name: "multiple errors from multiple files",
            // ... multi-file case data ...
        },
        {
            name: "errors from three files",
            // ... three-file case data ...
        },
    }
    // shared assertion loop...
}

3. Duplicated Stderr Capture Setup

The stderr capture setup is copy-pasted verbatim in TestParseAndDisplayActionlintOutput, TestParseAndDisplayActionlintOutputMultiFile, and TestDisplayActionlintSummary. This is brittle and hard to maintain.

Recommended Change: Extract a helper:

// captureStderr runs fn and returns what was written to stderr
func captureStderr(t *testing.T, fn func()) string {
    t.Helper()
    r, w, err := os.Pipe()
    require.NoError(t, err, "should create pipe for stderr capture")

    origStderr := os.Stderr
    os.Stderr = w
    t.Cleanup(func() { os.Stderr = origStderr })

    fn()

    w.Close()
    var buf bytes.Buffer
    _, err = buf.ReadFrom(r)
    require.NoError(t, err, "should read stderr output")
    return buf.String()
}

Usage:

output := captureStderr(t, func() {
    count, kinds, err = parseAndDisplayActionlintOutput(tt.stdout, tt.verbose)
})

4. TestInitActionlintStats — No Testify, Verbose Manual Checks

This function uses 5 separate ...

Custom agent used: agentic-workflows
GitHub Agentic Workflows (gh-aw) - Create, debug, and upgrade AI-powered workflows with intelligent prompt routing

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

---

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

[pelikhan]

@copilot find a helper for this or refactor into a test helper.

[Copilot]

Moved into testutil.CaptureStderr in pkg/testutil/tempdir.go so it's reusable across the codebase. The local captureStderr helper is removed and all call sites in actionlint_test.go now use testutil.CaptureStderr(...). (95a4b85)

[Copilot]

Pull request overview

This PR improves test quality in pkg/cli/actionlint_test.go by adopting testify assertions, reducing code duplication, and improving test maintainability. The changes align with codebase testing conventions seen throughout the repository where testify is used extensively.

Changes:

• Adopted testify/assert and testify/require for all test assertions
• Extracted stderr capture logic into a reusable testutil.CaptureStderr helper
• Consolidated test cases by merging multi-file tests into the main test function

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
pkg/testutil/tempdir.go	Added CaptureStderr helper function for reusable stderr capture across tests
pkg/cli/actionlint_test.go	Converted all manual assertions to testify, merged duplicate test functions, simplified test logic

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.