From 745ef63451657205ddb840b0cdd3979f21a70589 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 15 Sep 2017 16:41:32 +0200 Subject: [PATCH] Start implementing autoconfig.yml error handling --- qutebrowser/config/config.py | 19 ++++++++++++++---- qutebrowser/config/configfiles.py | 32 ++++++++++++++++++++++++++++--- tests/unit/config/test_config.py | 26 +++++++++++++++++++------ 3 files changed, 64 insertions(+), 13 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 044432e91..e39ac6623 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -671,10 +671,21 @@ def init(parent=None): text=e.to_html(), icon=QMessageBox.Warning, plain_text=False) - else: - _errbox = None - if getattr(config_api, 'load_autoconfig', True): - instance.read_yaml() + try: + if getattr(config_api, 'load_autoconfig', True): + try: + instance.read_yaml() + except configexc.ConfigFileErrors as e: + raise # caught in outer block + except configexc.Error as e: + desc = configexc.ConfigErrorDesc("Error", e) + raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) + except configexc.ConfigFileErrors as e: + _errbox = msgbox.msgbox(parent=None, + title="Error while reading config", + text=e.to_html(), + icon=QMessageBox.Warning, + plain_text=False) configfiles.init(instance) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 397a1d100..efa984f77 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -26,6 +26,7 @@ import traceback import configparser import contextlib +import yaml from PyQt5.QtCore import QSettings from qutebrowser.config import configexc @@ -86,12 +87,37 @@ class YamlConfig: def load(self): """Load self.values from the configured YAML file.""" - # FIXME:conf error handling try: with open(self._filename, 'r', encoding='utf-8') as f: - self.values = utils.yaml_load(f)['global'] + yaml_data = utils.yaml_load(f) except FileNotFoundError: - pass + return + except OSError as e: + desc = configexc.ConfigErrorDesc("While reading", e) + raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) + except yaml.YAMLError as e: + desc = configexc.ConfigErrorDesc("While parsing", e) + raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) + + try: + global_obj = yaml_data['global'] + except KeyError: + desc = configexc.ConfigErrorDesc( + "While loading data", + "Toplevel object does not contain 'global' key.") + raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) + except TypeError: + desc = configexc.ConfigErrorDesc("While loading data", + "Toplevel object is not a dict.") + raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) + + if not isinstance(global_obj, dict): + desc = configexc.ConfigErrorDesc( + "While loading data", + "'global' object is not a dict") + raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) + + self.values = global_obj class ConfigAPI: diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index adee2c9b3..384fcc16e 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -601,7 +601,6 @@ class TestConfig: assert caplog.records[0].message == expected_message def test_read_yaml(self, conf): - # FIXME:conf what about wrong values? assert not conf._yaml.loaded conf._yaml.values['content.plugins'] = True @@ -610,6 +609,11 @@ class TestConfig: assert conf._yaml.loaded assert conf._values['content.plugins'] is True + def test_read_yaml_invalid(self, conf): + conf._yaml.values['foo.bar'] = True + with pytest.raises(configexc.NoOptionError): + conf.read_yaml() + def test_get_opt_valid(self, conf): assert conf.get_opt('tabs.show') == configdata.DATA['tabs.show'] @@ -878,13 +882,17 @@ def init_patch(qapp, fake_save_manager, monkeypatch, config_tmpdir, @pytest.mark.parametrize('load_autoconfig', [True, False]) @pytest.mark.parametrize('config_py', [True, 'error', False]) +@pytest.mark.parametrize('invalid_yaml', [True, False]) def test_init(qtbot, init_patch, fake_save_manager, config_tmpdir, - load_autoconfig, config_py): + load_autoconfig, config_py, invalid_yaml): autoconfig_file = config_tmpdir / 'autoconfig.yml' config_py_file = config_tmpdir / 'config.py' - autoconfig_file.write_text('global:\n colors.hints.fg: magenta\n', - 'utf-8', ensure=True) + if invalid_yaml: + autoconfig_file.write_text('42', 'utf-8', ensure=True) + else: + autoconfig_file.write_text('global:\n colors.hints.fg: magenta\n', + 'utf-8', ensure=True) if config_py: config_py_lines = ['c.colors.hints.bg = "red"'] @@ -900,8 +908,12 @@ def test_init(qtbot, init_patch, fake_save_manager, config_tmpdir, qtbot.add_widget(config._errbox) expected = "Errors occurred while reading config.py:" assert config._errbox.text().strip().startswith(expected) + elif invalid_yaml and (load_autoconfig or not config_py): + qtbot.add_widget(config._errbox) + expected = "Errors occurred while reading autoconfig.yml:" + assert config._errbox.text().strip().startswith(expected) else: - assert config._errbox is None + assert config._errbox is None, config._errbox.text() objreg.get('config-commands') assert isinstance(config.instance, config.Config) @@ -911,13 +923,15 @@ def test_init(qtbot, init_patch, fake_save_manager, config_tmpdir, fake_save_manager.add_saveable.assert_any_call( 'yaml-config', unittest.mock.ANY) - if config_py and load_autoconfig: + if config_py and load_autoconfig and not invalid_yaml: assert config.instance._values == { 'colors.hints.bg': 'red', 'colors.hints.fg': 'magenta', } elif config_py: assert config.instance._values == {'colors.hints.bg': 'red'} + elif invalid_yaml: + assert config.instance._values == {} else: assert config.instance._values == {'colors.hints.fg': 'magenta'}