[testify-expert] Improve Test Quality: pkg/actionpins/actionpins_internal_test.go

[github-actions]

Overview

File: pkg/actionpins/actionpins_internal_test.go
Paired source: pkg/actionpins/actionpins.go (514 LOC, 27 functions)
Test count: 15 test functions (some with subtests) across 319 LOC
Analysis date: 2026-07-01

Strengths

• Correct require vs assert split: require for fatal preconditions, assert for soft checks.
• Descriptive behaviour-encoding test names (e.g. _StrictModeNoFallback, _SkipsForSHAInput).
• Table-driven pattern already in use for TestFormatPinnedActionWithResolution_ConsistentVersionComment.
• Subtests used appropriately for multi-scenario functions.

---

Prioritised Improvements

1. Missing / high-value tests

A. findCompatiblePin — zero direct coverage

findCompatiblePin is the semver-fallback heart of non-strict resolution. It is only exercised indirectly through the full resolution stack. A direct unit test makes the semver semantics explicit and guards against regressions.

// AFTER
func TestFindCompatiblePin_SemverFallback(t *testing.T) {
    pins := []ActionPin{
        {Repo: "actions/checkout", Version: "v4.2.0", SHA: "sha-4-2"},
        {Repo: "actions/checkout", Version: "v4.1.0", SHA: "sha-4-1"},
        {Repo: "actions/checkout", Version: "v3.0.0", SHA: "sha-3"},
    }

    tests := []struct {
        name    string
        version string
        wantOK  bool
        wantVer string
    }{
        {"exact major match returns first compatible", "v4", true, "v4.2.0"},
        {"exact version match", "v4.1.0", true, "v4.1.0"},
        {"no compatible version", "v2", false, ""},
        {"empty version", "", false, ""},
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            pin, ok := findCompatiblePin(pins, tt.version)
            assert.Equal(t, tt.wantOK, ok)
            if tt.wantOK {
                assert.Equal(t, tt.wantVer, pin.Version)
            }
        })
    }
}

B. resolveNonStrictHardcodedPin — two code paths untested

This function has two branches: (1) a semver-compatible pin was found, and (2) no compatible pin exists so the highest available is used. Neither is directly tested.

// AFTER
func TestResolveNonStrictHardcodedPin_CompatibleAndFallback(t *testing.T) {
    pins := []ActionPin{
        {Repo: "actions/checkout", Version: "v4.2.0", SHA: "sha-4-2"},
        {Repo: "actions/checkout", Version: "v3.0.0", SHA: "sha-3"},
    }

    t.Run("uses semver-compatible pin when available", func(t *testing.T) {
        ctx := &PinContext{Warnings: make(map[string]bool)}
        result := resolveNonStrictHardcodedPin("actions/checkout", "v4", pins, ctx)
        assert.Contains(t, result, "sha-4-2", "should use the v4-compatible pin")
    })

    t.Run("falls back to highest pin when no compatible version exists", func(t *testing.T) {
        ctx := &PinContext{Warnings: make(map[string]bool)}
        result := resolveNonStrictHardcodedPin("actions/checkout", "v2", pins, ctx)
        // matchingPins[0] is the highest (pins sorted descending)
        assert.Contains(t, result, "sha-4-2", "should fall back to highest available pin")
    })

    t.Run("deduplicates warning on repeated calls", func(t *testing.T) {
        ctx := &PinContext{Warnings: make(map[string]bool)}
        resolveNonStrictHardcodedPin("actions/checkout", "v4", pins, ctx)
        resolveNonStrictHardcodedPin("actions/checkout", "v4", pins, ctx)
        assert.Len(t, ctx.Warnings, 1, "warning should be recorded exactly once")
    })
}

C. resolveExactHardcodedPin — version-based branch not tested

The existing TestResolveExactHardcodedPin_BySHA only exercises the SHA-input path (isAlreadySHA=true). The version-exact-match path (isAlreadySHA=false) is never hit.

