VersionLessThan(): fix comparison for multi-digit version parts

The function was previously using lexicographical comparison of strings, which works as expected only for single-digit version parts. Fix the function to actually first convert version parts to integers before comparing them. This will ensure the correct numerical order. Signed-off-by: Tomáš Hozza <[email protected]>
osbuild · Oct 9, 2023 · 18a3031 · 18a3031
1 parent 76f5eed
commit 18a3031
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 2 deletions.
diff --git a/internal/common/distro.go b/internal/common/distro.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"io"
 	"os"
+	"strconv"
 	"strings"
 )
 
@@ -71,9 +72,17 @@ func VersionLessThan(a, b string) bool {
 	}
 
 	for idx := 0; idx < len(aParts); idx++ {
-		if aParts[idx] < bParts[idx] {
+		aInt, err := strconv.Atoi(aParts[idx])
+		if err != nil {
+			panic(err)
+		}
+		bInt, err := strconv.Atoi(bParts[idx])
+		if err != nil {
+			panic(err)
+		}
+		if aInt < bInt {
 			return true
-		} else if aParts[idx] > bParts[idx] {
+		} else if aInt > bInt {
 			return false
 		}
 	}

diff --git a/internal/common/distro_test.go b/internal/common/distro_test.go
@@ -52,3 +52,77 @@ VARIANT_ID=workstation`,
 		}
 	}
 }
+
+func TestVersionLessThan(t *testing.T) {
+	type testCases struct {
+		Name     string
+		VersionA string
+		VersionB string
+		Expected bool
+	}
+
+	cases := []testCases{
+		{
+			Name:     "8 < 8.1",
+			VersionA: "8",
+			VersionB: "8.1",
+			Expected: true,
+		},
+		{
+			Name:     "8.1 < 8.2",
+			VersionA: "8.1",
+			VersionB: "8.2",
+			Expected: true,
+		},
+		{
+			Name:     "8 < 9",
+			VersionA: "8",
+			VersionB: "9",
+			Expected: true,
+		},
+		{
+			Name:     "8.1 < 9",
+			VersionA: "8.1",
+			VersionB: "9",
+			Expected: true,
+		},
+		{
+			Name:     "8.1 < 9.1",
+			VersionA: "8.1",
+			VersionB: "9.1",
+			Expected: true,
+		},
+		{
+			Name:     "8 < 8.10",
+			VersionA: "8",
+			VersionB: "8.10",
+			Expected: true,
+		},
+		{
+			Name:     "8.1 < 8.10",
+			VersionA: "8.1",
+			VersionB: "8.10",
+			Expected: true,
+		},
+		{
+			Name:     "8.10 < 8.11",
+			VersionA: "8.10",
+			VersionB: "8.11",
+			Expected: true,
+		},
+		{
+			Name:     "8.10 > 8.6",
+			VersionA: "8.10",
+			VersionB: "8.6",
+			Expected: false,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.Name, func(t *testing.T) {
+			if VersionLessThan(c.VersionA, c.VersionB) != c.Expected {
+				t.Fatalf("VersionLessThan(%s, %s) returned unexpected result", c.VersionA, c.VersionB)
+			}
+		})
+	}
+}