From a9640d75531d6cbbfd254b65435f238c26bf5cd9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 22 Aug 2021 10:33:52 +0300 Subject: bpo-44955: Always call stopTestRun() for implicitly created TestResult objects (GH-27831) 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 3c771c0..625d27e 100644 --- a/Lib/unittest/case.py +++ b/Lib/unittest/case.py @@ -557,73 +557,71 @@ class TestCase(object): 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 1c178a9..3adde41 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 @@ class Test_TestSkipping(unittest.TestCase): 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 @@ class Test_TestSkipping(unittest.TestCase): 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 @@ class Test_TestSkipping(unittest.TestCase): 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 @@ class Test_TestSkipping(unittest.TestCase): 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 @@ class Test_TestSkipping(unittest.TestCase): 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 0000000..57d1da5 --- /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. -- cgit v0.12