conftypes: Various bugfixes/cleanups/tests.

qutebrowser/config/conftypes.py

@@ -126,8 +126,9 @@ class BaseType:
         Return:
             The transformed value.
         """
-        if self.none_ok and not value:
+        if not value:
             return None
+        else:
             return value
 
     def validate(self, value):
@@ -259,6 +260,7 @@ class Bool(BaseType):
     def transform(self, value):
         if not value:
             return None
+        else:
             return Bool._BOOLEAN_STATES[value.lower()]
 
     def validate(self, value):
@@ -294,8 +296,9 @@ class Int(BaseType):
         self.maxval = maxval
 
     def transform(self, value):
-        if self.none_ok and not value:
+        if not value:
             return None
+        else:
             return int(value)
 
     def validate(self, value):
@@ -328,9 +331,12 @@ class IntList(List):
 
     def validate(self, value):
         try:
-            self.transform(value)
+            vals = self.transform(value)
         except ValueError:
             raise ValidationError(value, "must be a list of integers!")
+        for v in vals:
+            if v is None and not self.none_ok:
+                raise ValidationError(value, "items may not be empty!")
 
 
 class Float(BaseType):
@@ -353,8 +359,9 @@ class Float(BaseType):
         self.maxval = maxval
 
     def transform(self, value):
-        if self.none_ok and not value:
+        if not value:
             return None
+        else:
             return float(value)
 
     def validate(self, value):
@@ -395,8 +402,9 @@ class Perc(BaseType):
         self.maxval = maxval
 
     def transform(self, value):
-        if self.none_ok and not value:
+        if not value:
             return
+        else:
             return int(value[:-1])
 
     def validate(self, value):
@@ -440,7 +448,7 @@ class PercList(List):
 
     def transform(self, value):
         vals = super().transform(value)
-        return [int(val[:-1]) for val in vals]
+        return [int(v[:-1]) if v is not None else None for v in vals]
 
     def validate(self, value):
         vals = super().transform(value)
@@ -556,6 +564,7 @@ class ColorSystem(BaseType):
     def transform(self, value):
         if not value:
             return None
+        else:
             mapping = {
                 'rgb': QColor.Rgb,
                 'hsv': QColor.Hsv,
@@ -582,6 +591,9 @@ class QtColor(BaseType):
             raise ValidationError(value, "must be a valid color")
 
     def transform(self, value):
+        if not value:
+            return None
+        else:
             return QColor(value)
 
 
@@ -654,6 +666,8 @@ class QtFont(Font):
     """A Font which gets converted to q QFont."""
 
     def transform(self, value):
+        if not value:
+            return None
         style_map = {
             'normal': QFont.StyleNormal,
             'italic': QFont.StyleItalic,
@@ -708,6 +722,9 @@ class Regex(BaseType):
             raise ValidationError(value, "must be a valid regex - " + str(e))
 
     def transform(self, value):
+        if not value:
+            return None
+        else:
             return re.compile(value, self.flags)
 
 
@@ -723,7 +740,8 @@ class RegexList(List):
 
     def transform(self, value):
         vals = super().transform(value)
-        return [re.compile(pattern, self.flags) for pattern in vals]
+        return [re.compile(v, self.flags) if v is not None else None
+                for v in vals]
 
     def validate(self, value):
         try:
@@ -752,6 +770,8 @@ class File(BaseType):
             raise ValidationError(value, "must be an absolute path!")
 
     def transform(self, value):
+        if not value:
+            return None
         return os.path.expanduser(value)
 
 
@@ -824,7 +844,7 @@ class WebKitBytes(BaseType):
     def transform(self, value):
         if not value:
             return None
-        if any(value.lower().endswith(c) for c in self.SUFFIXES):
+        elif any(value.lower().endswith(c) for c in self.SUFFIXES):
             suffix = value[-1].lower()
             val = value[:-1]
             multiplicator = self.SUFFIXES[suffix]
@@ -853,6 +873,7 @@ class WebKitBytesList(List):
     def transform(self, value):
         if value == '':
             return None
+        else:
             vals = super().transform(value)
             return [self.bytestype.transform(val) for val in vals]
 
@@ -894,6 +915,9 @@ class ShellCommand(String):
             raise ValidationError(value, "needs to contain a {}-placeholder.")
 
     def transform(self, value):
+        if not value:
+            return None
+        else:
             return shlex.split(value)
 
 
@@ -962,7 +986,9 @@ class Proxy(BaseType):
         return out
 
     def transform(self, value):
-        if value == 'system':
+        if not value:
+            return None
+        elif value == 'system':
             return None
         elif value == 'none':
             return QNetworkProxy(QNetworkProxy.NoProxy)
@@ -1011,7 +1037,11 @@ class KeyBindingName(BaseType):
     """The name (keys) of a keybinding."""
 
     def validate(self, value):
-        pass
+        if not value:
+            if self.none_ok:
+                return
+            else:
+                raise ValidationError(value, "may not be empty!")
 
 
 class KeyBinding(Command):
@@ -1046,13 +1076,20 @@ class AutoSearch(BaseType):
                                ('false', "Never search automatically."))
 
     def validate(self, value):
+        if not value:
+            if self.none_ok:
+                return
+            else:
+                raise ValidationError(value, "may not be empty!")
         if value.lower() in ('naive', 'dns'):
             pass
         else:
             Bool.validate(self, value)
 
     def transform(self, value):
-        if value.lower() in ('naive', 'dns'):
+        if not value:
+            return None
+        elif value.lower() in ('naive', 'dns'):
             return value.lower()
         elif super().transform(value):
             # boolean true is an alias for naive matching

qutebrowser/test/config/test_conftypes.py

@@ -20,6 +20,9 @@
 import unittest
 
 import qutebrowser.config.conftypes as conftypes
+from qutebrowser.test.stubs import FakeCmdUtils, FakeCommand
+
+from PyQt5.QtGui import QColor, QFont
 
 
 class ValidValuesTest(unittest.TestCase):
@@ -87,13 +90,8 @@ class BaseTypeTests(unittest.TestCase):
         self.assertEqual(self.t.transform("foobar"), "foobar")
 
     def test_transform_empty(self):
-        """Test transform with none_ok = False and an empty value."""
+        """Test transform with an empty value."""
-        self.assertEqual(self.t.transform(''), '')
+        self.assertIsNone(self.t.transform(''))
-
-    def test_transform_empty_none_ok(self):
-        """Test transform with none_ok = True and an empty value."""
-        t = conftypes.BaseType(none_ok=True)
-        self.assertIsNone(t.transform(''))
 
     def test_validate_not_implemented(self):
         """Test validate without valid_values set."""
@@ -166,6 +164,7 @@ class StringTests(unittest.TestCase):
     def test_validate_empty(self):
         """Test validate with empty string and none_ok = False."""
         t = conftypes.String()
+        with self.assertRaises(conftypes.ValidationError):
             t.validate("")
 
     def test_validate_empty_none_ok(self):
@@ -249,7 +248,7 @@ class ListTests(unittest.TestCase):
     def test_transform_empty(self):
         """Test transform with an empty value."""
         self.assertEqual(self.t.transform('foo,,baz'),
-                         ['foo', '', 'baz'])
+                         ['foo', None, 'baz'])
 
     def test_transform_empty_none_ok(self):
         """Test transform with an empty value and none_ok = True."""
@@ -271,7 +270,11 @@ class BoolTests(unittest.TestCase):
         """Test transform with all values."""
         for out, inputs in self.TESTS.items():
             for inp in inputs:
-                self.assertEqual(self.t.transform(inp), out)
+                self.assertEqual(self.t.transform(inp), out, inp)
+
+    def test_transform_empty(self):
+        """Test transform with none_ok = False and an empty value."""
+        self.assertIsNone(self.t.transform(''))
 
     def test_validate_valid(self):
         """Test validate with valid values."""
@@ -285,6 +288,18 @@ class BoolTests(unittest.TestCase):
             with self.assertRaises(conftypes.ValidationError):
                 self.t.validate(val)
 
+    def test_validate_empty(self):
+        """Test validate with empty string and none_ok = False."""
+        t = conftypes.Bool()
+        with self.assertRaises(conftypes.ValidationError):
+            t.validate('')
+
+    def test_validate_empty_none_ok(self):
+        """Test validate with empty string and none_ok = True."""
+        t = conftypes.Int(none_ok=True)
+        t.validate('')
+
+
 
 class IntTests(unittest.TestCase):
 
@@ -397,10 +412,9 @@ class IntListTests(unittest.TestCase):
         self.assertEqual(self.t.transform('23,42,1337'),
                          [23, 42, 1337])
 
-    def test_transform_empty_none_ok(self):
+    def test_transform_empty(self):
-        """Test transform with an empty value and none_ok = True."""
+        """Test transform with an empty value."""
-        t = conftypes.IntList(none_ok=True)
+        self.assertEqual(self.t.transform('23,,42'), [23, None, 42])
-        self.assertEqual(t.transform('23,,42'), [23, None, 42])
 
 
 class FloatTests(unittest.TestCase):
@@ -481,9 +495,9 @@ class FloatTests(unittest.TestCase):
         with self.assertRaises(conftypes.ValidationError):
             t.validate('3.01')
 
-    def test_transform_none(self):
+    def test_transform_empty(self):
         """Test transform with an empty value."""
-        t = conftypes.Float(none_ok=True)
+        t = conftypes.Float()
         self.assertIsNone(t.transform(''))
 
     def test_transform_float(self):
@@ -501,6 +515,9 @@ class PercTests(unittest.TestCase):
 
     """Test Perc."""
 
+    def setUp(self):
+        self.t = conftypes.Perc()
+
     def test_minval_gt_maxval(self):
         """Test __init__ with a minval bigger than the maxval."""
         with self.assertRaises(ValueError):
@@ -508,26 +525,22 @@ class PercTests(unittest.TestCase):
 
     def test_validate_int(self):
         """Test validate with a normal int (not a percentage)."""
-        t = conftypes.Perc()
         with self.assertRaises(ValueError):
-            t.validate('1337')
+            self.t.validate('1337')
 
     def test_validate_string(self):
         """Test validate with something which isn't a percentage."""
