Skip to content

Commit 664448d

Browse files
bpo-30856: Update TestResult early, without buffering in _Outcome (GH-28180)
TestResult methods addFailure(), addError(), addSkip() and addSubTest() are now called immediately after raising an exception in test or finishing a subtest. Previously they were called only after finishing the test clean up.
1 parent dea59cf commit 664448d

File tree

7 files changed

+76
-65
lines changed

7 files changed

+76
-65
lines changed

Lib/unittest/case.py

Lines changed: 30 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,10 @@ def __init__(self, result=None):
4747
self.result = result
4848
self.result_supports_subtests = hasattr(result, "addSubTest")
4949
self.success = True
50-
self.skipped = []
5150
self.expectedFailure = None
52-
self.errors = []
5351

5452
@contextlib.contextmanager
55-
def testPartExecutor(self, test_case, isTest=False):
53+
def testPartExecutor(self, test_case, subTest=False):
5654
old_success = self.success
5755
self.success = True
5856
try:
@@ -61,7 +59,7 @@ def testPartExecutor(self, test_case, isTest=False):
6159
raise
6260
except SkipTest as e:
6361
self.success = False
64-
self.skipped.append((test_case, str(e)))
62+
_addSkip(self.result, test_case, str(e))
6563
except _ShouldStop:
6664
pass
6765
except:
@@ -70,17 +68,36 @@ def testPartExecutor(self, test_case, isTest=False):
7068
self.expectedFailure = exc_info
7169
else:
7270
self.success = False
73-
self.errors.append((test_case, exc_info))
71+
if subTest:
72+
self.result.addSubTest(test_case.test_case, test_case, exc_info)
73+
else:
74+
_addError(self.result, test_case, exc_info)
7475
# explicitly break a reference cycle:
7576
# exc_info -> frame -> exc_info
7677
exc_info = None
7778
else:
78-
if self.result_supports_subtests and self.success:
79-
self.errors.append((test_case, None))
79+
if subTest and self.success:
80+
self.result.addSubTest(test_case.test_case, test_case, None)
8081
finally:
8182
self.success = self.success and old_success
8283

8384

85+
def _addSkip(result, test_case, reason):
86+
addSkip = getattr(result, 'addSkip', None)
87+
if addSkip is not None:
88+
addSkip(test_case, reason)
89+
else:
90+
warnings.warn("TestResult has no addSkip method, skips not reported",
91+
RuntimeWarning, 2)
92+
result.addSuccess(test_case)
93+
94+
def _addError(result, test, exc_info):
95+
if result is not None and exc_info is not None:
96+
if issubclass(exc_info[0], test.failureException):
97+
result.addFailure(test, exc_info)
98+
else:
99+
result.addError(test, exc_info)
100+
84101
def _id(obj):
85102
return obj
86103

@@ -467,15 +484,6 @@ def __repr__(self):
467484
return "<%s testMethod=%s>" % \
468485
(strclass(self.__class__), self._testMethodName)
469486

