Merge pull request #491 from redis/fix_stackbrew_generation

Fix stackbrew generation when 8.X and 8.X.Y formats are mixed toghether

Author: Peter-Sh

.github/workflows/build_release_automation.yml

@@ -62,12 +62,12 @@ jobs:
             --entrypoint /bin/bash \
             test-image:latest \
             -c "
-              cd release-automation
+              cd /release-automation
               set -e
               echo '=== Installing test dependencies ==='
-              pip install pytest pytest-cov
+              uv sync --frozen --all-extras
               echo '=== Running tests ==='
-              pytest -v tests/
+              uv run pytest
             "
 
       - name: Log in to Container Registry

release-automation/docker/Dockerfile

@@ -1,5 +1,6 @@
 """Data models for stackbrew library generation."""
 
+import functools
 import re
 from enum import Enum
 from typing import List, Optional
@@ -14,8 +15,12 @@ class DistroType(str, Enum):
     DEBIAN = "debian"
 
 
+@functools.total_ordering
 class RedisVersion(BaseModel):
-    """Represents a parsed Redis version."""
+    """Represents a parsed Redis version.
+
+    TODO: This class duplicates the code from redis-developer/redis-oss-release-automation
+    """
 
     major: int = Field(..., ge=1, description="Major version number")
     minor: int = Field(..., ge=0, description="Minor version number")
@@ -63,20 +68,46 @@ def is_eol(self) -> bool:
         """Check if this version is end-of-life."""
         return self.suffix.lower().endswith("-eol")
 
+    @property
+    def is_rc(self) -> bool:
+        """Check if this version is a release candidate."""
+        return self.suffix.lower().startswith("-rc")
+
+    @property
+    def is_ga(self) -> bool:
+        """Check if this version is a general availability (GA) release."""
+        return not self.is_milestone
+
+    @property
+    def is_internal(self) -> bool:
+        """Check if this version is an internal release."""
+        return bool(re.search(r"-int\d*$", self.suffix.lower()))
+
     @property
     def mainline_version(self) -> str:
         """Get the mainline version string (major.minor)."""
         return f"{self.major}.{self.minor}"
 
     @property
-    def sort_key(self) -> str:
-        suffix_weight = 0
-        if self.suffix.startswith("rc"):
-            suffix_weight = 100
-        elif self.suffix.startswith("m"):
-            suffix_weight = 50
+    def suffix_weight(self) -> str:
+        # warning: using lexicographic order, letters doesn't have any meaning except for ordering
+        suffix_weight = ""
+        if self.is_ga:
+            suffix_weight = "QQ"
+        if self.is_rc:
+            suffix_weight = "LL"
+        elif self.suffix.startswith("-m"):
+            suffix_weight = "II"
+
+        # internal versions are always lower than their GA/rc/m counterparts
+        if self.is_internal:
+            suffix_weight = suffix_weight[:1] + "E"
+
+        return suffix_weight
 
-        return f"{self.major}.{self.minor}.{self.patch or 0}.{suffix_weight}.{self.suffix}"
+    @property
+    def sort_key(self) -> str:
+        return f"{self.major}.{self.minor}.{self.patch or 0}.{self.suffix_weight}{self.suffix}"
 
     def __str__(self) -> str:
         """String representation of the version."""
@@ -90,21 +121,17 @@ def __lt__(self, other: "RedisVersion") -> bool:
         if not isinstance(other, RedisVersion):
             return NotImplemented
 
-        # Compare major.minor.patch first
-        self_tuple = (self.major, self.minor, self.patch or 0)
-        other_tuple = (other.major, other.minor, other.patch or 0)
+        return self.sort_key < other.sort_key
 
-        if self_tuple != other_tuple:
-            return self_tuple < other_tuple
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, RedisVersion):
+            return NotImplemented
 
-        # If numeric parts are equal, compare suffixes
-        # Empty suffix (GA) comes after suffixes (milestones)
-        if not self.suffix and other.suffix:
-            return False
-        if self.suffix and not other.suffix:
-            return True
+        return self.sort_key == other.sort_key
 
