From e4d966971e4a5619bca4dc176ed8450e430cf919 Mon Sep 17 00:00:00 2001 From: Quentin BEY Date: Thu, 26 Mar 2020 18:24:58 +0100 Subject: [PATCH] unittest: fix the `_GetOutOf_testPartExecutor` exception kind. Fixes https://github.com/pytest-dev/pytest/issues/6947 `_GetOutOf_testPartExecutor` was introduced in https://github.com/pytest-dev/pytest/commit/04f27d4eb469fb9c76fd2d100564a0f7d30028df and allow to catch Exception from a unittest TestCase, but as it inheritate from `KeyboardInterrupt` it is managed differently by unittest (see. https://github.com/python/cpython/blob/032de7324e30c6b44ef272cea3be205a3d768759/Lib/unittest/case.py#L61-L62) as the `testPartExecutor` reraise it directly. This prevent unittest to run the cleanup functions properly. Since this change enable the teardown methods to be ran properly by unittest, the explicit teardown is not required anymore. --- AUTHORS | 1 + changelog/6947.bugfix.rst | 1 + src/_pytest/unittest.py | 8 +--- testing/test_unittest.py | 85 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 changelog/6947.bugfix.rst diff --git a/AUTHORS b/AUTHORS index af0dc62c4d8..871f5b1bbbd 100644 --- a/AUTHORS +++ b/AUTHORS @@ -220,6 +220,7 @@ Pieter Mulder Piotr Banaszkiewicz Pulkit Goyal Punyashloka Biswal +Quentin Bey Quentin Pradet Ralf Schmitt Ralph Giles diff --git a/changelog/6947.bugfix.rst b/changelog/6947.bugfix.rst new file mode 100644 index 00000000000..28180cb129a --- /dev/null +++ b/changelog/6947.bugfix.rst @@ -0,0 +1 @@ +Unittest cleanup functions are called even if a test fails. diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index 2047876e5f1..6188a4120d6 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -113,15 +113,12 @@ 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 @@ -209,7 +206,7 @@ def runtest(self): testMethod = getattr(self._testcase, self._testcase._testMethodName) - class _GetOutOf_testPartExecutor(KeyboardInterrupt): + class _GetOutOf_testPartExecutor(Exception): """Helper exception to get out of unittests's testPartExecutor (see TestCase.run).""" @functools.wraps(testMethod) @@ -218,13 +215,12 @@ def wrapped_testMethod(*args, **kwargs): features can have a chance to kick in (notably --pdb)""" try: self.ihook.pytest_pyfunc_call(pyfuncitem=self) - except unittest.SkipTest: + except (unittest.SkipTest, exit.Exception): raise except Exception as exc: 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) diff --git a/testing/test_unittest.py b/testing/test_unittest.py index c5fc20239b1..c074ee3f778 100644 --- a/testing/test_unittest.py +++ b/testing/test_unittest.py @@ -209,6 +209,91 @@ def test_demo(self): assert type(obj).__name__ != "TestCaseObjectsShouldBeCleanedUp" +def test_cleanup_called_issue6947(testdir): + """ + Are unittest.TestCase cleanup functions not invoked on test failure? + When a test fails and the cleanup is not called, all following tests may fail. + This is truly visible for test using a database when the transaction is not rolled back at the end. + """ + testpath = testdir.makepyfile( + """ + import unittest + import pytest + + def increment_cleanup_count(): + TestCaseObjectsShouldBeCleanedUp.cleanup_counter += 1 + + class TestCaseObjectsShouldBeCleanedUp(unittest.TestCase): + cleanup_counter = 0 + def setUp(self): + self.addCleanup(increment_cleanup_count) + def test_1_goes_well(self): + # First test to be run + assert TestCaseObjectsShouldBeCleanedUp.cleanup_counter == 0 + def test_2_that_fails(self): + # Second test to be run, should fail + assert False + def test_3_goes_well_after_failure(self): + # Third test to be run + assert TestCaseObjectsShouldBeCleanedUp.cleanup_counter == 2 + def test_4_exit(self): + pytest.exit("pytest_exit called") + def test_5_not_run(self): + assert True + """ + ) + reprec = testdir.inline_run(testpath) + passed, skipped, failed = reprec.countoutcomes() + # test_1_goes_well passed + # test_2_that_fails failed + # test_3_goes_well_after_failure passed + # and finally test_4 quit + assert failed == 1, failed + assert passed == 2 + assert passed + skipped + failed == 3 + + +def test_teardown_called_issue6947(testdir): + """ + This test assert the TestCase.tearDown method is called once after every tests. + + Looks very similar to test_cleanup_called_issue6947 but testing a different mechanism. + """ + testpath = testdir.makepyfile( + """ + import unittest + import pytest + + class TestCaseObjectsShouldBeCleanedUp(unittest.TestCase): + cleanup_counter = 0 + def tearDown(self): + TestCaseObjectsShouldBeCleanedUp.cleanup_counter += 1 + def test_1_goes_well(self): + # First test to be run + assert TestCaseObjectsShouldBeCleanedUp.cleanup_counter == 0 + def test_2_that_fails(self): + # Second test to be run, should fail + assert False + def test_3_goes_well_after_failure(self): + # Third test to be run + assert TestCaseObjectsShouldBeCleanedUp.cleanup_counter == 2 + def test_4_exit(self): + pytest.exit("pytest_exit called") + def test_5_not_run(self): + assert True + """ + ) + reprec = testdir.inline_run(testpath) + passed, skipped, failed = reprec.countoutcomes() + # test_1_goes_well passed + # test_2_that_fails failed + # test_3_goes_well_after_failure passed + # and finally test_4 quit + assert failed == 1, failed + assert passed == 2 + assert passed + skipped + failed == 3 + + def test_unittest_skip_issue148(testdir): testpath = testdir.makepyfile( """