Bazaar

Merge lp:~spiv/bzr/traceback-accumulation-2.2 into lp:bzr/2.2

  1. traceback-accumulation-2.2
  2. Merge into 2.2
Proposed by Andrew Bennetts
Status: Merged
Approved by: Andrew Bennetts
Approved revision: no longer in the source branch.
Merged at revision: 5091
Proposed branch: lp:~spiv/bzr/traceback-accumulation-2.2
Merge into: lp:bzr/2.2
Diff against target: 81 lines (+16/-32)
2 files modified
NEWS (+4/-4)
bzrlib/tests/__init__.py (+12/-28)
To merge this branch: bzr merge lp:~spiv/bzr/traceback-accumulation-2.2
Reviewer Review Type Date Requested Status
Martin Pool Approve
Review via email: mp+35778@code.launchpad.net

Commit message

Replace TestCase__copy__ hack with more direct workaround in clone_test.

Description of the change

This is a more conservative workaround than the previous hack (which seems to mysteriously fail when sent to land in lp:bzr, but landed just fine in lp:bzr/2.2).

It removes the __copy__ method in favour of making clone_test explicitly reset just the details dict (given that the other mutable, shared attributes aren't causing us any grief).

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

+1, to my mind this is arguably a cleaner fix anyhow.

review: Approve
Revision history for this message
Andrew Bennetts (spiv) wrote :

Yes, I agree it's cleaner. With hindsight I'm not sure why I didn't do this in the first place. :)

Thanks for the speedy review.

Revision history for this message
Andrew Bennetts (spiv) wrote :

sent to pqm by email

Revision history for this message
Vincent Ladeuil (vila) wrote :

Anonymous XXX tend to lost their value over time, this will be later fixed in trunk anyway right ?

Revision history for this message
Andrew Bennetts (spiv) wrote :

Vincent Ladeuil wrote:
> Anonymous XXX tend to lost their value over time, this will be later fixed in trunk anyway right ?

The goal is to fix this in testtools. Then we can remove this
workaround (and probably also remove 'clone_test' entirely in favour of
the testtools equivalent, clone_test_with_new_id).

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-09-16 07:12:32 +0000
3+++ NEWS 2010-09-17 05:27:47 +0000
4@@ -75,10 +75,10 @@
5 *******
6
7 * Tracebacks from a parameterized test are no longer reported against every
8- parameterization of that test. This was done by adding a custom
9- ``__copy__`` method to TestCase, so that ``multiply_tests`` no longer
10- causes testtools.TestCase instances to share a details dict (or other
11- mutable attributes). (Andrew Bennetts, #625574)
12+ parameterization of that test. This was done by adding a hack to
13+ ``bzrlib.tests.clone_test`` so that it no longer causes
14+ testtools.TestCase instances to share a details dict.
15+ (Andrew Bennetts, #625574)
16
17
18 bzr 2.2
19
20=== modified file 'bzrlib/tests/__init__.py'
21--- bzrlib/tests/__init__.py 2010-09-15 06:44:55 +0000
22+++ bzrlib/tests/__init__.py 2010-09-17 05:27:47 +0000
23@@ -799,15 +799,6 @@
24 _log_file_name = None
25 # record lsprof data when performing benchmark calls.
26 _gather_lsprof_in_benchmarks = False
27- # Mutable attributes defined in bzrlib.tests.TestCase and
28- # testtools.TestCase that should not be shallow copied by clone_test.
29- # This is ugly, but probably the best we can do until testtools provide a
30- # nicer way to deal with this, see
31- # <https://bugs.launchpad.net/testtools/+bug/637725>.
32- _attrs_to_deepcopy = (
33- '_cleanups', 'exception_handlers', '_unique_id_gen',
34- '_traceback_id_gen', '_TestCase__details',
35- '_TestCase__exception_handlers')
36
37 def __init__(self, methodName='testMethod'):
38 super(TestCase, self).__init__(methodName)
39@@ -818,25 +809,6 @@
40 self.exception_handlers.insert(0,
41 (TestNotApplicable, self._do_not_applicable))
42
43- def __copy__(self):
44- # XXX: This method works around the lack of a way to correctly clone a
45- # test with current testtools. See
46- # <https://bugs.launchpad.net/testtools/+bug/637725>.
47- # The work around is to:
48- # - shallow copy self.__dict__
49- # - deep copy individual attributes in the _attrs_to_deepcopy black
50- # list.
51- # - create a new instance (skipping __init__) with the new __dict__.
52- attrs = self.__dict__.copy()
53- for attr_name in self._attrs_to_deepcopy:
54- if attr_name in attrs:
55- attrs[attr_name] = copy.deepcopy(attrs[attr_name])
56- # Some Python voodoo to create an instance without invoking its
57- # __init__, because we've already constructed the __dict__ to use.
58- new_instance = self.__class__.__new__(self.__class__)
59- new_instance.__dict__ = attrs
60- return new_instance
61-
62 def setUp(self):
63 super(TestCase, self).setUp()
64 for feature in getattr(self, '_test_needs_features', []):
65@@ -4080,6 +4052,18 @@
66 """
67 new_test = copy.copy(test)
68 new_test.id = lambda: new_id
69+ # XXX: Workaround <https://bugs.launchpad.net/testtools/+bug/637725>, which
70+ # causes cloned tests to share the 'details' dict. This makes it hard to
71+ # read the test output for parameterized tests, because tracebacks will be
72+ # associated with irrelevant tests.
73+ try:
74+ details = new_test._TestCase__details
75+ except AttributeError:
76+ # must be a different version of testtools than expected. Do nothing.
77+ pass
78+ else:
79+ # Reset the '__details' dict.
80+ new_test._TestCase__details = {}
81 return new_test
82
83

Subscribers

People subscribed via source and target branches