Merge pull request #1160 from phw/fix-config-loading

Fix loading BoolOption from config

picard/config.py

@@ -106,6 +106,7 @@ class ConfigSection(LockableObject):
                 return option_type.convert(self.raw_value(name))
             return default
         except Exception:
+            log.error('Error reading option value', exc_info=True)
             return default
         finally:
             self.unlock()
@@ -249,6 +250,7 @@ class Option(QtCore.QObject):
         return cls.registry.get((section, name))
 
 
+@staticmethod
 def _convert_to_bool(value):
     # The QSettings IniFormat saves boolean values as the strings "true"
     # and "false". Thus, explicit boolean and string comparisons are used
@@ -257,6 +259,8 @@ def _convert_to_bool(value):
     # PyQt >= 4.5, so that is not used.
     return value is True or value == "true"
 
+
+@staticmethod
 def _convert_to_int_list(values):
     return list(map(int, values))
 

test/test_config.py

@@ -0,0 +1,138 @@
+from test.picardtestcase import PicardTestCase
+
+from picard import config
+
+
+class CommonTests:
+
+    class CommonOptionTest(PicardTestCase):
+        opt_type = config.Option
+        default_value = 'somevalue'
+
+        def setUp(self):
+            self.opt_type.registry = {}
+
+        def test_constructor(self):
+            opt = self.opt_type('test', 'option1', self.default_value)
+            self.assertEqual('test', opt.section)
+            self.assertEqual('option1', opt.name)
+            self.assertEqual(self.default_value, opt.default)
+
+        def test_registry(self):
+            opt = self.opt_type('test', 'option1', self.default_value)
+            self.assertEqual(opt, self.opt_type.get('test', 'option1'))
+
+
+class OptionTest(CommonTests.CommonOptionTest):
+
+    def test_default_convert(self):
+        for default in ['somevalue', True, [], tuple(), 42]:
+            opt = config.Option('test', 'option1', default)
+            self.assertEqual(type(default), opt.convert)
+            self.assertEqual(default, opt.convert(default))
+
+
+class TextOptionTest(CommonTests.CommonOptionTest):
+    opt_type = config.TextOption
+    default_value = 'test'
+
+    def _test_convert(self, obj):
+        self.assertEqual('', obj.convert(''))
+        self.assertEqual('test', obj.convert('test'))
+        self.assertEqual('42', obj.convert(42))
+
+    def test_convert_instance(self):
+        opt = self.opt_type('test', 'option1', self.default_value)
+        self._test_convert(opt)
+
+    def test_convert_static(self):
+        self._test_convert(self.opt_type)
+
+
+class BoolOptionTest(CommonTests.CommonOptionTest):
+    opt_type = config.BoolOption
+    default_value = False
+
+    def _test_convert(self, obj):
+        self.assertTrue(obj.convert(True))
+        self.assertTrue(obj.convert('true'))
+        self.assertFalse(obj.convert(False))
+        self.assertFalse(obj.convert(None))
+        self.assertFalse(obj.convert('unknown'))
+        self.assertFalse(obj.convert('false'))
+
+    def test_convert_instance(self):
+        opt = self.opt_type('test', 'option1', self.default_value)
+        self._test_convert(opt)
+
+    def test_convert_static(self):
+        self._test_convert(self.opt_type)
+
+
+class IntOptionTest(CommonTests.CommonOptionTest):
+    opt_type = config.IntOption
+    default_value = 42
+
+    def _test_convert(self, obj):
+        self.assertEqual(42, obj.convert(42))
+        self.assertEqual(42, obj.convert('42'))
+        self.assertEqual(0, obj.convert(False))
+        self.assertEqual(1, obj.convert(True))
+        with self.assertRaises(ValueError):
+            obj.convert('notanumber')
+
+    def test_convert_instance(self):
+        opt = self.opt_type('test', 'option1', self.default_value)
+        self._test_convert(opt)
+
+    def test_convert_static(self):
+        self._test_convert(self.opt_type)
+
+
+class FloatOptionTest(CommonTests.CommonOptionTest):
+    opt_type = config.FloatOption
+    default_value = 42.5
+
+    def _test_convert(self, obj):
+        self.assertEqual(42.5, obj.convert(42.5))
+        self.assertEqual(42.5, obj.convert('42.5'))
+
+    def test_convert_instance(self):
+        opt = self.opt_type('test', 'option1', self.default_value)
+        self._test_convert(opt)
+
+    def test_convert_static(self):
+        self._test_convert(self.opt_type)
+
+
+class ListOptionTest(CommonTests.CommonOptionTest):
+    opt_type = config.ListOption
+    default_value = ['somevalue']
+
+    def _test_convert(self, obj):
+        self.assertEqual([], obj.convert([]))
+        self.assertEqual(['a', 'b', 'c'], obj.convert(('a', 'b', 'c')))
+        self.assertEqual(['a', 'b', 'c'], obj.convert('abc'))
+
+    def test_convert_instance(self):
+        opt = self.opt_type('test', 'option1', self.default_value)
+        self._test_convert(opt)
+
+    def test_convert_static(self):
+        self._test_convert(self.opt_type)
+
+
+class IntListOptionTest(CommonTests.CommonOptionTest):
+    opt_type = config.IntListOption
+    default_value = [1, 2, 3]
+
+    def _test_convert(self, obj):
+        self.assertEqual([], obj.convert([]))
+        self.assertEqual([1, 2, 3], obj.convert(('1', '2', '3')))
+
+    def test_convert_instance(self):
+        opt = self.opt_type('test', 'option1', self.default_value)
+        self._test_convert(opt)
+
+    def test_convert_static(self):
+        self._test_convert(self.opt_type)