From 5b9418c06ca6d51507468124250bb58046886be6 Mon Sep 17 00:00:00 2001
From: Felix Fontein <felix@fontein.de>
Date: Mon, 11 May 2020 18:58:15 +0200
Subject: [PATCH] Fix SemanticVersion comparison (#69395)

* Fix SemanticVersion comparison.

* Complete tests for _Alpha and _Numeric comparators.

* Linting, and add comment.
---
 lib/ansible/utils/version.py     | 16 +++++-----
 test/units/utils/test_version.py | 50 ++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/lib/ansible/utils/version.py b/lib/ansible/utils/version.py
index 0dc6687ed93..d69723b4731 100644
--- a/lib/ansible/utils/version.py
+++ b/lib/ansible/utils/version.py
@@ -72,14 +72,14 @@ class _Alpha:
 
         raise ValueError
 
-    def __gt__(self, other):
-        return not self.__lt__(other)
-
     def __le__(self, other):
         return self.__lt__(other) or self.__eq__(other)
 
+    def __gt__(self, other):
+        return not self.__le__(other)
+
     def __ge__(self, other):
-        return self.__gt__(other) or self.__eq__(other)
+        return not self.__lt__(other)
 
 
 class _Numeric:
@@ -115,14 +115,14 @@ class _Numeric:
 
         raise ValueError
 
-    def __gt__(self, other):
-        return not self.__lt__(other)
-
     def __le__(self, other):
         return self.__lt__(other) or self.__eq__(other)
 
+    def __gt__(self, other):
+        return not self.__le__(other)
+
     def __ge__(self, other):
-        return self.__gt__(other) or self.__eq__(other)
+        return not self.__lt__(other)
 
 
 class SemanticVersion(Version):
diff --git a/test/units/utils/test_version.py b/test/units/utils/test_version.py
index a46282876f2..7d04c112c5a 100644
--- a/test/units/utils/test_version.py
+++ b/test/units/utils/test_version.py
@@ -268,6 +268,31 @@ def test_alpha():
     assert _Alpha('b') >= _Alpha('a')
     assert _Alpha('b') >= _Alpha('b')
 
+    # The following 3*6 tests check that all comparison operators perform
+    # as expected. DO NOT remove any of them, or reformulate them (to remove
+    # the explicit `not`)!
+
+    assert _Alpha('a') == _Alpha('a')
+    assert not _Alpha('a') != _Alpha('a')  # pylint: disable=unneeded-not
+    assert not _Alpha('a') < _Alpha('a')  # pylint: disable=unneeded-not
+    assert _Alpha('a') <= _Alpha('a')
+    assert not _Alpha('a') > _Alpha('a')  # pylint: disable=unneeded-not
+    assert _Alpha('a') >= _Alpha('a')
+
+    assert not _Alpha('a') == _Alpha('b')  # pylint: disable=unneeded-not
+    assert _Alpha('a') != _Alpha('b')
+    assert _Alpha('a') < _Alpha('b')
+    assert _Alpha('a') <= _Alpha('b')
+    assert not _Alpha('a') > _Alpha('b')  # pylint: disable=unneeded-not
+    assert not _Alpha('a') >= _Alpha('b')  # pylint: disable=unneeded-not
+
+    assert not _Alpha('b') == _Alpha('a')  # pylint: disable=unneeded-not
+    assert _Alpha('b') != _Alpha('a')
+    assert not _Alpha('b') < _Alpha('a')  # pylint: disable=unneeded-not
+    assert not _Alpha('b') <= _Alpha('a')  # pylint: disable=unneeded-not
+    assert _Alpha('b') > _Alpha('a')
+    assert _Alpha('b') >= _Alpha('a')
+
 
 def test_numeric():
     assert _Numeric(1) == _Numeric(1)
@@ -283,3 +308,28 @@ def test_numeric():
     assert _Numeric(1) <= _Numeric(2)
     assert _Numeric(2) >= _Numeric(1)
     assert _Numeric(2) >= _Numeric(2)
+
+    # The following 3*6 tests check that all comparison operators perform
+    # as expected. DO NOT remove any of them, or reformulate them (to remove
+    # the explicit `not`)!
+
+    assert _Numeric(1) == _Numeric(1)
+    assert not _Numeric(1) != _Numeric(1)  # pylint: disable=unneeded-not
+    assert not _Numeric(1) < _Numeric(1)  # pylint: disable=unneeded-not
+    assert _Numeric(1) <= _Numeric(1)
+    assert not _Numeric(1) > _Numeric(1)  # pylint: disable=unneeded-not
+    assert _Numeric(1) >= _Numeric(1)
+
+    assert not _Numeric(1) == _Numeric(2)  # pylint: disable=unneeded-not
+    assert _Numeric(1) != _Numeric(2)
+    assert _Numeric(1) < _Numeric(2)
+    assert _Numeric(1) <= _Numeric(2)
+    assert not _Numeric(1) > _Numeric(2)  # pylint: disable=unneeded-not
+    assert not _Numeric(1) >= _Numeric(2)  # pylint: disable=unneeded-not
+
+    assert not _Numeric(2) == _Numeric(1)  # pylint: disable=unneeded-not
+    assert _Numeric(2) != _Numeric(1)
+    assert not _Numeric(2) < _Numeric(1)  # pylint: disable=unneeded-not
+    assert not _Numeric(2) <= _Numeric(1)  # pylint: disable=unneeded-not
+    assert _Numeric(2) > _Numeric(1)
+    assert _Numeric(2) >= _Numeric(1)