From 92243041c24b23178d4d0c860d52bc442490d291 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <me@the-compiler.org>
Date: Tue, 13 Jun 2023 19:51:22 +0200
Subject: qt: Stop trying other wrappers on ImportError

---
 qutebrowser/qt/machinery.py     |  8 +++++-
 tests/helpers/stubs.py          |  5 +++-
 tests/unit/test_qt_machinery.py | 57 +++++++++++++++++++++++++++++------------
 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/qutebrowser/qt/machinery.py b/qutebrowser/qt/machinery.py
index 4ef9017be..3ed90f27d 100644
--- a/qutebrowser/qt/machinery.py
+++ b/qutebrowser/qt/machinery.py
@@ -124,10 +124,16 @@ def _autoselect_wrapper() -> SelectionInfo:
     for wrapper in WRAPPERS:
         try:
             importlib.import_module(wrapper)
-        except ImportError as e:
+        except ModuleNotFoundError as e:
+            # Wrapper not available -> try the next one.
             info.set_module(wrapper, f"{type(e).__name__}: {e}")
             continue
+        except ImportError as e:
+            # Any other ImportError -> stop to surface the error.
+            info.set_module(wrapper, f"{type(e).__name__}: {e}")
+            break
 
+        # Wrapper imported successfully -> use it.
         info.set_module(wrapper, "success")
         info.wrapper = wrapper
         return info
diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py
index 134028012..dcc38ee10 100644
--- a/tests/helpers/stubs.py
+++ b/tests/helpers/stubs.py
@@ -695,7 +695,8 @@ class ImportFake:
 
     Attributes:
         modules: A dict mapping module names to bools. If True, the import will
-                 succeed. Otherwise, it'll fail with ImportError.
+                 succeed. If an exception is given, it will be raised.
+                 Otherwise, it'll fail with a fake ImportError.
         version_attribute: The name to use in the fake modules for the version
                            attribute.
         version: The version to use for the modules.
@@ -727,6 +728,8 @@ class ImportFake:
         if name not in self.modules:
             # Not one of the modules to test -> use real import
             return None
+        elif isinstance(self.modules[name], Exception):
+            raise self.modules[name]
         elif self.modules[name]:
             ns = types.SimpleNamespace()
             if self.version_attribute is not None:
diff --git a/tests/unit/test_qt_machinery.py b/tests/unit/test_qt_machinery.py
index eb2f6fa1e..ed1e53aa7 100644
--- a/tests/unit/test_qt_machinery.py
+++ b/tests/unit/test_qt_machinery.py
@@ -22,7 +22,7 @@
 import sys
 import argparse
 import typing
-from typing import Any, Optional, Dict, List
+from typing import Any, Optional, Dict, Union
 
 import pytest
 
@@ -126,51 +126,74 @@ def modules():
 @pytest.mark.parametrize(
     "available, expected",
     [
-        (
-            [],
+        pytest.param(
+            {
+                "PyQt5": ModuleNotFoundError("hiding somewhere"),
+                "PyQt6": ModuleNotFoundError("hiding somewhere"),
+            },
             machinery.SelectionInfo(
                 wrapper=None,
                 reason=machinery.SelectionReason.auto,
-                pyqt6="ImportError: Fake ImportError for PyQt6.",
-                pyqt5="ImportError: Fake ImportError for PyQt5.",
+                pyqt6="ModuleNotFoundError: hiding somewhere",
+                pyqt5="ModuleNotFoundError: hiding somewhere",
             ),
+            id="none-available",
         ),
-        (
-            ["PyQt6"],
+        pytest.param(
+            {
+                "PyQt5": ModuleNotFoundError("hiding somewhere"),
+                "PyQt6": True,
+            },
             machinery.SelectionInfo(
                 wrapper="PyQt6", reason=machinery.SelectionReason.auto, pyqt6="success"
             ),
+            id="only-pyqt6",
         ),
-        (
-            ["PyQt5"],
+        pytest.param(
+            {
+                "PyQt5": True,
+                "PyQt6": ModuleNotFoundError("hiding somewhere"),
+            },
             machinery.SelectionInfo(
                 wrapper="PyQt5",
                 reason=machinery.SelectionReason.auto,
-                pyqt6="ImportError: Fake ImportError for PyQt6.",
+                pyqt6="ModuleNotFoundError: hiding somewhere",
                 pyqt5="success",
             ),
+            id="only-pyqt5",
         ),
-        (
-            ["PyQt5", "PyQt6"],
+        pytest.param(
+            {"PyQt5": True, "PyQt6": True},
             machinery.SelectionInfo(
                 wrapper="PyQt6",
                 reason=machinery.SelectionReason.auto,
                 pyqt6="success",
                 pyqt5=None,
             ),
+            id="both",
+        ),
+        pytest.param(
+            {
+                "PyQt6": ImportError("Fake ImportError for PyQt6."),
+                "PyQt5": True,
+            },
+            machinery.SelectionInfo(
+                wrapper=None,
+                reason=machinery.SelectionReason.auto,
+                pyqt6="ImportError: Fake ImportError for PyQt6.",
+                pyqt5=None,
+            ),
+            id="import-error",
         ),
     ],
 )
 def test_autoselect(
     stubs: Any,
-    modules: Dict[str, bool],
-    available: List[str],
+    available: Dict[str, Union[bool, Exception]],
     expected: machinery.SelectionInfo,
     monkeypatch: pytest.MonkeyPatch,
 ):
-    for wrapper in available:
-        modules[wrapper] = True
-    stubs.ImportFake(modules, monkeypatch).patch()
+    stubs.ImportFake(available, monkeypatch).patch()
     assert machinery._autoselect_wrapper() == expected
 
 
-- 
cgit v1.2.3-54-g00ecf