Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly handle chained exceptions in Python 3 #216

Merged
merged 2 commits into from
Jun 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
2.4.1
-----

- Properly handle chained exceptions when capturing them inside
virtual methods (`#215`_). Thanks `@fabioz`_ for the report and sample
code with the fix.

.. _#215: https://github.com/pytest-dev/pytest-qt/pull/215


2.4.0
-----

Expand Down
17 changes: 12 additions & 5 deletions pytestqt/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,19 @@ def format_captured_exceptions(exceptions):
Formats exceptions given as (type, value, traceback) into a string
suitable to display as a test failure.
"""
message = 'Qt exceptions in virtual methods:\n'
message += '_' * 80 + '\n'
if sys.version_info.major == 2:
from StringIO import StringIO
else:
from io import StringIO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not have this import at the top of the file? Might also want to do except ImportError: instead so you don't need sys if you do it at the top.

Copy link
Member Author

@nicoddemus nicoddemus May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not have this import at the top of the file?

I put it here locally mostly because this is function is rarely used. If you prefer I can move it to the top though, no problem.

Might also want to do except ImportError: instead so you don't need sys if you do it at the top.

In general I tend to avoid that because it might lead to hard to diagnose errors (see pytest-dev/pytest-mock#68).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@The-Compiler gentle ping. 😁

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point - seems fine to keep it here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


stream = StringIO()
stream.write('Qt exceptions in virtual methods:\n')
sep = '_' * 80 + '\n'
stream.write(sep)
for (exc_type, value, tback) in exceptions:
message += ''.join(traceback.format_exception(exc_type, value, tback)) + '\n'
message += '_' * 80 + '\n'
return message
traceback.print_exception(exc_type, value, tback, file=stream)
stream.write(sep)
return stream.getvalue()


def _is_exception_capture_enabled(item):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_basics.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ def test_qt_api_ini_config(testdir, monkeypatch, option_api):
"""
from pytestqt.qt_compat import qt_api

monkeypatch.delenv("PYTEST_QT_API")
monkeypatch.delenv("PYTEST_QT_API", raising=False)

testdir.makeini("""
[pytest]
Expand Down
39 changes: 33 additions & 6 deletions tests/test_exceptions.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from pytestqt.exceptions import capture_exceptions, format_captured_exceptions
import pytest
import sys

import pytest

from pytestqt.exceptions import capture_exceptions, format_captured_exceptions


@pytest.mark.parametrize('raise_error', [False, True])
def test_catch_exceptions_in_virtual_methods(testdir, raise_error):
Expand All @@ -18,7 +20,11 @@ class Receiver(qt_api.QtCore.QObject):

def event(self, ev):
if {raise_error}:
raise ValueError('mistakes were made')
try:
raise RuntimeError('original error')
except RuntimeError:
raise ValueError('mistakes were made')

return qt_api.QtCore.QObject.event(self, ev)


Expand All @@ -32,11 +38,14 @@ def test_exceptions(qtbot):
'''.format(raise_error=raise_error))
result = testdir.runpytest()
if raise_error:
result.stdout.fnmatch_lines([
'*Qt exceptions in virtual methods:*',
expected_lines = ['*Qt exceptions in virtual methods:*']
if sys.version_info.major == 3:
expected_lines.append('RuntimeError: original error')
expected_lines.extend([
'*ValueError: mistakes were made*',
'*1 failed*',
'*1 failed*'
])
result.stdout.fnmatch_lines(expected_lines)
assert 'pytest.fail' not in '\n'.join(result.outlines)
else:
result.stdout.fnmatch_lines('*1 passed*')
Expand All @@ -55,6 +64,24 @@ def test_format_captured_exceptions():
assert 'ValueError: errors were made' in lines


@pytest.mark.skipif(sys.version_info.major == 2, reason='Python 3 only')
def test_format_captured_exceptions_chained():
try:
try:
raise ValueError('errors were made')
except ValueError:
raise RuntimeError('error handling value error')
except RuntimeError:
exceptions = [sys.exc_info()]

obtained_text = format_captured_exceptions(exceptions)
lines = obtained_text.splitlines()

assert 'Qt exceptions in virtual methods:' in lines
assert 'ValueError: errors were made' in lines
assert 'RuntimeError: error handling value error' in lines


@pytest.mark.parametrize('no_capture_by_marker', [True, False])
def test_no_capture(testdir, no_capture_by_marker):
"""
Expand Down