From 4e13a5fe3f43f4b40f91257d1d4e68c10ab8e610 Mon Sep 17 00:00:00 2001 From: Daniel Miller Date: Fri, 26 Apr 2024 10:24:58 -0400 Subject: [PATCH 1/4] Report class cleanup exceptions --- AUTHORS | 1 + changelog/11728.improvement.rst | 1 + src/_pytest/unittest.py | 15 +++++++++ testing/test_unittest.py | 58 +++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+) create mode 100644 changelog/11728.improvement.rst diff --git a/AUTHORS b/AUTHORS index d7148acfc51..4f61c05914b 100644 --- a/AUTHORS +++ b/AUTHORS @@ -101,6 +101,7 @@ Cyrus Maden Damian Skrzypczak Daniel Grana Daniel Hahler +Daniel Miller Daniel Nuri Daniel Sánchez Castelló Daniel Valenzuela Zenteno diff --git a/changelog/11728.improvement.rst b/changelog/11728.improvement.rst new file mode 100644 index 00000000000..3464bd90b5d --- /dev/null +++ b/changelog/11728.improvement.rst @@ -0,0 +1 @@ +Class cleanup exceptions are now reported. diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index 5099904fd41..10cd7212e52 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -111,6 +111,19 @@ def _register_unittest_setup_class_fixture(self, cls: type) -> None: return None cleanup = getattr(cls, "doClassCleanups", lambda: None) + def process_teardown_exceptions(raise_last: bool): + errors = getattr(cls, "tearDown_exceptions", None) + if not errors: + return + others = errors[:-1] if raise_last else errors + if others: + num = len(errors) + for n, (exc_type, exc, tb) in enumerate(others, start=1): + print(f"\nclass cleanup error ({n} of {num}):", file=sys.stderr) + traceback.print_exception(exc_type, exc, tb) + if raise_last: + raise errors[-1][1] + def unittest_setup_class_fixture( request: FixtureRequest, ) -> Generator[None, None, None]: @@ -125,6 +138,7 @@ def unittest_setup_class_fixture( # follow this here. except Exception: cleanup() + process_teardown_exceptions(raise_last=False) raise yield try: @@ -132,6 +146,7 @@ def unittest_setup_class_fixture( teardown() finally: cleanup() + process_teardown_exceptions(raise_last=True) self.session._fixturemanager._register_fixture( # Use a unique name to speed up lookup. diff --git a/testing/test_unittest.py b/testing/test_unittest.py index 9ecb548eeec..f9e7741dbb4 100644 --- a/testing/test_unittest.py +++ b/testing/test_unittest.py @@ -1500,6 +1500,64 @@ def test_cleanup_called_the_right_number_of_times(): assert passed == 1 +def test_class_cleanups_failure_in_setup(pytester: Pytester) -> None: + testpath = pytester.makepyfile( + """ + import unittest + class MyTestCase(unittest.TestCase): + @classmethod + def setUpClass(cls): + def cleanup(n): + raise Exception(f"fail {n}") + cls.addClassCleanup(cleanup, 2) + cls.addClassCleanup(cleanup, 1) + raise Exception("fail 0") + def test(self): + pass + """ + ) + result = pytester.runpytest('-s', testpath) + result.assert_outcomes(passed=0, errors=1) + result.stderr.fnmatch_lines([ + 'class cleanup error (1 of 2):', + 'Exception: fail 1', + 'class cleanup error (2 of 2):', + 'Exception: fail 2', + ]) + result.stdout.fnmatch_lines([ + '* ERROR at setup of MyTestCase.test *', + 'E * Exception: fail 0', + ]) + + +def test_class_cleanups_failure_in_teardown(pytester: Pytester) -> None: + testpath = pytester.makepyfile( + """ + import unittest + class MyTestCase(unittest.TestCase): + @classmethod + def setUpClass(cls): + def cleanup(n): + raise Exception(f"fail {n}") + cls.addClassCleanup(cleanup, 2) + cls.addClassCleanup(cleanup, 1) + def test(self): + pass + """ + ) + result = pytester.runpytest('-s', testpath) + result.assert_outcomes(passed=1, errors=1) + result.stderr.fnmatch_lines([ + 'class cleanup error (1 of 2):', + 'Traceback *', + 'Exception: fail 1', + ]) + result.stdout.fnmatch_lines([ + '* ERROR at teardown of MyTestCase.test *', + 'E * Exception: fail 2', + ]) + + def test_traceback_pruning(pytester: Pytester) -> None: """Regression test for #9610 - doesn't crash during traceback pruning.""" pytester.makepyfile( From e209a3db1576be5f755f90e2c5504471a02610f2 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 26 Apr 2024 14:43:10 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- testing/test_unittest.py | 50 +++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/testing/test_unittest.py b/testing/test_unittest.py index f9e7741dbb4..00d364fc172 100644 --- a/testing/test_unittest.py +++ b/testing/test_unittest.py @@ -1516,18 +1516,22 @@ def test(self): pass """ ) - result = pytester.runpytest('-s', testpath) + result = pytester.runpytest("-s", testpath) result.assert_outcomes(passed=0, errors=1) - result.stderr.fnmatch_lines([ - 'class cleanup error (1 of 2):', - 'Exception: fail 1', - 'class cleanup error (2 of 2):', - 'Exception: fail 2', - ]) - result.stdout.fnmatch_lines([ - '* ERROR at setup of MyTestCase.test *', - 'E * Exception: fail 0', - ]) + result.stderr.fnmatch_lines( + [ + "class cleanup error (1 of 2):", + "Exception: fail 1", + "class cleanup error (2 of 2):", + "Exception: fail 2", + ] + ) + result.stdout.fnmatch_lines( + [ + "* ERROR at setup of MyTestCase.test *", + "E * Exception: fail 0", + ] + ) def test_class_cleanups_failure_in_teardown(pytester: Pytester) -> None: @@ -1545,17 +1549,21 @@ def test(self): pass """ ) - result = pytester.runpytest('-s', testpath) + result = pytester.runpytest("-s", testpath) result.assert_outcomes(passed=1, errors=1) - result.stderr.fnmatch_lines([ - 'class cleanup error (1 of 2):', - 'Traceback *', - 'Exception: fail 1', - ]) - result.stdout.fnmatch_lines([ - '* ERROR at teardown of MyTestCase.test *', - 'E * Exception: fail 2', - ]) + result.stderr.fnmatch_lines( + [ + "class cleanup error (1 of 2):", + "Traceback *", + "Exception: fail 1", + ] + ) + result.stdout.fnmatch_lines( + [ + "* ERROR at teardown of MyTestCase.test *", + "E * Exception: fail 2", + ] + ) def test_traceback_pruning(pytester: Pytester) -> None: From 122fd05e1ef5cbc7023203121a51d9272db6b54a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 26 Apr 2024 20:10:32 -0300 Subject: [PATCH 3/4] Use ExceptionGroup instead of printing, improve changelog --- changelog/11728.improvement.rst | 2 +- src/_pytest/unittest.py | 30 ++++--- testing/test_unittest.py | 143 ++++++++++++++++++-------------- 3 files changed, 101 insertions(+), 74 deletions(-) diff --git a/changelog/11728.improvement.rst b/changelog/11728.improvement.rst index 3464bd90b5d..1e87fc5ed88 100644 --- a/changelog/11728.improvement.rst +++ b/changelog/11728.improvement.rst @@ -1 +1 @@ -Class cleanup exceptions are now reported. +For ``unittest``-based tests, exceptions during class cleanup (as raised by functions registered with :meth:`TestCase.addClassCleanup `) are now reported instead of silently failing. diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index 10cd7212e52..d6457c8782a 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -32,6 +32,9 @@ import pytest +if sys.version_info[:2] < (3, 11): + from exceptiongroup import BaseExceptionGroup + if TYPE_CHECKING: import unittest @@ -111,18 +114,19 @@ def _register_unittest_setup_class_fixture(self, cls: type) -> None: return None cleanup = getattr(cls, "doClassCleanups", lambda: None) - def process_teardown_exceptions(raise_last: bool): - errors = getattr(cls, "tearDown_exceptions", None) - if not errors: + def process_teardown_exceptions() -> None: + # tearDown_exceptions is a list set in the class containing exc_infos for errors during + # teardown for the class. + exc_infos = getattr(cls, "tearDown_exceptions", None) + if not exc_infos: return - others = errors[:-1] if raise_last else errors - if others: - num = len(errors) - for n, (exc_type, exc, tb) in enumerate(others, start=1): - print(f"\nclass cleanup error ({n} of {num}):", file=sys.stderr) - traceback.print_exception(exc_type, exc, tb) - if raise_last: - raise errors[-1][1] + exceptions = [exc for (_, exc, _) in exc_infos] + # If a single exception, raise it directly as this provides a more readable + # error. + if len(exceptions) == 1: + raise exceptions[0] + else: + raise BaseExceptionGroup("Unittest class cleanup errors", exceptions) def unittest_setup_class_fixture( request: FixtureRequest, @@ -138,7 +142,7 @@ def unittest_setup_class_fixture( # follow this here. except Exception: cleanup() - process_teardown_exceptions(raise_last=False) + process_teardown_exceptions() raise yield try: @@ -146,7 +150,7 @@ def unittest_setup_class_fixture( teardown() finally: cleanup() - process_teardown_exceptions(raise_last=True) + process_teardown_exceptions() self.session._fixturemanager._register_fixture( # Use a unique name to speed up lookup. diff --git a/testing/test_unittest.py b/testing/test_unittest.py index 00d364fc172..d726e74d603 100644 --- a/testing/test_unittest.py +++ b/testing/test_unittest.py @@ -1500,70 +1500,93 @@ def test_cleanup_called_the_right_number_of_times(): assert passed == 1 -def test_class_cleanups_failure_in_setup(pytester: Pytester) -> None: - testpath = pytester.makepyfile( - """ - import unittest - class MyTestCase(unittest.TestCase): - @classmethod - def setUpClass(cls): - def cleanup(n): - raise Exception(f"fail {n}") - cls.addClassCleanup(cleanup, 2) - cls.addClassCleanup(cleanup, 1) - raise Exception("fail 0") - def test(self): - pass +class TestClassCleanupErrors: """ - ) - result = pytester.runpytest("-s", testpath) - result.assert_outcomes(passed=0, errors=1) - result.stderr.fnmatch_lines( - [ - "class cleanup error (1 of 2):", - "Exception: fail 1", - "class cleanup error (2 of 2):", - "Exception: fail 2", - ] - ) - result.stdout.fnmatch_lines( - [ - "* ERROR at setup of MyTestCase.test *", - "E * Exception: fail 0", - ] - ) + Make sure to show exceptions raised during class cleanup function (those registered + via addClassCleanup()). + See #11728. + """ -def test_class_cleanups_failure_in_teardown(pytester: Pytester) -> None: - testpath = pytester.makepyfile( + def test_class_cleanups_failure_in_setup(self, pytester: Pytester) -> None: + testpath = pytester.makepyfile( + """ + import unittest + class MyTestCase(unittest.TestCase): + @classmethod + def setUpClass(cls): + def cleanup(n): + raise Exception(f"fail {n}") + cls.addClassCleanup(cleanup, 2) + cls.addClassCleanup(cleanup, 1) + raise Exception("fail 0") + def test(self): + pass """ - import unittest - class MyTestCase(unittest.TestCase): - @classmethod - def setUpClass(cls): - def cleanup(n): - raise Exception(f"fail {n}") - cls.addClassCleanup(cleanup, 2) - cls.addClassCleanup(cleanup, 1) - def test(self): - pass - """ - ) - result = pytester.runpytest("-s", testpath) - result.assert_outcomes(passed=1, errors=1) - result.stderr.fnmatch_lines( - [ - "class cleanup error (1 of 2):", - "Traceback *", - "Exception: fail 1", - ] - ) - result.stdout.fnmatch_lines( - [ - "* ERROR at teardown of MyTestCase.test *", - "E * Exception: fail 2", - ] - ) + ) + result = pytester.runpytest("-s", testpath) + result.assert_outcomes(passed=0, errors=1) + result.stdout.fnmatch_lines( + [ + "*Unittest class cleanup errors *2 sub-exceptions*", + "*Exception: fail 1", + "*Exception: fail 2", + ] + ) + result.stdout.fnmatch_lines( + [ + "* ERROR at setup of MyTestCase.test *", + "E * Exception: fail 0", + ] + ) + + def test_class_cleanups_failure_in_teardown(self, pytester: Pytester) -> None: + testpath = pytester.makepyfile( + """ + import unittest + class MyTestCase(unittest.TestCase): + @classmethod + def setUpClass(cls): + def cleanup(n): + raise Exception(f"fail {n}") + cls.addClassCleanup(cleanup, 2) + cls.addClassCleanup(cleanup, 1) + def test(self): + pass + """ + ) + result = pytester.runpytest("-s", testpath) + result.assert_outcomes(passed=1, errors=1) + result.stdout.fnmatch_lines( + [ + "*Unittest class cleanup errors *2 sub-exceptions*", + "*Exception: fail 1", + "*Exception: fail 2", + ] + ) + + def test_class_cleanup_1_failure_in_teardown(self, pytester: Pytester) -> None: + testpath = pytester.makepyfile( + """ + import unittest + class MyTestCase(unittest.TestCase): + @classmethod + def setUpClass(cls): + def cleanup(n): + raise Exception(f"fail {n}") + cls.addClassCleanup(cleanup, 1) + def test(self): + pass + """ + ) + result = pytester.runpytest("-s", testpath) + result.assert_outcomes(passed=1, errors=1) + result.stdout.fnmatch_lines( + [ + "*ERROR at teardown of MyTestCase.test*", + "*Exception: fail 1", + ] + ) def test_traceback_pruning(pytester: Pytester) -> None: From 8f9d6003f9ed533be48620bf3a638554cce3bb90 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 26 Apr 2024 20:18:08 -0300 Subject: [PATCH 4/4] Use ExceptionGroup instead of BaseExceptionGroup --- src/_pytest/unittest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index d6457c8782a..8f1791bf744 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -33,7 +33,7 @@ if sys.version_info[:2] < (3, 11): - from exceptiongroup import BaseExceptionGroup + from exceptiongroup import ExceptionGroup if TYPE_CHECKING: import unittest @@ -122,11 +122,11 @@ def process_teardown_exceptions() -> None: return exceptions = [exc for (_, exc, _) in exc_infos] # If a single exception, raise it directly as this provides a more readable - # error. + # error (hopefully this will improve in #12255). if len(exceptions) == 1: raise exceptions[0] else: - raise BaseExceptionGroup("Unittest class cleanup errors", exceptions) + raise ExceptionGroup("Unittest class cleanup errors", exceptions) def unittest_setup_class_fixture( request: FixtureRequest,