WIP: Make AbstractSignalBlocker a QObject and fix signals in threads

CHANGELOG.rst

@@ -1,9 +1,20 @@
 UNRELEASED
 ----------
 
-* Added official support for Python 3.13.
-* Dropped support for EOL Python 3.8.
-* Dropped support for EOL PySide 2.
+- Added official support for Python 3.13.
+- Dropped support for EOL Python 3.8.
+- Dropped support for EOL PySide 2.
+- Fixed PySide6 exceptions / warnings about being unable to disconnect signals
+  with ``qtbot.waitSignal`` (`#552`_, `#558`_).
+- Reduced the likelyhood of trouble when using ``qtbot.waitSignal(s)`` and
+  ``qtbot.waitCallback`` where the signal/callback is emitted from a non-main
+  thread. In theory, more problems remain and this isn't a proper fix yet. In
+  practice, it seems impossible to provoke any problems in pytest-qt's testsuite.
+  (`#586`_)
+
+..  _#552: https://github.com/pytest-dev/pytest-qt/issues/552
+..  _#558: https://github.com/pytest-dev/pytest-qt/issues/558
+..  _#586: https://github.com/pytest-dev/pytest-qt/issues/586
 
 4.4.0 (2024-02-07)
 ------------------

docs/wait_callback.rst

@@ -26,7 +26,7 @@ Anything following the ``with`` block will be run only after the callback has be
 
 If the callback doesn't get called during the given timeout,
 :class:`qtbot.TimeoutError <pytestqt.exceptions.TimeoutError>` is raised. If it is called more than once,
-:class:`qtbot.CallbackCalledTwiceError <pytestqt.wait_signal.CallbackCalledTwiceError>` is raised.
+:class:`qtbot.CallbackCalledTwiceError <pytestqt.exceptions.CallbackCalledTwiceError>` is raised.
 
 raising parameter
 -----------------

pytest.ini

@@ -2,5 +2,4 @@
 testpaths = tests
 addopts = --strict-markers --strict-config
 xfail_strict = true
-markers =
-  filterwarnings: pytest's filterwarnings marker
+filterwarnings = error

src/pytestqt/exceptions.py

@@ -116,3 +116,25 @@ class ScreenshotError(Exception):
 
         Access via ``qtbot.ScreenshotError``.
     """
+
+
+class SignalEmittedError(Exception):
+    """
+    .. versionadded:: 1.11
+
+    The exception thrown by :meth:`pytestqt.qtbot.QtBot.assertNotEmitted` if a
+    signal was emitted unexpectedly.
+
+    Access via ``qtbot.SignalEmittedError``.
+    """
+
+
+class CallbackCalledTwiceError(Exception):
+    """
+    .. versionadded:: 3.1
+
+    The exception thrown by :meth:`pytestqt.qtbot.QtBot.waitCallback` if a
+    callback was called twice.
+
+    Access via ``qtbot.CallbackCalledTwiceError``.
+    """

src/pytestqt/qtbot.py

@@ -2,16 +2,14 @@
 import weakref
 import warnings
 
-from pytestqt.exceptions import TimeoutError, ScreenshotError
-from pytestqt.qt_compat import qt_api
-from pytestqt.wait_signal import (
-    SignalBlocker,
-    MultiSignalBlocker,
-    SignalEmittedSpy,
+from pytestqt.exceptions import (
+    TimeoutError,
+    ScreenshotError,
     SignalEmittedError,
-    CallbackBlocker,
     CallbackCalledTwiceError,
 )
+from pytestqt.qt_compat import qt_api
+from pytestqt import wait_signal
 
 
 def _parse_ini_boolean(value):
@@ -350,7 +348,7 @@ def waitSignal(self, signal, *, timeout=5000, raising=None, check_params_cb=None
                 f"Passing None as signal isn't supported anymore, use qtbot.wait({timeout}) instead."
             )
         raising = self._should_raise(raising)
-        blocker = SignalBlocker(
+        blocker = wait_signal.SignalBlocker(
             timeout=timeout, raising=raising, check_params_cb=check_params_cb
         )
         blocker.connect(signal)
@@ -437,7 +435,7 @@ def waitSignals(
                         len(check_params_cbs), len(signals)
                     )
                 )
-        blocker = MultiSignalBlocker(
+        blocker = wait_signal.MultiSignalBlocker(
             timeout=timeout,
             raising=raising,
             order=order,
@@ -455,7 +453,7 @@ def wait(self, ms):
         While waiting, events will be processed and your test will stay
         responsive to user interface events or network communication.
         """
