refactoring and testing

Author: mirkoCrobu

internal/update/arduino/arduino.go

@@ -122,26 +122,45 @@ func (a *ArduinoPlatformUpdater) ListUpgradablePackages(cfg config.Configuration
 	if platformSummary == nil {
 		return nil, nil // No platform found
 	}
+	releasesMap := platformSummary.GetReleases()
 
-	installedVersionString := platformSummary.GetInstalledVersion()
+	releases := make([]string, 0, len(releasesMap))
 
-	installedV, err := semver.NewVersion(installedVersionString)
-	if err != nil {
-		slog.Warn("Failed to parse installed version", "version", installedVersionString, "error", err)
+	for k := range releasesMap {
+		releases = append(releases, k)
+	}
+	bestVersion, err := findBestCandidate(
+		platformSummary.GetInstalledVersion(),
+		releases,
+		cfg.MaxAllowedMajorVersion,
+	)
+
+	if bestVersion == "" || err != nil {
 		return nil, nil
 	}
+	return []update.UpgradablePackage{{
+		Type:        update.Arduino,
+		Name:        "arduino:zephyr",
+		FromVersion: platformSummary.GetInstalledVersion(),
+		ToVersion:   bestVersion,
+	}}, nil
+}
+func findBestCandidate(installedStr string, availableVersions []string, maxMajorConfig int) (string, error) {
+	installedV, err := semver.NewVersion(installedStr)
+	if err != nil {
+		return "", err
+	}
 
-	var maxMajor uint64
-	if cfg.MaxAllowedMajorVersion > 0 {
-		maxMajor = uint64(cfg.MaxAllowedMajorVersion)
+	maxMajor := uint64(maxMajorConfig)
+	if maxMajorConfig <= 0 {
+		maxMajor = installedV.Major()
 	}
+
 	var bestUpdateV *semver.Version
 
-	allReleases := platformSummary.GetReleases()
-	for versionString := range allReleases {
-		candidateV, err := semver.NewVersion(versionString)
+	for _, vStr := range availableVersions {
+		candidateV, err := semver.NewVersion(vStr)
 		if err != nil {
-			slog.Debug("Skipping unparsable version", "version", versionString, "error", err)
 			continue
 		}
 
@@ -152,23 +171,15 @@ func (a *ArduinoPlatformUpdater) ListUpgradablePackages(cfg config.Configuration
 		if !candidateV.GreaterThan(installedV) {
 			continue
 		}
-
 		if bestUpdateV == nil || candidateV.GreaterThan(bestUpdateV) {
 			bestUpdateV = candidateV
 		}
 	}
+
 	if bestUpdateV == nil {
-		slog.Debug("No suitable updates found within major version constraint")
-		return nil, nil
+		return "", nil
 	}
-	slog.Debug(" bestUpdateV.Original()", bestUpdateV.Original(), "")
-	slog.Debug(" bestUpdateV.String()", bestUpdateV.String(), "")
-	return []update.UpgradablePackage{{
-		Type:        update.Arduino,
-		Name:        "arduino:zephyr",
-		FromVersion: platformSummary.GetInstalledVersion(),
-		ToVersion:   bestUpdateV.Original(),
-	}}, nil
+	return bestUpdateV.Original(), nil
 }
 
 // UpgradePackages implements ServiceUpdater.

internal/update/arduino/arduino_test.go

@@ -0,0 +1,101 @@
+package arduino
+
+import "testing"
+
+func TestFindBestCandidate(t *testing.T) {
+	tests := []struct {
+		name            string
+		installed       string
+		available       []string
+		maxMajorConfig  int
+		expectedVersion string
+		expectError     bool
+	}{
+		{
+			name:            "Standard update: minor upgrade available",
+			installed:       "1.0.0",
+			available:       []string{"1.0.1", "1.1.0"},
+			maxMajorConfig:  0,
+			expectedVersion: "1.1.0",
+			expectError:     false,
+		},
+		{
+			name:            "Major update blocked by default (Config=0)",
+			installed:       "1.9.9",
+			available:       []string{"2.0.0", "1.9.10"},
+			maxMajorConfig:  0,
+			expectedVersion: "1.9.10",
+			expectError:     false,
+		},
+		{
+			name:            "Major update allowed by explicit config",
+			installed:       "1.9.9",
+			available:       []string{"2.0.0", "3.0.0"},
+			maxMajorConfig:  2,
+			expectedVersion: "2.0.0",
+			expectError:     false,
+		},
+		{
+			name:            "CRITICAL: Regression test for 'Zero Value' bug (Version 2+)",
+			installed:       "2.1.0",
+			available:       []string{"2.2.0", "3.0.0"},
+			maxMajorConfig:  0,
+			expectedVersion: "2.2.0",
+			expectError:     false,
+		},
+		{
+			name:            "No updates available (all older or same)",
+			installed:       "1.5.0",
+			available:       []string{"1.0.0", "1.5.0"},
+			maxMajorConfig:  0,
+			expectedVersion: "",
+			expectError:     false,
+		},
+		{
+			name:            "Handle unsorted list and pick highest valid",
+			installed:       "1.0.0",
+			available:       []string{"1.1.0", "1.5.0", "1.2.0"},
+			maxMajorConfig:  0,
+			expectedVersion: "1.5.0",
+			expectError:     false,
+		},
+		{
+			name:            "Skip invalid candidate strings",
+			installed:       "1.0.0",
+			available:       []string{"invalid-ver", "1.1.0"},
+			maxMajorConfig:  0,
+			expectedVersion: "1.1.0",
+			expectError:     false,
+		},
+		{
+			name:            "Error on invalid installed version string",
+			installed:       "not-a-semver",
+			available:       []string{"1.0.0"},
+			maxMajorConfig:  0,
+			expectedVersion: "",
+			expectError:     true,
+		},
+		{
+			name:            "Prerelease handling (standard logic ignores prereleases unless specifically handled)",
+			installed:       "1.0.0",
+			available:       []string{"1.0.1-beta"},
+			maxMajorConfig:  0,
+			expectedVersion: "1.0.1-beta",
+			expectError:     false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := findBestCandidate(tt.installed, tt.available, tt.maxMajorConfig)
+
+			if (err != nil) != tt.expectError {
+				t.Errorf("findBestCandidate() error = %v, expectError %v", err, tt.expectError)
+				return
+			}
+			if got != tt.expectedVersion {
+				t.Errorf("findBestCandidate() = %v, want %v", got, tt.expectedVersion)
+			}
+		})
+	}
+}