-        t = conftypes.Perc()
         with self.assertRaises(conftypes.ValidationError):
-            t.validate('1337%%')
+            self.t.validate('1337%%')
 
     def test_validate_perc(self):
         """Test validate with a percentage."""
-        t = conftypes.Perc()
+        self.t.validate('1337%')
-        t.validate('1337%')
 
     def test_validate_empty(self):
         """Test validate with empty string and none_ok = False."""
-        t = conftypes.Perc()
         with self.assertRaises(conftypes.ValidationError):
-            t.validate('')
+            self.t.validate('')
 
     def test_validate_empty_none_ok(self):
         """Test validate with empty string and none_ok = True."""
@@ -570,15 +583,13 @@ class PercTests(unittest.TestCase):
         with self.assertRaises(conftypes.ValidationError):
             t.validate('4%')
 
-    def test_transform_none(self):
+    def test_transform_empty(self):
         """Test transform with an empty value."""
-        t = conftypes.Perc(none_ok=True)
+        self.assertIsNone(self.t.transform(''))
-        self.assertIsNone(t.transform(''))
 
     def test_transform_perc(self):
         """Test transform with a percentage."""
-        t = conftypes.Perc()
+        self.assertEqual(self.t.transform('1337%'), 1337)
-        self.assertEqual(t.transform('1337%'), 1337)
 
 
 class PercListTests(unittest.TestCase):
