testtools

Merge lp:~lifeless/testtools/bug-335816-one-outcome-per-test into lp:~testtools-committers/testtools/trunk

  1. bug-335816-one-outcome-per-test
  2. Merge into trunk
Proposed by Robert Collins
Status: Merged
Approved by: Robert Collins
Approved revision: 63
Merged at revision: 63
Proposed branch: lp:~lifeless/testtools/bug-335816-one-outcome-per-test
Merge into: lp:~testtools-committers/testtools/trunk
Diff against target: 267 lines (+74/-32)
5 files modified
NEWS (+5/-0)
testtools/runtest.py (+20/-5)
testtools/testcase.py (+19/-12)
testtools/tests/test_runtest.py (+4/-8)
testtools/tests/test_testtools.py (+26/-7)
To merge this branch: bzr merge lp:~lifeless/testtools/bug-335816-one-outcome-per-test
Reviewer Review Type Date Requested Status
testtools developers Pending
Review via email: mp+25226@code.launchpad.net

Description of the change

Fix the bug where multiple exceptions in a single test corrupt test result objects output.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

jml says ok,

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-01-19 01:40:58 +0000
3+++ NEWS 2010-05-13 12:20:50 +0000
4@@ -34,6 +34,11 @@
5 * The backtrace test result output tests should now pass on windows and other
6 systems where os.sep is not '/'.
7
8+* When a cleanUp or tearDown exception occurs, it is now accumulated as a new
9+ traceback in the test details, rather than as a separate call to addError /
10+ addException. This makes testtools work better with most TestResult objects
11+ and fixes bug #335816.
12+
13
14 0.9.2
15 ~~~~~
16
17=== modified file 'testtools/runtest.py'
18--- testtools/runtest.py 2009-12-31 03:15:19 +0000
19+++ testtools/runtest.py 2010-05-13 12:20:50 +0000
20@@ -36,6 +36,8 @@
21 classes in the list).
22 :ivar exception_caught: An object returned when _run_user catches an
23 exception.
24+ :ivar _exceptions: A list of caught exceptions, used to do the single
25+ reporting of error/failure/skip etc.
26 """
27
28 def __init__(self, case, handlers=None):
29@@ -48,6 +50,7 @@
30 self.case = case
31 self.handlers = handlers or []
32 self.exception_caught = object()
33+ self._exceptions = []
34
35 def run(self, result=None):
36 """Run self.case reporting activity to result.
37@@ -86,7 +89,16 @@
38 result.startTest(self.case)
39 self.result = result
40 try:
41+ self._exceptions = []
42 self._run_core()
43+ if self._exceptions:
44+ # One or more caught exceptions, now trigger the test's
45+ # reporting method for just one.
46+ e = self._exceptions.pop()
47+ for exc_class, handler in self.handlers:
48+ if isinstance(e, exc_class):
49+ handler(self.case, self.result, e)
50+ break
51 finally:
52 result.stopTest(self.case)
53 return result
54@@ -96,7 +108,9 @@
55 if self.exception_caught == self._run_user(self.case._run_setup,
56 self.result):
57 # Don't run the test method if we failed getting here.
58- self.case._runCleanups(self.result)
59+ e = self.case._runCleanups(self.result)
60+ if e is not None:
61+ self._exceptions.append(e)
62 return
63 # Run everything from here on in. If any of the methods raise an
64 # exception we'll have failed.
65@@ -112,8 +126,9 @@
66 failed = True
67 finally:
68 try:
69- if not self._run_user(
70- self.case._runCleanups, self.result):
71+ e = self._run_user(self.case._runCleanups, self.result)
72+ if e is not None:
73+ self._exceptions.append(e)
74 failed = True
75 finally:
76 if not failed:
77@@ -134,9 +149,9 @@
78 # escape: but they are deprecated anyway.
79 exc_info = sys.exc_info()
80 e = exc_info[1]
81+ self.case.onException(exc_info)
82 for exc_class, handler in self.handlers:
83- self.case.onException(exc_info)
84 if isinstance(e, exc_class):
85- handler(self.case, self.result, e)
86+ self._exceptions.append(e)
87 return self.exception_caught
88 raise e
89
90=== modified file 'testtools/testcase.py'
91--- testtools/testcase.py 2010-01-16 04:36:08 +0000
92+++ testtools/testcase.py 2010-05-13 12:20:50 +0000
93@@ -76,6 +76,7 @@
94 unittest.TestCase.__init__(self, *args, **kwargs)
95 self._cleanups = []
96 self._unique_id_gen = itertools.count(1)
97+ self._traceback_id_gen = itertools.count(0)
98 self.__setup_called = False
99 self.__teardown_called = False
100 self.__details = {}
101@@ -145,9 +146,10 @@
102
103 See the docstring for addCleanup for more information.
104
105- Returns True if all cleanups ran without error, False otherwise.
106+ :return: None if all cleanups ran without error, the most recently
107+ raised exception from the cleanups otherwise.
108 """
109- ok = True
110+ last_exception = None
111 while self._cleanups:
112 function, arguments, keywordArguments = self._cleanups.pop()
113 try:
114@@ -155,9 +157,10 @@
115 except KeyboardInterrupt:
116 raise
117 except:
118- self._report_error(self, result, None)
119- ok = False
120- return ok
121+ exc_info = sys.exc_info()
122+ self._report_traceback(exc_info)
123+ last_exception = exc_info[1]
124+ return last_exception
125
126 def addCleanup(self, function, *arguments, **keywordArguments):
127 """Add a cleanup function to be called after tearDown.
128@@ -288,8 +291,7 @@
129 predicate(*args, **kwargs)
130 except self.failureException:
131 exc_info = sys.exc_info()
132- self.addDetail('traceback',
133- content.TracebackContent(exc_info, self))
134+ self._report_traceback(exc_info)
135 raise _ExpectedFailure(exc_info)
136 else:
137 raise _UnexpectedSuccess(reason)
138@@ -323,12 +325,14 @@
139
140 :seealso addOnException:
141 """
142+ if exc_info[0] not in [
143+ TestSkipped, _UnexpectedSuccess, _ExpectedFailure]:
144+ self._report_traceback(exc_info)
145 for handler in self.__exception_handlers:
146 handler(exc_info)
147
148 @staticmethod
149 def _report_error(self, result, err):
150- self._report_traceback()
151 result.addError(self, details=self.getDetails())
152
153 @staticmethod
154@@ -337,7 +341,6 @@
155
156 @staticmethod
157 def _report_failure(self, result, err):
158- self._report_traceback()
159 result.addFailure(self, details=self.getDetails())
160
161 @staticmethod
162@@ -349,9 +352,13 @@
163 self._add_reason(reason)
164 result.addSkip(self, details=self.getDetails())
165
166- def _report_traceback(self):
167- self.addDetail('traceback',
168- content.TracebackContent(sys.exc_info(), self))
169+ def _report_traceback(self, exc_info):
170+ tb_id = advance_iterator(self._traceback_id_gen)
171+ if tb_id:
172+ tb_label = 'traceback-%d' % tb_id
173+ else:
174+ tb_label = 'traceback'
175+ self.addDetail(tb_label, content.TracebackContent(exc_info, self))
176
177 @staticmethod
178 def _report_unexpected_success(self, result, err):
179
180=== modified file 'testtools/tests/test_runtest.py'
181--- testtools/tests/test_runtest.py 2009-12-31 03:15:19 +0000
182+++ testtools/tests/test_runtest.py 2010-05-13 12:20:50 +0000
183@@ -77,14 +77,12 @@
184 e = KeyError('Yo')
185 def raises():
186 raise e
187- def log_exc(self, result, err):
188- log.append((result, err))
189- run = RunTest(case, [(KeyError, log_exc)])
190+ run = RunTest(case, [(KeyError, None)])
191 run.result = ExtendedTestResult()
192 status = run._run_user(raises)
193 self.assertEqual(run.exception_caught, status)
194 self.assertEqual([], run.result._events)
195- self.assertEqual(["got it", (run.result, e)], log)
196+ self.assertEqual(["got it"], log)
197
198 def test__run_user_can_catch_Exception(self):
199 case = self.make_case()
200@@ -92,14 +90,12 @@
201 def raises():
202 raise e
203 log = []
204- def log_exc(self, result, err):
205- log.append((result, err))
206- run = RunTest(case, [(Exception, log_exc)])
207+ run = RunTest(case, [(Exception, None)])
208 run.result = ExtendedTestResult()
209 status = run._run_user(raises)
210 self.assertEqual(run.exception_caught, status)
211 self.assertEqual([], run.result._events)
212- self.assertEqual([(run.result, e)], log)
213+ self.assertEqual([], log)
214
215 def test__run_user_uncaught_Exception_raised(self):
216 case = self.make_case()
217
218=== modified file 'testtools/tests/test_testtools.py'
219--- testtools/tests/test_testtools.py 2010-01-16 04:20:26 +0000
220+++ testtools/tests/test_testtools.py 2010-05-13 12:20:50 +0000
221@@ -301,6 +301,9 @@
222 def runTest(self):
223 self._calls.append('runTest')
224
225+ def brokenTest(self):
226+ raise RuntimeError('Deliberate broken test')
227+
228 def tearDown(self):
229 self._calls.append('tearDown')
230 TestCase.tearDown(self)
231@@ -400,13 +403,29 @@
232 self.assertRaises(
233 KeyboardInterrupt, self.test.run, self.logging_result)
234
235- def test_multipleErrorsReported(self):
236- # Errors from all failing cleanups are reported.
237- self.test.addCleanup(lambda: 1/0)
238- self.test.addCleanup(lambda: 1/0)
239- self.test.run(self.logging_result)
240- self.assertErrorLogEqual(
241- ['startTest', 'addError', 'addError', 'stopTest'])
242+ def test_multipleCleanupErrorsReported(self):
243+ # Errors from all failing cleanups are reported as separate backtraces.
244+ self.test.addCleanup(lambda: 1/0)
245+ self.test.addCleanup(lambda: 1/0)
246+ self.logging_result = ExtendedTestResult()
247+ self.test.run(self.logging_result)
248+ self.assertEqual(['startTest', 'addError', 'stopTest'],
249+ [event[0] for event in self.logging_result._events])
250+ self.assertEqual(set(['traceback', 'traceback-1']),
251+ set(self.logging_result._events[1][2].keys()))
252+
253+ def test_multipleErrorsCoreAndCleanupReported(self):
254+ # Errors from all failing cleanups are reported, with stopTest,
255+ # startTest inserted.
256+ self.test = TestAddCleanup.LoggingTest('brokenTest')
257+ self.test.addCleanup(lambda: 1/0)
258+ self.test.addCleanup(lambda: 1/0)
259+ self.logging_result = ExtendedTestResult()
260+ self.test.run(self.logging_result)
261+ self.assertEqual(['startTest', 'addError', 'stopTest'],
262+ [event[0] for event in self.logging_result._events])
263+ self.assertEqual(set(['traceback', 'traceback-1', 'traceback-2']),
264+ set(self.logging_result._events[1][2].keys()))
265
266
267 class TestWithDetails(TestCase):

Subscribers

People subscribed via source and target branches