parseRevisionOption(): Raise an exception on invalid revisions

osc/core.py

@@ -6707,12 +6707,15 @@ def parseRevisionOption(string, allow_md5=True):
     revisions = [None, None]
     if string:
         parts = string.split(':')
-        for i, revision in enumerate(parts[0:2], 0):
+
+        if len(parts) > 2:
+            raise oscerr.OscInvalidRevision(string)
+
+        for i, revision in enumerate(parts, 0):
             if revision.isdigit() or (allow_md5 and revision.isalnum() and len(revision) == 32):
                 revisions[i] = revision
             elif revision != '' and revision != 'latest':
-                print('your revision \'%s\' will be ignored' % string, file=sys.stderr)
+                raise oscerr.OscInvalidRevision(string)
-                return None, None
 
     return tuple(revisions)
 

osc/oscerr.py

@@ -121,6 +121,22 @@ class OscIOError(OscBaseError):
         self.e = e
         self.msg = msg
 
+
+class OscValueError(OscBaseError):
+    """
+    Invalid argument value (of correct type).
+    """
+    pass
+
+
+class OscInvalidRevision(OscValueError):
+    """
+    Invalid revision value.
+    """
+    def __str__(self):
+        return "Invalid revision value: {}".format("".join(self.args))
+
+
 class PackageNotInstalled(OscBaseError):
     """
     Exception raised when a package is not installed on local system

tests/test_core.py

@@ -0,0 +1,52 @@
+from osc.core import parseRevisionOption
+from osc.oscerr import OscInvalidRevision
+
+import unittest
+
+
+class TestParseRevisionOption(unittest.TestCase):
+    def test_empty(self):
+        expected = None, None
+        actual = parseRevisionOption("")
+        self.assertEqual(expected, actual)
+
+    def test_colon(self):
+        expected = None, None
+        actual = parseRevisionOption(":")
+        # your revision ':' will be ignored
+        self.assertEqual(expected, actual)
+
+    def test_invalid_multiple_colons(self):
+        self.assertRaises(OscInvalidRevision, parseRevisionOption, ":::::")
+
+    def test_one_number(self):
+        expected = ("1", None)
+        actual = parseRevisionOption("1")
+        self.assertEqual(expected, actual)
+
+    def test_two_numbers(self):
+        expected = ("1", "2")
+        actual = parseRevisionOption("1:2")
+        self.assertEqual(expected, actual)
+
+    def test_invalid_multiple_numbers(self):
+        self.assertRaises(OscInvalidRevision, parseRevisionOption, "1:2:3:4:5")
+
+    def test_one_hash(self):
+        expected = "c4ca4238a0b923820dcc509a6f75849b", None
+        actual = parseRevisionOption("c4ca4238a0b923820dcc509a6f75849b")
+        self.assertEqual(expected, actual)
+
+    def test_two_hashes(self):
+        expected = ("d41d8cd98f00b204e9800998ecf8427e", "c4ca4238a0b923820dcc509a6f75849b")
+        actual = parseRevisionOption("d41d8cd98f00b204e9800998ecf8427e:c4ca4238a0b923820dcc509a6f75849b")
+        self.assertEqual(expected, actual)
+
+    def test_invalid_multiple_hashes(self):
+        rev = "d41d8cd98f00b204e9800998ecf8427e:c4ca4238a0b923820dcc509a6f75849b:c81e728d9d4c2f636f067f89cc14862c"
+        self.assertRaises(OscInvalidRevision, parseRevisionOption, rev)
+
+
+if __name__ == "__main__":
+    import unittest
+    unittest.main()