From 67d12a6cfbef95c35d4c65a28db1ecbddf4c23d5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:39:39 +0000 Subject: [PATCH 1/3] Initial plan From 9c755bcfa4785333796fd47845c4ccbbade82f99 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:53:51 +0000 Subject: [PATCH 2/3] Improve test quality: semver_precise_test.go with testify, new test cases, TestIsNewer table Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/semver_precise_test.go | 145 ++++++++++++++++++++++++++------- 1 file changed, 115 insertions(+), 30 deletions(-) diff --git a/pkg/cli/semver_precise_test.go b/pkg/cli/semver_precise_test.go index e3795cd0fc8..248fa1f92a3 100644 --- a/pkg/cli/semver_precise_test.go +++ b/pkg/cli/semver_precise_test.go @@ -1,15 +1,22 @@ //go:build !integration +// This file tests the parseVersion wrapper in pkg/cli and the IsPreciseVersion/IsNewer +// methods it exposes from pkg/semverutil. For full semverutil parsing-logic coverage +// see pkg/semverutil/semverutil_test.go. package cli import ( "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestIsPreciseVersion(t *testing.T) { tests := []struct { name string version string + wantNil bool expected bool }{ { @@ -52,53 +59,131 @@ func TestIsPreciseVersion(t *testing.T) { version: "v1.2.3", expected: true, }, + { + name: "invalid version string - returns nil", + version: "not-a-version", + wantNil: true, + }, + { + name: "empty string - returns nil", + version: "", + wantNil: true, + }, + { + name: "pre-release suffix - precise", + version: "v1.2.3-rc.1", + expected: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { v := parseVersion(tt.version) - if v == nil { - t.Fatalf("Failed to parse version: %s", tt.version) - } - - result := v.IsPreciseVersion() - if result != tt.expected { - t.Errorf("isPreciseVersion() for %q = %v, want %v", tt.version, result, tt.expected) + if tt.wantNil { + require.Nil(t, v, "parseVersion(%q) should return nil", tt.version) + return } + require.NotNil(t, v, "parseVersion(%q) should not return nil", tt.version) + assert.Equal(t, tt.expected, v.IsPreciseVersion(), "IsPreciseVersion() for %q", tt.version) }) } } -func TestPreciseVersionPreference(t *testing.T) { - // Test that when comparing equal versions, major-only versions are preferred - // This follows GitHub Actions convention of using major version tags (e.g., v8 instead of v8.0.0) - v6 := parseVersion("v6") - v600 := parseVersion("v6.0.0") - - if v6 == nil || v600 == nil { - t.Fatal("Failed to parse versions") +func TestIsNewer(t *testing.T) { + tests := []struct { + name string + v1 string + v2 string + newer bool + }{ + { + name: "strictly newer major", + v1: "v7.0.0", + v2: "v6.0.0", + newer: true, + }, + { + name: "strictly older major", + v1: "v5.0.0", + v2: "v6.0.0", + newer: false, + }, + { + name: "newer minor", + v1: "v6.1.0", + v2: "v6.0.0", + newer: true, + }, + { + name: "older minor", + v1: "v6.0.0", + v2: "v6.1.0", + newer: false, + }, + { + name: "newer patch", + v1: "v6.0.1", + v2: "v6.0.0", + newer: true, + }, + { + name: "older patch", + v1: "v6.0.0", + v2: "v6.0.1", + newer: false, + }, + { + name: "equal versions", + v1: "v6.0.0", + v2: "v6.0.0", + newer: false, + }, } - // They should parse to the same major.minor.patch - if v6.Major != v600.Major || v6.Minor != v600.Minor || v6.Patch != v600.Patch { - t.Errorf("v6 and v6.0.0 should parse to same major.minor.patch, got v6=%+v, v600=%+v", v6, v600) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v1 := parseVersion(tt.v1) + v2 := parseVersion(tt.v2) + require.NotNil(t, v1, "parseVersion(%q) should not return nil", tt.v1) + require.NotNil(t, v2, "parseVersion(%q) should not return nil", tt.v2) + assert.Equal(t, tt.newer, v1.IsNewer(v2), "IsNewer() for %q vs %q", tt.v1, tt.v2) + }) } +} - // v6.0.0 should be precise, v6 should not - if !v600.IsPreciseVersion() { - t.Error("v6.0.0 should be precise") +func TestPreciseVersionPreference(t *testing.T) { + // Tests that when comparing equivalent versions, imprecise tags (major-only or + // major.minor) are not considered precise, while full three-component versions are. + // This follows GitHub Actions convention of distinguishing major version pins + // (e.g. v8) from precise pins (e.g. v8.0.0). + tests := []struct { + name string + imprecise string + precise string + }{ + {name: "major vs major.minor.patch", imprecise: "v6", precise: "v6.0.0"}, + {name: "major.minor vs major.minor.patch", imprecise: "v6.0", precise: "v6.0.0"}, + {name: "no-prefix major.minor vs major.minor.patch", imprecise: "6.1", precise: "v6.1.0"}, } - if v6.IsPreciseVersion() { - t.Error("v6 should not be precise") - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + vImprecise := parseVersion(tt.imprecise) + vPrecise := parseVersion(tt.precise) + require.NotNil(t, vImprecise, "parseVersion(%q) should not return nil", tt.imprecise) + require.NotNil(t, vPrecise, "parseVersion(%q) should not return nil", tt.precise) - // Neither should be considered "newer" than the other - if v6.IsNewer(v600) { - t.Error("v6 should not be newer than v6.0.0") - } + // Both versions should share the same major.minor.patch + assert.Equal(t, vPrecise.Major, vImprecise.Major, "Major should match for %q and %q", tt.imprecise, tt.precise) + assert.Equal(t, vPrecise.Minor, vImprecise.Minor, "Minor should match for %q and %q", tt.imprecise, tt.precise) + assert.Equal(t, vPrecise.Patch, vImprecise.Patch, "Patch should match for %q and %q", tt.imprecise, tt.precise) - if v600.IsNewer(v6) { - t.Error("v6.0.0 should not be newer than v6") + assert.True(t, vPrecise.IsPreciseVersion(), "%q should be precise", tt.precise) + assert.False(t, vImprecise.IsPreciseVersion(), "%q should not be precise", tt.imprecise) + + // Neither should be considered newer than the other + assert.False(t, vImprecise.IsNewer(vPrecise), "%q should not be newer than %q", tt.imprecise, tt.precise) + assert.False(t, vPrecise.IsNewer(vImprecise), "%q should not be newer than %q", tt.precise, tt.imprecise) + }) } } From bd6083190cea2520b8a8b07336c6857ba599e7d4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:41:42 +0000 Subject: [PATCH 3/3] fix(semver): count only core-version dots in IsPreciseVersion; remove duplicate TestIsNewer; migrate TestVersionIsNewer to testify Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/semver_precise_test.go | 69 +++------------------------------- pkg/cli/semver_test.go | 13 +++---- pkg/semverutil/semverutil.go | 7 +++- 3 files changed, 18 insertions(+), 71 deletions(-) diff --git a/pkg/cli/semver_precise_test.go b/pkg/cli/semver_precise_test.go index 248fa1f92a3..9b192f449c8 100644 --- a/pkg/cli/semver_precise_test.go +++ b/pkg/cli/semver_precise_test.go @@ -70,10 +70,15 @@ func TestIsPreciseVersion(t *testing.T) { wantNil: true, }, { - name: "pre-release suffix - precise", + name: "pre-release on precise core - still precise", version: "v1.2.3-rc.1", expected: true, }, + { + name: "pre-release on imprecise core - invalid semver, returns nil", + version: "v1.2-rc.1", + wantNil: true, + }, } for _, tt := range tests { @@ -89,68 +94,6 @@ func TestIsPreciseVersion(t *testing.T) { } } -func TestIsNewer(t *testing.T) { - tests := []struct { - name string - v1 string - v2 string - newer bool - }{ - { - name: "strictly newer major", - v1: "v7.0.0", - v2: "v6.0.0", - newer: true, - }, - { - name: "strictly older major", - v1: "v5.0.0", - v2: "v6.0.0", - newer: false, - }, - { - name: "newer minor", - v1: "v6.1.0", - v2: "v6.0.0", - newer: true, - }, - { - name: "older minor", - v1: "v6.0.0", - v2: "v6.1.0", - newer: false, - }, - { - name: "newer patch", - v1: "v6.0.1", - v2: "v6.0.0", - newer: true, - }, - { - name: "older patch", - v1: "v6.0.0", - v2: "v6.0.1", - newer: false, - }, - { - name: "equal versions", - v1: "v6.0.0", - v2: "v6.0.0", - newer: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - v1 := parseVersion(tt.v1) - v2 := parseVersion(tt.v2) - require.NotNil(t, v1, "parseVersion(%q) should not return nil", tt.v1) - require.NotNil(t, v2, "parseVersion(%q) should not return nil", tt.v2) - assert.Equal(t, tt.newer, v1.IsNewer(v2), "IsNewer() for %q vs %q", tt.v1, tt.v2) - }) - } -} - func TestPreciseVersionPreference(t *testing.T) { // Tests that when comparing equivalent versions, imprecise tags (major-only or // major.minor) are not considered precise, while full three-component versions are. diff --git a/pkg/cli/semver_test.go b/pkg/cli/semver_test.go index 71c02a3f5b0..6056f8e1775 100644 --- a/pkg/cli/semver_test.go +++ b/pkg/cli/semver_test.go @@ -4,6 +4,9 @@ package cli import ( "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestIsSemanticVersionTag(t *testing.T) { @@ -157,14 +160,10 @@ func TestVersionIsNewer(t *testing.T) { v := parseVersion(tt.version) other := parseVersion(tt.other) - if v == nil || other == nil { - t.Fatalf("failed to parse versions: %q or %q", tt.version, tt.other) - } + require.NotNil(t, v, "parseVersion(%q) should not return nil", tt.version) + require.NotNil(t, other, "parseVersion(%q) should not return nil", tt.other) - got := v.IsNewer(other) - if got != tt.want { - t.Errorf("(%q).IsNewer(%q) = %v, want %v", tt.version, tt.other, got, tt.want) - } + assert.Equal(t, tt.want, v.IsNewer(other), "(%q).IsNewer(%q)", tt.version, tt.other) }) } } diff --git a/pkg/semverutil/semverutil.go b/pkg/semverutil/semverutil.go index 26fa6220d9d..535fb0bc793 100644 --- a/pkg/semverutil/semverutil.go +++ b/pkg/semverutil/semverutil.go @@ -148,9 +148,14 @@ func NormalizeGitDescribeSemver(v string) string { } // IsPreciseVersion returns true if the version has explicit minor and patch components -// (i.e., at least two dots in the version string, e.g. "v6.0.0" is precise, "v6" is not). +// (i.e., at least two dots in the core version, e.g. "v6.0.0" is precise, "v6" is not). +// Only core-version dots are counted; pre-release and build-metadata suffixes are excluded. func (v *SemanticVersion) IsPreciseVersion() bool { versionPart := strings.TrimPrefix(v.Raw, "v") + // Strip pre-release and build-metadata so their dots are not counted. + if idx := strings.IndexAny(versionPart, "-+"); idx >= 0 { + versionPart = versionPart[:idx] + } dotCount := strings.Count(versionPart, ".") return dotCount >= 2 }