From 63ff18f89a57608e17908454465af8216f66842f Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Mon, 22 Sep 2025 10:53:59 -0400 Subject: [PATCH 1/3] Add tests for RPM version sorting --- pulp_rpm/tests/unit/test_rpm_version.py | 305 ++++++++++++++++++++++++ 1 file changed, 305 insertions(+) create mode 100644 pulp_rpm/tests/unit/test_rpm_version.py diff --git a/pulp_rpm/tests/unit/test_rpm_version.py b/pulp_rpm/tests/unit/test_rpm_version.py new file mode 100644 index 0000000000..7649623dce --- /dev/null +++ b/pulp_rpm/tests/unit/test_rpm_version.py @@ -0,0 +1,305 @@ +""" +Unit tests for RPM version comparison functionality. + +These tests are designed to match the behavior tested in the Rust implementation +to ensure compatibility between different RPM version comparison implementations. +""" + +from pulp_rpm.app.rpm_version import ( + RpmVersion, + compare_rpm_versions, + compare_version_strings, + from_evr, +) + + +class TestRpmVersionComparison: + """Test basic RPM version comparison functionality.""" + + def test_evr_tostr(self): + """Test that EVRs are printed as expected.""" + evr = RpmVersion("", "1.2.3", "45") + assert str(evr) == "1.2.3-45" + + evr = RpmVersion("0", "1.2.3", "45") + assert str(evr) == "0:1.2.3-45" + + def test_evr_parse(self): + """Test that a correctly formed EVR string is parsed correctly.""" + evr = RpmVersion.from_string("1.2.3-45") + expected = RpmVersion("", "1.2.3", "45") + assert evr == expected + + evr = RpmVersion.from_string("0:1.2.3-45") + expected = RpmVersion("0", "1.2.3", "45") + assert evr == expected + + evr = RpmVersion.from_string("1:2.3.4-5") + expected = RpmVersion("1", "2.3.4", "5") + assert evr == expected + + def test_evr_parse_edge_cases(self): + """Test that various not-well-formed EVR strings still get parsed in a sensible way.""" + assert from_evr("-") == ("", "", "") + assert from_evr(".") == ("", ".", "") + assert from_evr(":") == ("", "", "") + assert from_evr(":-") == ("", "", "") + assert from_evr(".-") == ("", ".", "") + assert from_evr("0") == ("", "0", "") + assert from_evr("0-") == ("", "0", "") + assert from_evr(":0") == ("", "0", "") + assert from_evr(":0-") == ("", "0", "") + assert from_evr("0:") == ("0", "", "") + assert from_evr("asdf:") == ("asdf", "", "") + assert from_evr("~:") == ("~", "", "") + + def test_rpm_evr_compare(self): + """Test direct comparison of rpm EVR strings.""" + assert compare_rpm_versions("0:1.2.3-45", "1.2.3-45") == 0 + assert compare_rpm_versions("1.2.3-45", "1:1.2.3-45") < 0 + assert compare_rpm_versions("1.2.3-46", "1.2.3-45") > 0 + + def test_evr_ord(self): + """Test comparing EVRs using comparison operators.""" + # compare the same EVR without epoch as equal + evr1 = RpmVersion.from_string("1.2.3-45") + evr2 = RpmVersion.from_string("1.2.3-45") + assert evr1 == evr2 + + # compare the same EVR with epoch as equal + evr1 = RpmVersion.from_string("2:1.2.3-45") + evr2 = RpmVersion.from_string("2:1.2.3-45") + assert evr1 == evr2 + + # compare the same EVR with zero-epoch as equal to default-epoch + evr1 = RpmVersion.from_string("1.2.3-45") + evr2 = RpmVersion.from_string("0:1.2.3-45") + assert evr1 == evr2 + + # compare EVR with higher epoch and same version / release + evr1 = RpmVersion.from_string("1.2.3-45") + evr2 = RpmVersion.from_string("1:1.2.3-45") + assert evr1 < evr2 + + # compare EVR with higher epoch taken over EVR with higher version + evr1 = RpmVersion.from_string("4.2.3-45") + evr2 = RpmVersion.from_string("1:1.2.3-45") + assert evr1 < evr2 + + # compare EVR with higher version + evr1 = RpmVersion.from_string("1.2.3-45") + evr2 = RpmVersion.from_string("1.2.4-45") + assert evr1 < evr2 + + # compare EVR with higher version + evr1 = RpmVersion.from_string("1.23.3-45") + evr2 = RpmVersion.from_string("1.2.3-45") + assert evr1 > evr2 + + # compare EVR with higher version + evr1 = RpmVersion.from_string("12.2.3-45") + evr2 = RpmVersion.from_string("1.2.3-45") + assert evr1 > evr2 + + # compare EVR with higher version + evr1 = RpmVersion.from_string("1.2.3-45") + evr2 = RpmVersion.from_string("1.12.3-45") + assert evr1 < evr2 + + # compare versions with tilde parsing as older + evr1 = RpmVersion.from_string("~1.2.3-45") + evr2 = RpmVersion.from_string("1.2.3-45") + assert evr1 < evr2 + + # compare versions with tilde parsing as older + evr1 = RpmVersion.from_string("~12.2.3-45") + evr2 = RpmVersion.from_string("1.2.3-45") + assert evr1 < evr2 + + # compare versions with tilde parsing as older + evr1 = RpmVersion.from_string("~12.2.3-45") + evr2 = RpmVersion.from_string("~1.2.3-45") + assert evr1 > evr2 + + # compare versions with tilde parsing as older + evr1 = RpmVersion.from_string("~3:12.2.3-45") + evr2 = RpmVersion.from_string("0:1.2.3-45") + assert evr1 < evr2 + + # compare release + evr1 = RpmVersion.from_string("1.2.3-45") + evr2 = RpmVersion.from_string("1.2.3-46") + assert evr1 < evr2 + + # compare release + evr1 = RpmVersion.from_string("1.2.3-45.fc39") + evr2 = RpmVersion.from_string("1.2.3-46.fc38") + assert evr1 < evr2 + + # compare release + evr1 = RpmVersion.from_string("1.2.3-3") + evr2 = RpmVersion.from_string("1.2.3-10") + assert evr1 < evr2 + + # compare release + evr1 = RpmVersion.from_string("1.2.3-3.fc40") + evr2 = RpmVersion.from_string("1.2.3-10.fc39") + assert evr1 < evr2 + + def test_compare_version_string(self): + """Test many different combinations of version string comparison behavior.""" + assert compare_version_strings("1.0", "1.0") == 0 + assert compare_version_strings("1.0", "2.0") < 0 + assert compare_version_strings("2.0", "1.0") > 0 + + assert compare_version_strings("2.0.1", "2.0.1") == 0 + assert compare_version_strings("2.0", "2.0.1") < 0 + assert compare_version_strings("2.0.1", "2.0") > 0 + + assert compare_version_strings("5.0.1", "5.0.1a") < 0 + assert compare_version_strings("5.0.1a", "5.0.1") > 0 + + assert compare_version_strings("5.0.a1", "5.0.a1") == 0 + assert compare_version_strings("5.0.1a", "5.0.1a") == 0 + assert compare_version_strings("5.0.a1", "5.0.a2") < 0 + assert compare_version_strings("5.0.a2", "5.0.a1") > 0 + + assert compare_version_strings("10abc", "10.1abc") < 0 + assert compare_version_strings("10.1abc", "10abc") > 0 + + assert compare_version_strings("8.0", "8.0.rc1") < 0 + assert compare_version_strings("8.0.rc1", "8.0") > 0 + + assert compare_version_strings("10b2", "10a1") > 0 + assert compare_version_strings("10a2", "10b2") < 0 + + assert compare_version_strings("6.6p1", "7.5p1") < 0 + assert compare_version_strings("7.5p1", "6.6p1") > 0 + + assert compare_version_strings("6.5p1", "6.5p1") == 0 + assert compare_version_strings("6.5p1", "6.5p2") < 0 + assert compare_version_strings("6.5p2", "6.5p1") > 0 + assert compare_version_strings("6.5p2", "6.6p1") < 0 + assert compare_version_strings("6.6p1", "6.5p2") > 0 + + assert compare_version_strings("6.5p10", "6.5p10") == 0 + assert compare_version_strings("6.5p1", "6.5p10") < 0 + assert compare_version_strings("6.5p10", "6.5p1") > 0 + + assert compare_version_strings("abc10", "abc10") == 0 + assert compare_version_strings("abc10", "abc10.1") < 0 + assert compare_version_strings("abc10.1", "abc10") > 0 + + assert compare_version_strings("abc.4", "abc.4") == 0 + assert compare_version_strings("abc.4", "8") < 0 + assert compare_version_strings("8", "abc.4") > 0 + assert compare_version_strings("abc.4", "2") < 0 + assert compare_version_strings("2", "abc.4") > 0 + + assert compare_version_strings("1.0aa", "1.0aa") == 0 + assert compare_version_strings("1.0a", "1.0aa") < 0 + assert compare_version_strings("1.0aa", "1.0a") > 0 + + def test_version_comparison_numeric_handling(self): + """Test handling of numeric-like values in version strings.""" + assert compare_version_strings("10.0001", "10.0001") == 0 + # sequences of leading zeroes are meant to be ignored - it's not *actually* treated + # like a numeric value + assert compare_version_strings("10.0001", "10.1") == 0 + assert compare_version_strings("10.1", "10.0001") == 0 + assert compare_version_strings("10.0001", "10.0039") < 0 + assert compare_version_strings("10.0039", "10.0001") > 0 + # but sequences of zeroes within a numeric segment are not ignored + assert compare_version_strings("10.1", "10.10001") < 0 + assert compare_version_strings("10.1111", "10.10001") < 0 + assert compare_version_strings("10.11111", "10.10001") > 0 + + assert compare_version_strings("20240521", "20240521") == 0 + assert compare_version_strings("20240521", "20240522") < 0 + assert compare_version_strings("20240522", "20240521") > 0 + assert compare_version_strings("20240521", "202405210") < 0 + + def test_version_comparison_tilde_and_caret(self): + """Test behavior of tilde and caret operators.""" + assert compare_version_strings("1.0~rc1", "1.0~rc1") == 0 + assert compare_version_strings("1.0~rc1", "1.0") < 0 + assert compare_version_strings("1.0", "1.0~rc1") > 0 + assert compare_version_strings("1.0~rc1", "1.0~rc2") < 0 + assert compare_version_strings("1.0~rc2", "1.0~rc1") > 0 + assert compare_version_strings("1.0~rc1~git123", "1.0~rc1~git123") == 0 + assert compare_version_strings("1.0~rc1~git123", "1.0~rc1") < 0 + assert compare_version_strings("1.0~rc1", "1.0~rc1~git123") > 0 + + assert compare_version_strings("1.0^", "1.0^") == 0 + assert compare_version_strings("1.0", "1.0^") < 0 + assert compare_version_strings("1.0^", "1.0") > 0 + + assert compare_version_strings("1.0", "1.0git1^") < 0 + assert compare_version_strings("1.0^git1", "1.0^git2") < 0 + assert compare_version_strings("1.01", "1.0^git1") > 0 + assert compare_version_strings("1.0^20240501", "1.0^20240501") == 0 + assert compare_version_strings("1.0^20240501", "1.0.1") < 0 + assert compare_version_strings("1.0^20240501^git1", "1.0^20240501^git1") == 0 + assert compare_version_strings("1.0^20240502", "1.0^20240501^git1") > 0 + assert compare_version_strings("1.0~rc1^git1", "1.0~rc1^git1") == 0 + assert compare_version_strings("1.0~rc1", "1.0~rc1^git1") < 0 + assert compare_version_strings("1.0~rc1^git1", "1.0~rc1") > 0 + assert compare_version_strings("1.0^git1~pre", "1.0^git1~pre") == 0 + assert compare_version_strings("1.0^git1~pre", "1.0^git1") < 0 + assert compare_version_strings("1.0^git1", "1.0^git1~pre") > 0 + + def test_non_intuitive_comparison_behavior(self): + """Test some version comparison behavior that is a bit non-intuitive.""" + # (but needs to be maintained for compatibility) + assert compare_version_strings("1e.fc33", "1.fc33") < 0 + assert compare_version_strings("1g.fc33", "1.fc33") > 0 + + def test_non_alphanumeric_equivalence(self): + """Test handling of non-alphanumeric ascii characters (excluding separators).""" + # the existence of sequences of non-alphanumeric characters should not impact + # the version comparison at all + assert compare_version_strings("b", "b") == 0 + assert compare_version_strings("b+", "b+") == 0 + assert compare_version_strings("b+", "b_") == 0 + assert compare_version_strings("b_", "b+") == 0 + assert compare_version_strings("+b", "+b") == 0 + assert compare_version_strings("+b", "_b") == 0 + assert compare_version_strings("_b", "+b") == 0 + + assert compare_version_strings("+b", "++b") == 0 + assert compare_version_strings("+b", "+b+") == 0 + + assert compare_version_strings("+.", "+_") == 0 + assert compare_version_strings("_+", "+.") == 0 + assert compare_version_strings("+", ".") == 0 + assert compare_version_strings(",", "+") == 0 + + assert compare_version_strings("++", "_") == 0 + assert compare_version_strings("+", "..") == 0 + + assert compare_version_strings("4_0", "4_0") == 0 + assert compare_version_strings("4_0", "4.0") == 0 + assert compare_version_strings("4.0", "4_0") == 0 + + assert compare_version_strings("4.999", "5.0") < 0 + assert compare_version_strings("4.999.9", "5.0") < 0 + assert compare_version_strings("5.0", "4.999_9") > 0 + + # except when it comes to breaking up sequences of alphanumeric characters + # that do impact the comparison + assert compare_version_strings("4.999", "4.999.9") < 0 + assert compare_version_strings("4.999", "4.99.9") > 0 + + def test_non_ascii_character_equivalence(self): + """Test handling of non-ascii characters.""" + # the existence of sequences of non-ascii characters should not impact the + # version comparison at all + assert compare_version_strings("1.1.Á.1", "1.1.1") == 0 + assert compare_version_strings("1.1.Á", "1.1.Á") == 0 + assert compare_version_strings("1.1.Á", "1.1.Ê") == 0 + assert compare_version_strings("1.1.ÁÁ", "1.1.Á") == 0 + assert compare_version_strings("1.1.Á", "1.1.ÊÊ") == 0 + + # except when it comes to breaking up sequences of ascii characters that do + # impact the comparison + assert compare_version_strings("1.1Á1", "1.11") < 0 From fe5a30e8c298d605981826e50e74c422b703841a Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Mon, 22 Sep 2025 21:56:37 -0400 Subject: [PATCH 2/3] Change implementation of Python version sort --- pulp_rpm/app/rpm_version.py | 289 ++++++++++++++---------- pulp_rpm/tests/unit/test_rpm_version.py | 226 +++++++++--------- 2 files changed, 281 insertions(+), 234 deletions(-) diff --git a/pulp_rpm/app/rpm_version.py b/pulp_rpm/app/rpm_version.py index b7a30a2244..9e9066c909 100644 --- a/pulp_rpm/app/rpm_version.py +++ b/pulp_rpm/app/rpm_version.py @@ -12,7 +12,6 @@ # flake8: noqa -import re from typing import NamedTuple from typing import Union @@ -22,7 +21,7 @@ class RpmVersion(NamedTuple): Represent an RPM version. It is ordered. """ - epoch: int + epoch: str version: str release: str @@ -46,38 +45,36 @@ def from_string(cls, s): return cls(e, v, r) def __lt__(self, other): - return compare_rpm_versions(self, other) < 0 + return _compare_rpm_versions(self, other) < 0 def __gt__(self, other): - return compare_rpm_versions(self, other) > 0 + return _compare_rpm_versions(self, other) > 0 def __eq__(self, other): - return compare_rpm_versions(self, other) == 0 + return _compare_rpm_versions(self, other) == 0 def __le__(self, other): - return compare_rpm_versions(self, other) <= 0 + return _compare_rpm_versions(self, other) <= 0 def __ge__(self, other): - return compare_rpm_versions(self, other) >= 0 + return _compare_rpm_versions(self, other) >= 0 def from_evr(s): """ Return an (E, V, R) tuple given a string by splitting [e:]version-release into the three possible subcomponents. - Default epoch to 0, version and release to empty string if not specified. + Default epoch, version and release to empty string if not specified. - >>> assert from_evr("1:11.13.2.0-1") == (1, "11.13.2.0", "1") - >>> assert from_evr("11.13.2.0-1") == (0, "11.13.2.0", "1") + >>> assert from_evr("1:11.13.2.0-1") == ("1", "11.13.2.0", "1") + >>> assert from_evr("11.13.2.0-1") == ("", "11.13.2.0", "1") """ if ":" in s: e, _, vr = s.partition(":") else: - e = "0" + e = "" vr = s - e = int(e) - if "-" in vr: v, _, r = vr.partition("-") else: @@ -86,7 +83,7 @@ def from_evr(s): return e, v, r -def compare_rpm_versions(a: Union[RpmVersion, str], b: Union[RpmVersion, str]) -> int: +def _compare_rpm_versions(a: Union[RpmVersion, str], b: Union[RpmVersion, str]) -> int: """ Compare two RPM versions ``a`` and ``b`` and return: - 1 if the version of a is newer than b @@ -113,131 +110,181 @@ def compare_rpm_versions(a: Union[RpmVersion, str], b: Union[RpmVersion, str]) - if not isinstance(a, RpmVersion) and not isinstance(b, RpmVersion): raise TypeError(f"{a!r} and {b!r} must be RpmVersion or strings") + a_epoch = a.epoch or "0" + b_epoch = b.epoch or "0" + # First compare the epoch, if set. If the epoch's are not the same, then # the higher one wins no matter what the rest of the EVR is. - if a.epoch != b.epoch: - if a.epoch > b.epoch: - return 1 # a > b - else: - return -1 # a < b + if a_epoch != b_epoch: + epoch_compare = _compare_version_strings(a_epoch, b_epoch) + if epoch_compare != 0: + return epoch_compare # a > b # Epoch is the same, if version + release are the same we have a match if (a.version == b.version) and (a.release == b.release): return 0 # a == b # Compare version first, if version is equal then compare release - compare_res = vercmp(a.version, b.version) - if compare_res != 0: # a > b || a < b - return compare_res - else: - return vercmp(a.release, b.release) - - -class Vercmp: - R_NONALNUMTILDE_CARET = re.compile(rb"^([^a-zA-Z0-9~\^]*)(.*)$") - R_NUM = re.compile(rb"^([\d]+)(.*)$") - R_ALPHA = re.compile(rb"^([a-zA-Z]+)(.*)$") + version_compare = _compare_version_strings(a.version, b.version) + if version_compare != 0: # a > b || a < b + return version_compare + + return _compare_version_strings(a.release, b.release) + + +# internal use: each individual component of the EVR is compared using this function +def _compare_version_strings(first, second): + first = first.encode("utf-8") + second = second.encode("utf-8") + + if first == second: + return 0 + + def not_alphanumeric_tilde_or_caret(c): + return not ( + (ord(b"a") <= c <= ord(b"z")) + or (ord(b"A") <= c <= ord(b"Z")) + or (ord(b"0") <= c <= ord(b"9")) + or c == ord(b"~") + or c == ord(b"^") + ) + + def trim_start_matches(data, predicate): + """Trim leading bytes that match the predicate""" + start = 0 + while start < len(data) and predicate(data[start]): + start += 1 + return data[start:] + + def strip_prefix(data, prefix): + """Strip prefix from data, return (stripped_data, was_stripped)""" + if data.startswith(prefix): + return data[len(prefix) :], True + return data, False + + def matching_contiguous(data, predicate): + """Match contiguous characters that satisfy predicate""" + if not data: + return None, data + + if not predicate(data[0]): + return None, data + + end = 0 + while end < len(data) and predicate(data[end]): + end += 1 + + return data[:end], data[end:] + + version1_part = first + version2_part = second + + while True: + # Strip any leading non-alphanumeric, non-tilde, non-caret characters + version1_part = trim_start_matches(version1_part, not_alphanumeric_tilde_or_caret) + version2_part = trim_start_matches(version2_part, not_alphanumeric_tilde_or_caret) + + # Tilde separator parses as "older" or lesser version + version1_stripped, version1_had_tilde = strip_prefix(version1_part, b"~") + version2_stripped, version2_had_tilde = strip_prefix(version2_part, b"~") + + if version1_had_tilde and not version2_had_tilde: + return -1 + elif not version1_had_tilde and version2_had_tilde: + return 1 + elif version1_had_tilde and version2_had_tilde: + version1_part = version1_stripped + version2_part = version2_stripped + continue - @classmethod - def compare(cls, first, second): - # Rpm versions can only be ascii, anything else is just ignored - first = first.encode("ascii", "ignore") - second = second.encode("ascii", "ignore") + # Caret means the version is less... Unless the other version + # has ended, then do the exact opposite. + version1_stripped, version1_had_caret = strip_prefix(version1_part, b"^") + version2_stripped, version2_had_caret = strip_prefix(version2_part, b"^") + + if version1_had_caret and not version2_had_caret: + # first has caret but second doesn't + if not version2_part: # second has ended + return 1 # first > second + else: # second continues + return -1 # first < second + elif not version1_had_caret and version2_had_caret: + # second has caret but first doesn't + if not version1_part: # first has ended + return -1 # first < second + else: # first continues + return 1 # first > second + elif version1_had_caret and version2_had_caret: + # both have caret, strip and continue + version1_part = version1_stripped + version2_part = version2_stripped + continue - if first == second: + # Check if we've run out of characters + if not version1_part and not version2_part: return 0 + elif not version1_part: + return -1 + elif not version2_part: + return 1 - while first or second: - m1 = cls.R_NONALNUMTILDE_CARET.match(first) - m2 = cls.R_NONALNUMTILDE_CARET.match(second) - m1_head, first = m1.group(1), m1.group(2) - m2_head, second = m2.group(1), m2.group(2) - if m1_head or m2_head: - # Ignore junk at the beginning - continue - - # handle the tilde separator, it sorts before everything else - if first.startswith(b"~"): - if not second.startswith(b"~"): - return -1 - first, second = first[1:], second[1:] - continue - if second.startswith(b"~"): - return 1 + # Parse numeric or alphabetic segments + def is_digit(c): + return ord(b"0") <= c <= ord(b"9") - # Now look at the caret, which is like the tilde but pointier. - if first.startswith(b"^"): - # first has a caret but second has ended - if not second: - return 1 # first > second - - # first has a caret but second continues on - elif not second.startswith(b"^"): - return -1 # first < second - - # strip the ^ and start again - first, second = first[1:], second[1:] - continue - - # Caret means the version is less... Unless the other version - # has ended, then do the exact opposite. - if second.startswith(b"^"): - return -1 if not first else 1 - - # We've run out of characters to compare. - # Note: we have to do this after we compare the ~ and ^ madness - # because ~'s and ^'s take precedance. - # If we ran to the end of either, we are finished with the loop - if not first or not second: - break - - # grab first completely alpha or completely numeric segment - m1 = cls.R_NUM.match(first) - if m1: - m2 = cls.R_NUM.match(second) - if not m2: - # numeric segments are always newer than alpha segments - return 1 - isnum = True - else: - m1 = cls.R_ALPHA.match(first) - m2 = cls.R_ALPHA.match(second) - if not m2: - return -1 - isnum = False + def is_alpha(c): + return (ord(b"a") <= c <= ord(b"z")) or (ord(b"A") <= c <= ord(b"Z")) + + if version1_part and is_digit(version1_part[0]): + # First starts with digit - extract numeric segment + segment1, version1_part = matching_contiguous(version1_part, is_digit) - m1_head, first = m1.group(1), m1.group(2) - m2_head, second = m2.group(1), m2.group(2) + if version2_part and is_digit(version2_part[0]): + # Both numeric + segment2, version2_part = matching_contiguous(version2_part, is_digit) - if isnum: - # throw away any leading zeros - it's a number, right? - m1_head = m1_head.lstrip(b"0") - m2_head = m2_head.lstrip(b"0") + # Strip leading zeros + segment1 = segment1.lstrip(b"0") + segment2 = segment2.lstrip(b"0") - # whichever number has more digits wins - m1hlen = len(m1_head) - m2hlen = len(m2_head) - if m1hlen < m2hlen: + # Compare by length first (more digits = larger number) + if len(segment1) < len(segment2): return -1 - if m1hlen > m2hlen: + elif len(segment1) > len(segment2): return 1 - - # Same number of chars - if m1_head < m2_head: - return -1 - if m1_head > m2_head: + else: + # Same length, compare lexicographically + if segment1 < segment2: + return -1 + elif segment1 > segment2: + return 1 + # Equal, continue to next segment + else: + # First is numeric, second is not - numeric wins return 1 - # Both segments equal - continue - - m1len = len(first) - m2len = len(second) - if m1len == m2len == 0: - return 0 - if m1len != 0: - return 1 - return -1 + else: + # First starts with alpha or we're at end + if version1_part: + segment1, version1_part = matching_contiguous(version1_part, is_alpha) + else: + segment1 = b"" + if version2_part and is_digit(version2_part[0]): + # First is alpha, second is numeric - numeric wins + return -1 + else: + # Both alpha or at least one is empty + if version2_part: + segment2, version2_part = matching_contiguous(version2_part, is_alpha) + else: + segment2 = b"" + + # Compare alphabetically + if segment1 < segment2: + return -1 + elif segment1 > segment2: + return 1 + # Equal, continue to next segment -def vercmp(first, second): - return Vercmp.compare(first, second) + # Should not reach here due to the checks above, but just in case + return 0 diff --git a/pulp_rpm/tests/unit/test_rpm_version.py b/pulp_rpm/tests/unit/test_rpm_version.py index 7649623dce..476ee8a4dc 100644 --- a/pulp_rpm/tests/unit/test_rpm_version.py +++ b/pulp_rpm/tests/unit/test_rpm_version.py @@ -7,8 +7,8 @@ from pulp_rpm.app.rpm_version import ( RpmVersion, - compare_rpm_versions, - compare_version_strings, + _compare_rpm_versions, + _compare_version_strings, from_evr, ) @@ -55,9 +55,9 @@ def test_evr_parse_edge_cases(self): def test_rpm_evr_compare(self): """Test direct comparison of rpm EVR strings.""" - assert compare_rpm_versions("0:1.2.3-45", "1.2.3-45") == 0 - assert compare_rpm_versions("1.2.3-45", "1:1.2.3-45") < 0 - assert compare_rpm_versions("1.2.3-46", "1.2.3-45") > 0 + assert _compare_rpm_versions("0:1.2.3-45", "1.2.3-45") == 0 + assert _compare_rpm_versions("1.2.3-45", "1:1.2.3-45") < 0 + assert _compare_rpm_versions("1.2.3-46", "1.2.3-45") > 0 def test_evr_ord(self): """Test comparing EVRs using comparison operators.""" @@ -148,158 +148,158 @@ def test_evr_ord(self): def test_compare_version_string(self): """Test many different combinations of version string comparison behavior.""" - assert compare_version_strings("1.0", "1.0") == 0 - assert compare_version_strings("1.0", "2.0") < 0 - assert compare_version_strings("2.0", "1.0") > 0 + assert _compare_version_strings("1.0", "1.0") == 0 + assert _compare_version_strings("1.0", "2.0") < 0 + assert _compare_version_strings("2.0", "1.0") > 0 - assert compare_version_strings("2.0.1", "2.0.1") == 0 - assert compare_version_strings("2.0", "2.0.1") < 0 - assert compare_version_strings("2.0.1", "2.0") > 0 + assert _compare_version_strings("2.0.1", "2.0.1") == 0 + assert _compare_version_strings("2.0", "2.0.1") < 0 + assert _compare_version_strings("2.0.1", "2.0") > 0 - assert compare_version_strings("5.0.1", "5.0.1a") < 0 - assert compare_version_strings("5.0.1a", "5.0.1") > 0 + assert _compare_version_strings("5.0.1", "5.0.1a") < 0 + assert _compare_version_strings("5.0.1a", "5.0.1") > 0 - assert compare_version_strings("5.0.a1", "5.0.a1") == 0 - assert compare_version_strings("5.0.1a", "5.0.1a") == 0 - assert compare_version_strings("5.0.a1", "5.0.a2") < 0 - assert compare_version_strings("5.0.a2", "5.0.a1") > 0 + assert _compare_version_strings("5.0.a1", "5.0.a1") == 0 + assert _compare_version_strings("5.0.1a", "5.0.1a") == 0 + assert _compare_version_strings("5.0.a1", "5.0.a2") < 0 + assert _compare_version_strings("5.0.a2", "5.0.a1") > 0 - assert compare_version_strings("10abc", "10.1abc") < 0 - assert compare_version_strings("10.1abc", "10abc") > 0 + assert _compare_version_strings("10abc", "10.1abc") < 0 + assert _compare_version_strings("10.1abc", "10abc") > 0 - assert compare_version_strings("8.0", "8.0.rc1") < 0 - assert compare_version_strings("8.0.rc1", "8.0") > 0 + assert _compare_version_strings("8.0", "8.0.rc1") < 0 + assert _compare_version_strings("8.0.rc1", "8.0") > 0 - assert compare_version_strings("10b2", "10a1") > 0 - assert compare_version_strings("10a2", "10b2") < 0 + assert _compare_version_strings("10b2", "10a1") > 0 + assert _compare_version_strings("10a2", "10b2") < 0 - assert compare_version_strings("6.6p1", "7.5p1") < 0 - assert compare_version_strings("7.5p1", "6.6p1") > 0 + assert _compare_version_strings("6.6p1", "7.5p1") < 0 + assert _compare_version_strings("7.5p1", "6.6p1") > 0 - assert compare_version_strings("6.5p1", "6.5p1") == 0 - assert compare_version_strings("6.5p1", "6.5p2") < 0 - assert compare_version_strings("6.5p2", "6.5p1") > 0 - assert compare_version_strings("6.5p2", "6.6p1") < 0 - assert compare_version_strings("6.6p1", "6.5p2") > 0 + assert _compare_version_strings("6.5p1", "6.5p1") == 0 + assert _compare_version_strings("6.5p1", "6.5p2") < 0 + assert _compare_version_strings("6.5p2", "6.5p1") > 0 + assert _compare_version_strings("6.5p2", "6.6p1") < 0 + assert _compare_version_strings("6.6p1", "6.5p2") > 0 - assert compare_version_strings("6.5p10", "6.5p10") == 0 - assert compare_version_strings("6.5p1", "6.5p10") < 0 - assert compare_version_strings("6.5p10", "6.5p1") > 0 + assert _compare_version_strings("6.5p10", "6.5p10") == 0 + assert _compare_version_strings("6.5p1", "6.5p10") < 0 + assert _compare_version_strings("6.5p10", "6.5p1") > 0 - assert compare_version_strings("abc10", "abc10") == 0 - assert compare_version_strings("abc10", "abc10.1") < 0 - assert compare_version_strings("abc10.1", "abc10") > 0 + assert _compare_version_strings("abc10", "abc10") == 0 + assert _compare_version_strings("abc10", "abc10.1") < 0 + assert _compare_version_strings("abc10.1", "abc10") > 0 - assert compare_version_strings("abc.4", "abc.4") == 0 - assert compare_version_strings("abc.4", "8") < 0 - assert compare_version_strings("8", "abc.4") > 0 - assert compare_version_strings("abc.4", "2") < 0 - assert compare_version_strings("2", "abc.4") > 0 + assert _compare_version_strings("abc.4", "abc.4") == 0 + assert _compare_version_strings("abc.4", "8") < 0 + assert _compare_version_strings("8", "abc.4") > 0 + assert _compare_version_strings("abc.4", "2") < 0 + assert _compare_version_strings("2", "abc.4") > 0 - assert compare_version_strings("1.0aa", "1.0aa") == 0 - assert compare_version_strings("1.0a", "1.0aa") < 0 - assert compare_version_strings("1.0aa", "1.0a") > 0 + assert _compare_version_strings("1.0aa", "1.0aa") == 0 + assert _compare_version_strings("1.0a", "1.0aa") < 0 + assert _compare_version_strings("1.0aa", "1.0a") > 0 def test_version_comparison_numeric_handling(self): """Test handling of numeric-like values in version strings.""" - assert compare_version_strings("10.0001", "10.0001") == 0 + assert _compare_version_strings("10.0001", "10.0001") == 0 # sequences of leading zeroes are meant to be ignored - it's not *actually* treated # like a numeric value - assert compare_version_strings("10.0001", "10.1") == 0 - assert compare_version_strings("10.1", "10.0001") == 0 - assert compare_version_strings("10.0001", "10.0039") < 0 - assert compare_version_strings("10.0039", "10.0001") > 0 + assert _compare_version_strings("10.0001", "10.1") == 0 + assert _compare_version_strings("10.1", "10.0001") == 0 + assert _compare_version_strings("10.0001", "10.0039") < 0 + assert _compare_version_strings("10.0039", "10.0001") > 0 # but sequences of zeroes within a numeric segment are not ignored - assert compare_version_strings("10.1", "10.10001") < 0 - assert compare_version_strings("10.1111", "10.10001") < 0 - assert compare_version_strings("10.11111", "10.10001") > 0 + assert _compare_version_strings("10.1", "10.10001") < 0 + assert _compare_version_strings("10.1111", "10.10001") < 0 + assert _compare_version_strings("10.11111", "10.10001") > 0 - assert compare_version_strings("20240521", "20240521") == 0 - assert compare_version_strings("20240521", "20240522") < 0 - assert compare_version_strings("20240522", "20240521") > 0 - assert compare_version_strings("20240521", "202405210") < 0 + assert _compare_version_strings("20240521", "20240521") == 0 + assert _compare_version_strings("20240521", "20240522") < 0 + assert _compare_version_strings("20240522", "20240521") > 0 + assert _compare_version_strings("20240521", "202405210") < 0 def test_version_comparison_tilde_and_caret(self): """Test behavior of tilde and caret operators.""" - assert compare_version_strings("1.0~rc1", "1.0~rc1") == 0 - assert compare_version_strings("1.0~rc1", "1.0") < 0 - assert compare_version_strings("1.0", "1.0~rc1") > 0 - assert compare_version_strings("1.0~rc1", "1.0~rc2") < 0 - assert compare_version_strings("1.0~rc2", "1.0~rc1") > 0 - assert compare_version_strings("1.0~rc1~git123", "1.0~rc1~git123") == 0 - assert compare_version_strings("1.0~rc1~git123", "1.0~rc1") < 0 - assert compare_version_strings("1.0~rc1", "1.0~rc1~git123") > 0 - - assert compare_version_strings("1.0^", "1.0^") == 0 - assert compare_version_strings("1.0", "1.0^") < 0 - assert compare_version_strings("1.0^", "1.0") > 0 - - assert compare_version_strings("1.0", "1.0git1^") < 0 - assert compare_version_strings("1.0^git1", "1.0^git2") < 0 - assert compare_version_strings("1.01", "1.0^git1") > 0 - assert compare_version_strings("1.0^20240501", "1.0^20240501") == 0 - assert compare_version_strings("1.0^20240501", "1.0.1") < 0 - assert compare_version_strings("1.0^20240501^git1", "1.0^20240501^git1") == 0 - assert compare_version_strings("1.0^20240502", "1.0^20240501^git1") > 0 - assert compare_version_strings("1.0~rc1^git1", "1.0~rc1^git1") == 0 - assert compare_version_strings("1.0~rc1", "1.0~rc1^git1") < 0 - assert compare_version_strings("1.0~rc1^git1", "1.0~rc1") > 0 - assert compare_version_strings("1.0^git1~pre", "1.0^git1~pre") == 0 - assert compare_version_strings("1.0^git1~pre", "1.0^git1") < 0 - assert compare_version_strings("1.0^git1", "1.0^git1~pre") > 0 + assert _compare_version_strings("1.0~rc1", "1.0~rc1") == 0 + assert _compare_version_strings("1.0~rc1", "1.0") < 0 + assert _compare_version_strings("1.0", "1.0~rc1") > 0 + assert _compare_version_strings("1.0~rc1", "1.0~rc2") < 0 + assert _compare_version_strings("1.0~rc2", "1.0~rc1") > 0 + assert _compare_version_strings("1.0~rc1~git123", "1.0~rc1~git123") == 0 + assert _compare_version_strings("1.0~rc1~git123", "1.0~rc1") < 0 + assert _compare_version_strings("1.0~rc1", "1.0~rc1~git123") > 0 + + assert _compare_version_strings("1.0^", "1.0^") == 0 + assert _compare_version_strings("1.0", "1.0^") < 0 + assert _compare_version_strings("1.0^", "1.0") > 0 + + assert _compare_version_strings("1.0", "1.0git1^") < 0 + assert _compare_version_strings("1.0^git1", "1.0^git2") < 0 + assert _compare_version_strings("1.01", "1.0^git1") > 0 + assert _compare_version_strings("1.0^20240501", "1.0^20240501") == 0 + assert _compare_version_strings("1.0^20240501", "1.0.1") < 0 + assert _compare_version_strings("1.0^20240501^git1", "1.0^20240501^git1") == 0 + assert _compare_version_strings("1.0^20240502", "1.0^20240501^git1") > 0 + assert _compare_version_strings("1.0~rc1^git1", "1.0~rc1^git1") == 0 + assert _compare_version_strings("1.0~rc1", "1.0~rc1^git1") < 0 + assert _compare_version_strings("1.0~rc1^git1", "1.0~rc1") > 0 + assert _compare_version_strings("1.0^git1~pre", "1.0^git1~pre") == 0 + assert _compare_version_strings("1.0^git1~pre", "1.0^git1") < 0 + assert _compare_version_strings("1.0^git1", "1.0^git1~pre") > 0 def test_non_intuitive_comparison_behavior(self): """Test some version comparison behavior that is a bit non-intuitive.""" # (but needs to be maintained for compatibility) - assert compare_version_strings("1e.fc33", "1.fc33") < 0 - assert compare_version_strings("1g.fc33", "1.fc33") > 0 + assert _compare_version_strings("1e.fc33", "1.fc33") < 0 + assert _compare_version_strings("1g.fc33", "1.fc33") > 0 def test_non_alphanumeric_equivalence(self): """Test handling of non-alphanumeric ascii characters (excluding separators).""" # the existence of sequences of non-alphanumeric characters should not impact # the version comparison at all - assert compare_version_strings("b", "b") == 0 - assert compare_version_strings("b+", "b+") == 0 - assert compare_version_strings("b+", "b_") == 0 - assert compare_version_strings("b_", "b+") == 0 - assert compare_version_strings("+b", "+b") == 0 - assert compare_version_strings("+b", "_b") == 0 - assert compare_version_strings("_b", "+b") == 0 + assert _compare_version_strings("b", "b") == 0 + assert _compare_version_strings("b+", "b+") == 0 + assert _compare_version_strings("b+", "b_") == 0 + assert _compare_version_strings("b_", "b+") == 0 + assert _compare_version_strings("+b", "+b") == 0 + assert _compare_version_strings("+b", "_b") == 0 + assert _compare_version_strings("_b", "+b") == 0 - assert compare_version_strings("+b", "++b") == 0 - assert compare_version_strings("+b", "+b+") == 0 + assert _compare_version_strings("+b", "++b") == 0 + assert _compare_version_strings("+b", "+b+") == 0 - assert compare_version_strings("+.", "+_") == 0 - assert compare_version_strings("_+", "+.") == 0 - assert compare_version_strings("+", ".") == 0 - assert compare_version_strings(",", "+") == 0 + assert _compare_version_strings("+.", "+_") == 0 + assert _compare_version_strings("_+", "+.") == 0 + assert _compare_version_strings("+", ".") == 0 + assert _compare_version_strings(",", "+") == 0 - assert compare_version_strings("++", "_") == 0 - assert compare_version_strings("+", "..") == 0 + assert _compare_version_strings("++", "_") == 0 + assert _compare_version_strings("+", "..") == 0 - assert compare_version_strings("4_0", "4_0") == 0 - assert compare_version_strings("4_0", "4.0") == 0 - assert compare_version_strings("4.0", "4_0") == 0 + assert _compare_version_strings("4_0", "4_0") == 0 + assert _compare_version_strings("4_0", "4.0") == 0 + assert _compare_version_strings("4.0", "4_0") == 0 - assert compare_version_strings("4.999", "5.0") < 0 - assert compare_version_strings("4.999.9", "5.0") < 0 - assert compare_version_strings("5.0", "4.999_9") > 0 + assert _compare_version_strings("4.999", "5.0") < 0 + assert _compare_version_strings("4.999.9", "5.0") < 0 + assert _compare_version_strings("5.0", "4.999_9") > 0 # except when it comes to breaking up sequences of alphanumeric characters # that do impact the comparison - assert compare_version_strings("4.999", "4.999.9") < 0 - assert compare_version_strings("4.999", "4.99.9") > 0 + assert _compare_version_strings("4.999", "4.999.9") < 0 + assert _compare_version_strings("4.999", "4.99.9") > 0 def test_non_ascii_character_equivalence(self): """Test handling of non-ascii characters.""" # the existence of sequences of non-ascii characters should not impact the # version comparison at all - assert compare_version_strings("1.1.Á.1", "1.1.1") == 0 - assert compare_version_strings("1.1.Á", "1.1.Á") == 0 - assert compare_version_strings("1.1.Á", "1.1.Ê") == 0 - assert compare_version_strings("1.1.ÁÁ", "1.1.Á") == 0 - assert compare_version_strings("1.1.Á", "1.1.ÊÊ") == 0 + assert _compare_version_strings("1.1.Á.1", "1.1.1") == 0 + assert _compare_version_strings("1.1.Á", "1.1.Á") == 0 + assert _compare_version_strings("1.1.Á", "1.1.Ê") == 0 + assert _compare_version_strings("1.1.ÁÁ", "1.1.Á") == 0 + assert _compare_version_strings("1.1.Á", "1.1.ÊÊ") == 0 # except when it comes to breaking up sequences of ascii characters that do # impact the comparison - assert compare_version_strings("1.1Á1", "1.11") < 0 + assert _compare_version_strings("1.1Á1", "1.11") < 0 From e336ee5fa64359e07e80e67ce353d822dbb861a8 Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Tue, 23 Sep 2025 01:21:03 -0400 Subject: [PATCH 3/3] Add unit tests for Package.objects.with_age() --- pulp_rpm/app/rpm_version.py | 5 +- pulp_rpm/tests/unit/test_package_age.py | 602 ++++++++++++++++++++++++ pulp_rpm/tests/unit/test_rpm_version.py | 3 - 3 files changed, 603 insertions(+), 7 deletions(-) create mode 100644 pulp_rpm/tests/unit/test_package_age.py diff --git a/pulp_rpm/app/rpm_version.py b/pulp_rpm/app/rpm_version.py index 9e9066c909..88885d239b 100644 --- a/pulp_rpm/app/rpm_version.py +++ b/pulp_rpm/app/rpm_version.py @@ -203,19 +203,16 @@ def matching_contiguous(data, predicate): version2_stripped, version2_had_caret = strip_prefix(version2_part, b"^") if version1_had_caret and not version2_had_caret: - # first has caret but second doesn't if not version2_part: # second has ended return 1 # first > second else: # second continues return -1 # first < second elif not version1_had_caret and version2_had_caret: - # second has caret but first doesn't if not version1_part: # first has ended return -1 # first < second else: # first continues return 1 # first > second elif version1_had_caret and version2_had_caret: - # both have caret, strip and continue version1_part = version1_stripped version2_part = version2_stripped continue @@ -287,4 +284,4 @@ def is_alpha(c): # Equal, continue to next segment # Should not reach here due to the checks above, but just in case - return 0 + raise RuntimeError("somehow escaped the loop during version comparison") diff --git a/pulp_rpm/tests/unit/test_package_age.py b/pulp_rpm/tests/unit/test_package_age.py new file mode 100644 index 0000000000..e36e1b9573 --- /dev/null +++ b/pulp_rpm/tests/unit/test_package_age.py @@ -0,0 +1,602 @@ +""" +Unit tests for Package.objects.with_age() functionality. +""" + +from django.test import TestCase +from unittest import skip + +from pulp_rpm.app.models import Package + + +class TestPackageAge(TestCase): + """Test Package age calculation functionality.""" + + def setUp(self): + """Set up test packages with various version combinations.""" + # Package group 1: same name and arch, different versions + self.pkg1_v1 = Package.objects.create( + name="testpkg", + epoch="0", + version="1.0.0", + release="1.el8", + arch="x86_64", + pkgId="checksum1", + checksum_type="sha256", + ) + + self.pkg1_v2 = Package.objects.create( + name="testpkg", + epoch="0", + version="2.0.0", + release="1.el8", + arch="x86_64", + pkgId="checksum2", + checksum_type="sha256", + ) + + self.pkg1_v3 = Package.objects.create( + name="testpkg", + epoch="0", + version="1.5.0", + release="2.el8", + arch="x86_64", + pkgId="checksum3", + checksum_type="sha256", + ) + + # Package group 2: same name but different arch + self.pkg2_i686 = Package.objects.create( + name="testpkg", + epoch="0", + version="1.0.0", + release="1.el8", + arch="i686", + pkgId="checksum4", + checksum_type="sha256", + ) + + self.pkg2_i686_v2 = Package.objects.create( + name="testpkg", + epoch="0", + version="3.0.0", + release="1.el8", + arch="i686", + pkgId="checksum5", + checksum_type="sha256", + ) + + # Package group 3: different name + self.pkg3_other = Package.objects.create( + name="otherpkg", + epoch="0", + version="1.0.0", + release="1.el8", + arch="x86_64", + pkgId="checksum6", + checksum_type="sha256", + ) + + # Package group 4: epoch differences + self.pkg4_epoch0 = Package.objects.create( + name="epochpkg", + epoch="0", + version="2.0.0", + release="1.el8", + arch="x86_64", + pkgId="checksum7", + checksum_type="sha256", + ) + + self.pkg4_epoch1 = Package.objects.create( + name="epochpkg", + epoch="1", + version="1.0.0", + release="1.el8", + arch="x86_64", + pkgId="checksum8", + checksum_type="sha256", + ) + + def test_age_calculation_basic_versions(self): + """Test that age is calculated correctly for basic version differences.""" + packages = Package.objects.with_age().filter(name="testpkg", arch="x86_64").order_by("age") + + # Expected order: 2.0.0 (age=1), 1.5.0-2 (age=2), 1.0.0 (age=3) + self.assertEqual(packages.count(), 3) + + # Newest version should have age=1 + newest = packages[0] + self.assertEqual(newest.version, "2.0.0") + self.assertEqual(newest.age, 1) + + # Middle version should have age=2 + middle = packages[1] + self.assertEqual(middle.version, "1.5.0") + self.assertEqual(middle.release, "2.el8") + self.assertEqual(middle.age, 2) + + # Oldest version should have age=3 + oldest = packages[2] + self.assertEqual(oldest.version, "1.0.0") + self.assertEqual(oldest.age, 3) + + def test_age_calculation_different_architectures(self): + """Test that packages with different architectures are aged separately.""" + # x86_64 packages + x86_packages = ( + Package.objects.with_age().filter(name="testpkg", arch="x86_64").order_by("age") + ) + self.assertEqual(x86_packages.count(), 3) + self.assertEqual(x86_packages[0].age, 1) # 2.0.0 + self.assertEqual(x86_packages[1].age, 2) # 1.5.0-2 + self.assertEqual(x86_packages[2].age, 3) # 1.0.0 + + # i686 packages + i686_packages = ( + Package.objects.with_age().filter(name="testpkg", arch="i686").order_by("age") + ) + self.assertEqual(i686_packages.count(), 2) + + # Even though i686 3.0.0 is newer than any x86_64 version, it should still have age=1 + # within its own architecture group + self.assertEqual(i686_packages[0].version, "3.0.0") + self.assertEqual(i686_packages[0].age, 1) + self.assertEqual(i686_packages[1].version, "1.0.0") + self.assertEqual(i686_packages[1].age, 2) + + def test_age_calculation_different_names(self): + """Test that packages with different names are aged separately.""" + # Different package name should have its own age calculation + other_packages = Package.objects.with_age().filter(name="otherpkg") + self.assertEqual(other_packages.count(), 1) + self.assertEqual(other_packages[0].age, 1) + + def test_age_calculation_with_epochs(self): + """Test that epoch is considered in age calculation.""" + epoch_packages = Package.objects.with_age().filter(name="epochpkg").order_by("age") + self.assertEqual(epoch_packages.count(), 2) + + # Epoch 1 should be newer than epoch 0, regardless of version numbers + newest = epoch_packages[0] + self.assertEqual(newest.epoch, "1") + self.assertEqual(newest.version, "1.0.0") + self.assertEqual(newest.age, 1) + + oldest = epoch_packages[1] + self.assertEqual(oldest.epoch, "0") + self.assertEqual(oldest.version, "2.0.0") + self.assertEqual(oldest.age, 2) + + def test_age_all_packages(self): + """Test age calculation when querying all packages.""" + all_packages = Package.objects.with_age() + + # Verify each package has an age + for pkg in all_packages: + self.assertIsNotNone(pkg.age) + self.assertGreater(pkg.age, 0) + + # Check that packages are grouped correctly by name+arch + testpkg_x86_ages = [ + p.age for p in all_packages if p.name == "testpkg" and p.arch == "x86_64" + ] + testpkg_i686_ages = [ + p.age for p in all_packages if p.name == "testpkg" and p.arch == "i686" + ] + otherpkg_ages = [p.age for p in all_packages if p.name == "otherpkg"] + epochpkg_ages = [p.age for p in all_packages if p.name == "epochpkg"] + + self.assertEqual(sorted(testpkg_x86_ages), [1, 2, 3]) + self.assertEqual(sorted(testpkg_i686_ages), [1, 2]) + self.assertEqual(sorted(otherpkg_ages), [1]) + self.assertEqual(sorted(epochpkg_ages), [1, 2]) + + def test_age_with_release_differences(self): + """Test age calculation when versions are same but releases differ.""" + # Create packages with same version but different releases + rel_pkg1 = Package.objects.create( # noqa: F841 + name="relpkg", + epoch="0", + version="1.0.0", + release="1.el8", + arch="x86_64", + pkgId="rel1", + checksum_type="sha256", + ) + + rel_pkg2 = Package.objects.create( # noqa: F841 + name="relpkg", + epoch="0", + version="1.0.0", + release="2.el8", + arch="x86_64", + pkgId="rel2", + checksum_type="sha256", + ) + + rel_pkg3 = Package.objects.create( # noqa: F841 + name="relpkg", + epoch="0", + version="1.0.0", + release="10.el8", + arch="x86_64", + pkgId="rel3", + checksum_type="sha256", + ) + + packages = Package.objects.with_age().filter(name="relpkg").order_by("age") + + # Expected order: 10.el8 > 2.el8 > 1.el8 (numeric comparison of release) + self.assertEqual(packages[0].release, "10.el8") # age=1 + self.assertEqual(packages[0].age, 1) + + self.assertEqual(packages[1].release, "2.el8") # age=2 + self.assertEqual(packages[1].age, 2) + + self.assertEqual(packages[2].release, "1.el8") # age=3 + self.assertEqual(packages[2].age, 3) + + @skip("The implementation of package age is broken w/r/t '^' and '~' characters") + def test_age_with_tilde_and_caret_versions(self): + """Test age calculation with tilde and caret version characters.""" + # Create packages with tilde and caret versions + Package.objects.create( + name="tildepkg", + epoch="0", + version="1.0~rc1", + release="1.el8", + arch="x86_64", + pkgId="tilde1", + checksum_type="sha256", + ) + + Package.objects.create( + name="tildepkg", + epoch="0", + version="1.0", + release="1.el8", + arch="x86_64", + pkgId="tilde2", + checksum_type="sha256", + ) + + Package.objects.create( + name="tildepkg", + epoch="0", + version="1.0~rc2", + release="1.el8", + arch="x86_64", + pkgId="tilde3", + checksum_type="sha256", + ) + + Package.objects.create( + name="tildepkg", + epoch="0", + version="1.0^git123", + release="1.el8", + arch="x86_64", + pkgId="tilde4", + checksum_type="sha256", + ) + + packages = Package.objects.with_age().filter(name="tildepkg").order_by("age") + + # Expected order: 1.0^git123 (age=1), 1.0 (age=2), 1.0~rc2 (age=3), 1.0~rc1 (age=4) + # Caret sorts higher than regular, regular sorts higher than tilde + self.assertEqual(packages[0].version, "1.0^git123") + self.assertEqual(packages[0].age, 1) + + self.assertEqual(packages[1].version, "1.0") + self.assertEqual(packages[1].age, 2) + + self.assertEqual(packages[2].version, "1.0~rc2") + self.assertEqual(packages[2].age, 3) + + self.assertEqual(packages[3].version, "1.0~rc1") + self.assertEqual(packages[3].age, 4) + + def test_age_with_numeric_handling_versions(self): + """Test age calculation with numeric version handling edge cases.""" + # Create packages with leading zeros and numeric comparisons + Package.objects.create( + name="numpkg", + epoch="0", + version="10.0001", + release="1.el8", + arch="x86_64", + pkgId="num1", + checksum_type="sha256", + ) + + Package.objects.create( + name="numpkg", + epoch="0", + version="10.1", + release="1.el8", + arch="x86_64", + pkgId="num2", + checksum_type="sha256", + ) + + Package.objects.create( + name="numpkg", + epoch="0", + version="10.10001", + release="1.el8", + arch="x86_64", + pkgId="num3", + checksum_type="sha256", + ) + + Package.objects.create( + name="numpkg", + epoch="0", + version="20240521", + release="1.el8", + arch="x86_64", + pkgId="num4", + checksum_type="sha256", + ) + + Package.objects.create( + name="numpkg", + epoch="0", + version="202405210", + release="1.el8", + arch="x86_64", + pkgId="num5", + checksum_type="sha256", + ) + + packages = Package.objects.with_age().filter(name="numpkg").order_by("age") + + # Expected order: 202405210 > 20240521 > 10.10001 > 10.1 == 10.0001 + # Leading zeros are ignored in numeric segments + self.assertEqual(packages[0].version, "202405210") + self.assertEqual(packages[0].age, 1) + + self.assertEqual(packages[1].version, "20240521") + self.assertEqual(packages[1].age, 2) + + self.assertEqual(packages[2].version, "10.10001") + self.assertEqual(packages[2].age, 3) + + # 10.1 and 10.0001 should be considered equal (leading zeros ignored) + # but one will have age=4 and the other age=5 based on creation order + remaining_versions = [packages[3].version, packages[4].version] + self.assertIn("10.1", remaining_versions) + self.assertIn("10.0001", remaining_versions) + + def test_age_with_non_intuitive_comparison_versions(self): + """Test age calculation with non-intuitive version comparison behavior.""" + # Create packages that test the 'e' vs numeric behavior + Package.objects.create( + name="intuitivepkg", + epoch="0", + version="1e.fc33", + release="1.el8", + arch="x86_64", + pkgId="intuit1", + checksum_type="sha256", + ) + + Package.objects.create( + name="intuitivepkg", + epoch="0", + version="1.fc33", + release="1.el8", + arch="x86_64", + pkgId="intuit2", + checksum_type="sha256", + ) + + Package.objects.create( + name="intuitivepkg", + epoch="0", + version="1g.fc33", + release="1.el8", + arch="x86_64", + pkgId="intuit3", + checksum_type="sha256", + ) + + packages = Package.objects.with_age().filter(name="intuitivepkg").order_by("age") + + # Expected order: 1g.fc33 > 1.fc33 > 1e.fc33 + # 'e' comes before numeric in comparison, 'g' comes after numeric + self.assertEqual(packages[0].version, "1g.fc33") + self.assertEqual(packages[0].age, 1) + + self.assertEqual(packages[1].version, "1.fc33") + self.assertEqual(packages[1].age, 2) + + self.assertEqual(packages[2].version, "1e.fc33") + self.assertEqual(packages[2].age, 3) + + def test_age_with_non_alphanumeric_equivalence_versions(self): + """Test age calculation with non-alphanumeric character equivalence.""" + # Create packages with various non-alphanumeric separators + Package.objects.create( + name="alphapkg", + epoch="0", + version="4.0", + release="1.el8", + arch="x86_64", + pkgId="alpha1", + checksum_type="sha256", + ) + + Package.objects.create( + name="alphapkg", + epoch="0", + version="4_0", + release="1.el8", + arch="x86_64", + pkgId="alpha2", + checksum_type="sha256", + ) + + Package.objects.create( + name="alphapkg", + epoch="0", + version="4+0", + release="1.el8", + arch="x86_64", + pkgId="alpha3", + checksum_type="sha256", + ) + + Package.objects.create( + name="alphapkg", + epoch="0", + version="4.999", + release="1.el8", + arch="x86_64", + pkgId="alpha4", + checksum_type="sha256", + ) + + Package.objects.create( + name="alphapkg", + epoch="0", + version="4.999.9", + release="1.el8", + arch="x86_64", + pkgId="alpha5", + checksum_type="sha256", + ) + + packages = Package.objects.with_age().filter(name="alphapkg").order_by("age") + + # Expected behavior: 4.999.9 > 4.999 > 4.0 == 4_0 == 4+0 + # Non-alphanumeric characters are treated as equivalent separators + self.assertEqual(packages[0].version, "4.999.9") + self.assertEqual(packages[0].age, 1) + + self.assertEqual(packages[1].version, "4.999") + self.assertEqual(packages[1].age, 2) + + # The remaining three should be considered equivalent versions + # but will have different ages based on creation order + remaining_versions = [packages[2].version, packages[3].version, packages[4].version] + self.assertIn("4.0", remaining_versions) + self.assertIn("4_0", remaining_versions) + self.assertIn("4+0", remaining_versions) + + @skip("Non-ASCII character handling do not work correctly with current implementation") + def test_age_with_non_ascii_character_versions(self): + """Test age calculation with non-ASCII character handling.""" + # Create packages with non-ASCII characters + Package.objects.create( + name="asciipkg", + epoch="0", + version="1.1.1", + release="1.el8", + arch="x86_64", + pkgId="ascii1", + checksum_type="sha256", + ) + + Package.objects.create( + name="asciipkg", + epoch="0", + version="1.1.Á.1", + release="1.el8", + arch="x86_64", + pkgId="ascii2", + checksum_type="sha256", + ) + + Package.objects.create( + name="asciipkg", + epoch="0", + version="1.11", + release="1.el8", + arch="x86_64", + pkgId="ascii3", + checksum_type="sha256", + ) + + Package.objects.create( + name="asciipkg", + epoch="0", + version="1.1Á1", + release="1.el8", + arch="x86_64", + pkgId="ascii4", + checksum_type="sha256", + ) + + packages = Package.objects.with_age().filter(name="asciipkg").order_by("age") + + # Expected behavior: 1.11 > 1.1.1 == 1.1.Á.1 > 1.1Á1 + # Non-ASCII chars are ignored unless they break up alphanumeric sequences + self.assertEqual(packages[0].version, "1.11") + self.assertEqual(packages[0].age, 1) + + # 1.1.1 and 1.1.Á.1 should be equivalent + equivalent_versions = [packages[1].version, packages[2].version] + self.assertIn("1.1.1", equivalent_versions) + self.assertIn("1.1.Á.1", equivalent_versions) + + # 1.1Á1 should be last (non-ASCII breaks up the sequence) + self.assertEqual(packages[3].version, "1.1Á1") + self.assertEqual(packages[3].age, 4) + + def test_age_with_mixed_complex_scenarios(self): + """Test age calculation with mixed complex version scenarios.""" + # Create a comprehensive mix of different version types + Package.objects.create( + name="mixedpkg", + epoch="0", + version="2.0", + release="1.el8", + arch="x86_64", + pkgId="mixed1", + checksum_type="sha256", + ) + + Package.objects.create( + name="mixedpkg", + epoch="0", + version="2.0.rc1", + release="1.el8", + arch="x86_64", + pkgId="mixed2", + checksum_type="sha256", + ) + + Package.objects.create( + name="mixedpkg", + epoch="0", + version="2.0.0", + release="1.el8", + arch="x86_64", + pkgId="mixed3", + checksum_type="sha256", + ) + + Package.objects.create( + name="mixedpkg", + epoch="1", + version="1.0", + release="1.el8", + arch="x86_64", + pkgId="mixed4", + checksum_type="sha256", + ) + + packages = Package.objects.with_age().filter(name="mixedpkg").order_by("age") + + # Expected order: 1:1.0 (epoch wins) > 2.0.0 > 2.0 > 2.0.rc1 + self.assertEqual(packages[0].epoch, "1") + self.assertEqual(packages[0].version, "1.0") + self.assertEqual(packages[0].age, 1) + + # Rest should be ordered by version comparison + remaining = packages[1:] + versions = [p.version for p in remaining] + self.assertIn("2.0.0", versions) + self.assertIn("2.0", versions) + self.assertIn("2.0.rc1", versions) diff --git a/pulp_rpm/tests/unit/test_rpm_version.py b/pulp_rpm/tests/unit/test_rpm_version.py index 476ee8a4dc..ae23bc4b12 100644 --- a/pulp_rpm/tests/unit/test_rpm_version.py +++ b/pulp_rpm/tests/unit/test_rpm_version.py @@ -1,8 +1,5 @@ """ Unit tests for RPM version comparison functionality. - -These tests are designed to match the behavior tested in the Rust implementation -to ensure compatibility between different RPM version comparison implementations. """ from pulp_rpm.app.rpm_version import (