From 11caac8ec84e7dcabbda104d42be33eabcef50d8 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 27 Oct 2015 18:22:33 -0200 Subject: [PATCH 1/7] Setup/teardown exception capturing as early/late as possible to catch all exceptions Fixes #105 Fixes #106 --- CHANGELOG.rst | 13 +++++- pytestqt/exceptions.py | 61 +++++++++++++++++++++++----- pytestqt/plugin.py | 57 ++++++++++++++++---------- tests/test_exceptions.py | 86 ++++++++++++++++++++++++++++++++++++---- 4 files changed, 178 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8ede4968..ed41a990 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,3 +1,14 @@ +1.9.0 +----- + +- Exception capturing now happens as early/late as possible in order to catch + all possible exceptions (including fixtures). Thanks + `@The-Compiler`_ for the request (`105`_, `106`_). + +.. _105: https://github.com/pytest-dev/pytest-qt/issues/105 +.. _106: https://github.com/pytest-dev/pytest-qt/issues/106 + + 1.8.0 ----- @@ -244,4 +255,4 @@ First working version. .. _@datalyze-solutions: https://github.com/datalyze-solutions .. _@fabioz: https://github.com/fabioz .. _@baudren: https://github.com/baudren -.. _@itghisi: https://github.com/itghisi \ No newline at end of file +.. _@itghisi: https://github.com/itghisi diff --git a/pytestqt/exceptions.py b/pytestqt/exceptions.py index ee790d59..e926d3e4 100644 --- a/pytestqt/exceptions.py +++ b/pytestqt/exceptions.py @@ -1,6 +1,7 @@ from contextlib import contextmanager import sys import traceback +import pytest @contextmanager @@ -10,17 +11,54 @@ def capture_exceptions(): and returns them as a list of (type, value, traceback) after the context ends. """ - result = [] - - def hook(type_, value, tback): - result.append((type_, value, tback)) - - old_hook = sys.excepthook - sys.excepthook = hook + manager = _QtExceptionCaptureManager() + manager.start() try: - yield result + yield manager.exceptions finally: - sys.excepthook = old_hook + manager.finish() + + +class _QtExceptionCaptureManager(object): + """ + Manages exception capture context. + """ + + def __init__(self): + self.old_hook = None + self.exceptions = [] + + def start(self): + """Start exception capturing by installing a hook into sys.excepthook + that records exceptions received into ``self.exceptions``. + """ + def hook(type_, value, tback): + self.exceptions.append((type_, value, tback)) + + self.old_hook = sys.excepthook + sys.excepthook = hook + + def finish(self): + """Stop exception capturing, restoring the original hook. + + Can be called multiple times. + """ + if self.old_hook is not None: + sys.excepthook = self.old_hook + self.old_hook = None + + def fail_if_exceptions_occurred(self, when): + """calls pytest.fail() with an informative message if exceptions + have been captured so far. Before pytest.fail() is called, also + finish capturing. + """ + if self.exceptions: + self.finish() + exceptions = self.exceptions[:] + self.exceptions[:] = [] + prefix = '%s ERROR: ' % when + pytest.fail(prefix + format_captured_exceptions(exceptions), + pytrace=False) def format_captured_exceptions(exceptions): @@ -41,4 +79,7 @@ def _is_exception_capture_disabled(item): """returns if exception capture is disabled for the given test item. """ return item.get_marker('qt_no_exception_capture') or \ - item.config.getini('qt_no_exception_capture') \ No newline at end of file + item.config.getini('qt_no_exception_capture') + +def _is_exception_capture_enabled(item): + return not _is_exception_capture_disabled(item) diff --git a/pytestqt/plugin.py b/pytestqt/plugin.py index 3be002eb..646bc6b0 100644 --- a/pytestqt/plugin.py +++ b/pytestqt/plugin.py @@ -1,7 +1,8 @@ import pytest from pytestqt.exceptions import capture_exceptions, format_captured_exceptions, \ - _is_exception_capture_disabled + _is_exception_capture_disabled, _is_exception_capture_enabled, \ + _QtExceptionCaptureManager from pytestqt.logging import QtLoggingPlugin, _QtMessageCapture, Record from pytestqt.qt_compat import QApplication, QT_API from pytestqt.qtbot import QtBot @@ -43,15 +44,7 @@ def qtbot(qapp, request): that they are properly closed after the test ends. """ result = QtBot() - no_capture = _is_exception_capture_disabled(request.node) - if no_capture: - yield result # pragma: no cover - else: - with capture_exceptions() as exceptions: - yield result - if exceptions: - pytest.fail(format_captured_exceptions(exceptions), pytrace=False) - + yield result result._close() @@ -90,15 +83,45 @@ def pytest_addoption(parser): 'default: "{0}"'.format(default)) -@pytest.mark.hookwrapper +@pytest.hookimpl(hookwrapper=True, tryfirst=True) +def pytest_runtest_setup(item): + """ + Hook called after before test setup starts, to start capturing exceptions + as early as possible. + """ + capture_enabled = _is_exception_capture_enabled(item) + if capture_enabled: + item.qt_exception_capture_manager = _QtExceptionCaptureManager() + item.qt_exception_capture_manager.start() + yield + _process_events(item) + if capture_enabled: + item.qt_exception_capture_manager.fail_if_exceptions_occurred('SETUP') + + +@pytest.hookimpl(hookwrapper=True, trylast=True) +def pytest_runtest_call(item): + yield + _process_events(item) + capture_enabled = _is_exception_capture_enabled(item) + if capture_enabled: + item.qt_exception_capture_manager.fail_if_exceptions_occurred('CALL') + + +@pytest.hookimpl(hookwrapper=True, trylast=True) def pytest_runtest_teardown(item): """ Hook called after each test tear down, to process any pending events and - avoiding leaking events to the next test. + avoiding leaking events to the next test. Also, if exceptions have + been captured during fixtures teardown, fail the test. """ _process_events(item) yield _process_events(item) + capture_enabled = _is_exception_capture_enabled(item) + if capture_enabled: + item.qt_exception_capture_manager.fail_if_exceptions_occurred('TEARDOWN') + item.qt_exception_capture_manager.finish() def _process_events(item): @@ -107,15 +130,7 @@ def _process_events(item): """ app = QApplication.instance() if app is not None: - if _is_exception_capture_disabled(item): - app.processEvents() - else: - with capture_exceptions() as exceptions: - app.processEvents() - if exceptions: - pytest.fail('TEARDOWN ERROR: ' + - format_captured_exceptions(exceptions), - pytrace=False) + app.processEvents() def pytest_configure(config): diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index e24298ba..75cda41d 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -35,7 +35,7 @@ def test_exceptions(qtbot): result.stdout.fnmatch_lines([ '*Qt exceptions in virtual methods:*', '*ValueError: mistakes were made*', - '*1 error*', + '*1 failed*', ]) assert 'pytest.fail' not in '\n'.join(result.outlines) else: @@ -121,9 +121,9 @@ def test_capture(qtbot): res.stdout.fnmatch_lines(['*2 passed*']) -def test_exception_capture_on_teardown(testdir): +def test_exception_capture_on_call(testdir): """ - Exceptions should also be captured during test teardown. + Exceptions should also be captured during test execution. :type testdir: TmpTestdir """ @@ -138,14 +138,86 @@ def event(self, ev): def test_widget(qtbot, qapp): w = MyWidget() - # keep a reference to the widget so it will lives after the test - # ends. This will in turn trigger its event() during test tear down, - # raising the exception during its event processing - test_widget.w = w qapp.postEvent(w, QEvent(QEvent.User)) + qapp.processEvents() ''') res = testdir.runpytest('-s') res.stdout.fnmatch_lines([ + "*RuntimeError('event processed')*", + '*1 failed*', + ]) + + +def test_exception_capture_on_widget_close(testdir): + """ + Exceptions should also be captured when widget is being closed. + + :type testdir: TmpTestdir + """ + testdir.makepyfile(''' + import pytest + from pytestqt.qt_compat import QWidget, QtCore, QEvent + + class MyWidget(QWidget): + + def closeEvent(self, ev): + raise RuntimeError('close error') + + def test_widget(qtbot, qapp): + w = MyWidget() + test_widget.w = w # keep it alive + qtbot.addWidget(w) + ''') + res = testdir.runpytest('-s') + res.stdout.fnmatch_lines([ + "*RuntimeError('close error')*", + '*1 error*', + ]) + + +@pytest.mark.parametrize('mode', ['setup', 'teardown']) +def test_exception_capture_on_fixture_setup_and_teardown(testdir, mode): + """ + Setup/teardown exception capturing as early/late as possible to catch + all exceptions, even from other fixtures (#105). + + :type testdir: TmpTestdir + """ + if mode == 'setup': + setup_code = 'send_event(w, qapp)' + teardown_code = '' + else: + setup_code = '' + teardown_code = 'send_event(w, qapp)' + + testdir.makepyfile(''' + import pytest + from pytestqt.qt_compat import QWidget, QtCore, QEvent, QApplication + + class MyWidget(QWidget): + + def event(self, ev): + if ev.type() == QEvent.User: + raise RuntimeError('event processed') + return True + + @pytest.yield_fixture + def widget(qapp): + w = MyWidget() + {setup_code} + yield w + {teardown_code} + + def send_event(w, qapp): + qapp.postEvent(w, QEvent(QEvent.User)) + qapp.processEvents() + + def test_capture(widget): + pass + '''.format(setup_code=setup_code, teardown_code=teardown_code)) + res = testdir.runpytest('-s') + res.stdout.fnmatch_lines([ + '*__ ERROR at %s of test_capture __*' % mode, "*RuntimeError('event processed')*", '*1 error*', ]) From b477525cf1492bdfc75bff6589f19dcea8dc9538 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 28 Oct 2015 19:32:36 -0200 Subject: [PATCH 2/7] Use pytest.mark.hook* marks to keep 2.7 compatibility --- pytestqt/plugin.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pytestqt/plugin.py b/pytestqt/plugin.py index 646bc6b0..f64b550a 100644 --- a/pytestqt/plugin.py +++ b/pytestqt/plugin.py @@ -99,7 +99,8 @@ def pytest_runtest_setup(item): item.qt_exception_capture_manager.fail_if_exceptions_occurred('SETUP') -@pytest.hookimpl(hookwrapper=True, trylast=True) +@pytest.mark.hookwrapper +@pytest.mark.tryfirst def pytest_runtest_call(item): yield _process_events(item) @@ -108,7 +109,8 @@ def pytest_runtest_call(item): item.qt_exception_capture_manager.fail_if_exceptions_occurred('CALL') -@pytest.hookimpl(hookwrapper=True, trylast=True) +@pytest.mark.hookwrapper +@pytest.mark.trylast def pytest_runtest_teardown(item): """ Hook called after each test tear down, to process any pending events and From 039a4f8ab614ed24970c6183539407493e79297c Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 28 Oct 2015 19:32:46 -0200 Subject: [PATCH 3/7] Simplify exception list cleaning logic --- pytestqt/exceptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pytestqt/exceptions.py b/pytestqt/exceptions.py index e926d3e4..5ee6da8b 100644 --- a/pytestqt/exceptions.py +++ b/pytestqt/exceptions.py @@ -54,8 +54,8 @@ def fail_if_exceptions_occurred(self, when): """ if self.exceptions: self.finish() - exceptions = self.exceptions[:] - self.exceptions[:] = [] + exceptions = self.exceptions + self.exceptions = [] prefix = '%s ERROR: ' % when pytest.fail(prefix + format_captured_exceptions(exceptions), pytrace=False) From 49b9c0ee456f5f6a718895b8f74803ec976cf3a7 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 28 Oct 2015 19:56:40 -0200 Subject: [PATCH 4/7] Close widgets before tearing down other fixtures Fix #106 --- CHANGELOG.rst | 7 ++++-- pytestqt/plugin.py | 11 +++++----- pytestqt/qtbot.py | 51 ++++++++++++++++++++++++++++++-------------- tests/test_basics.py | 33 ++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ed41a990..0a076ab7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,8 +2,11 @@ ----- - Exception capturing now happens as early/late as possible in order to catch - all possible exceptions (including fixtures). Thanks - `@The-Compiler`_ for the request (`105`_, `106`_). + all possible exceptions (including fixtures)(`105`_). Thanks + `@The-Compiler`_ for the request. + +- Widgets registered by ``qtbot.addWidget`` are now closed before all other + fixtures are tear down (`106`_). Thanks `@The-Compiler`_ for request. .. _105: https://github.com/pytest-dev/pytest-qt/issues/105 .. _106: https://github.com/pytest-dev/pytest-qt/issues/106 diff --git a/pytestqt/plugin.py b/pytestqt/plugin.py index f64b550a..284880a6 100644 --- a/pytestqt/plugin.py +++ b/pytestqt/plugin.py @@ -5,7 +5,7 @@ _QtExceptionCaptureManager from pytestqt.logging import QtLoggingPlugin, _QtMessageCapture, Record from pytestqt.qt_compat import QApplication, QT_API -from pytestqt.qtbot import QtBot +from pytestqt.qtbot import QtBot, _close_widgets from pytestqt.wait_signal import SignalBlocker, MultiSignalBlocker, SignalTimeoutError # modules imported here just for backward compatibility before we have split the implementation @@ -35,7 +35,7 @@ def qapp(): _qapp_instance = None -@pytest.yield_fixture +@pytest.fixture def qtbot(qapp, request): """ Fixture used to create a QtBot instance for using during testing. @@ -43,9 +43,8 @@ def qtbot(qapp, request): Make sure to call addWidget for each top-level widget you create to ensure that they are properly closed after the test ends. """ - result = QtBot() - yield result - result._close() + result = QtBot(request) + return result @pytest.fixture @@ -118,6 +117,8 @@ def pytest_runtest_teardown(item): been captured during fixtures teardown, fail the test. """ _process_events(item) + _close_widgets(item) + _process_events(item) yield _process_events(item) capture_enabled = _is_exception_capture_enabled(item) diff --git a/pytestqt/qtbot.py b/pytestqt/qtbot.py index 818ee321..fce1de6c 100644 --- a/pytestqt/qtbot.py +++ b/pytestqt/qtbot.py @@ -156,19 +156,8 @@ class QtBot(object): """ - def __init__(self): - self._widgets = [] # list of weakref to QWidget instances - - def _close(self): - """ - Clear up method. Called at the end of each test that uses a ``qtbot`` fixture. - """ - for w in self._widgets: - w = w() - if w is not None: - w.close() - w.deleteLater() - self._widgets[:] = [] + def __init__(self, request): + self._request = request def addWidget(self, widget): """ @@ -178,7 +167,7 @@ def addWidget(self, widget): :param QWidget widget: Widget to keep track of. """ - self._widgets.append(weakref.ref(widget)) + _add_widget(self._request.node, widget) add_widget = addWidget # pep-8 alias @@ -216,7 +205,7 @@ def stopForInteraction(self): .. note:: As a convenience, it is also aliased as `stop`. """ widget_and_visibility = [] - for weak_widget in self._widgets: + for weak_widget in _iter_widgets(self._request.node): widget = weak_widget() if widget is not None: widget_and_visibility.append((widget, widget.isVisible())) @@ -324,4 +313,34 @@ def waitSignals(self, signals=None, timeout=1000, raising=False): # provide easy access to SignalTimeoutError to qtbot fixtures -QtBot.SignalTimeoutError = SignalTimeoutError \ No newline at end of file +QtBot.SignalTimeoutError = SignalTimeoutError + + +def _add_widget(item, widget): + """ + Register a widget into the given pytest item for later closing. + """ + qt_widgets = getattr(item, 'qt_widgets', []) + qt_widgets.append(weakref.ref(widget)) + item.qt_widgets = qt_widgets + + +def _close_widgets(item): + """ + Close all widgets registered in the pytest item. + """ + widgets = getattr(item, 'qt_widgets', None) + if widgets: + for w in item.qt_widgets: + w = w() + if w is not None: + w.close() + w.deleteLater() + del item.qt_widgets + + +def _iter_widgets(item): + """ + Iterates over widgets registered in the given pytest item. + """ + return iter(getattr(item, 'qt_widgets', [])) diff --git a/tests/test_basics.py b/tests/test_basics.py index 7603c7fd..1e4a55ea 100644 --- a/tests/test_basics.py +++ b/tests/test_basics.py @@ -171,6 +171,39 @@ def test_public_api_backward_compatibility(): assert pytestqt.plugin.Record +def test_widgets_closed_before_fixtures(testdir): + """ + Ensure widgets added by "qtbot.add_widget" are closed before all other + fixtures are teardown. (#106). + """ + testdir.makepyfile(''' + import pytest + from pytestqt.qt_compat import QWidget + + class Widget(QWidget): + + closed = False + + def closeEvent(self, e): + e.accept() + self.closed = True + + @pytest.yield_fixture + def widget(qtbot): + w = Widget() + qtbot.add_widget(w) + yield w + assert w.closed + + def test_foo(widget): + pass + ''') + result = testdir.runpytest() + result.stdout.fnmatch_lines([ + '*= 1 passed in *' + ]) + + class EventRecorder(QWidget): """ From 1db1527fedbb342dceb0b6eae67f462edaca86ad Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 28 Oct 2015 20:01:47 -0200 Subject: [PATCH 5/7] Clean up imports and _is_exception_capture_disabled --- pytestqt/exceptions.py | 10 ++++------ pytestqt/plugin.py | 21 +++++++++++---------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/pytestqt/exceptions.py b/pytestqt/exceptions.py index 5ee6da8b..f22cb2cc 100644 --- a/pytestqt/exceptions.py +++ b/pytestqt/exceptions.py @@ -75,11 +75,9 @@ def format_captured_exceptions(exceptions): return message -def _is_exception_capture_disabled(item): +def _is_exception_capture_enabled(item): """returns if exception capture is disabled for the given test item. """ - return item.get_marker('qt_no_exception_capture') or \ - item.config.getini('qt_no_exception_capture') - -def _is_exception_capture_enabled(item): - return not _is_exception_capture_disabled(item) + disabled = item.get_marker('qt_no_exception_capture') or \ + item.config.getini('qt_no_exception_capture') + return not disabled diff --git a/pytestqt/plugin.py b/pytestqt/plugin.py index 284880a6..dfa23cf4 100644 --- a/pytestqt/plugin.py +++ b/pytestqt/plugin.py @@ -1,19 +1,20 @@ import pytest from pytestqt.exceptions import capture_exceptions, format_captured_exceptions, \ - _is_exception_capture_disabled, _is_exception_capture_enabled, \ - _QtExceptionCaptureManager + _is_exception_capture_enabled, _QtExceptionCaptureManager from pytestqt.logging import QtLoggingPlugin, _QtMessageCapture, Record from pytestqt.qt_compat import QApplication, QT_API from pytestqt.qtbot import QtBot, _close_widgets from pytestqt.wait_signal import SignalBlocker, MultiSignalBlocker, SignalTimeoutError -# modules imported here just for backward compatibility before we have split the implementation -# of this file in several modules +# classes/functions imported here just for backward compatibility before we +# split the implementation of this file in several modules assert SignalBlocker assert MultiSignalBlocker assert SignalTimeoutError assert Record +assert capture_exceptions +assert format_captured_exceptions @pytest.yield_fixture(scope='session') @@ -93,7 +94,7 @@ def pytest_runtest_setup(item): item.qt_exception_capture_manager = _QtExceptionCaptureManager() item.qt_exception_capture_manager.start() yield - _process_events(item) + _process_events() if capture_enabled: item.qt_exception_capture_manager.fail_if_exceptions_occurred('SETUP') @@ -102,7 +103,7 @@ def pytest_runtest_setup(item): @pytest.mark.tryfirst def pytest_runtest_call(item): yield - _process_events(item) + _process_events() capture_enabled = _is_exception_capture_enabled(item) if capture_enabled: item.qt_exception_capture_manager.fail_if_exceptions_occurred('CALL') @@ -116,18 +117,18 @@ def pytest_runtest_teardown(item): avoiding leaking events to the next test. Also, if exceptions have been captured during fixtures teardown, fail the test. """ - _process_events(item) + _process_events() _close_widgets(item) - _process_events(item) + _process_events() yield - _process_events(item) + _process_events() capture_enabled = _is_exception_capture_enabled(item) if capture_enabled: item.qt_exception_capture_manager.fail_if_exceptions_occurred('TEARDOWN') item.qt_exception_capture_manager.finish() -def _process_events(item): +def _process_events(): """Calls app.processEvents() while taking care of capturing exceptions or not based on the given item's configuration. """ From 9dd8fd9d5bcc81f2eed600603f72189a88b4387f Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 28 Oct 2015 20:44:17 -0200 Subject: [PATCH 6/7] Fix CHANGELOG formatting --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0a076ab7..af1f782d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,7 +6,7 @@ `@The-Compiler`_ for the request. - Widgets registered by ``qtbot.addWidget`` are now closed before all other - fixtures are tear down (`106`_). Thanks `@The-Compiler`_ for request. + fixtures are tear down (`106`_). Thanks `@The-Compiler`_ for request. .. _105: https://github.com/pytest-dev/pytest-qt/issues/105 .. _106: https://github.com/pytest-dev/pytest-qt/issues/106 From e64014a4d25599dc3505ce66b0110cbf33568eac Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 30 Oct 2015 09:36:13 -0200 Subject: [PATCH 7/7] Use pytest-2.7-compatible hookwrapper/tryfirst markers --- pytestqt/plugin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pytestqt/plugin.py b/pytestqt/plugin.py index dfa23cf4..131b593a 100644 --- a/pytestqt/plugin.py +++ b/pytestqt/plugin.py @@ -83,7 +83,8 @@ def pytest_addoption(parser): 'default: "{0}"'.format(default)) -@pytest.hookimpl(hookwrapper=True, tryfirst=True) +@pytest.mark.hookwrapper +@pytest.mark.tryfirst def pytest_runtest_setup(item): """ Hook called after before test setup starts, to start capturing exceptions