@@ -663,6 +674,9 @@ class PercOrIntTests(unittest.TestCase):
 
     """Test PercOrInt."""
 
+    def setUp(self):
+        self.t = conftypes.PercOrInt()
+
     def test_minint_gt_maxint(self):
         """Test __init__ with a minint bigger than the maxint."""
         with self.assertRaises(ValueError):
@@ -675,25 +689,21 @@ class PercOrIntTests(unittest.TestCase):
 
     def test_validate_string(self):
         """Test validate with something which isn't a percentage."""
-        t = conftypes.PercOrInt()
         with self.assertRaises(conftypes.ValidationError):
-            t.validate('1337%%')
+            self.t.validate('1337%%')
 
     def test_validate_perc(self):
         """Test validate with a percentage."""
-        t = conftypes.PercOrInt()
+        self.t.validate('1337%')
-        t.validate('1337%')
 
     def test_validate_int(self):
         """Test validate with a normal int."""
-        t = conftypes.PercOrInt()
+        self.t.validate('1337')
-        t.validate('1337')
 
     def test_validate_empty(self):
         """Test validate with empty string and none_ok = False."""
-        t = conftypes.PercOrInt()
         with self.assertRaises(conftypes.ValidationError):
-            t.validate('')
+            self.t.validate('')
 
     def test_validate_empty_none_ok(self):
         """Test validate with empty string and none_ok = True."""
@@ -786,21 +796,125 @@ class PercOrIntTests(unittest.TestCase):
 
     def test_transform_none(self):
         """Test transform with an empty value."""
-        t = conftypes.PercOrInt(none_ok=True)
+        self.assertIsNone(self.t.transform(''))
-        self.assertIsNone(t.transform(''))
 
     def test_transform_perc(self):
         """Test transform with a percentage."""
