From 04f27d4eb469fb9c76fd2d100564a0f7d30028df Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 18 Oct 2019 18:26:03 +0200 Subject: [PATCH 1/3] unittest: do not use TestCase.debug() with `--pdb` Fixes https://github.com/pytest-dev/pytest/issues/5991 Fixes https://github.com/pytest-dev/pytest/issues/3823 Ref: https://github.com/pytest-dev/pytest-django/issues/772 Ref: https://github.com/pytest-dev/pytest/pull/1890 Ref: https://github.com/pytest-dev/pytest-django/pull/782 - inject wrapped testMethod - adjust test_trial_error - add test for `--trace` with unittests --- changelog/3823.bugfix.rst | 1 + changelog/5991.bugfix.rst | 1 + doc/en/unittest.rst | 11 ------- src/_pytest/unittest.py | 60 ++++++++++++++++++++++++++------------- testing/test_unittest.py | 47 +++++++++++++++++++++++++----- 5 files changed, 82 insertions(+), 38 deletions(-) create mode 100644 changelog/3823.bugfix.rst create mode 100644 changelog/5991.bugfix.rst diff --git a/changelog/3823.bugfix.rst b/changelog/3823.bugfix.rst new file mode 100644 index 00000000000..a653fecdd92 --- /dev/null +++ b/changelog/3823.bugfix.rst @@ -0,0 +1 @@ +``--trace`` now works with unittests. diff --git a/changelog/5991.bugfix.rst b/changelog/5991.bugfix.rst new file mode 100644 index 00000000000..5659069da3e --- /dev/null +++ b/changelog/5991.bugfix.rst @@ -0,0 +1 @@ +Fix interaction with ``--pdb`` and unittests: do not use unittest's ``TestCase.debug()``. diff --git a/doc/en/unittest.rst b/doc/en/unittest.rst index 0f6737c0dc0..cd7858190fb 100644 --- a/doc/en/unittest.rst +++ b/doc/en/unittest.rst @@ -238,17 +238,6 @@ was executed ahead of the ``test_method``. .. _pdb-unittest-note: -.. note:: - - Running tests from ``unittest.TestCase`` subclasses with ``--pdb`` will - disable tearDown and cleanup methods for the case that an Exception - occurs. This allows proper post mortem debugging for all applications - which have significant logic in their tearDown machinery. However, - supporting this feature has the following side effect: If people - overwrite ``unittest.TestCase`` ``__call__`` or ``run``, they need to - to overwrite ``debug`` in the same way (this is also true for standard - unittest). - .. note:: Due to architectural differences between the two frameworks, setup and diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index 11dc77cc4ff..bef209b0df5 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -1,4 +1,5 @@ """ discovery and running of std-library "unittest" style tests. """ +import functools import sys import traceback @@ -107,6 +108,7 @@ class TestCaseFunction(Function): nofuncargs = True _excinfo = None _testcase = None + _need_tearDown = None def setup(self): self._testcase = self.parent.obj(self.name) @@ -115,6 +117,8 @@ def setup(self): self._request._fillfixtures() def teardown(self): + if self._need_tearDown: + self._testcase.tearDown() self._testcase = None self._obj = None @@ -187,29 +191,45 @@ def addSuccess(self, testcase): def stopTest(self, testcase): pass - def _handle_skip(self): - # implements the skipping machinery (see #2137) - # analog to pythons Lib/unittest/case.py:run + def runtest(self): testMethod = getattr(self._testcase, self._testcase._testMethodName) - if getattr(self._testcase.__class__, "__unittest_skip__", False) or getattr( - testMethod, "__unittest_skip__", False - ): - # If the class or method was skipped. - skip_why = getattr( - self._testcase.__class__, "__unittest_skip_why__", "" - ) or getattr(testMethod, "__unittest_skip_why__", "") - self._testcase._addSkip(self, self._testcase, skip_why) - return True - return False - def runtest(self): - if self.config.pluginmanager.get_plugin("pdbinvoke") is None: + class _GetOutOf_testPartExecutor(KeyboardInterrupt): + """Helper exception to get out of unittests's testPartExecutor.""" + + unittest = sys.modules.get("unittest") + + reraise = () + if unittest: + reraise += (unittest.SkipTest,) + + @functools.wraps(testMethod) + def wrapped_testMethod(*args, **kwargs): + try: + self.ihook.pytest_pyfunc_call(pyfuncitem=self) + except reraise: + raise + except Exception as exc: + expecting_failure_method = getattr( + testMethod, "__unittest_expecting_failure__", False + ) + expecting_failure_class = getattr( + self, "__unittest_expecting_failure__", False + ) + expecting_failure = expecting_failure_class or expecting_failure_method + self._need_tearDown = True + + if expecting_failure: + raise + + raise _GetOutOf_testPartExecutor(exc) + + self._testcase._wrapped_testMethod = wrapped_testMethod + self._testcase._testMethodName = "_wrapped_testMethod" + try: self._testcase(result=self) - else: - # disables tearDown and cleanups for post mortem debugging (see #1890) - if self._handle_skip(): - return - self._testcase.debug() + except _GetOutOf_testPartExecutor as exc: + raise exc.args[0] from exc.args[0] def _prunetraceback(self, excinfo): Function._prunetraceback(self, excinfo) diff --git a/testing/test_unittest.py b/testing/test_unittest.py index f56284d8510..b34f543135f 100644 --- a/testing/test_unittest.py +++ b/testing/test_unittest.py @@ -537,24 +537,28 @@ def f(_): ) result.stdout.fnmatch_lines( [ - "test_trial_error.py::TC::test_four FAILED", + "test_trial_error.py::TC::test_four SKIPPED", "test_trial_error.py::TC::test_four ERROR", "test_trial_error.py::TC::test_one FAILED", "test_trial_error.py::TC::test_three FAILED", - "test_trial_error.py::TC::test_two FAILED", + "test_trial_error.py::TC::test_two SKIPPED", + "test_trial_error.py::TC::test_two ERROR", "*ERRORS*", "*_ ERROR at teardown of TC.test_four _*", + "NOTE: Incompatible Exception Representation, displaying natively:", + "*DelayedCalls*", + "*_ ERROR at teardown of TC.test_two _*", + "NOTE: Incompatible Exception Representation, displaying natively:", "*DelayedCalls*", "*= FAILURES =*", - "*_ TC.test_four _*", - "*NameError*crash*", + # "*_ TC.test_four _*", + # "*NameError*crash*", "*_ TC.test_one _*", "*NameError*crash*", "*_ TC.test_three _*", + "NOTE: Incompatible Exception Representation, displaying natively:", "*DelayedCalls*", - "*_ TC.test_two _*", - "*NameError*crash*", - "*= 4 failed, 1 error in *", + "*= 2 failed, 2 skipped, 2 errors in *", ] ) @@ -1096,3 +1100,32 @@ def test_should_not_run(self): ) result = testdir.runpytest() result.stdout.fnmatch_lines(["*Exit: pytest_exit called*", "*= no tests ran in *"]) + + +def test_trace(testdir, monkeypatch): + calls = [] + + def check_call(*args, **kwargs): + calls.append((args, kwargs)) + assert args == ("runcall",) + + class _pdb: + def runcall(*args, **kwargs): + calls.append((args, kwargs)) + + return _pdb + + monkeypatch.setattr("_pytest.debugging.pytestPDB._init_pdb", check_call) + + p1 = testdir.makepyfile( + """ + import unittest + + class MyTestCase(unittest.TestCase): + def test(self): + self.assertEqual('foo', 'foo') + """ + ) + result = testdir.runpytest("--trace", str(p1)) + assert len(calls) == 2 + assert result.ret == 0 From f7b1de70c037d0ca43adc25966677d5a78034abc Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 12 Nov 2019 12:47:03 -0300 Subject: [PATCH 2/3] No need to call tearDown on expected failures - Isolate logic for getting expected exceptions - Use original method name, as users see it when entering the debugger --- src/_pytest/unittest.py | 42 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index bef209b0df5..de6be7dc5e2 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -108,7 +108,6 @@ class TestCaseFunction(Function): nofuncargs = True _excinfo = None _testcase = None - _need_tearDown = None def setup(self): self._testcase = self.parent.obj(self.name) @@ -117,8 +116,6 @@ def setup(self): self._request._fillfixtures() def teardown(self): - if self._need_tearDown: - self._testcase.tearDown() self._testcase = None self._obj = None @@ -191,45 +188,44 @@ def addSuccess(self, testcase): def stopTest(self, testcase): pass + def _expecting_failure(self, test_method) -> bool: + """Return True if the given unittest method (or the entire class) is marked + with @expectedFailure""" + expecting_failure_method = getattr( + test_method, "__unittest_expecting_failure__", False + ) + expecting_failure_class = getattr(self, "__unittest_expecting_failure__", False) + return bool(expecting_failure_class or expecting_failure_method) + def runtest(self): + import unittest + testMethod = getattr(self._testcase, self._testcase._testMethodName) class _GetOutOf_testPartExecutor(KeyboardInterrupt): - """Helper exception to get out of unittests's testPartExecutor.""" - - unittest = sys.modules.get("unittest") - - reraise = () - if unittest: - reraise += (unittest.SkipTest,) + """Helper exception to get out of unittests's testPartExecutor (see TestCase.run).""" @functools.wraps(testMethod) def wrapped_testMethod(*args, **kwargs): + """Wrap the original method to call into pytest's machinery, so other pytest + features can have a chance to kick in (notably --pdb)""" try: self.ihook.pytest_pyfunc_call(pyfuncitem=self) - except reraise: + except unittest.SkipTest: raise except Exception as exc: - expecting_failure_method = getattr( - testMethod, "__unittest_expecting_failure__", False - ) - expecting_failure_class = getattr( - self, "__unittest_expecting_failure__", False - ) - expecting_failure = expecting_failure_class or expecting_failure_method - self._need_tearDown = True - + expecting_failure = self._expecting_failure(testMethod) if expecting_failure: raise - raise _GetOutOf_testPartExecutor(exc) - self._testcase._wrapped_testMethod = wrapped_testMethod - self._testcase._testMethodName = "_wrapped_testMethod" + setattr(self._testcase, self._testcase._testMethodName, wrapped_testMethod) try: self._testcase(result=self) except _GetOutOf_testPartExecutor as exc: raise exc.args[0] from exc.args[0] + finally: + delattr(self._testcase, self._testcase._testMethodName) def _prunetraceback(self, excinfo): Function._prunetraceback(self, excinfo) From 59369651dbe6a3bac420e16dcded9ad095b1680b Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 12 Nov 2019 14:03:40 -0300 Subject: [PATCH 3/3] Bring back explicit tear down Otherwise 'normal' failures won't call teardown explicitly --- src/_pytest/unittest.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index de6be7dc5e2..71ff580a6da 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -110,12 +110,15 @@ class TestCaseFunction(Function): _testcase = None def setup(self): + self._needs_explicit_tearDown = False self._testcase = self.parent.obj(self.name) self._obj = getattr(self._testcase, self.name) if hasattr(self, "_request"): self._request._fillfixtures() def teardown(self): + if self._needs_explicit_tearDown: + self._testcase.tearDown() self._testcase = None self._obj = None @@ -217,6 +220,7 @@ def wrapped_testMethod(*args, **kwargs): expecting_failure = self._expecting_failure(testMethod) if expecting_failure: raise + self._needs_explicit_tearDown = True raise _GetOutOf_testPartExecutor(exc) setattr(self._testcase, self._testcase._testMethodName, wrapped_testMethod)