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

[Andrew Bennetts]

Replace TestCase__copy__ hack with more direct workaround in clone_test.

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).

[Martin Pool (mbp)]

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

review: Approve

[Andrew Bennetts (spiv)]

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.

[Andrew Bennetts (spiv)]

sent to pqm by email

[Vincent Ladeuil (vila)]

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

[Andrew Bennetts (spiv)]

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).

=== modified file 'NEWS'
--- NEWS	2010-09-16 07:12:32 +0000
+++ NEWS	2010-09-17 05:27:47 +0000
@@ -75,10 +75,10 @@
 *******
 
 * Tracebacks from a parameterized test are no longer reported against every
-  parameterization of that test.  This was done by adding a custom
-  ``__copy__`` method to TestCase, so that ``multiply_tests`` no longer
-  causes testtools.TestCase instances to share a details dict (or other
-  mutable attributes).  (Andrew Bennetts, #625574)
+  parameterization of that test.  This was done by adding a hack to
+  ``bzrlib.tests.clone_test`` so that it no longer causes
+  testtools.TestCase instances to share a details dict.
+  (Andrew Bennetts, #625574)
 
 
 bzr 2.2

=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py	2010-09-15 06:44:55 +0000
+++ bzrlib/tests/__init__.py	2010-09-17 05:27:47 +0000
@@ -799,15 +799,6 @@
     _log_file_name = None
     # record lsprof data when performing benchmark calls.
     _gather_lsprof_in_benchmarks = False
-    # Mutable attributes defined in bzrlib.tests.TestCase and
-    # testtools.TestCase that should not be shallow copied by clone_test.
-    # This is ugly, but probably the best we can do until testtools provide a
-    # nicer way to deal with this, see
-    # <https://bugs.launchpad.net/testtools/+bug/637725>.
-    _attrs_to_deepcopy = (
-        '_cleanups', 'exception_handlers', '_unique_id_gen',
-        '_traceback_id_gen', '_TestCase__details',
-        '_TestCase__exception_handlers')
 
     def __init__(self, methodName='testMethod'):
         super(TestCase, self).__init__(methodName)
@@ -818,25 +809,6 @@
         self.exception_handlers.insert(0,
             (TestNotApplicable, self._do_not_applicable))
 
-    def __copy__(self):
-        # XXX: This method works around the lack of a way to correctly clone a
-        # test with current testtools.  See
-        # <https://bugs.launchpad.net/testtools/+bug/637725>.
-        # The work around is to:
-        #  - shallow copy self.__dict__
-        #  - deep copy individual attributes in the _attrs_to_deepcopy black
-        #    list.
-        #  - create a new instance (skipping __init__) with the new __dict__.
-        attrs = self.__dict__.copy()
-        for attr_name in self._attrs_to_deepcopy:
-            if attr_name in attrs:
-                attrs[attr_name] = copy.deepcopy(attrs[attr_name])
-        # Some Python voodoo to create an instance without invoking its
-        # __init__, because we've already constructed the __dict__ to use.
-        new_instance = self.__class__.__new__(self.__class__)
-        new_instance.__dict__ = attrs
-        return new_instance
-
     def setUp(self):
         super(TestCase, self).setUp()
         for feature in getattr(self, '_test_needs_features', []):
@@ -4080,6 +4052,18 @@
     """
     new_test = copy.copy(test)
     new_test.id = lambda: new_id
+    # XXX: Workaround <https://bugs.launchpad.net/testtools/+bug/637725>, which
+    # causes cloned tests to share the 'details' dict.  This makes it hard to
+    # read the test output for parameterized tests, because tracebacks will be
+    # associated with irrelevant tests.
+    try:
+        details = new_test._TestCase__details
+    except AttributeError:
+        # must be a different version of testtools than expected.  Do nothing.
+        pass
+    else:
+        # Reset the '__details' dict.
+        new_test._TestCase__details = {}
     return new_test