-        t = conftypes.PercOrInt()
+        self.assertEqual(self.t.transform('1337%'), '1337%')
-        self.assertEqual(t.transform('1337%'), '1337%')
 
     def test_transform_int(self):
         """Test transform with an int."""
-        t = conftypes.PercOrInt()
+        self.assertEqual(self.t.transform('1337'), '1337')
-        self.assertEqual(t.transform('1337'), '1337')
 
 
+class CommandTests(unittest.TestCase):
 
+    """Test Command."""
+
+    def setUp(self):
+        self.old_cmdutils = conftypes.cmdutils
+        commands = {
+            'cmd1': FakeCommand("desc 1"),
+            'cmd2': FakeCommand("desc 2"),
+        }
+        conftypes.cmdutils = FakeCmdUtils(commands)
+        self.t = conftypes.Command()
+
+    def tearDown(self):
+        conftypes.cmdutils = self.old_cmdutils
+
+    def test_validate_empty(self):
+        """Test validate with an empty string."""
+        with self.assertRaises(conftypes.ValidationError):
+            self.t.validate('')
+
+    def test_validate_empty_none_ok(self):
+        """Test validate with an empty string and none_ok=True."""
+        t = conftypes.Command(none_ok=True)
+        t.validate('')
+
+    def test_validate_command(self):
+        """Test validate with a command."""
+        self.t.validate('cmd1')
+        self.t.validate('cmd2')
+
+    def test_validate_command_args(self):
+        """Test validate with a command and arguments."""
+        self.t.validate('cmd1  foo bar')
+        self.t.validate('cmd2  baz fish')
+
+    def test_validate_invalid_command(self):
+        """Test validate with an invalid command."""
+        with self.assertRaises(conftypes.ValidationError):
+            self.t.validate('cmd3')
+
+    def test_validate_invalid_command_args(self):
+        """Test validate with an invalid command and arguments."""
+        with self.assertRaises(conftypes.ValidationError):
+            self.t.validate('cmd3  foo bar')
+
+    def test_transform(self):
+        """Make sure transform doesn't alter values."""
+        self.assertEqual(self.t.transform('foo bar'), 'foo bar')
+
+    def test_transform_empty(self):
+        """Test transform with an empty value."""
+        self.assertIsNone(self.t.transform(''))
+
+    def test_complete(self):
+        """Test complete."""
+        items = self.t.complete()
+        self.assertEqual(len(items), 2)
+        self.assertIn(('cmd1', "desc 1"), items)
+        self.assertIn(('cmd2', "desc 2"), items)
+
+
+class ColorSystemTests(unittest.TestCase):
+
+    """Test ColorSystem."""
+
+    TESTS = {
+        'RGB': QColor.Rgb,
+        'rgb': QColor.Rgb,
+        'HSV': QColor.Hsv,
+        'hsv': QColor.Hsv,
+        'HSL': QColor.Hsl,
+        'hsl': QColor.Hsl,
+    }
+    INVALID = ['RRGB', 'HSV ']
+
+    def setUp(self):
+        self.t = conftypes.ColorSystem()
+
+    def test_validate_empty(self):
+        """Test validate with an empty string."""
+        with self.assertRaises(conftypes.ValidationError):
+            self.t.validate('')
+
+    def test_validate_empty_none_ok(self):
+        """Test validate with an empty string and none_ok=True."""
+        t = conftypes.ColorSystem(none_ok=True)
+        t.validate('')
+
+    def test_validate_valid(self):
+        """Test validate with valid values."""
+        for val in self.TESTS:
+            self.t.validate(val)
+
+    def test_validate_invalid(self):
+        """Test validate with invalid values."""
+        for val in self.INVALID:
+            with self.assertRaises(conftypes.ValidationError, msg=val):
+                self.t.validate(val)
+
+    def test_transform(self):
+        """Test transform."""
+        for k, v in self.TESTS.items():
+            self.assertEqual(self.t.transform(k), v, k)
+
+    def test_transform_empty(self):
+        """Test transform with an empty value."""
+        self.assertIsNone(self.t.transform(''))
 
 if __name__ == '__main__':
     unittest.main()

qutebrowser/test/stubs.py

@@ -279,3 +279,19 @@ class FakeSignal:
 
     def __init__(self, name='fake'):
         self.signal = '2{}(int, int)'.format(name)
+
+
+class FakeCmdUtils:
+
+    """Stub for cmdutils which provides a cmd_dict."""
+
+    def __init__(self, commands):
+        self.cmd_dict = commands
+
+
+class FakeCommand:
+
+    """A simple command stub which has a description."""
+
+    def __init__(self, desc):
+        self.desc = desc