From 35f8c030a3e8a532752f201ab5dc85fe379e2ffa Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 19 Aug 2021 15:04:29 +0300 Subject: [PATCH] bpo-44955: Always call stopTestRun() for implicitly created TestResult objects Method stopTestRun() is now always called in pair with method startTestRun() for TestResult objects implicitly created in TestCase.run(). Previously it was not called for test methods and classes decorated with a skipping decorator. --- Lib/unittest/case.py | 102 +++++++++--------- Lib/unittest/test/test_skipping.py | 50 ++++++++- .../2021-08-19-15-03-54.bpo-44955.1mxFQS.rst | 5 + 3 files changed, 104 insertions(+), 53 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-08-19-15-03-54.bpo-44955.1mxFQS.rst diff --git a/Lib/unittest/case.py b/Lib/unittest/case.py index 3c771c0ca8e102..625d27ef6dc61e 100644 --- a/Lib/unittest/case.py +++ b/Lib/unittest/case.py @@ -557,73 +557,71 @@ def _callCleanup(self, function, /, *args, **kwargs): function(*args, **kwargs) def run(self, result=None): - orig_result = result if result is None: result = self.defaultTestResult() startTestRun = getattr(result, 'startTestRun', None) + stopTestRun = getattr(result, 'stopTestRun', None) if startTestRun is not None: startTestRun() + else: + stopTestRun = None result.startTest(self) - - testMethod = getattr(self, self._testMethodName) - if (getattr(self.__class__, "__unittest_skip__", False) or - getattr(testMethod, "__unittest_skip__", False)): - # If the class or method was skipped. - try: + try: + testMethod = getattr(self, self._testMethodName) + if (getattr(self.__class__, "__unittest_skip__", False) or + getattr(testMethod, "__unittest_skip__", False)): + # If the class or method was skipped. skip_why = (getattr(self.__class__, '__unittest_skip_why__', '') or getattr(testMethod, '__unittest_skip_why__', '')) self._addSkip(result, self, skip_why) - finally: - result.stopTest(self) - return - 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 - outcome = _Outcome(result) - try: - self._outcome = outcome + return + + expecting_failure = ( + getattr(self, "__unittest_expecting_failure__", False) or + getattr(testMethod, "__unittest_expecting_failure__", False) + ) + outcome = _Outcome(result) + try: + self._outcome = outcome - with outcome.testPartExecutor(self): - self._callSetUp() - if outcome.success: - outcome.expecting_failure = expecting_failure - with outcome.testPartExecutor(self, isTest=True): - self._callTestMethod(testMethod) - outcome.expecting_failure = False with outcome.testPartExecutor(self): - self._callTearDown() - - self.doCleanups() - for test, reason in outcome.skipped: - self._addSkip(result, test, reason) - self._feedErrorsToResult(result, outcome.errors) - if outcome.success: - if expecting_failure: - if outcome.expectedFailure: - self._addExpectedFailure(result, outcome.expectedFailure) + self._callSetUp() + if outcome.success: + outcome.expecting_failure = expecting_failure + with outcome.testPartExecutor(self, isTest=True): + self._callTestMethod(testMethod) + outcome.expecting_failure = False + with outcome.testPartExecutor(self): + self._callTearDown() + + self.doCleanups() + for test, reason in outcome.skipped: + self._addSkip(result, test, reason) + self._feedErrorsToResult(result, outcome.errors) + if outcome.success: + if expecting_failure: + if outcome.expectedFailure: + self._addExpectedFailure(result, outcome.expectedFailure) + else: + self._addUnexpectedSuccess(result) else: - self._addUnexpectedSuccess(result) - else: - result.addSuccess(self) - return result + result.addSuccess(self) + return result + finally: + # explicitly break reference cycles: + # outcome.errors -> frame -> outcome -> outcome.errors + # outcome.expectedFailure -> frame -> outcome -> outcome.expectedFailure + outcome.errors.clear() + outcome.expectedFailure = None + + # clear the outcome, no more needed + self._outcome = None + finally: result.stopTest(self) - if orig_result is None: - stopTestRun = getattr(result, 'stopTestRun', None) - if stopTestRun is not None: - stopTestRun() - - # explicitly break reference cycles: - # outcome.errors -> frame -> outcome -> outcome.errors - # outcome.expectedFailure -> frame -> outcome -> outcome.expectedFailure - outcome.errors.clear() - outcome.expectedFailure = None - - # clear the outcome, no more needed - self._outcome = None + if stopTestRun is not None: + stopTestRun() def doCleanups(self): """Execute all cleanup functions. Normally called for you after diff --git a/Lib/unittest/test/test_skipping.py b/Lib/unittest/test/test_skipping.py index 1c178a95f750ff..3adde41b04e737 100644 --- a/Lib/unittest/test/test_skipping.py +++ b/Lib/unittest/test/test_skipping.py @@ -7,6 +7,8 @@ class Test_TestSkipping(unittest.TestCase): def test_skipping(self): class Foo(unittest.TestCase): + def defaultTestResult(self): + return LoggingResult(events) def test_skip_me(self): self.skipTest("skip") events = [] @@ -16,8 +18,15 @@ def test_skip_me(self): self.assertEqual(events, ['startTest', 'addSkip', 'stopTest']) self.assertEqual(result.skipped, [(test, "skip")]) + events = [] + test.run() + self.assertEqual(events, ['startTestRun', 'startTest', 'addSkip', + 'stopTest', 'stopTestRun']) + # Try letting setUp skip the test now. class Foo(unittest.TestCase): + def defaultTestResult(self): + return LoggingResult(events) def setUp(self): self.skipTest("testing") def test_nothing(self): pass @@ -29,8 +38,15 @@ def test_nothing(self): pass self.assertEqual(result.skipped, [(test, "testing")]) self.assertEqual(result.testsRun, 1) + events = [] + test.run() + self.assertEqual(events, ['startTestRun', 'startTest', 'addSkip', + 'stopTest', 'stopTestRun']) + def test_skipping_subtests(self): class Foo(unittest.TestCase): + def defaultTestResult(self): + return LoggingResult(events) def test_skip_me(self): with self.subTest(a=1): with self.subTest(b=2): @@ -54,11 +70,20 @@ def test_skip_me(self): self.assertIsNot(subtest, test) self.assertEqual(result.skipped[2], (test, "skip 3")) + events = [] + test.run() + self.assertEqual(events, + ['startTestRun', 'startTest', 'addSkip', 'addSkip', + 'addSkip', 'stopTest', 'stopTestRun']) + def test_skipping_decorators(self): op_table = ((unittest.skipUnless, False, True), (unittest.skipIf, True, False)) for deco, do_skip, dont_skip in op_table: class Foo(unittest.TestCase): + def defaultTestResult(self): + return LoggingResult(events) + @deco(do_skip, "testing") def test_skip(self): pass @@ -66,6 +91,7 @@ def test_skip(self): pass def test_dont_skip(self): pass test_do_skip = Foo("test_skip") test_dont_skip = Foo("test_dont_skip") + suite = unittest.TestSuite([test_do_skip, test_dont_skip]) events = [] result = LoggingResult(events) @@ -78,19 +104,41 @@ def test_dont_skip(self): pass self.assertEqual(result.skipped, [(test_do_skip, "testing")]) self.assertTrue(result.wasSuccessful()) + events = [] + test_do_skip.run() + self.assertEqual(len(result.skipped), 1) + self.assertEqual(events, ['startTestRun', 'startTest', 'addSkip', + 'stopTest', 'stopTestRun']) + + events = [] + test_dont_skip.run() + self.assertEqual(len(result.skipped), 1) + self.assertEqual(events, ['startTestRun', 'startTest', 'addSuccess', + 'stopTest', 'stopTestRun']) + def test_skip_class(self): @unittest.skip("testing") class Foo(unittest.TestCase): + def defaultTestResult(self): + return LoggingResult(events) def test_1(self): record.append(1) + events = [] record = [] - result = unittest.TestResult() + result = LoggingResult(events) test = Foo("test_1") suite = unittest.TestSuite([test]) suite.run(result) + self.assertEqual(events, ['startTest', 'addSkip', 'stopTest']) self.assertEqual(result.skipped, [(test, "testing")]) self.assertEqual(record, []) + events = [] + test.run() + self.assertEqual(events, ['startTestRun', 'startTest', 'addSkip', + 'stopTest', 'stopTestRun']) + self.assertEqual(record, []) + def test_skip_non_unittest_class(self): @unittest.skip("testing") class Mixin: diff --git a/Misc/NEWS.d/next/Library/2021-08-19-15-03-54.bpo-44955.1mxFQS.rst b/Misc/NEWS.d/next/Library/2021-08-19-15-03-54.bpo-44955.1mxFQS.rst new file mode 100644 index 00000000000000..57d1da533cde0d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-08-19-15-03-54.bpo-44955.1mxFQS.rst @@ -0,0 +1,5 @@ +Method :meth:`~unittest.TestResult.stopTestRun` is now always called in pair +with method :meth:`~unittest.TestResult.startTestRun` for +:class:`~unittest.TestResult` objects implicitly created in +:meth:`~unittest.TestCase.run`. Previously it was not called for test +methods and classes decorated with a skipping decorator.