test(semver): upgrade semver_precise_test.go to testify + expand coverage

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,74 @@ 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 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 {
 		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")
-	}
-
-	// 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)
+	// 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"},
 	}
 
-	// v6.0.0 should be precise, v6 should not
-	if !v600.IsPreciseVersion() {
-		t.Error("v6.0.0 should 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)
 
-	if v6.IsPreciseVersion() {
-		t.Error("v6 should not be precise")
-	}
+			// 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)
 
-	// Neither should be considered "newer" than the other
-	if v6.IsNewer(v600) {
-		t.Error("v6 should not be newer than v6.0.0")
-	}
+			assert.True(t, vPrecise.IsPreciseVersion(), "%q should be precise", tt.precise)
+			assert.False(t, vImprecise.IsPreciseVersion(), "%q should not be precise", tt.imprecise)
 
-	if v600.IsNewer(v6) {
-		t.Error("v6.0.0 should not be newer than v6")
+			// 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)
+		})
 	}
 }

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)
 		})
 	}
 }

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
 }