470-
def _addSkip(self, result, test_case, reason):
471-
addSkip = getattr(result, 'addSkip', None)
472-
if addSkip is not None:
473-
addSkip(test_case, reason)
474-
else:
475-
warnings.warn("TestResult has no addSkip method, skips not reported",
476-
RuntimeWarning, 2)
477-
result.addSuccess(test_case)
478-
479487
@contextlib.contextmanager
480488
def subTest(self, msg=_subtest_msg_sentinel, **params):
481489
"""Return a context manager that will return the enclosed block
@@ -494,7 +502,7 @@ def subTest(self, msg=_subtest_msg_sentinel, **params):
494502
params_map = parent.params.new_child(params)
495503
self._subtest = _SubTest(self, msg, params_map)
496504
try:
497-
with self._outcome.testPartExecutor(self._subtest, isTest=True):
505+
with self._outcome.testPartExecutor(self._subtest, subTest=True):
498506
yield
499507
if not self._outcome.success:
500508
result = self._outcome.result
@@ -507,16 +515,6 @@ def subTest(self, msg=_subtest_msg_sentinel, **params):
507515
finally:
508516
self._subtest = parent
509517

510-
def _feedErrorsToResult(self, result, errors):
511-
for test, exc_info in errors:
512-
if isinstance(test, _SubTest):
513-
result.addSubTest(test.test_case, test, exc_info)
514-
elif exc_info is not None:
515-
if issubclass(exc_info[0], self.failureException):
516-
result.addFailure(test, exc_info)
517-
else:
518-
result.addError(test, exc_info)
519-
520518
def _addExpectedFailure(self, result, exc_info):
521519
try:
522520
addExpectedFailure = result.addExpectedFailure
@@ -574,7 +572,7 @@ def run(self, result=None):
574572
# If the class or method was skipped.
575573
skip_why = (getattr(self.__class__, '__unittest_skip_why__', '')
576574
or getattr(testMethod, '__unittest_skip_why__', ''))
577-
self._addSkip(result, self, skip_why)
575+
_addSkip(result, self, skip_why)
578576
return result
579577

580578
expecting_failure = (
@@ -589,16 +587,13 @@ def run(self, result=None):
589587
self._callSetUp()
590588
if outcome.success:
591589
outcome.expecting_failure = expecting_failure
592-
with outcome.testPartExecutor(self, isTest=True):
590+
with outcome.testPartExecutor(self):
593591
self._callTestMethod(testMethod)
594592
outcome.expecting_failure = False
595593
with outcome.testPartExecutor(self):
596594
self._callTearDown()
597-
598595
self.doCleanups()
599-
for test, reason in outcome.skipped:
600-
self._addSkip(result, test, reason)
601-
self._feedErrorsToResult(result, outcome.errors)
596+
602597
if outcome.success:
603598
if expecting_failure:
604599
if outcome.expectedFailure:
@@ -609,11 +604,10 @@ def run(self, result=None):
609604
result.addSuccess(self)
610605
return result
611606
finally:
612-
# explicitly break reference cycles:
613-
# outcome.errors -> frame -> outcome -> outcome.errors
607+
# explicitly break reference cycle:
614608
# outcome.expectedFailure -> frame -> outcome -> outcome.expectedFailure
615-
outcome.errors.clear()
616609
outcome.expectedFailure = None
610+
outcome = None
617611

618612
# clear the outcome, no more needed
619613
self._outcome = None

Lib/unittest/test/test_case.py

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,8 @@ def test(self):
197197
super(Foo, self).test()
198198
raise RuntimeError('raised by Foo.test')
199199

200-
expected = ['startTest', 'setUp', 'test', 'tearDown',
201-
'addError', 'stopTest']
200+
expected = ['startTest', 'setUp', 'test',
201+
'addError', 'tearDown', 'stopTest']
202202
Foo(events).run(result)
203203
self.assertEqual(events, expected)
204204

@@ -216,7 +216,7 @@ def test(self):
216216
raise RuntimeError('raised by Foo.test')
217217

218218
expected = ['startTestRun', 'startTest', 'setUp', 'test',
219-
'tearDown', 'addError', 'stopTest', 'stopTestRun']
219+
'addError', 'tearDown', 'stopTest', 'stopTestRun']
220220
Foo(events).run()
221221
self.assertEqual(events, expected)
222222

@@ -236,8 +236,8 @@ def test(self):
236236
super(Foo, self).test()
237237
self.fail('raised by Foo.test')
238238

239-
expected = ['startTest', 'setUp', 'test', 'tearDown',
240-
'addFailure', 'stopTest']
239+
expected = ['startTest', 'setUp', 'test',
240+
'addFailure', 'tearDown', 'stopTest']
241241
Foo(events).run(result)
242242
self.assertEqual(events, expected)
243243

@@ -252,7 +252,7 @@ def test(self):
252252
self.fail('raised by Foo.test')
253253

254254
expected = ['startTestRun', 'startTest', 'setUp', 'test',
255-
'tearDown', 'addFailure', 'stopTest', 'stopTestRun']
255+
'addFailure', 'tearDown', 'stopTest', 'stopTestRun']
256256
events = []
257257
Foo(events).run()
258258
self.assertEqual(events, expected)
@@ -353,19 +353,19 @@ def test(self):
353353
def test_run_call_order__subtests(self):
354354
events = []
355355
result = LoggingResult(events)
356-
expected = ['startTest', 'setUp', 'test', 'tearDown',
356+
expected = ['startTest', 'setUp', 'test',
357357
'addSubTestFailure', 'addSubTestSuccess',
358358
'addSubTestFailure', 'addSubTestFailure',
359-
'addSubTestSuccess', 'addError', 'stopTest']
359+
'addSubTestSuccess', 'addError', 'tearDown', 'stopTest']
360360
self._check_call_order__subtests(result, events, expected)
361361

362362
def test_run_call_order__subtests_legacy(self):
363363
# With a legacy result object (without an addSubTest method),
364364
# text execution stops after the first subtest failure.
365365
events = []
366366
result = LegacyLoggingResult(events)
367-
expected = ['startTest', 'setUp', 'test', 'tearDown',
368-
'addFailure', 'stopTest']
367+
expected = ['startTest', 'setUp', 'test',
368+
'addFailure', 'tearDown', 'stopTest']
369369
self._check_call_order__subtests(result, events, expected)
370370

371371
def _check_call_order__subtests_success(self, result, events, expected_events):
@@ -386,9 +386,9 @@ def test_run_call_order__subtests_success(self):
386386
result = LoggingResult(events)
387387
# The 6 subtest successes are individually recorded, in addition
388388
# to the whole test success.
389-
expected = (['startTest', 'setUp', 'test', 'tearDown']
389+
expected = (['startTest', 'setUp', 'test']
390390
+ 6 * ['addSubTestSuccess']
391-
+ ['addSuccess', 'stopTest'])
391+
+ ['tearDown', 'addSuccess', 'stopTest'])
392392
self._check_call_order__subtests_success(result, events, expected)
393393

394394
def test_run_call_order__subtests_success_legacy(self):
@@ -413,8 +413,8 @@ def test(self):
413413
self.fail('failure')
414414
self.fail('failure')
415415

416-
expected = ['startTest', 'setUp', 'test', 'tearDown',
417-
'addSubTestFailure', 'stopTest']
416+
expected = ['startTest', 'setUp', 'test',
417+
'addSubTestFailure', 'tearDown', 'stopTest']
418418
Foo(events).run(result)
419419
self.assertEqual(events, expected)
420420

Lib/unittest/test/test_functiontestcase.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ def test():
5858
def tearDown():
5959
events.append('tearDown')
6060

61-
expected = ['startTest', 'setUp', 'test', 'tearDown',
62-
'addError', 'stopTest']
61+
expected = ['startTest', 'setUp', 'test',
62+
'addError', 'tearDown', 'stopTest']
6363
unittest.FunctionTestCase(test, setUp, tearDown).run(result)
6464
self.assertEqual(events, expected)
6565

@@ -84,8 +84,8 @@ def test():
8484
def tearDown():
8585
events.append('tearDown')
8686

87-
expected = ['startTest', 'setUp', 'test', 'tearDown',
88-
'addFailure', 'stopTest']
87+
expected = ['startTest', 'setUp', 'test',
88+
'addFailure', 'tearDown', 'stopTest']
8989
unittest.FunctionTestCase(test, setUp, tearDown).run(result)
9090
self.assertEqual(events, expected)
9191

Lib/unittest/test/test_result.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,8 @@ def test_foo(self):
816816
self.assertEqual(str(test_case), description)
817817
self.assertIn('ValueError: bad cleanup2', formatted_exc)
818818
self.assertNotIn('TypeError', formatted_exc)
819-
self.assertIn(expected_out, formatted_exc)
819+
self.assertIn('\nStdout:\nset up\ndo cleanup2\n', formatted_exc)
820+
self.assertNotIn('\ndo cleanup1\n', formatted_exc)
820821
test_case, formatted_exc = result.errors[1]
821822
self.assertEqual(str(test_case), description)
822823
self.assertIn('TypeError: bad cleanup1', formatted_exc)
@@ -847,13 +848,16 @@ def test_foo(self):
847848
self.assertIn('ZeroDivisionError: division by zero', formatted_exc)
848849
self.assertNotIn('ValueError', formatted_exc)
849850
self.assertNotIn('TypeError', formatted_exc)
850-
self.assertIn(expected_out, formatted_exc)
851+
self.assertIn('\nStdout:\nset up\n', formatted_exc)
852+
self.assertNotIn('\ndo cleanup2\n', formatted_exc)
853+
self.assertNotIn('\ndo cleanup1\n', formatted_exc)
851854
test_case, formatted_exc = result.errors[1]
852855
self.assertEqual(str(test_case), description)
853856
self.assertIn('ValueError: bad cleanup2', formatted_exc)
854857
self.assertNotIn('ZeroDivisionError', formatted_exc)
855858
self.assertNotIn('TypeError', formatted_exc)
856-
self.assertIn(expected_out, formatted_exc)
859+
self.assertIn('\nStdout:\nset up\ndo cleanup2\n', formatted_exc)
860+
self.assertNotIn('\ndo cleanup1\n', formatted_exc)
857861
test_case, formatted_exc = result.errors[2]
858862
self.assertEqual(str(test_case), description)
859863
self.assertIn('TypeError: bad cleanup1', formatted_exc)
@@ -887,13 +891,16 @@ def test_foo(self):
887891
self.assertIn('ZeroDivisionError: division by zero', formatted_exc)
888892
self.assertNotIn('ValueError', formatted_exc)
889893
self.assertNotIn('TypeError', formatted_exc)
890-
self.assertIn(expected_out, formatted_exc)
894+
self.assertIn('\nStdout:\nset up\ntear down\n', formatted_exc)
895+
self.assertNotIn('\ndo cleanup2\n', formatted_exc)
896+
self.assertNotIn('\ndo cleanup1\n', formatted_exc)
891897
test_case, formatted_exc = result.errors[1]
892898
self.assertEqual(str(test_case), description)
893899
self.assertIn('ValueError: bad cleanup2', formatted_exc)
894900
self.assertNotIn('ZeroDivisionError', formatted_exc)
895901
self.assertNotIn('TypeError', formatted_exc)
896-
self.assertIn(expected_out, formatted_exc)
902+
self.assertIn('\nStdout:\nset up\ntear down\ndo cleanup2\n', formatted_exc)
903+
self.assertNotIn('\ndo cleanup1\n', formatted_exc)
897904
test_case, formatted_exc = result.errors[2]
898905
self.assertEqual(str(test_case), description)
899906
self.assertIn('TypeError: bad cleanup1', formatted_exc)

Lib/unittest/test/test_runner.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ def testNothing(self):
7878
pass
7979

8080
test = TestableTest('testNothing')
81-
outcome = test._outcome = _Outcome()
81+
result = unittest.TestResult()
82+
outcome = test._outcome = _Outcome(result=result)
8283

8384
CleanUpExc = Exception('foo')
8485
exc2 = Exception('bar')
@@ -94,10 +95,13 @@ def cleanup2():
9495
self.assertFalse(test.doCleanups())
9596
self.assertFalse(outcome.success)
9697

97-
((_, (Type1, instance1, _)),
98-
(_, (Type2, instance2, _))) = reversed(outcome.errors)
99-
self.assertEqual((Type1, instance1), (Exception, CleanUpExc))
100-
self.assertEqual((Type2, instance2), (Exception, exc2))
98+
(_, msg2), (_, msg1) = result.errors
99+
self.assertIn('in cleanup1', msg1)
100+
self.assertIn('raise CleanUpExc', msg1)
101+
self.assertIn('Exception: foo', msg1)
102+
self.assertIn('in cleanup2', msg2)
103+
self.assertIn('raise exc2', msg2)
104+
self.assertIn('Exception: bar', msg2)
101105

102106
def testCleanupInRun(self):
103107
blowUp = False

Lib/unittest/test/test_skipping.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ def tearDown(self):
197197
result = LoggingResult(events)
198198
test = Foo("test_skip_me")
199199
self.assertIs(test.run(result), result)
200-
self.assertEqual(events, ['startTest', 'addSkip', 'addFailure', 'stopTest'])
200+
self.assertEqual(events, ['startTest', 'addFailure', 'addSkip', 'stopTest'])
201201
self.assertEqual(result.skipped, [(test, "skip")])
202202

203203
def test_skipping_and_fail_in_cleanup(self):
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
:class:`unittest.TestResult` methods
2+
:meth:`~unittest.TestResult.addFailure`,
3+
:meth:`~unittest.TestResult.addError`, :meth:`~unittest.TestResult.addSkip`
4+
and :meth:`~unittest.TestResult.addSubTest` are now called immediately after
5+
raising an exception in test or finishing a subtest. Previously they were
6+
called only after finishing the test clean up.

0 commit comments

Comments
 (0)