From c5a4008db1bf875b1ebea247ce58a72d902ac7c4 Mon Sep 17 00:00:00 2001 From: matthiasL-scality Date: Wed, 16 Jul 2025 15:38:48 +0200 Subject: [PATCH 1/2] (PTFE-2534) Remove sort and modify test assertion --- bert_e/tests/test_bert_e.py | 2 +- bert_e/tests/unit/test_sorted.py | 4 +- bert_e/workflow/gitwaterflow/branches.py | 50 +++++++++++++++--------- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/bert_e/tests/test_bert_e.py b/bert_e/tests/test_bert_e.py index 10547ca6..90009e56 100644 --- a/bert_e/tests/test_bert_e.py +++ b/bert_e/tests/test_bert_e.py @@ -569,7 +569,7 @@ def test_branch_cascade_mixed_versions(self): 2: {'name': 'development/4.3', 'ignore': True}, 3: {'name': 'development/5.1.8', 'ignore': True}, 4: {'name': 'development/5.1', 'ignore': False}, - 5: {'name': 'development/10', 'ignore': False} + 5: {'name': 'development/10.0', 'ignore': False} }) tags = ['4.3.16', '4.3.18', '5.1.3', '5.1.7'] fixver = ['5.1.9', '10.0.0'] diff --git a/bert_e/tests/unit/test_sorted.py b/bert_e/tests/unit/test_sorted.py index 33cb80df..73fd49e7 100644 --- a/bert_e/tests/unit/test_sorted.py +++ b/bert_e/tests/unit/test_sorted.py @@ -74,10 +74,10 @@ def test_compare_branches_major_only_vs_major_minor(): def test_compare_branches_major_minor_micro_vs_major_minor(): branch1 = ((4, 3, 2),) branch2 = ((4, 3),) - assert compare_branches(branch1, branch2) == -997 + assert compare_branches(branch1, branch2) == -1 def test_compare_branches_major_minor_vs_major_minor_micro(): branch1 = ((4, 3),) branch2 = ((4, 3, 2),) - assert compare_branches(branch1, branch2) == 997 + assert compare_branches(branch1, branch2) == 1 diff --git a/bert_e/workflow/gitwaterflow/branches.py b/bert_e/workflow/gitwaterflow/branches.py index 3829f5a9..d899dc48 100644 --- a/bert_e/workflow/gitwaterflow/branches.py +++ b/bert_e/workflow/gitwaterflow/branches.py @@ -63,24 +63,38 @@ def compare_branches(branch1, branch2): return -1 # Both are major.minor or longer - compare normally - minor1 = version1[1] if len(version1) > 1 else 999 - minor2 = version2[1] if len(version2) > 1 else 999 - - # Compare minor versions - if minor1 != minor2: - return minor1 - minor2 - - # Same major.minor - extract micro versions - # Default to 0 if no micro - micro1 = version1[2] if len(version1) > 2 else 999 - # Default to 0 if no micro - micro2 = version2[2] if len(version2) > 2 else 999 - - # Compare micro versions - if micro1 != micro2: - return micro1 - micro2 - else: - return 0 + # Handle cases where one version has missing components + len1 = len(version1) + len2 = len(version2) + + # Compare minor versions if both exist + if len1 > 1 and len2 > 1: + minor1 = version1[1] + minor2 = version2[1] + if minor1 != minor2: + return minor1 - minor2 + elif len1 > 1 and len2 == 1: + # version1 has minor, version2 doesn't -> version1 comes first + return -1 + elif len1 == 1 and len2 > 1: + # version2 has minor, version1 doesn't -> version2 comes first + return 1 + # Both have same length (1) -> major versions already compared above + + # Same major.minor - compare micro versions if both exist + if len1 > 2 and len2 > 2: + micro1 = version1[2] + micro2 = version2[2] + if micro1 != micro2: + return micro1 - micro2 + elif len1 > 2 and len2 <= 2: + # version1 has micro, version2 doesn't -> version1 comes first + return -1 + elif len1 <= 2 and len2 > 2: + # version2 has micro, version1 doesn't -> version2 comes first + return 1 + + return 0 def compare_queues(version1, version2): From a17b81d9d850e5ecb6ab7f960cf75a0ede90065e Mon Sep 17 00:00:00 2001 From: matthiasL-scality Date: Thu, 17 Jul 2025 13:57:38 +0200 Subject: [PATCH 2/2] Fix flake8 --- bert_e/tests/unit/test_sorted.py | 4 +- bert_e/workflow/gitwaterflow/branches.py | 87 +++++++++++++----------- 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/bert_e/tests/unit/test_sorted.py b/bert_e/tests/unit/test_sorted.py index 73fd49e7..d1cd0127 100644 --- a/bert_e/tests/unit/test_sorted.py +++ b/bert_e/tests/unit/test_sorted.py @@ -55,7 +55,7 @@ def test_sorted_versions(): def test_compare_branches_major_minor_vs_major_only(): branch1 = ((4, 3),) - branch2 = ((4, None),) + branch2 = ((4, ),) assert compare_branches(branch1, branch2) == -1 @@ -66,7 +66,7 @@ def test_compare_branches_major_only_vs_major_only_returns_0(): def test_compare_branches_major_only_vs_major_minor(): - branch1 = ((4, None),) + branch1 = ((4, ),) branch2 = ((4, 3),) assert compare_branches(branch1, branch2) == 1 diff --git a/bert_e/workflow/gitwaterflow/branches.py b/bert_e/workflow/gitwaterflow/branches.py index d899dc48..50ca1814 100644 --- a/bert_e/workflow/gitwaterflow/branches.py +++ b/bert_e/workflow/gitwaterflow/branches.py @@ -29,6 +29,45 @@ LOG = logging.getLogger(__name__) +def _compare_version_component(version1, version2, component_index): + """ + Helper method to compare version components at a specific level. + + Args: + version1, version2: Version tuples to compare + component_index: Index of the component to compare + (0 for major, 1 for minor, 2 for micro) + + Returns: + int: -1 if version1 < version2, 1 if version1 > version2, + 0 if equal or both missing + None: if comparison should continue to next level + """ + len1 = len(version1) + len2 = len(version2) + + # Both have the component - compare them + if len1 > component_index and len2 > component_index: + comp1 = version1[component_index] + comp2 = version2[component_index] + if comp1 != comp2: + return comp1 - comp2 + return None # Equal, continue to next level + + # Only one has the component + elif len1 > component_index and len2 <= component_index: + # version1 has component, version2 doesn't + # -> version1 comes first in the cascade + return -1 + elif len1 <= component_index and len2 > component_index: + # version2 has component, version1 doesn't + # -> version2 comes first in the cascade + return 1 + + # Neither has the component at this level + return None + + def compare_branches(branch1, branch2): """ Compare GitWaterflow branches for sorting. @@ -39,13 +78,11 @@ def compare_branches(branch1, branch2): # Safely extract version components version1 = branch1[0] version2 = branch2[0] - # Extract major versions (always present) - major1 = version1[0] if len(version1) > 0 else 0 - major2 = version2[0] if len(version2) > 0 else 0 - # Compare major versions first - if major1 != major2: - return major1 - major2 + # Compare major versions first using helper + major_result = _compare_version_component(version1, version2, 0) + if major_result is not None: + return major_result # Same major version - check if one is major-only vs major.minor # Major-only branches have None as minor version, e.g., (4, None) @@ -62,38 +99,12 @@ def compare_branches(branch1, branch2): # major.minor comes before return -1 - # Both are major.minor or longer - compare normally - # Handle cases where one version has missing components - len1 = len(version1) - len2 = len(version2) - - # Compare minor versions if both exist - if len1 > 1 and len2 > 1: - minor1 = version1[1] - minor2 = version2[1] - if minor1 != minor2: - return minor1 - minor2 - elif len1 > 1 and len2 == 1: - # version1 has minor, version2 doesn't -> version1 comes first - return -1 - elif len1 == 1 and len2 > 1: - # version2 has minor, version1 doesn't -> version2 comes first - return 1 - # Both have same length (1) -> major versions already compared above - - # Same major.minor - compare micro versions if both exist - if len1 > 2 and len2 > 2: - micro1 = version1[2] - micro2 = version2[2] - if micro1 != micro2: - return micro1 - micro2 - elif len1 > 2 and len2 <= 2: - # version1 has micro, version2 doesn't -> version1 comes first - return -1 - elif len1 <= 2 and len2 > 2: - # version2 has micro, version1 doesn't -> version2 comes first - return 1 - + # Compare remaining version components (minor, micro) using helper + for level in range(1, 3): # 1=minor, 2=micro + result = _compare_version_component(version1, version2, level) + if result is not None: + return result + return 0