-        return self.suffix < other.suffix
+    def __hash__(self) -> int:
+        """Hash for use in sets and dicts."""
+        return hash((self.major, self.minor, self.patch or 0, self.suffix))
 
 
 class Distribution(BaseModel):

release-automation/src/stackbrew_generator/models.py

@@ -105,14 +105,13 @@ def filter_actual_versions(self, versions: List[Tuple[RedisVersion, str, str]])
         patch_versions = OrderedDict()
 
         for version, commit, tag_ref in versions:
-            patch_key = (version.major, version.minor, version.patch)
+            patch_key = (version.major, version.minor, version.patch or 0)
             if patch_key not in patch_versions:
                 patch_versions[patch_key] = (version, commit, tag_ref)
             elif patch_versions[patch_key][0].is_milestone and not version.is_milestone:
                 # GA always takes precedence over milestone for the same major.minor.patch
                 patch_versions[patch_key] = (version, commit, tag_ref)
 
-        print(patch_versions.values())
         filtered_versions = []
         mainlines_with_ga = set()
 

release-automation/src/stackbrew_generator/version_filter.py

@@ -26,7 +26,7 @@ def test_invalid_major_version(self):
         result = self.runner.invoke(app, ["generate-stackbrew-content", "0"])
         assert result.exit_code != 0
 
-    @patch('stackbrew_generator.git_operations.GitClient')
+    @patch('stackbrew_generator.cli.GitClient')
     def test_no_tags_found(self, mock_git_client_class):
         """Test handling when no tags are found."""
         # Mock git client to return no tags
@@ -36,7 +36,7 @@ def test_no_tags_found(self, mock_git_client_class):
 
         result = self.runner.invoke(app, ["generate-stackbrew-content", "99"])
         assert result.exit_code == 1
-        assert "No tags found" in result.stderr
+        assert "No versions found for major version 99" in result.stderr
 
     @patch('stackbrew_generator.version_filter.VersionFilter.get_actual_major_redis_versions')
     def test_no_versions_found(self, mock_get_versions):

release-automation/tests/test_integration.py

@@ -49,6 +49,25 @@ def test_parse_eol_version(self):
         assert version.suffix == "-eol"
         assert version.is_eol is True
 
+    def test_parse_rc_internal_version(self):
+        """Test parsing RC internal version."""
+        version = RedisVersion.parse("8.2.1-rc2-int3")
+        assert version.major == 8
+        assert version.minor == 2
+        assert version.patch == 1
+        assert version.suffix == "-rc2-int3"
+        assert version.is_rc is True
+        assert version.is_internal is True
+        assert len(version.sort_key) > 0
+
+        version = RedisVersion.parse("8.4-int")
+        assert version.major == 8
+        assert version.minor == 4
+        assert version.patch == None
+        assert version.suffix == "-int"
+        assert version.is_internal is True
+        assert len(version.sort_key) > 0
+
     def test_parse_invalid_version(self):
         """Test parsing invalid version strings."""
         with pytest.raises(ValueError):
@@ -82,22 +101,67 @@ def test_string_representation(self):
 
     def test_version_comparison(self):
         """Test version comparison for sorting."""
-        v1 = RedisVersion.parse("8.2.1")
-        v2 = RedisVersion.parse("8.2.2")
-        v3 = RedisVersion.parse("8.2.1-m01")
-        v4 = RedisVersion.parse("8.3.0")
+        v8_2_1 = RedisVersion.parse("8.2.1")
+        v8_2_2 = RedisVersion.parse("8.2.2")
+        v8_2_1_m_01 = RedisVersion.parse("8.2.1-m01")
+        v8_2_1_rc_01 = RedisVersion.parse("8.2.1-rc01")
+        v8_2_1_rc_01_int_1 = RedisVersion.parse("8.2.1-rc01-int1")
+        v8_3_0 = RedisVersion.parse("8.3.0")
+        v8_3_0_rc_1 = RedisVersion.parse("8.3.0-rc1")
+        v8_3_0_rc_1_int_1 = RedisVersion.parse("8.3.0-rc1-int1")
+        v8_3_0_rc_1_int_2 = RedisVersion.parse("8.3.0-rc1-int2")
+        v8_4 = RedisVersion.parse("8.4")
+        v8_4_rc_1 = RedisVersion.parse("8.4-rc1")
+        v8_6_int = RedisVersion.parse("8.6-int")
 
         # Test numeric comparison
