From 63e96fa3fe2c86a840cf84ade7fa0429843a9cdf Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 13 Jun 2023 17:01:15 +0200 Subject: qt: Split machinery.init() and init_explicit() into two functions This also moves the checking for sys.modules into _select_wrapper. --- qutebrowser/qt/core.py | 2 +- qutebrowser/qt/dbus.py | 2 +- qutebrowser/qt/gui.py | 2 +- qutebrowser/qt/machinery.py | 108 ++++++++++-------- qutebrowser/qt/network.py | 2 +- qutebrowser/qt/opengl.py | 2 +- qutebrowser/qt/printsupport.py | 2 +- qutebrowser/qt/qml.py | 2 +- qutebrowser/qt/sip.py | 2 +- qutebrowser/qt/sql.py | 2 +- qutebrowser/qt/test.py | 2 +- qutebrowser/qt/webenginecore.py | 2 +- qutebrowser/qt/webenginewidgets.py | 2 +- qutebrowser/qt/webkit.py | 2 +- qutebrowser/qt/webkitwidgets.py | 2 +- qutebrowser/qt/widgets.py | 2 +- tests/unit/test_qt_machinery.py | 222 ++++++++++++++++++++----------------- 17 files changed, 198 insertions(+), 162 deletions(-) diff --git a/qutebrowser/qt/core.py b/qutebrowser/qt/core.py index 5cc437552..d9323f877 100644 --- a/qutebrowser/qt/core.py +++ b/qutebrowser/qt/core.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtcore-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/dbus.py b/qutebrowser/qt/dbus.py index e9ee15ff8..350c98dc5 100644 --- a/qutebrowser/qt/dbus.py +++ b/qutebrowser/qt/dbus.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtdbus-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/gui.py b/qutebrowser/qt/gui.py index edc3adaea..6c08bc3c4 100644 --- a/qutebrowser/qt/gui.py +++ b/qutebrowser/qt/gui.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtgui-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/machinery.py b/qutebrowser/qt/machinery.py index f86edaf40..d4479a247 100644 --- a/qutebrowser/qt/machinery.py +++ b/qutebrowser/qt/machinery.py @@ -139,6 +139,11 @@ def _select_wrapper(args: Optional[argparse.Namespace]) -> SelectionInfo: - Otherwise, if the QUTE_QT_WRAPPER environment variable is set, use that. - Otherwise, use PyQt5 (FIXME:qt6 autoselect). """ + for name in WRAPPERS: + # If any Qt wrapper has been imported before this, all hope is lost. + if name in sys.modules: + raise Error(f"{name} already imported") + if args is not None and args.qt_wrapper is not None: assert args.qt_wrapper in WRAPPERS, args.qt_wrapper # ensured by argparse return SelectionInfo(wrapper=args.qt_wrapper, reason=SelectionReason.cli) @@ -190,56 +195,24 @@ IS_PYSIDE: bool _initialized = False -def init(args: Optional[argparse.Namespace] = None) -> SelectionInfo: - """Initialize Qt wrapper globals. +def _set_globals(info: SelectionInfo) -> None: + """Set all global variables in this module based on the given SelectionInfo. - There is two ways how this function can be called: - - - Explicitly, during qutebrowser startup, where it gets called before - earlyinit.early_init() in qutebrowser.py (i.e. after we have an argument - parser, but before any kinds of Qt usage). This allows `args` to be passed, - which is used to select the Qt wrapper (if --qt-wrapper is given). - - - Implicitly, when any of the qutebrowser.qt.* modules in this package is imported. - This should never happen during normal qutebrowser usage, but means that any - qutebrowser module can be imported without having to worry about machinery.init(). - This is useful for e.g. tests or manual interactive usage of the qutebrowser code. - In this case, `args` will be None. + Those are split into multiple global variables because that way we can teach mypy + about them via --always-true and --always-false, see tox.ini. """ global INFO, USE_PYQT5, USE_PYQT6, USE_PYSIDE6, IS_QT5, IS_QT6, \ IS_PYQT, IS_PYSIDE, _initialized - if args is None: - # Implicit initialization can happen multiple times - # (all subsequent calls are a no-op) - if _initialized: - return None # FIXME:qt6 - else: - # Explicit initialization can happen exactly once, and if it's used, there - # should not be any implicit initialization (qutebrowser.qt imports) before it. - if _initialized: # pylint: disable=else-if-used - raise Error("init() already called before application init") - _initialized = True - - for name in WRAPPERS: - # If any Qt wrapper has been imported before this, all hope is lost. - if name in sys.modules: - raise Error(f"{name} already imported") + assert info.wrapper is not None, info + assert not _initialized - INFO = _select_wrapper(args) - if INFO.wrapper is None: - # No Qt wrapper was importable. - if args is None: - # Implicit initialization -> raise error immediately - raise NoWrapperAvailableError(INFO) - else: - # Explicit initialization -> show error in earlyinit.py - return INFO - - USE_PYQT5 = INFO.wrapper == "PyQt5" - USE_PYQT6 = INFO.wrapper == "PyQt6" - USE_PYSIDE6 = INFO.wrapper == "PySide6" - assert USE_PYQT5 ^ USE_PYQT6 ^ USE_PYSIDE6 + _initialized = True + INFO = info + USE_PYQT5 = info.wrapper == "PyQt5" + USE_PYQT6 = info.wrapper == "PyQt6" + USE_PYSIDE6 = info.wrapper == "PySide6" + assert USE_PYQT5 + USE_PYQT6 + USE_PYSIDE6 == 1 IS_QT5 = USE_PYQT5 IS_QT6 = USE_PYQT6 or USE_PYSIDE6 @@ -248,4 +221,49 @@ def init(args: Optional[argparse.Namespace] = None) -> SelectionInfo: assert IS_QT5 ^ IS_QT6 assert IS_PYQT ^ IS_PYSIDE - return INFO + +def init_implicit() -> None: + """Initialize Qt wrapper globals implicitly at Qt import time. + + This gets called when any qutebrowser.qt module is imported, and implicitly + initializes the Qt wrapper globals. + + After this is called, no explicit initialization via machinery.init() is possible + anymore - thus, this should never be called before init() when running qutebrowser + as an application (and any further calls will be a no-op). + + However, this ensures that any qutebrowser module can be imported without + having to worry about machinery.init(). This is useful for e.g. tests or + manual interactive usage of the qutebrowser code. + """ + if _initialized: + # Implicit initialization can happen multiple times + # (all subsequent calls are a no-op) + return None + + info = _select_wrapper(args=None) + if info.wrapper is None: + raise NoWrapperAvailableError(info) + + _set_globals(info) + + +def init(args: argparse.Namespace) -> SelectionInfo: + """Initialize Qt wrapper globals during qutebrowser application start. + + This gets called from earlyinit.py, i.e. after we have an argument parser, + but before any kinds of Qt usage. This allows `args` to be passed, which is + used to select the Qt wrapper (if --qt-wrapper is given). + + If any qutebrowser.qt module is imported before this, init_implicit() will be called + instead, which means this can't be called anymore. + """ + if _initialized: + raise Error("init() already called before application init") + + info = _select_wrapper(args) + # If info is None here (no Qt wrapper available), we'll show an error later + # in earlyinit.py. + + _set_globals(info) + return info diff --git a/qutebrowser/qt/network.py b/qutebrowser/qt/network.py index 2a665a3a9..9fbd91ec3 100644 --- a/qutebrowser/qt/network.py +++ b/qutebrowser/qt/network.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtnetwork-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/opengl.py b/qutebrowser/qt/opengl.py index 5c3af3a5a..6f94fa3d9 100644 --- a/qutebrowser/qt/opengl.py +++ b/qutebrowser/qt/opengl.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtopengl-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/printsupport.py b/qutebrowser/qt/printsupport.py index e733bf828..c1736b7e3 100644 --- a/qutebrowser/qt/printsupport.py +++ b/qutebrowser/qt/printsupport.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtprintsupport-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/qml.py b/qutebrowser/qt/qml.py index efb6901f2..da45afb36 100644 --- a/qutebrowser/qt/qml.py +++ b/qutebrowser/qt/qml.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtqml-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/sip.py b/qutebrowser/qt/sip.py index 860c1ee4f..7e4bf246d 100644 --- a/qutebrowser/qt/sip.py +++ b/qutebrowser/qt/sip.py @@ -16,7 +16,7 @@ Note that we don't yet abstract between PySide/PyQt here. from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: # pylint: disable=no-else-raise raise machinery.Unavailable() diff --git a/qutebrowser/qt/sql.py b/qutebrowser/qt/sql.py index 825d0acc8..c2ed624ac 100644 --- a/qutebrowser/qt/sql.py +++ b/qutebrowser/qt/sql.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtsql-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/test.py b/qutebrowser/qt/test.py index 11753a622..d1d266902 100644 --- a/qutebrowser/qt/test.py +++ b/qutebrowser/qt/test.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qttest-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/webenginecore.py b/qutebrowser/qt/webenginecore.py index fcffef9b9..f45d13f54 100644 --- a/qutebrowser/qt/webenginecore.py +++ b/qutebrowser/qt/webenginecore.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtwebenginecore-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/webenginewidgets.py b/qutebrowser/qt/webenginewidgets.py index 9bb62db1f..9d11d5f79 100644 --- a/qutebrowser/qt/webenginewidgets.py +++ b/qutebrowser/qt/webenginewidgets.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtwebenginewidgets-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/webkit.py b/qutebrowser/qt/webkit.py index c8b5c0b39..17516d96c 100644 --- a/qutebrowser/qt/webkit.py +++ b/qutebrowser/qt/webkit.py @@ -15,7 +15,7 @@ https://qtwebkit.github.io/doc/qtwebkit/qtwebkit-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: # pylint: disable=no-else-raise diff --git a/qutebrowser/qt/webkitwidgets.py b/qutebrowser/qt/webkitwidgets.py index 2c60909cc..d6e7254f6 100644 --- a/qutebrowser/qt/webkitwidgets.py +++ b/qutebrowser/qt/webkitwidgets.py @@ -15,7 +15,7 @@ https://qtwebkit.github.io/doc/qtwebkit/qtwebkitwidgets-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/qutebrowser/qt/widgets.py b/qutebrowser/qt/widgets.py index 44b677f09..0f43779a6 100644 --- a/qutebrowser/qt/widgets.py +++ b/qutebrowser/qt/widgets.py @@ -14,7 +14,7 @@ https://doc.qt.io/qt-6/qtwidgets-index.html from qutebrowser.qt import machinery -machinery.init() +machinery.init_implicit() if machinery.USE_PYSIDE6: diff --git a/tests/unit/test_qt_machinery.py b/tests/unit/test_qt_machinery.py index e9caf5ff0..4af5a78f3 100644 --- a/tests/unit/test_qt_machinery.py +++ b/tests/unit/test_qt_machinery.py @@ -29,6 +29,15 @@ import pytest from qutebrowser.qt import machinery +@pytest.fixture +def undo_init(monkeypatch: pytest.MonkeyPatch) -> None: + """Pretend Qt support isn't initialized yet and Qt was never imported.""" + monkeypatch.setattr(machinery, "_initialized", False) + monkeypatch.delenv("QUTE_QT_WRAPPER", raising=False) + for wrapper in machinery.WRAPPERS: + monkeypatch.delitem(sys.modules, wrapper, raising=False) + + @pytest.mark.parametrize( "exception", [ @@ -81,9 +90,10 @@ def test_selectioninfo_set_module(): "PyQt5: ImportError: Python imploded\n" "PyQt6: success\n" "selected: PyQt6 (via autoselect)" - ) + ), ), - ]) + ], +) def test_selectioninfo_str(info: machinery.SelectionInfo, expected: str): assert str(info) == expected @@ -236,6 +246,7 @@ def test_select_wrapper( env: Optional[str], expected: machinery.SelectionInfo, monkeypatch: pytest.MonkeyPatch, + undo_init: None, ): if env is None: monkeypatch.delenv("QUTE_QT_WRAPPER", raising=False) @@ -245,109 +256,116 @@ def test_select_wrapper( assert machinery._select_wrapper(args) == expected -@pytest.fixture -def undo_init(monkeypatch: pytest.MonkeyPatch) -> None: - """Pretend Qt support isn't initialized yet and Qt was never imported.""" - for wrapper in machinery.WRAPPERS: - monkeypatch.delitem(sys.modules, wrapper, raising=False) - monkeypatch.setattr(machinery, "_initialized", False) - monkeypatch.delenv("QUTE_QT_WRAPPER", raising=False) - - -def test_init_multiple_implicit(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setattr(machinery, "_initialized", True) - machinery.init() - machinery.init() - - -def test_init_multiple_explicit(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setattr(machinery, "_initialized", True) - machinery.init() - - with pytest.raises( - machinery.Error, match=r"init\(\) already called before application init" - ): - machinery.init(args=argparse.Namespace(qt_wrapper="PyQt6")) - - -def test_init_after_qt_import(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setattr(machinery, "_initialized", False) +def test_select_wrapper_after_qt_import(): + assert any(wrapper in sys.modules for wrapper in machinery.WRAPPERS) with pytest.raises(machinery.Error, match="Py.* already imported"): - machinery.init() - + machinery._select_wrapper(args=None) -@pytest.mark.xfail(reason="autodetect not used yet") -def test_init_none_available_implicit( - stubs: Any, - modules: Dict[str, bool], - monkeypatch: pytest.MonkeyPatch, - undo_init: None, -): - stubs.ImportFake(modules, monkeypatch).patch() - message = "No Qt wrapper was importable." # FIXME maybe check info too - with pytest.raises(machinery.NoWrapperAvailableError, match=message): - machinery.init(args=None) +class TestInit: + @pytest.fixture + def empty_args(self) -> argparse.Namespace: + return argparse.Namespace(qt_wrapper=None) -@pytest.mark.xfail(reason="autodetect not used yet") -def test_init_none_available_explicit( - stubs: Any, - modules: Dict[str, bool], - monkeypatch: pytest.MonkeyPatch, - undo_init: None, -): - stubs.ImportFake(modules, monkeypatch).patch() - info = machinery.init(args=argparse.Namespace(qt_wrapper=None)) - assert info == machinery.SelectionInfo( - wrapper=None, - reason=machinery.SelectionReason.default, - pyqt6="ImportError: Fake ImportError for PyQt6.", - pyqt5="ImportError: Fake ImportError for PyQt5.", - ) - + def test_multiple_implicit(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.setattr(machinery, "_initialized", True) + machinery.init_implicit() + machinery.init_implicit() -@pytest.mark.parametrize( - "selected_wrapper, true_vars", - [ - ("PyQt6", ["USE_PYQT6", "IS_QT6", "IS_PYQT"]), - ("PyQt5", ["USE_PYQT5", "IS_QT5", "IS_PYQT"]), - ("PySide6", ["USE_PYSIDE6", "IS_QT6", "IS_PYSIDE"]), - ], -) -def test_init_properly( - monkeypatch: pytest.MonkeyPatch, - selected_wrapper: str, - true_vars: str, - undo_init: None, -): - bool_vars = [ - "USE_PYQT5", - "USE_PYQT6", - "USE_PYSIDE6", - "IS_QT5", - "IS_QT6", - "IS_PYQT", - "IS_PYSIDE", - ] - all_vars = bool_vars + ["INFO"] - # Make sure we didn't forget anything that's declared in the module. - # Not sure if this is a good idea. Might remove it in the future if it breaks. - assert set(typing.get_type_hints(machinery).keys()) == set(all_vars) - - for var in all_vars: - monkeypatch.delattr(machinery, var) - - info = machinery.SelectionInfo( - wrapper=selected_wrapper, - reason=machinery.SelectionReason.fake, + def test_multiple_explicit( + self, + monkeypatch: pytest.MonkeyPatch, + empty_args: argparse.Namespace, + ): + monkeypatch.setattr(machinery, "_initialized", True) + + with pytest.raises( + machinery.Error, match=r"init\(\) already called before application init" + ): + machinery.init(args=empty_args) + + @pytest.mark.xfail(reason="autodetect not used yet") + def test_none_available_implicit( + self, + stubs: Any, + modules: Dict[str, bool], + monkeypatch: pytest.MonkeyPatch, + undo_init: None, + ): + stubs.ImportFake(modules, monkeypatch).patch() + message = "No Qt wrapper was importable." # FIXME maybe check info too + with pytest.raises(machinery.NoWrapperAvailableError, match=message): + machinery.init_implicit() + + @pytest.mark.xfail(reason="autodetect not used yet") + def test_none_available_explicit( + self, + stubs: Any, + modules: Dict[str, bool], + monkeypatch: pytest.MonkeyPatch, + empty_args: argparse.Namespace, + undo_init: None, + ): + stubs.ImportFake(modules, monkeypatch).patch() + info = machinery.init(args=empty_args) + assert info == machinery.SelectionInfo( + wrapper=None, + reason=machinery.SelectionReason.default, + pyqt6="ImportError: Fake ImportError for PyQt6.", + pyqt5="ImportError: Fake ImportError for PyQt5.", + ) + + @pytest.mark.parametrize( + "selected_wrapper, true_vars", + [ + ("PyQt6", ["USE_PYQT6", "IS_QT6", "IS_PYQT"]), + ("PyQt5", ["USE_PYQT5", "IS_QT5", "IS_PYQT"]), + ("PySide6", ["USE_PYSIDE6", "IS_QT6", "IS_PYSIDE"]), + ], ) - monkeypatch.setattr(machinery, "_select_wrapper", lambda args: info) - - machinery.init() - assert machinery.INFO == info - - expected_vars = dict.fromkeys(bool_vars, False) - expected_vars.update(dict.fromkeys(true_vars, True)) - actual_vars = {var: getattr(machinery, var) for var in bool_vars} - - assert expected_vars == actual_vars + @pytest.mark.parametrize("explicit", [True, False]) + def test_properly( + self, + monkeypatch: pytest.MonkeyPatch, + selected_wrapper: str, + true_vars: str, + explicit: bool, + empty_args: argparse.Namespace, + undo_init: None, + ): + bool_vars = [ + "USE_PYQT5", + "USE_PYQT6", + "USE_PYSIDE6", + "IS_QT5", + "IS_QT6", + "IS_PYQT", + "IS_PYSIDE", + ] + all_vars = bool_vars + ["INFO"] + # Make sure we didn't forget anything that's declared in the module. + # Not sure if this is a good idea. Might remove it in the future if it breaks. + assert set(typing.get_type_hints(machinery).keys()) == set(all_vars) + + for var in all_vars: + monkeypatch.delattr(machinery, var) + + info = machinery.SelectionInfo( + wrapper=selected_wrapper, + reason=machinery.SelectionReason.fake, + ) + monkeypatch.setattr(machinery, "_select_wrapper", lambda args: info) + + if explicit: + ret = machinery.init(empty_args) + assert ret == info + else: + machinery.init_implicit() + + assert machinery.INFO == info + + expected_vars = dict.fromkeys(bool_vars, False) + expected_vars.update(dict.fromkeys(true_vars, True)) + actual_vars = {var: getattr(machinery, var) for var in bool_vars} + + assert expected_vars == actual_vars -- cgit v1.2.3-54-g00ecf