diff --git a/pkg/cli/semver_precise_test.go b/pkg/cli/semver_precise_test.go index e3795cd0fc8..9b192f449c8 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,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) + }) } } 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 }