-        assert v1 < v2
-        assert v2 < v4
+        assert v8_2_1 < v8_2_2
+        assert v8_2_2 < v8_3_0
 
         # Test milestone vs GA (GA comes after milestone)
-        assert v3 < v1
+        assert v8_2_1_m_01 < v8_2_1
+
+        assert v8_3_0_rc_1 < v8_3_0
+
+        assert v8_2_1_rc_01 > v8_2_1_m_01
+        assert v8_2_1_rc_01_int_1 > v8_2_1_m_01
+        assert v8_2_1_rc_01_int_1 < v8_2_1_rc_01
+
+        assert v8_3_0_rc_1_int_1 < v8_3_0_rc_1_int_2
+
+        assert v8_3_0_rc_1 > v8_3_0_rc_1_int_1
+        assert v8_3_0_rc_1 > v8_3_0_rc_1_int_2
 
         # Test sorting
-        versions = [v4, v1, v3, v2]
+        versions = [
+            v8_3_0,
+            v8_2_1,
+            v8_2_1_m_01,
+            v8_2_2,
+            v8_3_0_rc_1,
+            v8_3_0_rc_1_int_1,
+            v8_3_0_rc_1_int_2,
+            v8_6_int,
+            v8_4,
+            v8_4_rc_1,
+            v8_2_1_rc_01,
+            v8_2_1_rc_01_int_1,
+        ]
         sorted_versions = sorted(versions)
-        assert sorted_versions == [v3, v1, v2, v4]
+        assert sorted_versions == [
+            v8_2_1_m_01,
+            v8_2_1_rc_01_int_1,
+            v8_2_1_rc_01,
+            v8_2_1,
+            v8_2_2,
+            v8_3_0_rc_1_int_1,
+            v8_3_0_rc_1_int_2,
+            v8_3_0_rc_1,
+            v8_3_0,
+            v8_4_rc_1,
+            v8_4,
+            v8_6_int,
+        ]
 
 
 class TestDistribution:

release-automation/tests/test_models.py

@@ -261,6 +261,54 @@ def test_filter_actual_versions_milestone_only(self):
         expected_versions = ["8.2.1-m02", "8.1.0-m01"]
         assert version_strings == expected_versions
 
+    def test_filter_actual_versions_with_short_rc(self):
+        """Test actual version filtering with short (without patch) versions."""
+        version_filter = VersionFilter(MockGitClient())
+
+        versions = create_version_tuples([
+            "v8.4.0",
+            "v8.4-rc1",
+            "v8.2.3",
+        ])
+
+        result = version_filter.filter_actual_versions(versions)
+
+        version_strings = [str(v[0]) for v in result]
+        expected_versions = ["8.4.0", "8.2.3"]
+        assert version_strings == expected_versions
+
+    def test_filter_actual_versions_with_short_ga(self):
+        """Test actual version filtering with short (without patch) versions."""
+        version_filter = VersionFilter(MockGitClient())
+
+        versions = create_version_tuples([
+            "v8.4",
+            "v8.4.0-rc1",
+            "v8.2.3",
+        ])
+
+        result = version_filter.filter_actual_versions(versions)
+
+        version_strings = [str(v[0]) for v in result]
+        expected_versions = ["8.4", "8.2.3"]
+        assert version_strings == expected_versions
+
+    def test_filter_actual_versions_with_short_both_ga_and_rc(self):
+        """Test actual version filtering with short (without patch) versions."""
+        version_filter = VersionFilter(MockGitClient())
+
+        versions = create_version_tuples([
+            "v8.4",
+            "v8.4-rc1",
+            "v8.2.3",
+        ])
+
+        result = version_filter.filter_actual_versions(versions)
+
+        version_strings = [str(v[0]) for v in result]
+        expected_versions = ["8.4", "8.2.3"]
+        assert version_strings == expected_versions
+
     def test_filter_actual_versions_empty(self):
         """Test actual version filtering with empty input."""
         version_filter = VersionFilter(MockGitClient())