From 7c8fd587a766988af213c3aec148e62bdd1c6af2 Mon Sep 17 00:00:00 2001 From: Florian Strzelecki Date: Fri, 4 Sep 2020 19:59:10 +0200 Subject: [PATCH] config: small refactoring and unit tests for types Co-authored-by: dgw --- sopel/config/types.py | 147 +++++++------ sopel/modules/admin.py | 5 +- test/config/test_config_types.py | 364 +++++++++++++++++++++++++++++++ 3 files changed, 444 insertions(+), 72 deletions(-) create mode 100644 test/config/test_config_types.py diff --git a/sopel/config/types.py b/sopel/config/types.py index 93db3b0393..cc09a5dfb6 100644 --- a/sopel/config/types.py +++ b/sopel/config/types.py @@ -95,19 +95,20 @@ def configure_setting(self, name, prompt, default=NO_DEFAULT): passed, the current value of the setting will be ignored, even if it is not the attribute's default. """ - clazz = getattr(self.__class__, name) - if default is NO_DEFAULT and not clazz.is_secret: + attribute = getattr(self.__class__, name) + if default is NO_DEFAULT and not attribute.is_secret: try: default = getattr(self, name) except AttributeError: pass except ValueError: print('The configured value for this option was invalid.') - if clazz.default is not NO_DEFAULT: - default = clazz.default + if attribute.default is not NO_DEFAULT: + default = attribute.default while True: try: - value = clazz.configure(prompt, default, self._parent, self._section_name) + value = attribute.configure( + prompt, default, self._parent, self._section_name) except ValueError as exc: print(exc) else: @@ -194,7 +195,9 @@ def configure(self, prompt, default, parent, section_name): raise ValueError("You must provide a value for this option.") value = value or default - return self.parse(value) + section = getattr(parent, section_name) + + return self._parse(value, parent, section) def serialize(self, value, *args, **kwargs): """Take some object, and return the string to be saved to the file. @@ -212,25 +215,32 @@ def parse(self, value, *args, **kwargs): def __get__(self, instance, owner=None): if instance is None: # If instance is None, we're getting from a section class, not an - # instance of a session class. It makes the wizard code simpler + # instance of a section class. It makes the wizard code simpler # (and is really just more intuitive) to return the descriptor # instance here. return self + value = None env_name = 'SOPEL_%s_%s' % (instance._section_name.upper(), self.name.upper()) if env_name in os.environ: value = os.environ.get(env_name) elif instance._parser.has_option(instance._section_name, self.name): value = instance._parser.get(instance._section_name, self.name) - else: - if self.default is not NO_DEFAULT: - return self.default - raise AttributeError( - "Missing required value for {}.{}".format( - instance._section_name, self.name - ) + + settings = instance._parent + section = getattr(settings, instance._section_name) + return self._parse(value, settings, section) + + def _parse(self, value, settings, section): + if value is not None: + return self.parse(value) + if self.default is not NO_DEFAULT: + return self.default + raise AttributeError( + "Missing required value for {}.{}".format( + section._section_name, self.name ) - return self.parse(value) + ) def __set__(self, instance, value): if value is None: @@ -238,9 +248,15 @@ def __set__(self, instance, value): raise ValueError('Cannot unset an option with a required value.') instance._parser.remove_option(instance._section_name, self.name) return - value = self.serialize(value) + + settings = instance._parent + section = getattr(settings, instance._section_name) + value = self._serialize(value, settings, section) instance._parser.set(instance._section_name, self.name, value) + def _serialize(self, value, settings, section): + return self.serialize(value) + def __delete__(self, instance): instance._parser.remove_option(instance._section_name, self.name) @@ -503,7 +519,10 @@ def configure(self, prompt, default, parent, section_name): else: values.append(value) value = get_input(each_prompt + ' ') - return self.parse(self.serialize(values)) + + section = getattr(parent, section_name) + values = self._serialize(values, parent, section) + return self._parse(values, parent, section) class ChoiceAttribute(BaseValidated): @@ -569,62 +588,55 @@ def __init__(self, name, relative=True, directory=False, default=None): self.relative = relative self.directory = directory - def __get__(self, instance, owner=None): - if instance is None: - return self - env_name = 'SOPEL_%s_%s' % (instance._section_name.upper(), self.name.upper()) - if env_name in os.environ: - value = os.environ.get(env_name) - elif instance._parser.has_option(instance._section_name, self.name): - value = instance._parser.get(instance._section_name, self.name) - else: - if self.default is not NO_DEFAULT: - value = self.default - else: + def _parse(self, value, settings, section): + if value is None: + if self.default == NO_DEFAULT: raise AttributeError( "Missing required value for {}.{}".format( - instance._section_name, self.name + section._section_name, self.name ) ) - main_config = instance._parent - this_section = getattr(main_config, instance._section_name) - return self.parse(value, main_config, this_section) + value = self.default - def __set__(self, instance, value): - main_config = instance._parent - this_section = getattr(main_config, instance._section_name) - value = self.serialize(value, main_config, this_section) - instance._parser.set(instance._section_name, self.name, value) + if not value: + return self.parse(value) - def configure(self, prompt, default, parent, section_name): - if default is not NO_DEFAULT and default is not None: - prompt = '{} [{}]'.format(prompt, default) - value = get_input(prompt + ' ') - if not value and default is NO_DEFAULT: - raise ValueError("You must provide a value for this option.") - value = value or default - return self.parse(value, parent, section_name) + result = os.path.expanduser(value) + if not os.path.isabs(result): + if not self.relative: + raise ValueError("Value must be an absolute path.") + result = os.path.join(settings.homedir, result) + + return self.parse(result) - def parse(self, value, main_config, this_section): - """Used to validate ``value`` when loading the config. + def _serialize(self, value, settings, section): + """Used to validate ``value`` when it is changed at runtime. - :param main_config: the config object which contains this attribute - :type main_config: :class:`~sopel.config.Config` - :param this_section: the config section which contains this attribute - :type this_section: :class:`~StaticSection` + :param settings: the config object which contains this attribute + :type settings: :class:`~sopel.config.Config` + :param section: the config section which contains this attribute + :type section: :class:`~StaticSection` :return: the ``value``, if it is valid :rtype: str :raise ValueError: if the ``value`` is not valid """ - if value is None: - return + self._parse(value, settings, section) + return self.serialize(value) - value = os.path.expanduser(value) + def parse(self, value): + """Parse ``value`` as a path on the filesystem to check. - if not os.path.isabs(value): - if not self.relative: - raise ValueError("Value must be an absolute path.") - value = os.path.join(main_config.homedir, value) + :param str value: the path to check + :rtype: str + :raise ValueError: if the directory or file doesn't exist and cannot + be created + + If there is no ``value``, then this returns ``None``. Otherwise, it'll + check if the directory or file exists. If it doesn't, it'll try to + create it. + """ + if not value: + return None if self.directory and not os.path.isdir(value): try: @@ -639,16 +651,15 @@ def parse(self, value, main_config, this_section): raise ValueError("Value must be an existing or creatable file.") return value - def serialize(self, value, main_config, this_section): - """Used to validate ``value`` when it is changed at runtime. + def serialize(self, value): + """Directly return the ``value`` without modification. - :param main_config: the config object which contains this attribute - :type main_config: :class:`~sopel.config.Config` - :param this_section: the config section which contains this attribute - :type this_section: :class:`~StaticSection` - :return: the ``value``, if it is valid + :param str value: the value needing to be saved + :return: the unaltered ``value``, if it is valid :rtype: str - :raise ValueError: if the ``value`` is not valid + + Managing the filename is done by other methods (:meth:`parse`), and + this method is a no-op: this way it ensures that relative paths won't + be replaced by absolute ones. """ - self.parse(value, main_config, this_section) return value # So that it's still relative diff --git a/sopel/modules/admin.py b/sopel/modules/admin.py index 1bbde8bea0..b679e8562d 100644 --- a/sopel/modules/admin.py +++ b/sopel/modules/admin.py @@ -352,10 +352,7 @@ def set_config(bot, trigger): # Otherwise, set the value to one given if descriptor is not None: try: - if isinstance(descriptor, types.FilenameAttribute): - value = descriptor.parse(value, bot.config, descriptor) - else: - value = descriptor.parse(value) + value = descriptor._parse(value, bot.config, section) except ValueError as exc: bot.say("Can't set attribute: " + str(exc)) return diff --git a/test/config/test_config_types.py b/test/config/test_config_types.py new file mode 100644 index 0000000000..89a8ed1c19 --- /dev/null +++ b/test/config/test_config_types.py @@ -0,0 +1,364 @@ +# coding=utf-8 +from __future__ import absolute_import, division, print_function, unicode_literals + +import os + +import pytest + +from sopel.config import types + + +def test_validated_attribute(): + option = types.ValidatedAttribute('foo') + assert option.name == 'foo' + assert option.default is None + assert option.is_secret is False + + +def test_validated_parse(): + option = types.ValidatedAttribute('foo') + assert option.parse('string') == 'string' + assert option.parse('1') == '1' + assert option.parse('') == '' + assert option.parse(None) is None + assert option.parse(1) == 1 + + +def test_validated_parse_custom(): + def _fixed_parser(value): + return 'fixed value' + + option = types.ValidatedAttribute('foo', parse=_fixed_parser) + assert option.parse('string') == 'fixed value' + assert option.parse('1') == 'fixed value' + assert option.parse('') == 'fixed value' + assert option.parse(None) == 'fixed value' + assert option.parse(1) == 'fixed value' + + +def test_validated_parse_bool(): + option = types.ValidatedAttribute('foo', parse=bool) + assert option.parse('string') is False + assert option.parse('1') is True + assert option.parse('') is False + assert option.parse(None) is False + assert option.parse(1) == 1 # TODO: cast as ``True``? + + # true-ish values + assert option.parse('yes') is True + assert option.parse('YES') is True + assert option.parse('yES') is True + assert option.parse('y') is True + assert option.parse('Y') is True + assert option.parse('true') is True + assert option.parse('True') is True + assert option.parse('TRUE') is True + assert option.parse('trUE') is True + assert option.parse('on') is True + assert option.parse('ON') is True + assert option.parse('On') is True + + # everything else + assert option.parse('no') is False + assert option.parse('disable') is False + assert option.parse('disabled') is False + assert option.parse('enable') is False # TODO: maybe true-ish? + assert option.parse('enabled') is False # TODO: maybe true-ish? + + +def test_validated_parse_int(): + option = types.ValidatedAttribute('foo', parse=int) + assert option.parse('0') == 0 + assert option.parse('1') == 1 + assert option.parse('7814') == 7814 + assert option.parse('-1') == -1 + assert option.parse(1) == 1 + + with pytest.raises(ValueError): + option.parse('785.56') + + with pytest.raises(ValueError): + option.parse('string') + + with pytest.raises(ValueError): + option.parse('') + + with pytest.raises(TypeError): + option.parse(None) + + +def test_validated_serialize(): + option = types.ValidatedAttribute('foo') + assert option.serialize('string') == 'string' + assert option.serialize('1') == '1' + assert option.serialize('') == '' + assert option.serialize(None) == 'None' # TODO: empty string instead? + assert option.serialize(1) == '1' + + +def test_validated_serialize_custom(): + def _fixed_serialize(value): + return 'fixed value' + + option = types.ValidatedAttribute('foo', serialize=_fixed_serialize) + assert option.serialize('string') == 'fixed value' + assert option.serialize('1') == 'fixed value' + assert option.serialize('') == 'fixed value' + assert option.serialize(None) == 'fixed value' + assert option.serialize(1) == 'fixed value' + + +def test_validated_serialize_bool(): + option = types.ValidatedAttribute('foo', parse=bool) + assert option.serialize(True) == 'true' + assert option.serialize('string') == 'false' + assert option.serialize('1') == 'true' + assert option.serialize('') == 'false' + assert option.serialize(None) == 'false' + assert option.serialize(1) == 'true' + + # true-ish values + assert option.serialize('yes') == 'true' + assert option.serialize('YES') == 'true' + assert option.serialize('yES') == 'true' + assert option.serialize('y') == 'true' + assert option.serialize('Y') == 'true' + assert option.serialize('true') == 'true' + assert option.serialize('True') == 'true' + assert option.serialize('TRUE') == 'true' + assert option.serialize('trUE') == 'true' + assert option.serialize('on') == 'true' + assert option.serialize('ON') == 'true' + assert option.serialize('On') == 'true' + + # everything else + assert option.serialize('no') == 'false' + assert option.serialize('disable') == 'false' + assert option.serialize('disabled') == 'false' + assert option.serialize('enable') == 'false' # TODO: maybe true-ish? + assert option.serialize('enabled') == 'false' # TODO: maybe true-ish? + + +def test_validated_serialize_bool_custom(): + def _fixed_serialized(value): + return 'fixed value' + + option = types.ValidatedAttribute( + 'foo', parse=bool, serialize=_fixed_serialized) + assert option.serialize(True) == 'fixed value' + assert option.serialize('string') == 'fixed value' + assert option.serialize('1') == 'fixed value' + assert option.serialize('') == 'fixed value' + assert option.serialize(None) == 'fixed value' + assert option.serialize(1) == 'fixed value' + + # true-ish values + assert option.serialize('yes') == 'fixed value' + assert option.serialize('YES') == 'fixed value' + assert option.serialize('yES') == 'fixed value' + assert option.serialize('y') == 'fixed value' + assert option.serialize('Y') == 'fixed value' + assert option.serialize('true') == 'fixed value' + assert option.serialize('True') == 'fixed value' + assert option.serialize('TRUE') == 'fixed value' + assert option.serialize('trUE') == 'fixed value' + assert option.serialize('on') == 'fixed value' + assert option.serialize('ON') == 'fixed value' + assert option.serialize('On') == 'fixed value' + + # everything else + assert option.serialize('no') == 'fixed value' + assert option.serialize('disable') == 'fixed value' + assert option.serialize('disabled') == 'fixed value' + assert option.serialize('enable') == 'fixed value' + assert option.serialize('enabled') == 'fixed value' + + +def test_secret_attribute(): + option = types.SecretAttribute('foo') + assert option.name == 'foo' + assert option.default is None + assert option.is_secret is True + + +def test_list_attribute(): + option = types.ListAttribute('foo') + assert option.name == 'foo' + assert option.default == [] + assert option.is_secret is False + + +def test_list_parse_single_value(): + option = types.ListAttribute('foo') + assert option.parse('string') == ['string'] + assert option.parse('1') == ['1'] + assert option.parse('') == [] + + with pytest.raises(TypeError): + option.parse(None) + + with pytest.raises(TypeError): + option.parse(1) + + +def test_list_parse_new_lines(): + option = types.ListAttribute('foo') + assert option.parse(""" + value 1 + "# value 2" + value 3 + """) == [ + 'value 1', + '# value 2', + 'value 3', + ] + + +def test_list_parse_new_lines_no_strip(): + option = types.ListAttribute('foo', strip=False) + # strip isn't used for newline-based list attribute + assert option.parse(""" + value 1 + "# value 2" + value 3 + """) == [ + 'value 1', + '# value 2', + 'value 3', + ] + + +def test_list_parse_legacy_comma(): + option = types.ListAttribute('foo') + assert option.parse("""value 1, # value 2, value 3""") == [ + 'value 1', + '# value 2', + 'value 3', + ] + + +def test_list_parse_legacy_comma_no_strip(): + option = types.ListAttribute('foo', strip=False) + assert option.parse("""value 1, # value 2 , value 3""") == [ + 'value 1', + ' # value 2 ', + ' value 3', + ] + + +def test_list_parse_new_lines_legacy_comma(): + option = types.ListAttribute('foo') + assert option.parse(""" + value 1, value 2, + value 3 + """) == [ + 'value 1, value 2', + 'value 3', + ] + + +def test_list_serialize(): + option = types.ListAttribute('foo') + assert option.serialize([]) == '' + assert option.serialize(['value 1', 'value 2', 'value 3']) == ( + '\n' + 'value 1\n' + 'value 2\n' + 'value 3' + ) + + assert option.serialize(set()) == '' + assert option.serialize(set(['1', '2', '3'])) == ( + '\n' + '\n'.join(set(['1', '2', '3'])) + ) + + +def test_list_serialize_quote(): + option = types.ListAttribute('foo') + assert option.serialize(['value 1', '# value 2', 'value 3']) == ( + '\n' + 'value 1\n' + '"# value 2"\n' + 'value 3' + ) + + +def test_list_serialize_value_error(): + option = types.ListAttribute('foo') + + with pytest.raises(ValueError): + option.serialize('value 1') + + with pytest.raises(ValueError): + option.serialize(('1', '2', '3')) # tuple is not allowed + + +def test_choice_attribute(): + option = types.ChoiceAttribute('foo', choices=['a', 'b', 'c']) + assert option.name == 'foo' + assert option.default is None + assert option.is_secret is False + + +def test_choice_parse(): + option = types.ChoiceAttribute('foo', choices=['a', 'b', 'c']) + assert option.parse('a') == 'a' + assert option.parse('b') == 'b' + assert option.parse('c') == 'c' + + with pytest.raises(ValueError): + option.parse('d') + + +def test_choice_serialize(): + option = types.ChoiceAttribute('foo', choices=['a', 'b', 'c']) + assert option.serialize('a') == 'a' + assert option.serialize('b') == 'b' + assert option.serialize('c') == 'c' + + with pytest.raises(ValueError): + option.serialize('d') + + +def test_filename_attribute(): + option = types.FilenameAttribute('foo') + assert option.name == 'foo' + assert option.default is None + assert option.is_secret is False + assert option.directory is False + assert option.relative is True + + +def test_filename_parse(tmpdir): + testfile = tmpdir.join('foo.txt') + testfile.write('') + option = types.FilenameAttribute('foo') + assert option.parse(testfile.strpath) == testfile.strpath + assert option.parse(None) is None + assert option.parse('') is None + + +def test_filename_parse_create(tmpdir): + testfile = tmpdir.join('foo.txt') + + option = types.FilenameAttribute('foo') + assert not os.path.exists(testfile.strpath), 'Test file must not exist yet' + assert option.parse(testfile.strpath) == testfile.strpath + assert os.path.exists(testfile.strpath), 'Test file must exist now' + assert os.path.isfile(testfile.strpath), 'Test file must be a file' + + +def test_filename_parse_directory(tmpdir): + testdir = tmpdir.join('foo') + testdir.mkdir() + option = types.FilenameAttribute('foo', directory=True) + assert option.parse(testdir.strpath) == testdir.strpath + + +def test_filename_parse_directory_create(tmpdir): + testdir = tmpdir.join('foo') + option = types.FilenameAttribute('foo', directory=True) + assert not os.path.exists(testdir.strpath), 'Test dir must not exist yet' + assert option.parse(testdir.strpath) == testdir.strpath + assert os.path.exists(testdir.strpath), 'Test dir must exist now' + assert os.path.isdir(testdir.strpath), 'Test dir must be a directory'