From 560f565c062abb60fbba44719649e50e56f49cca Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 4 Aug 2015 21:31:11 +0200 Subject: [PATCH 1/5] Disconnect SignalBlocker after exiting loop. Otherwise, a second signal emission will still call the functions of the already garbage-collected SignalBlocker with PyQt5 < 5.3. The signals could already be disconnected, so the TypeError (older PyQt versions) or RuntimeError (newer PyQt versions) which is raised in that case is ignored. Fixes #69. --- pytestqt/_tests/test_wait_signal.py | 27 +++++++++++++++++++++++++++ pytestqt/plugin.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/pytestqt/_tests/test_wait_signal.py b/pytestqt/_tests/test_wait_signal.py index addfbfbd..4240a0e2 100644 --- a/pytestqt/_tests/test_wait_signal.py +++ b/pytestqt/_tests/test_wait_signal.py @@ -239,3 +239,30 @@ class TestException(Exception): with pytest.raises(TestException): with func(arg, timeout=10, raising=raising): raise TestException + + +@pytest.mark.parametrize('multiple, do_timeout', + [(True, False), (True, True), (False, False), (False, True)]) +def test_wait_twice(qtbot, single_shot, multiple, do_timeout): + """ + https://github.com/pytest-dev/pytest-qt/issues/69 + """ + signaller = Signaller() + + if multiple: + func = qtbot.waitSignals + arg = [signaller.signal] + else: + func = qtbot.waitSignal + arg = signaller.signal + + if do_timeout: + with func(arg, timeout=100): + single_shot(signaller.signal, 200) + with func(arg, timeout=100): + single_shot(signaller.signal, 200) + else: + with func(arg): + signaller.signal.emit() + with func(arg): + signaller.signal.emit() diff --git a/pytestqt/plugin.py b/pytestqt/plugin.py index 7ca92883..ddebbfaa 100644 --- a/pytestqt/plugin.py +++ b/pytestqt/plugin.py @@ -355,6 +355,10 @@ def __init__(self, timeout=1000, raising=False): self.timeout = timeout self.signal_triggered = False self.raising = raising + self._timer = QtCore.QTimer() + self._timer.setSingleShot(True) + if timeout is not None: + self._timer.setInterval(timeout) def wait(self): """ @@ -368,12 +372,25 @@ def wait(self): if self.timeout is None and not self._signals: raise ValueError("No signals or timeout specified.") if self.timeout is not None: - QtCore.QTimer.singleShot(self.timeout, self._loop.quit) + self._timer.timeout.connect(self._quit_loop_by_timeout) + self._timer.start() self._loop.exec_() if not self.signal_triggered and self.raising: raise SignalTimeoutError("Didn't get signal after %sms." % self.timeout) + def _quit_loop_by_timeout(self): + self._loop.quit() + self._cleanup() + + def _cleanup(self): + if self.timeout is not None: + try: + self._timer.timeout.disconnect(self._quit_loop_by_timeout) + except (TypeError, RuntimeError): + # already disconnected by Qt? + pass + def __enter__(self): return self @@ -426,6 +443,16 @@ def _quit_loop_by_signal(self): """ self.signal_triggered = True self._loop.quit() + self._cleanup() + + def _cleanup(self): + super(SignalBlocker, self)._cleanup() + for signal in self._signals: + try: + signal.disconnect(self._quit_loop_by_signal) + except (TypeError, RuntimeError): + # already disconnected by Qt? + pass class MultiSignalBlocker(_AbstractSignalBlocker): From efb6000624790b01e25b35bd8904d7eff6ec65a3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 5 Aug 2015 07:09:59 +0200 Subject: [PATCH 2/5] Use multiple parametrize calls. --- pytestqt/_tests/test_wait_signal.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pytestqt/_tests/test_wait_signal.py b/pytestqt/_tests/test_wait_signal.py index 4240a0e2..99cba4ea 100644 --- a/pytestqt/_tests/test_wait_signal.py +++ b/pytestqt/_tests/test_wait_signal.py @@ -217,9 +217,8 @@ def check(self, timeout, *delays): return StopWatch() -@pytest.mark.parametrize('multiple, raising', - [(True, True), (True, False), (False, True), - (False, False)]) +@pytest.mark.parametrize('multiple', [True, False]) +@pytest.mark.parametrize('raising', [True, False]) def test_wait_signals_handles_exceptions(qtbot, multiple, raising): """ Make sure waitSignal handles exceptions correctly. @@ -241,8 +240,8 @@ class TestException(Exception): raise TestException -@pytest.mark.parametrize('multiple, do_timeout', - [(True, False), (True, True), (False, False), (False, True)]) +@pytest.mark.parametrize('multiple', [True, False]) +@pytest.mark.parametrize('do_timeout', [True, False]) def test_wait_twice(qtbot, single_shot, multiple, do_timeout): """ https://github.com/pytest-dev/pytest-qt/issues/69 From 8a5177138ef752fb32427fa974ec2b3641d16d51 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 5 Aug 2015 07:11:13 +0200 Subject: [PATCH 3/5] Set self._timer to None. --- pytestqt/plugin.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pytestqt/plugin.py b/pytestqt/plugin.py index ddebbfaa..377b0e99 100644 --- a/pytestqt/plugin.py +++ b/pytestqt/plugin.py @@ -355,9 +355,11 @@ def __init__(self, timeout=1000, raising=False): self.timeout = timeout self.signal_triggered = False self.raising = raising - self._timer = QtCore.QTimer() - self._timer.setSingleShot(True) - if timeout is not None: + if timeout is None: + self._timer = None + else: + self._timer = QtCore.QTimer() + self._timer.setSingleShot(True) self._timer.setInterval(timeout) def wait(self): @@ -371,7 +373,7 @@ def wait(self): return if self.timeout is None and not self._signals: raise ValueError("No signals or timeout specified.") - if self.timeout is not None: + if self._timer is not None: self._timer.timeout.connect(self._quit_loop_by_timeout) self._timer.start() self._loop.exec_() @@ -384,7 +386,7 @@ def _quit_loop_by_timeout(self): self._cleanup() def _cleanup(self): - if self.timeout is not None: + if self._timer is not None: try: self._timer.timeout.disconnect(self._quit_loop_by_timeout) except (TypeError, RuntimeError): From a4ca81a192db9554f61f0edde43f1b1a0682e683 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 5 Aug 2015 07:12:37 +0200 Subject: [PATCH 4/5] Always call self._cleanup(). --- pytestqt/plugin.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pytestqt/plugin.py b/pytestqt/plugin.py index 377b0e99..388a6c9d 100644 --- a/pytestqt/plugin.py +++ b/pytestqt/plugin.py @@ -377,13 +377,13 @@ def wait(self): self._timer.timeout.connect(self._quit_loop_by_timeout) self._timer.start() self._loop.exec_() + self._cleanup() if not self.signal_triggered and self.raising: raise SignalTimeoutError("Didn't get signal after %sms." % self.timeout) def _quit_loop_by_timeout(self): self._loop.quit() - self._cleanup() def _cleanup(self): if self._timer is not None: @@ -445,7 +445,6 @@ def _quit_loop_by_signal(self): """ self.signal_triggered = True self._loop.quit() - self._cleanup() def _cleanup(self): super(SignalBlocker, self)._cleanup() From a7d8ddf227d56fe784f4d825d0317545e0cf4db9 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 6 Aug 2015 08:17:00 +0200 Subject: [PATCH 5/5] Revert "Always call self._cleanup()." This reverts commit a4ca81a192db9554f61f0edde43f1b1a0682e683. For some reason this makes the test fail on Trusty again, and nobody seems to understand why - so let's just keep the old way. --- pytestqt/plugin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pytestqt/plugin.py b/pytestqt/plugin.py index 388a6c9d..377b0e99 100644 --- a/pytestqt/plugin.py +++ b/pytestqt/plugin.py @@ -377,13 +377,13 @@ def wait(self): self._timer.timeout.connect(self._quit_loop_by_timeout) self._timer.start() self._loop.exec_() - self._cleanup() if not self.signal_triggered and self.raising: raise SignalTimeoutError("Didn't get signal after %sms." % self.timeout) def _quit_loop_by_timeout(self): self._loop.quit() + self._cleanup() def _cleanup(self): if self._timer is not None: @@ -445,6 +445,7 @@ def _quit_loop_by_signal(self): """ self.signal_triggered = True self._loop.quit() + self._cleanup() def _cleanup(self): super(SignalBlocker, self)._cleanup()