From 0d63909a164ef9dfadead744cf3f590017d824ac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 20:23:37 +0000 Subject: [PATCH 1/5] Initial plan From 9f7dac7369322cbf587de4707bb6a78767c79317 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 20:37:46 +0000 Subject: [PATCH 2/5] test(actionpins): expand internal pin-resolution coverage Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/actionpins/actionpins_internal_test.go | 195 ++++++++++++++++++--- 1 file changed, 170 insertions(+), 25 deletions(-) diff --git a/pkg/actionpins/actionpins_internal_test.go b/pkg/actionpins/actionpins_internal_test.go index fbae401e041..eb0bde8db09 100644 --- a/pkg/actionpins/actionpins_internal_test.go +++ b/pkg/actionpins/actionpins_internal_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -32,15 +33,33 @@ func TestBuildByRepoIndex_GroupsByRepoAndSortsDescending(t *testing.T) { } func TestCountPinKeyMismatches_ReturnsOnlyVersionMismatches(t *testing.T) { - entries := map[string]ActionPin{ - "actions/checkout@v5": {Repo: "actions/checkout", Version: "v5", SHA: "sha-1"}, - "actions/setup-go@v5": {Repo: "actions/setup-go", Version: "v4", SHA: "sha-2"}, - "invalid-key": {Repo: "actions/cache", Version: "v4", SHA: "sha-3"}, - } + t.Run("returns zero for empty entries", func(t *testing.T) { + assert.Zero(t, countPinKeyMismatches(map[string]ActionPin{}), "Expected empty input to produce zero mismatches") + }) - count := countPinKeyMismatches(entries) + t.Run("returns zero when all key versions match", func(t *testing.T) { + entries := map[string]ActionPin{ + "actions/checkout@v5": {Repo: "actions/checkout", Version: "v5", SHA: "sha-1"}, + "actions/setup-go@v4": {Repo: "actions/setup-go", Version: "v4", SHA: "sha-2"}, + } + assert.Zero(t, countPinKeyMismatches(entries), "Expected zero mismatches when key versions match pin versions") + }) - assert.Equal(t, 1, count, "Expected only one key/version mismatch to be counted") + t.Run("ignores invalid keys without version suffix", func(t *testing.T) { + entries := map[string]ActionPin{ + "invalid-key": {Repo: "actions/cache", Version: "v4", SHA: "sha-3"}, + } + assert.Zero(t, countPinKeyMismatches(entries), "Expected invalid keys without @version to be ignored") + }) + + t.Run("counts only true key-version mismatches", func(t *testing.T) { + entries := map[string]ActionPin{ + "actions/checkout@v5": {Repo: "actions/checkout", Version: "v5", SHA: "sha-1"}, + "actions/setup-go@v5": {Repo: "actions/setup-go", Version: "v4", SHA: "sha-2"}, + "invalid-key": {Repo: "actions/cache", Version: "v4", SHA: "sha-3"}, + } + assert.Equal(t, 1, countPinKeyMismatches(entries), "Expected only one key/version mismatch to be counted") + }) } func TestInitWarnings_InitializesAndPreservesMap(t *testing.T) { @@ -113,6 +132,65 @@ func TestFormatPinnedActionWithResolution_ConsistentVersionComment(t *testing.T) } } +func TestFindCompatiblePin_SemverFallback(t *testing.T) { + pins := []ActionPin{ + {Repo: "actions/checkout", Version: "v5.2.0", SHA: "sha-v5-2"}, + {Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-v5-0"}, + {Repo: "actions/checkout", Version: "v4.9.9", SHA: "sha-v4-9"}, + } + + tests := []struct { + name string + version string + wantFound bool + wantVersion string + availablePins []ActionPin + }{ + { + name: "exact-major", + version: "v5", + wantFound: true, + wantVersion: "v5.2.0", + availablePins: pins, + }, + { + name: "exact-version", + version: "v5.0.0", + wantFound: true, + wantVersion: "v5.0.0", + availablePins: []ActionPin{ + {Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-v5-0"}, + {Repo: "actions/checkout", Version: "v4.9.9", SHA: "sha-v4-9"}, + }, + }, + { + name: "no-match", + version: "v6", + wantFound: false, + availablePins: pins, + }, + { + name: "empty-version", + version: "", + wantFound: false, + availablePins: pins, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pin, found := findCompatiblePin(tt.availablePins, tt.version) + assert.Equal(t, tt.wantFound, found) + if tt.wantFound { + assert.Equal(t, tt.wantVersion, pin.Version) + } else { + assert.Empty(t, pin.Version) + assert.Empty(t, pin.SHA) + } + }) + } +} + func TestFindVersionBySHA_ReturnsVersionForKnownSHA(t *testing.T) { t.Run("returns version for a known SHA in embedded data", func(t *testing.T) { pins := GetActionPinsByRepo("actions/checkout") @@ -142,24 +220,33 @@ func TestGetContainerPin_ReturnsPinnedImage(t *testing.T) { assert.Contains(t, pin.PinnedImage, "@sha256:", "Expected pinned image to include digest") } -func TestGetContainerPin_MCPGatewayV036IsPinned(t *testing.T) { - const image = "ghcr.io/github/gh-aw-mcpg:v0.3.6" - - pin, ok := GetContainerPin(image) - require.True(t, ok, "Expected embedded container pin for %s", image) - assert.Equal(t, image, pin.Image, "Expected image name to match key") - assert.Equal(t, "sha256:2bb8eef86006a4c5963c55616a9c51c32f27bfdecb023b8aa6f91f6718d9171c", pin.Digest, "Expected v0.3.6 digest to match") - assert.Equal(t, image+"@sha256:2bb8eef86006a4c5963c55616a9c51c32f27bfdecb023b8aa6f91f6718d9171c", pin.PinnedImage, "Expected pinned image to include v0.3.6 digest") -} - -func TestGetContainerPin_MCPGatewayV039IsPinned(t *testing.T) { - const image = "ghcr.io/github/gh-aw-mcpg:v0.3.9" +func TestGetContainerPin_MCPGatewayVersionsArePinned(t *testing.T) { + tests := []struct { + name string + image string + digest string + }{ + { + name: "v0.3.6", + image: "ghcr.io/github/gh-aw-mcpg:v0.3.6", + digest: "sha256:2bb8eef86006a4c5963c55616a9c51c32f27bfdecb023b8aa6f91f6718d9171c", + }, + { + name: "v0.3.9", + image: "ghcr.io/github/gh-aw-mcpg:v0.3.9", + digest: "sha256:64828b42a4482f58fab16509d7f8f495a6d97c972a98a68aff20543531ac0388", + }, + } - pin, ok := GetContainerPin(image) - require.True(t, ok, "Expected embedded container pin for %s", image) - assert.Equal(t, image, pin.Image, "Expected image name to match key") - assert.Equal(t, "sha256:64828b42a4482f58fab16509d7f8f495a6d97c972a98a68aff20543531ac0388", pin.Digest, "Expected v0.3.9 digest to match") - assert.Equal(t, image+"@sha256:64828b42a4482f58fab16509d7f8f495a6d97c972a98a68aff20543531ac0388", pin.PinnedImage, "Expected pinned image to include v0.3.9 digest") + for _, tt := range tests { + t.Run(tt.name, 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, "Expected image name to match key") + assert.Equal(t, tt.digest, pin.Digest, "Expected digest to match for %s", tt.image) + assert.Equal(t, tt.image+"@"+tt.digest, pin.PinnedImage, "Expected pinned image to include digest for %s", tt.image) + }) + } } func TestGetContainerPin_DefaultMCPImagesArePinned(t *testing.T) { @@ -199,7 +286,7 @@ func TestResolveActionPinDynamically_SkipsForSHAInput(t *testing.T) { assert.False(t, ok, "Expected no dynamic resolution for SHA input") assert.Empty(t, result, "Expected empty result when dynamic resolution is skipped") - assert.Equal(t, 0, resolver.called, "Expected resolver not to be called for SHA input") + assert.Zero(t, resolver.called, "Expected resolver not to be called for SHA input") } func TestResolveActionPinFromHardcodedPins_StrictModeNoFallback(t *testing.T) { @@ -220,6 +307,64 @@ func TestResolveExactHardcodedPin_BySHA(t *testing.T) { assert.Contains(t, result, "sha-v5", "Expected result to include matched SHA") } +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 matched SHA") + assert.Contains(t, result, "v5.0.0", "Expected result to include matched version") +} + +func TestResolveExactHardcodedPin_NoMatch(t *testing.T) { + pins := []ActionPin{{Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-v5"}} + + result, ok := resolveExactHardcodedPin("actions/checkout", "v4.0.0", false, pins) + + assert.False(t, ok, "Expected no resolution when version does not match and input is not SHA") + assert.Empty(t, result, "Expected empty result when no exact match is found") +} + +func TestResolveNonStrictHardcodedPin_CompatibleAndFallback(t *testing.T) { + pins := []ActionPin{ + {Repo: "actions/checkout", Version: "v5.2.0", SHA: "sha-v5-2"}, + {Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-v5-0"}, + {Repo: "actions/checkout", Version: "v4.9.9", SHA: "sha-v4-9"}, + } + + t.Run("uses semver-compatible pin when available", func(t *testing.T) { + ctx := &PinContext{Warnings: make(map[string]bool)} + expectedWarning := "Unable to resolve actions/checkout@v5 dynamically, using hardcoded pin for actions/checkout@v5.2.0" + var result string + + stderrOutput := testutil.CaptureStderr(t, func() { + result = resolveNonStrictHardcodedPin("actions/checkout", "v5", pins, ctx) + }) + + assert.Contains(t, result, "sha-v5-2", "Expected highest semver-compatible pin to be selected") + assert.True(t, ctx.Warnings["actions/checkout@v5"], "Expected warning cache key to be recorded") + assert.Contains(t, stderrOutput, expectedWarning, "Expected warning to be emitted for compatible fallback") + }) + + t.Run("falls back to highest available pin and deduplicates warnings", func(t *testing.T) { + ctx := &PinContext{Warnings: make(map[string]bool)} + expectedWarning := "Unable to resolve actions/checkout@v9 dynamically, using hardcoded pin for actions/checkout@v5.2.0" + var first, second string + + stderrOutput := testutil.CaptureStderr(t, func() { + first = resolveNonStrictHardcodedPin("actions/checkout", "v9", pins, ctx) + second = resolveNonStrictHardcodedPin("actions/checkout", "v9", pins, ctx) + }) + + assert.Contains(t, first, "sha-v5-2", "Expected fallback to highest available pin when no compatible version exists") + assert.Contains(t, second, "sha-v5-2", "Expected consistent fallback result on repeated calls") + assert.True(t, ctx.Warnings["actions/checkout@v9"], "Expected warning cache key to be recorded") + assert.Len(t, ctx.Warnings, 1, "Expected warning deduplication for repeated resolution attempts") + assert.Equal(t, 1, strings.Count(stderrOutput, expectedWarning), "Expected warning to be emitted exactly once for repeated fallback resolution") + }) +} + func TestResolveActionPinFromHardcodedPins_SkipHardcodedFallback(t *testing.T) { t.Run("returns false immediately when SkipHardcodedFallback is set", func(t *testing.T) { ctx := &PinContext{SkipHardcodedFallback: true, Warnings: make(map[string]bool)} From 14b3a8581828a9cc05e1a2e2c88efae6a9f80666 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 2 Jul 2026 01:11:17 +0000 Subject: [PATCH 3/5] docs(adr): draft ADR-42834 for table-driven subtests in actionpins pin-resolution --- ...-subtests-for-actionpins-pin-resolution.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 docs/adr/42834-table-driven-subtests-for-actionpins-pin-resolution.md diff --git a/docs/adr/42834-table-driven-subtests-for-actionpins-pin-resolution.md b/docs/adr/42834-table-driven-subtests-for-actionpins-pin-resolution.md new file mode 100644 index 00000000000..f2843442f90 --- /dev/null +++ b/docs/adr/42834-table-driven-subtests-for-actionpins-pin-resolution.md @@ -0,0 +1,44 @@ +# ADR-42834: Table-Driven Subtests for actionpins Pin-Resolution Coverage + +**Date**: 2026-07-02 +**Status**: Draft +**Deciders**: pelikhan (PR author) + +--- + +### Context + +The `pkg/actionpins` package implements critical GitHub Actions pin-resolution logic: semver fallback selection, exact-match hardcoded-pin resolution, and per-call warning deduplication via `PinContext.Warnings`. Prior to this PR, several internal branches (`findCompatiblePin`, `resolveExactHardcodedPin`, `resolveNonStrictHardcodedPin`) had no test coverage, leaving regression risk invisible. Test functions that did exist used flat top-level assertions with no subtest grouping, making failure attribution coarse-grained. The package also emits user-visible warnings to stderr as a side effect of resolution; that behavior was entirely untested. + +### Decision + +We will convert actionpins internal tests to table-driven subtests (`t.Run`) for parameterized cases and introduce `testutil.CaptureStderr` for asserting stderr side-effects of resolution functions. New dedicated test functions will cover the previously untested branches: semver major-version fallback, exact-version and exact-SHA resolution, no-match paths, and warning-dedup semantics across repeated calls. + +### Alternatives Considered + +#### Alternative 1: Flat top-level test functions (status quo) + +Continue adding one `TestX` function per scenario without subtests. Simple to write and requires no `testutil` helper. Rejected because failure output names only the top-level function, making it harder to identify which of many inputs triggered the failure; it also encourages copy-paste duplication across similar cases. + +#### Alternative 2: Interface-based stderr mock / io.Writer injection + +Inject an `io.Writer` into the resolution functions to capture output in tests, rather than adding a `testutil.CaptureStderr` helper that redirects `os.Stderr`. Considered because it avoids OS-level file descriptor manipulation in tests. Rejected because it would require changing the signatures of internal (unexported) functions or adding an exported `Writer` field to `PinContext`, widening the public surface of the package solely for test concerns. The `testutil.CaptureStderr` approach keeps production code unchanged. + +### Consequences + +#### Positive +- Subtests provide precise failure attribution (the failing subtest name identifies the exact input, e.g., `TestFindCompatiblePin_SemverFallback/exact-major`) with no extra tooling. +- Previously untested resolution branches (semver fallback, exact-match, no-match, warning dedup) are now covered, reducing regression risk for critical pin-resolution logic. +- Table-driven structure consolidates related cases (e.g., two separate `TestGetContainerPin_MCPGatewayV*` functions merged into one) and makes adding future cases low-friction. + +#### Negative +- Test file grows significantly (+170 lines), increasing the maintenance surface. +- `testutil.CaptureStderr` introduces a shared test-helper dependency; future changes to that utility can affect these tests indirectly. + +#### Neutral +- No production code (`actionpins.go`) is modified; the decision affects only the test file and the `testutil` package import. +- Adoption of `assert.Zero` over `assert.Equal(t, 0, ...)` is a stylistic cleanup bundled into this PR; no behavioral change. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 9fd0ebedd807b2adf7d387ca48555e0e184c7ae8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Jul 2026 01:48:46 +0000 Subject: [PATCH 4/5] test(actionpins): address all 8 review-thread improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename 'exact-version' → 'major-compatible-match' and add 'first-compatible-not-exact' case clarifying first-match semantics (thread 1) - Add 'returns-first-compatible-not-highest' case making list-ordering contract explicit (thread 2) - Assert on stable unstyled fragment to remove ANSI fragility (thread 3) - Replace assert.Empty×2 with assert.Equal(t, ActionPin{}, pin) in not-found branch (thread 4) - Add TestResolveExactHardcodedPin_VersionTakesPrecedenceOverSHA (thread 5) - Add 'deduplicates warning on compatible path' subtest (thread 6) - Add 'minor-version-constraint' case for v5.1 major-only matching (thread 7) - Split TestResolveNonStrictHardcodedPin_CompatibleAndFallback into TestResolveNonStrictHardcodedPin_SelectsHighestCompatible and TestResolveNonStrictHardcodedPin_FallsBackToHighestWhenNoCompatible (thread 8) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/actionpins/actionpins_internal_test.go | 98 ++++++++++++++++++---- 1 file changed, 82 insertions(+), 16 deletions(-) diff --git a/pkg/actionpins/actionpins_internal_test.go b/pkg/actionpins/actionpins_internal_test.go index eb0bde8db09..d2ca23dd96f 100644 --- a/pkg/actionpins/actionpins_internal_test.go +++ b/pkg/actionpins/actionpins_internal_test.go @@ -154,7 +154,9 @@ func TestFindCompatiblePin_SemverFallback(t *testing.T) { availablePins: pins, }, { - name: "exact-version", + // major-compatible-match: requesting v5.0.0 from a list without v5.2.0 returns v5.0.0 — + // the function performs major-compatible matching, not exact-version matching. + name: "major-compatible-match", version: "v5.0.0", wantFound: true, wantVersion: "v5.0.0", @@ -163,6 +165,41 @@ func TestFindCompatiblePin_SemverFallback(t *testing.T) { {Repo: "actions/checkout", Version: "v4.9.9", SHA: "sha-v4-9"}, }, }, + { + // first-compatible-not-exact: requesting v5.0.0 from [v5.2.0, v5.0.0] returns v5.2.0 + // because findCompatiblePin returns the FIRST major-compatible match, not the exact one. + name: "first-compatible-not-exact", + version: "v5.0.0", + wantFound: true, + wantVersion: "v5.2.0", + availablePins: pins, + }, + { + // returns-first-compatible-not-highest: list order determines the result. + // With an unsorted list [v5.0.0, v5.2.0], requesting v5 returns v5.0.0 (the first + // major-compatible entry), not v5.2.0. Callers (e.g. resolveNonStrictHardcodedPin) must + // supply a pre-sorted (descending) slice via GetActionPinsByRepo to get the highest pin. + name: "returns-first-compatible-not-highest", + version: "v5", + wantFound: true, + wantVersion: "v5.0.0", + availablePins: []ActionPin{ + {Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-low"}, + {Repo: "actions/checkout", Version: "v5.2.0", SHA: "sha-high"}, + }, + }, + { + // minor-version-constraint: IsCompatible performs major-only comparison, so + // requesting v5.1 from [v5.2.0, v5.0.0] returns v5.2.0 (same major, first match). + name: "minor-version-constraint", + version: "v5.1", + wantFound: true, + wantVersion: "v5.2.0", + availablePins: []ActionPin{ + {Repo: "actions/checkout", Version: "v5.2.0", SHA: "sha-a"}, + {Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-b"}, + }, + }, { name: "no-match", version: "v6", @@ -184,8 +221,7 @@ func TestFindCompatiblePin_SemverFallback(t *testing.T) { if tt.wantFound { assert.Equal(t, tt.wantVersion, pin.Version) } else { - assert.Empty(t, pin.Version) - assert.Empty(t, pin.SHA) + assert.Equal(t, ActionPin{}, pin, "expected zero-value ActionPin when not found") } }) } @@ -326,7 +362,18 @@ func TestResolveExactHardcodedPin_NoMatch(t *testing.T) { assert.Empty(t, result, "Expected empty result when no exact match is found") } -func TestResolveNonStrictHardcodedPin_CompatibleAndFallback(t *testing.T) { +func TestResolveExactHardcodedPin_VersionTakesPrecedenceOverSHA(t *testing.T) { + // When isAlreadySHA=false, only the version-match path runs; the SHA-match loop is + // skipped entirely. This test uses a pin whose Version and SHA fields are identical + // to make the path selection explicit: the version loop matches and returns before + // the SHA loop would ever execute. + pins := []ActionPin{{Repo: "actions/checkout", Version: "sha-v5", SHA: "sha-v5"}} + result, ok := resolveExactHardcodedPin("actions/checkout", "sha-v5", false, pins) + require.True(t, ok, "Expected version-match path to resolve when isAlreadySHA=false") + assert.Contains(t, result, "sha-v5", "Expected result to include the matched pin's SHA/version") +} + +func TestResolveNonStrictHardcodedPin_SelectsHighestCompatible(t *testing.T) { pins := []ActionPin{ {Repo: "actions/checkout", Version: "v5.2.0", SHA: "sha-v5-2"}, {Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-v5-0"}, @@ -335,7 +382,6 @@ func TestResolveNonStrictHardcodedPin_CompatibleAndFallback(t *testing.T) { t.Run("uses semver-compatible pin when available", func(t *testing.T) { ctx := &PinContext{Warnings: make(map[string]bool)} - expectedWarning := "Unable to resolve actions/checkout@v5 dynamically, using hardcoded pin for actions/checkout@v5.2.0" var result string stderrOutput := testutil.CaptureStderr(t, func() { @@ -344,27 +390,47 @@ func TestResolveNonStrictHardcodedPin_CompatibleAndFallback(t *testing.T) { assert.Contains(t, result, "sha-v5-2", "Expected highest semver-compatible pin to be selected") assert.True(t, ctx.Warnings["actions/checkout@v5"], "Expected warning cache key to be recorded") - assert.Contains(t, stderrOutput, expectedWarning, "Expected warning to be emitted for compatible fallback") + // Assert on the stable unstyled message fragment to avoid ANSI-code fragility. + assert.Contains(t, stderrOutput, "using hardcoded pin for actions/checkout@v5.2.0", "Expected warning to be emitted for compatible fallback") }) - t.Run("falls back to highest available pin and deduplicates warnings", func(t *testing.T) { + t.Run("deduplicates warning on compatible path", func(t *testing.T) { ctx := &PinContext{Warnings: make(map[string]bool)} - expectedWarning := "Unable to resolve actions/checkout@v9 dynamically, using hardcoded pin for actions/checkout@v5.2.0" - var first, second string stderrOutput := testutil.CaptureStderr(t, func() { - first = resolveNonStrictHardcodedPin("actions/checkout", "v9", pins, ctx) - second = resolveNonStrictHardcodedPin("actions/checkout", "v9", pins, ctx) + resolveNonStrictHardcodedPin("actions/checkout", "v5", pins, ctx) + resolveNonStrictHardcodedPin("actions/checkout", "v5", pins, ctx) }) - assert.Contains(t, first, "sha-v5-2", "Expected fallback to highest available pin when no compatible version exists") - assert.Contains(t, second, "sha-v5-2", "Expected consistent fallback result on repeated calls") - assert.True(t, ctx.Warnings["actions/checkout@v9"], "Expected warning cache key to be recorded") - assert.Len(t, ctx.Warnings, 1, "Expected warning deduplication for repeated resolution attempts") - assert.Equal(t, 1, strings.Count(stderrOutput, expectedWarning), "Expected warning to be emitted exactly once for repeated fallback resolution") + assert.Equal(t, 1, strings.Count(stderrOutput, "using hardcoded pin for actions/checkout@v5.2.0"), + "Expected warning emitted exactly once for repeated compatible resolution") + assert.Len(t, ctx.Warnings, 1) }) } +func TestResolveNonStrictHardcodedPin_FallsBackToHighestWhenNoCompatible(t *testing.T) { + pins := []ActionPin{ + {Repo: "actions/checkout", Version: "v5.2.0", SHA: "sha-v5-2"}, + {Repo: "actions/checkout", Version: "v5.0.0", SHA: "sha-v5-0"}, + {Repo: "actions/checkout", Version: "v4.9.9", SHA: "sha-v4-9"}, + } + + ctx := &PinContext{Warnings: make(map[string]bool)} + var first, second string + + stderrOutput := testutil.CaptureStderr(t, func() { + first = resolveNonStrictHardcodedPin("actions/checkout", "v9", pins, ctx) + second = resolveNonStrictHardcodedPin("actions/checkout", "v9", pins, ctx) + }) + + assert.Contains(t, first, "sha-v5-2", "Expected fallback to highest available pin when no compatible version exists") + assert.Contains(t, second, "sha-v5-2", "Expected consistent fallback result on repeated calls") + assert.True(t, ctx.Warnings["actions/checkout@v9"], "Expected warning cache key to be recorded") + assert.Len(t, ctx.Warnings, 1, "Expected warning deduplication for repeated resolution attempts") + // Assert on the stable unstyled message fragment to avoid ANSI-code fragility. + assert.Equal(t, 1, strings.Count(stderrOutput, "using hardcoded pin for actions/checkout@v5.2.0"), "Expected warning to be emitted exactly once for repeated fallback resolution") +} + func TestResolveActionPinFromHardcodedPins_SkipHardcodedFallback(t *testing.T) { t.Run("returns false immediately when SkipHardcodedFallback is set", func(t *testing.T) { ctx := &PinContext{SkipHardcodedFallback: true, Warnings: make(map[string]bool)} From 5a8df358753d37c8ee819a297a710dd5083ca1f8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Jul 2026 01:49:53 +0000 Subject: [PATCH 5/5] test(actionpins): clarify comments on major-compatible-match test cases Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/actionpins/actionpins_internal_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/actionpins/actionpins_internal_test.go b/pkg/actionpins/actionpins_internal_test.go index d2ca23dd96f..75fb4469763 100644 --- a/pkg/actionpins/actionpins_internal_test.go +++ b/pkg/actionpins/actionpins_internal_test.go @@ -154,8 +154,9 @@ func TestFindCompatiblePin_SemverFallback(t *testing.T) { availablePins: pins, }, { - // major-compatible-match: requesting v5.0.0 from a list without v5.2.0 returns v5.0.0 — - // the function performs major-compatible matching, not exact-version matching. + // major-compatible-match: requesting v5.0.0 from a list that contains only v5.0.0 and + // v4.9.9 returns v5.0.0 — the function uses major-compatible matching, not exact-version + // matching. The only v5.x present happens to be v5.0.0, so the result looks exact. name: "major-compatible-match", version: "v5.0.0", wantFound: true, @@ -167,7 +168,9 @@ func TestFindCompatiblePin_SemverFallback(t *testing.T) { }, { // first-compatible-not-exact: requesting v5.0.0 from [v5.2.0, v5.0.0] returns v5.2.0 - // because findCompatiblePin returns the FIRST major-compatible match, not the exact one. + // because findCompatiblePin returns the first major-compatible match, not the exact one. + // This is a consequence of the pre-sorted (descending) slice contract; see + // returns-first-compatible-not-highest for the complementary case with an unsorted list. name: "first-compatible-not-exact", version: "v5.0.0", wantFound: true,