diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index a155726a8..2ee973730 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -21,6 +21,7 @@ import types import os.path +import sys import textwrap import traceback import configparser @@ -250,7 +251,15 @@ def read_config_py(filename=None): raise configexc.ConfigFileErrors(basename, [desc]) try: - exec(code, module.__dict__) + # Save and restore sys variables + with saved_sys_properties(): + # Add config directory to python path, so config.py can import + # other files in logical places + config_dir = os.path.dirname(filename) + if config_dir not in sys.path: + sys.path.insert(0, config_dir) + + exec(code, module.__dict__) except Exception as e: api.errors.append(configexc.ConfigErrorDesc( "Unhandled exception", @@ -260,6 +269,20 @@ def read_config_py(filename=None): return api +@contextlib.contextmanager +def saved_sys_properties(): + """Save various sys properties such as sys.path and sys.modules.""" + old_path = sys.path.copy() + old_modules = sys.modules.copy() + + try: + yield + finally: + sys.path = old_path + for module in set(sys.modules).difference(old_modules): + del sys.modules[module] + + def init(): """Initialize config storage not related to the main config.""" global state diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 975e9c46a..bf214fed3 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -19,6 +19,7 @@ """Tests for qutebrowser.config.configfiles.""" import os +import sys import pytest @@ -207,32 +208,103 @@ class TestYaml: assert error.traceback is None +class ConfPy: + + """Helper class to get a confpy fixture.""" + + def __init__(self, tmpdir, filename: str = "config.py"): + self._file = tmpdir / filename + self.filename = str(self._file) + + def write(self, *lines): + text = '\n'.join(lines) + self._file.write_text(text, 'utf-8', ensure=True) + + def read(self): + """Read the config.py via configfiles and check for errors.""" + api = configfiles.read_config_py(self.filename) + assert not api.errors + + def write_qbmodule(self): + self.write('import qbmodule', + 'qbmodule.run(config)') + + +class TestConfigPyModules: + + pytestmark = pytest.mark.usefixtures('config_stub', 'key_config_stub') + + @pytest.fixture + def confpy(self, tmpdir): + return ConfPy(tmpdir) + + @pytest.fixture + def qbmodulepy(self, tmpdir): + return ConfPy(tmpdir, filename="qbmodule.py") + + @pytest.fixture(autouse=True) + def restore_sys_path(self): + old_path = sys.path.copy() + yield + sys.path = old_path + + def test_bind_in_module(self, confpy, qbmodulepy, tmpdir): + qbmodulepy.write('def run(config):', + ' config.bind(",a", "message-info foo", mode="normal")') + confpy.write_qbmodule() + confpy.read() + expected = {'normal': {',a': 'message-info foo'}} + assert config.instance._values['bindings.commands'] == expected + assert "qbmodule" not in sys.modules.keys() + assert tmpdir not in sys.path + + def test_restore_sys_on_err(self, confpy, qbmodulepy, tmpdir): + confpy.write_qbmodule() + qbmodulepy.write('def run(config):', + ' 1/0') + api = configfiles.read_config_py(confpy.filename) + + assert len(api.errors) == 1 + error = api.errors[0] + assert error.text == "Unhandled exception" + assert isinstance(error.exception, ZeroDivisionError) + assert "qbmodule" not in sys.modules.keys() + assert tmpdir not in sys.path + + def test_fail_on_nonexistent_module(self, confpy, qbmodulepy, tmpdir): + qbmodulepy.write('def run(config):', + ' pass') + confpy.write('import foobar', + 'foobar.run(config)') + api = configfiles.read_config_py(confpy.filename) + + assert len(api.errors) == 1 + error = api.errors[0] + assert error.text == "Unhandled exception" + assert isinstance(error.exception, ImportError) + + tblines = error.traceback.strip().splitlines() + assert tblines[0] == "Traceback (most recent call last):" + assert tblines[-1].endswith("Error: No module named 'foobar'") + + def test_no_double_if_path_exists(self, confpy, qbmodulepy, tmpdir): + sys.path.insert(0, tmpdir) + confpy.write('import sys', + 'if sys.path[0] in sys.path[1:]:', + ' raise Exception("Path not expected")') + confpy.read() + assert sys.path.count(tmpdir) == 1 + + class TestConfigPy: """Tests for ConfigAPI and read_config_py().""" pytestmark = pytest.mark.usefixtures('config_stub', 'key_config_stub') - class ConfPy: - - """Helper class to get a confpy fixture.""" - - def __init__(self, tmpdir): - self._confpy = tmpdir / 'config.py' - self.filename = str(self._confpy) - - def write(self, *lines): - text = '\n'.join(lines) - self._confpy.write_text(text, 'utf-8', ensure=True) - - def read(self): - """Read the config.py via configfiles and check for errors.""" - api = configfiles.read_config_py(self.filename) - assert not api.errors - @pytest.fixture def confpy(self, tmpdir): - return self.ConfPy(tmpdir) + return ConfPy(tmpdir) @pytest.mark.parametrize('line', [ 'c.colors.hints.bg = "red"',