-        blocker = MultiSignalBlocker(timeout=ms, raising=False)
+        blocker = wait_signal.MultiSignalBlocker(timeout=ms, raising=False)
         blocker.wait()
 
     @contextlib.contextmanager
@@ -475,7 +473,7 @@ def assertNotEmitted(self, signal, *, wait=0):
         .. note:: This method is also available as ``assert_not_emitted``
                   (pep-8 alias)
         """
-        spy = SignalEmittedSpy(signal)
+        spy = wait_signal.SignalEmittedSpy(signal)
         with spy, self.waitSignal(signal, timeout=wait, raising=False):
             yield
         spy.assert_not_emitted()
@@ -589,7 +587,7 @@ def waitCallback(self, *, timeout=5000, raising=None):
         .. note:: This method is also available as ``wait_callback`` (pep-8 alias)
         """
         raising = self._should_raise(raising)
-        blocker = CallbackBlocker(timeout=timeout, raising=raising)
+        blocker = wait_signal.CallbackBlocker(timeout=timeout, raising=raising)
         return blocker
 
     @contextlib.contextmanager

src/pytestqt/utils.py

@@ -1,3 +1,7 @@
+import dataclasses
+from typing import Any
+
+
 def get_marker(item, name):
     """Get a marker from a pytest item.
 
@@ -9,3 +13,19 @@ def get_marker(item, name):
     except AttributeError:
         # pytest < 3.6
         return item.get_marker(name)
+
+
+@dataclasses.dataclass
+class SignalAndArgs:
+    signal_name: str
+    args: list[Any]
+
+    def __str__(self) -> str:
+        args = repr(self.args) if self.args else ""
+
+        # remove signal parameter signature, e.g. turn "some_signal(str,int)" to "some_signal", because we're adding
+        # the actual parameters anyways
+        signal_name = self.signal_name
+        signal_name = signal_name.partition("(")[0]
+
+        return signal_name + args

tests/conftest.py

@@ -1,4 +1,3 @@
-import functools
 import time
 
 import pytest
@@ -63,10 +62,9 @@ def shutdown(self):
         def single_shot(self, signal, delay):
             t = qt_api.QtCore.QTimer(self)
             t.setSingleShot(True)
-            slot = functools.partial(self._emit, signal)
-            t.timeout.connect(slot)
+            t.timeout.connect(signal)
             t.start(delay)
-            self.timers_and_slots.append((t, slot))
+            self.timers_and_slots.append((t, signal))
 
         def single_shot_callback(self, callback, delay):
             t = qt_api.QtCore.QTimer(self)
@@ -75,9 +73,6 @@ def single_shot_callback(self, callback, delay):
             t.start(delay)
             self.timers_and_slots.append((t, callback))
 
-        def _emit(self, signal):
-            signal.emit()
-
     timer = Timer()
     yield timer
     timer.shutdown()

tests/test_wait_signal.py

@@ -5,10 +5,10 @@
 import sys
 
 from pytestqt.qt_compat import qt_api
-from pytestqt.wait_signal import (
+from pytestqt.utils import SignalAndArgs
+from pytestqt.exceptions import (
     SignalEmittedError,
     TimeoutError,
-    SignalAndArgs,
     CallbackCalledTwiceError,
 )
 
@@ -1364,3 +1364,134 @@ def test_timeout_not_raising(self, qtbot):
         assert not callback.called
         assert callback.args is None
         assert callback.kwargs is None
+
+
+@pytest.mark.parametrize(
+    "check_warnings, count",
+    [
+        # Checking for warnings
+        pytest.param(
+            True,  # check warnings
+            200,  # gets output reliably even with only few runs (often the first)
+            id="stderr",
+        ),
+        # Triggering AttributeError
+        pytest.param(
+            False,  # don't check warnings
+            # Hopefully enough to trigger the AttributeError race condition reliably.
+            # With 500 runs, only 1 of 5 Windows PySide6 CI jobs triggered it (but all
+            # Ubuntu/macOS jobs did).  With 1500 runs, Windows jobs still only triggered
+            # it 0-2 times.
+            #
+            # On my machine (Linux, Intel Core Ultra 9 185H), 500 runs trigger it
+            # reliably and take ~1s in total.
+            2500 if sys.platform == "win32" else 500,
+            id="attributeerror",
+        ),
+    ],
+)
+@pytest.mark.parametrize("multi_blocker", [True, False])
+def test_signal_raised_from_thread(
+    monkeypatch: pytest.MonkeyPatch,
+    pytester: pytest.Pytester,
+    check_warnings: bool,
+    multi_blocker: bool,
+    count: int,
+) -> None:
+    """Wait for a signal with a thread.
+
+    Extracted from https://github.com/pytest-dev/pytest-qt/issues/586
+    """
+    pytester.makepyfile(
+        f"""
+        import pytest
+        from pytestqt.qt_compat import qt_api
+
+
+        class Worker(qt_api.QtCore.QObject):
+            signal = qt_api.Signal()
+
+
+        @pytest.mark.parametrize("_", range({count}))
+        def test_thread(qtbot, _):
+            worker = Worker()
+            thread = qt_api.QtCore.QThread()
+            worker.moveToThread(thread)
+            thread.start()
+
+            try:
+                if {multi_blocker}:  # multi_blocker
+                    with qtbot.waitSignals([worker.signal], timeout=500) as blocker:
+                        worker.signal.emit()
+                else:
+                    with qtbot.waitSignal(worker.signal, timeout=500) as blocker:
+                        worker.signal.emit()
+            finally:
+                thread.quit()
+                thread.wait()
+    """
+    )
+    if check_warnings:
+        monkeypatch.setenv("QT_FATAL_WARNINGS", "1")
+    res = pytester.runpytest_subprocess("-x", "-s")
+
+    qtimer_message = "QObject::killTimer: Timers cannot be stopped from another thread"
+    if (
+        qtimer_message in res.stderr.str()
+        and multi_blocker
+        and check_warnings
+        and qt_api.pytest_qt_api == "pyside6"
+    ):
+        # We haven't fixed MultiSignalBlocker yet...
+        pytest.xfail(f"Qt error: {qtimer_message}")
+
+    outcomes = res.parseoutcomes()
+    res.assert_outcomes(passed=outcomes["passed"])  # no failed/error
+
+
+@pytest.mark.skip(reason="Runs ~1min to reproduce bug reliably")
+def test_callback_in_thread(pytester: pytest.Pytester) -> None:
+    """Wait for a callback with a thread.
+
+    Inspired by https://github.com/pytest-dev/pytest-qt/issues/586
+    """
+    # Hopefully enough to trigger the bug reliably.
+    #
+    # On my machine (Linux, Intel Core Ultra 9 185H), sometimes the bug only
+    # triggers after ~30k runs (~44s). Thus, we skip this test by default.
+    count = 50_000
+
+    pytester.makepyfile(
+        f"""
+        import pytest
+        from pytestqt.qt_compat import qt_api
+
+
+        class Worker(qt_api.QtCore.QObject):
+            def __init__(self, callback):
+                super().__init__()
+                self.callback = callback
+
+            def call_callback(self):
+                self.callback()
+
+
+        @pytest.mark.parametrize("_", range({count}))
+        def test_thread(qtbot, _):
+            thread = qt_api.QtCore.QThread()
+
+            try:
+                with qtbot.waitCallback() as callback:
+                    worker = Worker(callback)
+                    worker.moveToThread(thread)
+                    thread.started.connect(worker.call_callback)
+                    thread.start()
+            finally:
+                thread.quit()
+                thread.wait()
+    """
+    )
+
+    res = pytester.runpytest_subprocess("-x")
+    outcomes = res.parseoutcomes()
+    res.assert_outcomes(passed=outcomes["passed"])  # no failed/error