diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5f354b4..88aeb9b 100644 --- a/CHANGELOG.rst +++ b/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) ------------------ diff --git a/pytest.ini b/pytest.ini index 9ade678..26b0a16 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,5 +2,4 @@ testpaths = tests addopts = --strict-markers --strict-config xfail_strict = true -markers = - filterwarnings: pytest's filterwarnings marker +filterwarnings = error diff --git a/src/pytestqt/wait_signal.py b/src/pytestqt/wait_signal.py index 8da3836..04c66b6 100644 --- a/src/pytestqt/wait_signal.py +++ b/src/pytestqt/wait_signal.py @@ -24,12 +24,12 @@ def __init__(self, timeout=5000, raising=True): self.raising = raising self._signals = None # will be initialized by inheriting implementations self._timeout_message = "" - if timeout is None or timeout == 0: - self._timer = None - else: - self._timer = qt_api.QtCore.QTimer(self._loop) - self._timer.setSingleShot(True) + + self._timer = qt_api.QtCore.QTimer(self._loop) + self._timer.setSingleShot(True) + if timeout is not None: self._timer.setInterval(timeout) + self._timer.timeout.connect(self._quit_loop_by_timeout) def wait(self): """ @@ -43,11 +43,13 @@ def wait(self): return if self.timeout is None and not self._signals: raise ValueError("No signals or timeout specified.") - if self._timer is not None: - self._timer.timeout.connect(self._quit_loop_by_timeout) - self._timer.start() if self.timeout != 0: + if self.timeout is not None: + # asserts as a stop-gap for possible multithreading issues + assert not self.signal_triggered + self._timer.start() + assert not self.signal_triggered qt_api.exec(self._loop) if not self.signal_triggered and self.raising: @@ -62,10 +64,7 @@ def _quit_loop_by_timeout(self): def _cleanup(self): # store timeout message before the data to construct it is lost self._timeout_message = self._get_timeout_error_message() - if self._timer is not None: - _silent_disconnect(self._timer.timeout, self._quit_loop_by_timeout) - self._timer.stop() - self._timer = None + self._timer.stop() def _get_timeout_error_message(self): """Subclasses have to implement this, returning an appropriate error message for a TimeoutError.""" @@ -649,12 +648,12 @@ def __init__(self, timeout=5000, raising=True): self.kwargs = None self.called = False self._loop = qt_api.QtCore.QEventLoop() - if timeout is None: - self._timer = None - else: - self._timer = qt_api.QtCore.QTimer(self._loop) - self._timer.setSingleShot(True) + + self._timer = qt_api.QtCore.QTimer(self._loop) + self._timer.setSingleShot(True) + if timeout is not None: self._timer.setInterval(timeout) + self._timer.timeout.connect(self._quit_loop_by_timeout) def wait(self): """ @@ -664,8 +663,7 @@ def wait(self): __tracebackhide__ = True if self.called: return - if self._timer is not None: - self._timer.timeout.connect(self._quit_loop_by_timeout) + if self.timeout is not None: self._timer.start() qt_api.exec(self._loop) if not self.called and self.raising: @@ -687,10 +685,7 @@ def _quit_loop_by_timeout(self): self._loop.quit() def _cleanup(self): - if self._timer is not None: - _silent_disconnect(self._timer.timeout, self._quit_loop_by_timeout) - self._timer.stop() - self._timer = None + self._timer.stop() def __call__(self, *args, **kwargs): # Not inside the try: block, as if self.called is True, we did quit the diff --git a/tests/conftest.py b/tests/conftest.py index 6010c31..e09ea8b 100644 --- a/tests/conftest.py +++ b/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() diff --git a/tests/test_wait_signal.py b/tests/test_wait_signal.py index 87ca324..8903dd5 100644 --- a/tests/test_wait_signal.py +++ b/tests/test_wait_signal.py @@ -1364,3 +1364,132 @@ 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_stderr, count", + [ + # Checking stderr messages + pytest.param( + True, # check stderr + 200, # gets output reliably even with only few runs (often the first) + id="stderr", + ), + # Triggering AttributeError + pytest.param( + False, # don't check stderr + # 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( + pytester: pytest.Pytester, check_stderr: 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, capfd, _): + 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_stderr}: # check_stderr + out, err = capfd.readouterr() + assert not err + """ + ) + + res = pytester.runpytest_subprocess("-x", "-s") + outcomes = res.parseoutcomes() + + if outcomes.get("failed", 0) and check_stderr and qt_api.pytest_qt_api == "pyside6": + # The test succeeds on PyQt (unsure why!), but we can't check + # qt_api.pytest_qt_api at import time, so we can't use + # pytest.mark.xfail conditionally. + pytest.xfail( + "Qt error: QObject::killTimer: " + "Timers cannot be stopped from another thread" + ) + + 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