Skip to content

Improve test quality: migrate action_reference_test.go to testify#19310

Merged
pelikhan merged 2 commits intomainfrom
copilot/improve-test-quality-action-reference
Mar 3, 2026
Merged

Improve test quality: migrate action_reference_test.go to testify#19310
pelikhan merged 2 commits intomainfrom
copilot/improve-test-quality-action-reference

Conversation

Copy link
Contributor

Copilot AI commented Mar 3, 2026

pkg/workflow/action_reference_test.go had zero testify usage — all assertions were raw if/t.Errorf comparisons, and two test functions used repetitive per-case t.Run blocks instead of table-driven patterns.

Changes

  • Added testify/assert import — replaces all manual comparison blocks
  • TestConvertToRemoteActionRef — converted 10 individual t.Run subtests to a single table-driven test; added nilData bool and setEmptyTag bool fields for clean nil/empty edge-case handling
  • TestCompilerActionTag — converted 5 individual t.Run subtests to a single table-driven test; added useResolve bool to distinguish convertToRemoteActionRef vs resolveActionReference call paths
  • TestResolveActionReference / TestResolveSetupActionReference — existing description struct fields now flow into assertion messages instead of being embedded in t.Errorf format strings
  • TestResolveSetupActionReferenceWithData — assertions replaced with descriptive assert.Equal calls

Before:

if ref != expected {
    t.Errorf("Expected %q, got %q", expected, ref)
}
if ref != "" {
    t.Errorf("Expected empty string with 'dev' version, got %q", ref)
}

After:

assert.Equal(t, tt.expectedRef, ref, tt.description)
assert.Empty(t, ref, "should return empty string for invalid/dev version")

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GO111MODULE 64/bin/go git rev-�� --show-toplevel go /usr/bin/git cli/install.sh..git GO111MODULE 64/bin/go git (http block)
  • https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1
    • Triggering command: /usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha GOMODCACHE erignore /usr/bin/git e=false GO111MODULE 64/bin/go git conf�� user.email resolved$ /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/vet -json GO111MODULE 64/bin/go /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v3
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha -json GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /usr/bin/git GOINSECURE GOMOD GOMODCACHE git (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v5
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env 3980905719/custom/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha /tmp/gh-aw-test-runs/20260303-021924-27764/test-1969773630 status /usr/bin/git .github/workflowgit GOPROXY 64/bin/go git rev-�� --git-dir sh /opt/hostedtoolcache/node/24.13.1/x64/bin/node "prettier" --chegit sh 64/bin/go node (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel go /usr/bin/git ithub/workflows GO111MODULE 64/bin/go git rev-�� --show-toplevel go /usr/bin/git plorer.md GO111MODULE aw.test git (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v6
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha s/test.md GOPROXY /usr/bin/infocmp GOSUMDB GOWORK 64/bin/go infocmp -1 xterm-color go /usr/bin/git -json GO111MODULE 64/bin/go git (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha list --json /usr/bin/git -json GO111MODULE 64/bin/go git -C /home/REDACTED/work/gh-aw/gh-aw/.github/workflows rev-parse /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/vet -json GO111MODULE 64/bin/go /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha --show-toplevel x_amd64/link /usr/bin/git -json GO111MODULE x_amd64/link git rev-�� --show-toplevel x_amd64/link /usr/bin/git -json GO111MODULE 64/bin/go git (http block)
  • https://api.github.com/repos/actions/github-script/git/ref/tags/v8
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go env ck 'scripts/**/*GOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha f9dd00c75c79c7baGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD erignore ache/go/1.25.0/xGO111MODULE env 1098574/b411/_pkGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD erignore go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/actions/setup-go/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha runs/20260303-021924-27764/test-1969773630 -trimpath /usr/bin/git s/test.md github.com/githurev-parse -lang=go1.25 git -C /home/REDACTED/work/gh-aw/gh-aw/.github/workflows rev-parse /usr/bin/gh -c=4 -nolocalimports -importcfg gh (http block)
  • https://api.github.com/repos/actions/setup-node/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha -test.paniconexit0 -test.v=true /usr/bin/git -test.timeout=10git -test.run=^Test -test.short=true--show-toplevel git rev-�� --show-toplevel go (http block)
  • https://api.github.com/repos/actions/upload-artifact/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/upload-artifact/git/ref/tags/v4 --jq .object.sha -json GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env 1924-27764/test-1969773630 GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/1/artifacts
    • Triggering command: /usr/bin/gh gh run download 1 --dir test-logs/run-1 GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12345/artifacts
    • Triggering command: /usr/bin/gh gh run download 12345 --dir test-logs/run-12345 GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12346/artifacts
    • Triggering command: /usr/bin/gh gh run download 12346 --dir test-logs/run-12346 GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/2/artifacts
    • Triggering command: /usr/bin/gh gh run download 2 --dir test-logs/run-2 GO111MODULE x_amd64/link GOINSECURE GOMOD GOMODCACHE x_amd64/link env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE Xe/zd3aBy0104K8ZfExHKi7/u8U4vpCVJ6IIRF6zALNG (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/3/artifacts
    • Triggering command: /usr/bin/gh gh run download 3 --dir test-logs/run-3 GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/4/artifacts
    • Triggering command: /usr/bin/gh gh run download 4 --dir test-logs/run-4 GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/5/artifacts
    • Triggering command: /usr/bin/gh gh run download 5 --dir test-logs/run-5 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go estl�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path -json GO111MODULE 64/bin/go GOINSECURE GOMOD erignore go m/_n�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE ; \ fi (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 100 GOMOD GOMODCACHE o fix."; \ exitGOPROXY env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 6 GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha (http block)
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/nonexistent/action/git/ref/tags/v999.999.999
    • Triggering command: /usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env 2716143650/.github/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/nonexistent/repo/actions/runs/12345
    • Triggering command: /usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion GOINSECURE GOMOD GOMODCACHE 51RFpkM/58sERvdotCvyK1taGOl6 env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/owner/repo/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/bin/go GOINSECURE GOMOD GOMODCACHE erignore env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/owner/repo/contents/file.md
    • Triggering command: /tmp/go-build3514322672/b383/cli.test /tmp/go-build3514322672/b383/cli.test -test.testlogfile=/tmp/go-build3514322672/b383/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true GOINSECURE GOMOD GOMODCACHE go k/gh�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD erignore go (http block)
  • https://api.github.com/repos/test-owner/test-repo/actions/secrets
    • Triggering command: /usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go m/_n�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

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

Current State

  • Test File: pkg/workflow/action_reference_test.go
  • Source File: pkg/workflow/action_reference.go
  • Test Functions: 5 test functions (TestConvertToRemoteActionRef, TestResolveActionReference, TestCompilerActionTag, TestResolveSetupActionReference, TestResolveSetupActionReferenceWithData)
  • Lines of Code: 367 lines
  • Testify Usage: ❌ None — file imports only "testing"

Test Quality Analysis

Strengths ✅

  1. Good use of t.Run() subtests — tests are logically grouped and clearly named
  2. Table-driven tests presentTestResolveActionReference and TestResolveSetupActionReference already use table-driven patterns
  3. Good test case coverage — covers happy path, dev mode, release mode, action-tag override, empty/nil inputs

Areas for Improvement 🎯

1. No Testify Assertions — Replace All Manual Comparisons

The entire file uses raw if ref != expected { t.Errorf(...) } comparisons. Zero testify usage. This is the most impactful change.

Current Issues (throughout the file):

// ❌ CURRENT — manual comparison, weak failure message
if ref != expected {
    t.Errorf("Expected %q, got %q", expected, ref)
}

// ❌ CURRENT — manual nil check
if ref != "" {
    t.Errorf("Expected empty string with 'dev' version, got %q", ref)
}

Recommended Changes:

// ✅ IMPROVED — testify assertion with descriptive message
assert.Equal(t, expected, ref, "local path with ./ prefix should resolve to versioned remote ref")

// ✅ IMPROVED — testify assertion for empty string
assert.Empty(t, ref, "dev version should return empty string")

Add this import:

import (
    "testing"

    "github.com/stretchr/testify/assert"
    "github.com/stretchr/testify/require"
)
2. Convert TestConvertToRemoteActionRef to Table-Driven Test

TestConvertToRemoteActionRef uses many repetitive t.Run subtests with the same pattern: construct a compiler, call convertToRemoteActionRef, compare result. These should be a single table-driven test.

Current Pattern (repeated 9 times):

// ❌ CURRENT — repetitive t.Run subtests
func TestConvertToRemoteActionRef(t *testing.T) {
    t.Run("local path with ./ prefix and version tag", func(t *testing.T) {
        compiler := NewCompilerWithVersion("v1.2.3")
        data := &WorkflowData{}
        ref := compiler.convertToRemoteActionRef("./actions/create-issue", data)
        expected := "github/gh-aw/actions/create-issue@v1.2.3"
        if ref != expected {
            t.Errorf("Expected %q, got %q", expected, ref)
        }
    })
    // ... 8 more identical-pattern subtests
}

Recommended Change:

// ✅ IMPROVED — table-driven test
func TestConvertToRemoteActionRef(t *testing.T) {
    tests := []struct {
        name        string
        version     string
        actionTag   string
        localPath   string
        expectedRef string
        shouldBeEmpty bool
    }{
        {
            name:        "local path with ./ prefix and version tag",
            version:     "v1.2.3",
            localPath:   "./actions/create-issue",
            expectedRef: "github/gh-aw/actions/create-issue@v1.2.3",
        },
        {
            name:        "local path without ./ prefix and version tag",
            version:     "v1.0.0",
            localPath:   "actions/create-issue",
            expectedRef: "github/gh-aw/actions/create-issue@v1.0.0",
        },
        {
            name:          "dev version returns empty",
            version:       "dev",
            localPath:     "./actions/create-issue",
            shouldBeEmpty: true,
        },
        {
            name:          "empty version returns empty",
            version:       "",
            localPath:     "./actions/create-issue",
            shouldBeEmpty: true,
        },
        {
            name:        "action-tag overrides version",
            version:     "v1.0.0",
            actionTag:   "latest",
            localPath:   "./actions/create-issue",
            expectedRef: "github/gh-aw/actions/create-issue@latest",
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            compiler := NewCompilerWithVersion(tt.version)
            data := &WorkflowData{}
            if tt.actionTag != "" {
                data.Features = map[string]any{"action-tag": tt.actionTag}
            }

            ref := compiler.convertToRemoteActionRef(tt.localPath, data)

            if tt.shouldBeEmpt...

</details>



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

- Fixes github/gh-aw#19245

<!-- START COPILOT CODING AGENT TIPS -->
---Let Copilot coding agent [set things up for you](https://github.com/github/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.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve test quality in action_reference_test.go Improve test quality: migrate action_reference_test.go to testify Mar 3, 2026
@pelikhan pelikhan marked this pull request as ready for review March 3, 2026 02:26
Copilot AI review requested due to automatic review settings March 3, 2026 02:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates pkg/workflow/action_reference_test.go from manual if/t.Errorf assertions and repetitive subtests to a more maintainable style using testify/assert and table-driven tests.

Changes:

  • Replace manual equality/empty checks with assert.Equal / assert.Empty.
  • Refactor TestConvertToRemoteActionRef and TestCompilerActionTag into table-driven subtests.
  • Use description fields as assertion messages for clearer failures.

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

Comment on lines 349 to +358
// Create mock action resolver and cache
cache := NewActionCache("")
resolver := NewActionResolver(cache)

// The resolver will fail to resolve github/gh-aw/actions/setup@v1.0.0
// since it's not a real tag, but it should fall back gracefully
ref := ResolveSetupActionReference(ActionModeRelease, "v1.0.0", "", resolver)

// Without a valid pin or successful resolution, should return tag-based reference
expectedRef := "github/gh-aw/actions/setup@v1.0.0"
if ref != expectedRef {
t.Errorf("Expected %q, got %q", expectedRef, ref)
}
assert.Equal(t, "github/gh-aw/actions/setup@v1.0.0", ref, "should return tag-based reference when SHA resolution fails")
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestResolveSetupActionReferenceWithData constructs a real ActionResolver, which calls gh api and can block on network (20s timeout) and is subject to CI/firewall constraints. Since this test only needs to verify fallback behavior when SHA resolution fails, use a small stub ActionSHAResolver that immediately returns an error/empty SHA instead of the real resolver to keep unit tests deterministic and fast.

Copilot uses AI. Check for mistakes.
Comment on lines +224 to +229
name: "compiler actionTag with dev mode forces release behavior",
version: "v1.0.0",
compilerActionTag: "v2.0.0",
localPath: "./actions/setup",
useResolve: true,
expectedRef: "github/gh-aw/actions/setup@v2.0.0",
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table case name "compiler actionTag with dev mode forces release behavior" is misleading: the test always sets ActionModeRelease and the version is v1.0.0 (not a dev build). Rename this case to reflect what’s actually being asserted (e.g., action-tag flag priority/behavior in release mode) to avoid confusion when diagnosing failures.

Copilot uses AI. Check for mistakes.
@pelikhan pelikhan merged commit 02c3907 into main Mar 3, 2026
110 checks passed
@pelikhan pelikhan deleted the copilot/improve-test-quality-action-reference branch March 3, 2026 02:30
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.

3 participants