From 0fb8dcdf0ab597383d9fe96444828d6ba8ad9d3f Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Nov 22 2020 21:01:17 +0000
Subject: [PATCH 1/7] Fix docstring in version.py


---

diff --git a/opam2rpm/version.py b/opam2rpm/version.py
index b361c03..53ea44e 100644
--- a/opam2rpm/version.py
+++ b/opam2rpm/version.py
@@ -30,18 +30,18 @@ https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version.
 
 To use this class, create a Version object from the upstream version number:
 
-    >>> v = Version('v1.2.3')
+>>> v = Version('v1.2.3')
 
 Version objects can be compared as though they were numbers:
 
-    >>> w = Version('v1.2.3b')
-    >>> x = Version('v1.2.3~2')
-    >>> v < w
-    True
-    >>> v < x
-    False
-"""
+>>> w = Version('v1.2.3b')
+>>> x = Version('v1.2.3~2')
+>>> v < w
+True
+>>> v < x
+False
 
+"""
 from itertools import zip_longest
 import re
 

From ae0388a9fe4eb3ce15289d39dc543980a7abb4df Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Nov 22 2020 21:01:44 +0000
Subject: [PATCH 2/7] Fix BeautifulSoup package name


---

diff --git a/requirements.txt b/requirements.txt
index e0db9cf..3a8a991 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,4 @@
-BeautifulSoup
+BeautifulSoup4
 jinja2
 pyparsing
 requests

From a79ded87a05bcf8467d443352aabb2d8367cfda3 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Nov 22 2020 21:17:36 +0000
Subject: [PATCH 3/7] Add type hints to version module


---

diff --git a/opam2rpm/version.py b/opam2rpm/version.py
index 53ea44e..5b0f297 100644
--- a/opam2rpm/version.py
+++ b/opam2rpm/version.py
@@ -42,7 +42,10 @@ True
 False
 
 """
+from __future__ import annotations
+
 from itertools import zip_longest
+from typing import List, Tuple, Literal, Optional
 import re
 
 def char_class(char):
@@ -67,9 +70,9 @@ class Version:
 
     __slots__ = ['elems']
 
-    def __init__(self, verstring):
+    def __init__(self, verstring: str) -> None:
         """Initialize a version object from a version string."""
-        self.elems = []
+        self.elems: List[str] = []
         while verstring:
             digits = re.search(r'\d+', verstring)
             if digits:
@@ -81,9 +84,9 @@ class Version:
                 verstring = verstring[end:]
             else:
                 self.elems.append(verstring)
-                verstring = None
+                break
 
-    def compare(self, other):
+    def compare(self, other: Version) -> Literal[1, -1, 0]:
         """Compare two versions.
 
         Returns a negative value, zero, or a positive value as the first
@@ -91,8 +94,8 @@ class Version:
         respectively.
         """
         digit = False
-        comparison = 0
         for el1, el2 in zip_longest(self.elems, other.elems, fillvalue=''):
+        comparison: Literal[1, -1, 0] = 0
             if digit:
                 if int(el1) < int(el2):
                     comparison = -1
@@ -141,7 +144,7 @@ class Version:
         return ''.join(self.elems)
 
     @staticmethod
-    def _to_fedora(elems):
+    def _to_fedora(elems: List[str]) -> Tuple[str, str]:
         """Convert elems to Fedora version and release numbers."""
         ver = str(elems.pop(0))
         while elems and \
@@ -156,7 +159,7 @@ class Version:
                 ver += '.0'
         return ver, '0.1.'.join(elems) if elems else '1'
 
-    def to_fedora(self):
+    def to_fedora(self) -> Tuple[str, str]:
         """Convert this version number to Fedora version and release numbers."""
         if not self.elems:
             return '0', '0.1'

From 61b4154d45048302139b92d254d1871c92807849 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Nov 22 2020 21:17:36 +0000
Subject: [PATCH 4/7] switch char_class to use an enumeration


---

diff --git a/opam2rpm/version.py b/opam2rpm/version.py
index 5b0f297..89020ca 100644
--- a/opam2rpm/version.py
+++ b/opam2rpm/version.py
@@ -44,11 +44,21 @@ False
 """
 from __future__ import annotations
 
+from enum import IntEnum
 from itertools import zip_longest
 from typing import List, Tuple, Literal, Optional
 import re
 
-def char_class(char):
+
+class CharClass(IntEnum):
+    """Character classes inside a version string"""
+    TILDE = 0
+    EMPTY = 1
+    LETTER = 2
+    REST = 3
+
+
+def char_class(char: Optional[str]) -> CharClass:
     """Determine which class this character belongs to:
     0: tilde
     1: empty
@@ -56,12 +66,13 @@ def char_class(char):
     3: anything else
     """
     if char == '~':
-        return 0
-    if char == '':
-        return 1
+        return CharClass.TILDE
+    if char == '' or char is None:
+        return CharClass.EMPTY
     if char.isalpha():