// AFTER
func TestResolveExactHardcodedPin_ByVersion(t *testing.T) {
    pins := []ActionPin{{Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-v5"}}

    result, ok := resolveExactHardcodedPin("actions/checkout", "v5.0.0", false, pins)

    require.True(t, ok, "Expected exact version match to resolve")
    assert.Contains(t, result, "sha-v5", "Expected result to include the matched SHA")
}

func TestResolveExactHardcodedPin_NoMatch(t *testing.T) {
    pins := []ActionPin{{Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-v5"}}

    _, ok := resolveExactHardcodedPin("actions/checkout", "v4.0.0", false, pins)

    assert.False(t, ok, "Expected no match for a version not present in pins")
}

2. Table-driven refactors

Merge two near-identical container-pin tests

TestGetContainerPin_MCPGatewayV036IsPinned and TestGetContainerPin_MCPGatewayV039IsPinned are structurally identical. Merge them:

// AFTER
func TestGetContainerPin_MCPGatewayVersionsArePinned(t *testing.T) {
    tests := []struct {
        image          string
        expectedDigest string
    }{
        {
            image:          "ghcr.io/github/gh-aw-mcpg:v0.3.6",
            expectedDigest: "sha256:2bb8eef86006a4c5963c55616a9c51c32f27bfdecb023b8aa6f91f6718d9171c",
        },
        {
            image:          "ghcr.io/github/gh-aw-mcpg:v0.3.9",
            expectedDigest: "sha256:64828b42a4482f58fab16509d7f8f495a6d97c972a98a68aff20543531ac0388",
        },
    }

    for _, tt := range tests {
        t.Run(tt.image, func(t *testing.T) {
            pin, ok := GetContainerPin(tt.image)
            require.True(t, ok, "Expected embedded container pin for %s", tt.image)
            assert.Equal(t, tt.image, pin.Image)
            assert.Equal(t, tt.expectedDigest, pin.Digest)
            assert.Equal(t, tt.image+"@"+tt.expectedDigest, pin.PinnedImage)
        })
    }
}

Expand TestCountPinKeyMismatches with boundary cases

// AFTER — table-driven with boundary cases
func TestCountPinKeyMismatches_BoundaryCases(t *testing.T) {
    tests := []struct {
        name     string
        entries  map[string]ActionPin
        expected int
    }{
        {"empty map", map[string]ActionPin{}, 0},
        {"all keys match", map[string]ActionPin{
            "actions/checkout@v5": {Repo: "actions/checkout", Version: "v5"},
        }, 0},
        {"one mismatch", map[string]ActionPin{
            "actions/setup-go@v5": {Repo: "actions/setup-go", Version: "v4"},
        }, 1},
        {"invalid key format not counted", map[string]ActionPin{
            "invalid-key": {Repo: "actions/cache", Version: "v4"},
        }, 0},
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            assert.Equal(t, tt.expected, countPinKeyMismatches(tt.entries))
        })
    }
}

3. Minor assertion clarity

TestResolveActionPinDynamically_SkipsForSHAInput uses assert.Equal(t, 0, resolver.called). Prefer assert.Zero(t, resolver.called) to make zero-value intent clearer.

---

Acceptance Checklist

• Add TestFindCompatiblePin_SemverFallback covering exact-major, exact-version, no-match, and empty-version cases.
• Add TestResolveNonStrictHardcodedPin_CompatibleAndFallback covering both branches and warning dedup.
• Add TestResolveExactHardcodedPin_ByVersion and TestResolveExactHardcodedPin_NoMatch.
• Merge TestGetContainerPin_MCPGatewayV036IsPinned / V039 into a single table-driven test.
• Expand TestCountPinKeyMismatches with empty/no-mismatch/invalid-key boundary cases.
• Replace assert.Equal(t, 0, ...) with assert.Zero where applicable.
• make test-unit passes with all new tests green.

Generated by 🧪 Daily Testify Uber Super Expert · 47.1 AIC · ⌖ 17.9 AIC · ⊞ 1.6K · ◷

• expires on Jul 3, 2026, 10:40 AM UTC-08:00

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · 5.09 AIC · ⌖ 11.2 AIC · ⊞ 8.1K · ◷