Merge lp:~lifeless/testtools/bug-812793 into lp:~testtools-committers/testtools/trunk

Proposed by Robert Collins
Status: Merged
Approved by: Jonathan Lange
Approved revision: 305
Merged at revision: 305
Proposed branch: lp:~lifeless/testtools/bug-812793
Merge into: lp:~testtools-committers/testtools/trunk
Diff against target: 71 lines (+29/-4)
3 files modified
NEWS (+7/-0)
testtools/testcase.py (+6/-3)
testtools/tests/test_testcase.py (+16/-1)
To merge this branch: bzr merge lp:~lifeless/testtools/bug-812793
Reviewer Review Type Date Requested Status
Jonathan Lange Approve
Review via email: mp+143860@code.launchpad.net

Description of the change

Minor tweak, should be self evident.

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

The code change is indeed self-evident and good. Thanks.

This might have unintended consequences. _details_to_str treats the 'traceback' detail specially, rendering it not so much as a detail but as a traditional test traceback. With this change, if a fixture has a detail named 'traceback', that will be rendered as the main error. This could be confusing to users.

On balance, I'm OK with this patch landing, but would be much happier if we could resolve (or at least document) that potential source of confusion.

(Note: I think the _details_to_str special casing of 'traceback' is a bit of a kludge and would be glad to see it go.)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2013-01-18 09:17:19 +0000
+++ NEWS 2013-01-18 11:27:25 +0000
@@ -17,6 +17,13 @@
17* Testtools now uses setuptools rather than distutils so that we can document17* Testtools now uses setuptools rather than distutils so that we can document
18 the extras dependency. (Robert Collins)18 the extras dependency. (Robert Collins)
1919
20Improvements
21------------
22
23* Testtools will no longer override test code registered details called
24 'traceback' when reporting caught exceptions from test code.
25 (Robert Collins, #812793)
26
200.9.24270.9.24
21~~~~~~28~~~~~~
2229
2330
=== modified file 'testtools/testcase.py'
--- testtools/testcase.py 2013-01-18 09:17:19 +0000
+++ testtools/testcase.py 2013-01-18 11:27:25 +0000
@@ -506,9 +506,12 @@
506 def _report_traceback(self, exc_info, tb_label='traceback'):506 def _report_traceback(self, exc_info, tb_label='traceback'):
507 id_gen = self._traceback_id_gens.setdefault(507 id_gen = self._traceback_id_gens.setdefault(
508 tb_label, itertools.count(0))508 tb_label, itertools.count(0))
509 tb_id = advance_iterator(id_gen)509 while True:
510 if tb_id:510 tb_id = advance_iterator(id_gen)
511 tb_label = '%s-%d' % (tb_label, tb_id)511 if tb_id:
512 tb_label = '%s-%d' % (tb_label, tb_id)
513 if tb_label not in self.getDetails():
514 break
512 self.addDetail(tb_label, content.TracebackContent(exc_info, self))515 self.addDetail(tb_label, content.TracebackContent(exc_info, self))
513516
514 @staticmethod517 @staticmethod
515518
=== modified file 'testtools/tests/test_testcase.py'
--- testtools/tests/test_testcase.py 2012-07-27 18:53:44 +0000
+++ testtools/tests/test_testcase.py 2013-01-18 11:27:25 +0000
@@ -23,7 +23,10 @@
23 _b,23 _b,
24 _u,24 _u,
25 )25 )
26from testtools.content import TracebackContent26from testtools.content import (
27 text_content,
28 TracebackContent,
29 )
27from testtools.matchers import (30from testtools.matchers import (
28 Annotate,31 Annotate,
29 DocTestMatches,32 DocTestMatches,
@@ -609,6 +612,18 @@
609 self.assertFails(expected_error, self.assertIsNotNone, None)612 self.assertFails(expected_error, self.assertIsNotNone, None)
610613
611614
615 def test_fail_preserves_traceback_detail(self):
616 class Test(TestCase):
617 def test(self):
618 self.addDetail('traceback', text_content('foo'))
619 self.fail('bar')
620 test = Test('test')
621 result = ExtendedTestResult()
622 test.run(result)
623 self.assertEqual(set(['traceback', 'traceback-1']),
624 set(result._events[1][2].keys()))
625
626
612class TestAddCleanup(TestCase):627class TestAddCleanup(TestCase):
613 """Tests for TestCase.addCleanup."""628 """Tests for TestCase.addCleanup."""
614629

Subscribers

People subscribed via source and target branches