-        return 2
-    return 3
+        return CharClass.LETTER
+    return CharClass.REST
+
 
 class Version:
     """Representation of a package version.

From 101034e998dc643d8b147d90c13a7fab5451aeb1 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Nov 22 2020 21:17:36 +0000
Subject: [PATCH 5/7] Add pytest & coverage config files


---

diff --git a/.coveragerc b/.coveragerc
new file mode 100644
index 0000000..3219c69
--- /dev/null
+++ b/.coveragerc
@@ -0,0 +1,4 @@
+[run]
+branch = True
+[tool:pytest]
+addopts = --cov=opam2rpm --cov-report html
diff --git a/.gitignore b/.gitignore
index 0b73876..07caaf1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,5 @@
 /build
 /opam2rpm/__pycache__
+/htmlcov
+.coverage
+opam2rpm.egg-info
diff --git a/dev-requirements.txt b/dev-requirements.txt
new file mode 100644
index 0000000..3918028
--- /dev/null
+++ b/dev-requirements.txt
@@ -0,0 +1,5 @@
+-r requirements.txt
+
+pytest
+pytest-cov
+mypy
diff --git a/pytest.ini b/pytest.ini
new file mode 100644
index 0000000..b95b354
--- /dev/null
+++ b/pytest.ini
@@ -0,0 +1,2 @@
+[pytest]
+addopts = --doctest-modules --cov=opam2rpm --cov-report html --cov-report term

From 2b0364d2422953fd23bb448c4347dacf660b0c1d Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Nov 22 2020 21:17:36 +0000
Subject: [PATCH 6/7] Add unit tests for the version module


---

diff --git a/tests/test_version.py b/tests/test_version.py
new file mode 100644
index 0000000..1c49990
--- /dev/null
+++ b/tests/test_version.py
@@ -0,0 +1,56 @@
+from opam2rpm.version import Version
+
+v_middle = Version("v1.2.3")
+v_highest = Version("v1.2.3b")
+v_lowest = Version('v1.2.3~2')
+
+
+def test_version_greater():
+    assert(Version("v2.4b") > Version("v0.2.4~"))
+    assert(Version("v2.4b") > Version("v0.2.4"))
+    assert(Version("v1") > Version("v0.4"))
+
+    assert(Version("v2.4b") > Version("v2.4a"))
+
+    assert(v_highest > v_middle > v_lowest)
+
+
+def test_version_equal():
+    assert(Version("v0.1c") == Version("v0.1c"))
+    assert(v_highest == v_highest)
+
+
+def test_version_smaller():
+    assert(Version("v0.2.4~") < Version("v2.4b"))
+    assert(Version("v0.2.4") < Version("v2.4b"))
+    assert(Version("v0.4") < Version("v1"))
+
+    assert(Version("v2.4a") < Version("v2.4b"))
+
+    assert(v_lowest < v_middle < v_highest)
+
+
+def test_version_not_equal():
+    assert(v_middle != v_highest)
+    assert(v_middle != v_lowest)
+    assert(v_lowest != v_highest)
+
+
+def test_str():
+    for s in ["v", "v1.2", "v0.16~", "3.4.8a"]:
+        assert(str(Version(s)) == s)
+
+
+class TestToFedora:
+    def test_from_empty(self):
+        assert(Version("").to_fedora() == ("0", "0.1"))
+
+    def _test_from_single_digit(self):
+        # FIXME: this does not look right:
+        assert(Version("b1.2").to_fedora() == ('0', 'b0.1.10.1..0.1.2'))
+
+    def test_general_case(self):
+        assert(Version("v0.1.2~").to_fedora() == ("0.1.2.0", "1"))
+        assert(Version("v0.1.2").to_fedora() == ("0.1.2", "1"))
+
+        assert(Version("v0.1.2_3").to_fedora() == ("0.1.2.3", "1"))

From acd8cc7e60c1bd23eb6a9ab05385f539d0598a81 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Nov 22 2020 21:17:37 +0000
Subject: [PATCH 7/7] Fix comparison failure when a character is compared to None


The current implementation of the version comparison has a bug and would claim
that v1.2.3~2 > v1.2.3 as the comparison loop contained only a break out of the
inner loop and not out of the outer loop, thus resulting in the next characters
being compared, although a comparison was already found

---

diff --git a/opam2rpm/version.py b/opam2rpm/version.py
index 89020ca..a48a9a3 100644
--- a/opam2rpm/version.py
+++ b/opam2rpm/version.py
@@ -58,6 +58,16 @@ class CharClass(IntEnum):
     REST = 3
 
 
+def str_to_int(number_or_not: Optional[str],
+               fallback: int=0) -> int:
+    if number_or_not is None:
+        return fallback
+    try:
+        return int(number_or_not)
+    except ValueError:
+        return fallback
+
+
 def char_class(char: Optional[str]) -> CharClass:
     """Determine which class this character belongs to:
     0: tilde
@@ -105,19 +115,19 @@ class Version:
         respectively.
         """
         digit = False
-        for el1, el2 in zip_longest(self.elems, other.elems, fillvalue=''):
         comparison: Literal[1, -1, 0] = 0
+        for el1, el2 in zip_longest(self.elems, other.elems):
             if digit:
-                if int(el1) < int(el2):
+                int_el1, int_el2 = str_to_int(el1), str_to_int(el2)
+                if int_el1 < int_el2:
                     comparison = -1
                     break
-                if int(el1) > int(el2):
+                if int_el1 > int_el2:
                     comparison = 1
                     break
             else:
-                for ch1, ch2 in zip_longest(el1, el2, fillvalue=''):
-                    cl1 = char_class(ch1)
-                    cl2 = char_class(ch2)
+                for ch1, ch2 in zip_longest(el1 or '', el2 or ''):
+                    cl1, cl2 = char_class(ch1), char_class(ch2)
                     if cl1 < cl2:
                         comparison = -1
                         break
@@ -130,6 +140,10 @@ class Version:
                     if ch1 > ch2:
                         comparison = 1
                         break
+                # need to break out of the outer loop as well if we found a
+                # valid comparison
+                if comparison != 0:
+                    break
             digit = not digit
         return comparison
 
@@ -159,7 +173,7 @@ class Version:
         """Convert elems to Fedora version and release numbers."""
         ver = str(elems.pop(0))
         while elems and \
-              (elems[0] == '.' or elems[0] == '_' or elems[0].startswith('~')):
+                (elems[0] == '.' or elems[0] == '_' or elems[0].startswith('~')):
             dot = elems.pop(0)
             if dot == '_':
                 dot = '.'