testtools-dev team mailing list archive
-
testtools-dev team
-
Mailing list archive
-
Message #00077
[Merge] lp:~spiv/testtools/cloned-details-637725 into lp:testtools
Andrew Bennetts has proposed merging lp:~spiv/testtools/cloned-details-637725 into lp:testtools.
Requested reviews:
testtools developers (testtools-dev)
Related bugs:
#637725 clone_test_with_new_id shares details dict between test case instances
https://bugs.launchpad.net/bugs/637725
This is a minimal fix for the accidental sharing of a single details dict among cloned TestCases. It works by making self.__details get initialised in a lazy fashion by addDetails/getDetails.
I'm a bit uncomfortable that it ignores the larger question of how cloning should behave with other mutable state a freshly-initialised TestCase could have. e.g. self._cleanups is shared... but because _runCleanups should always empty it completely that doesn't have any effect — unless a subclass does add something to it during __init__, or cloned tests are run concurrently in threads. And there are other minor issues I can see, such as self._unique_id_gen will be effectively set to itertools.count(0) in a clone (because copy.copy seems to always produce a count(0) regardless of the state).
So I don't think this is the end of the story here, but it's a simple patch that solves the most obvious problem... and it has a test :)
--
https://code.launchpad.net/~spiv/testtools/cloned-details-637725/+merge/35780
Your team testtools developers is requested to review the proposed merge of lp:~spiv/testtools/cloned-details-637725 into lp:testtools.
=== modified file 'testtools/testcase.py'
--- testtools/testcase.py 2010-09-12 00:36:10 +0000
+++ testtools/testcase.py 2010-09-17 06:36:52 +0000
@@ -84,7 +84,9 @@
self._traceback_id_gen = itertools.count(0)
self.__setup_called = False
self.__teardown_called = False
- self.__details = {}
+ # __details is lazy-initialized so that a constructed-but-not-run
+ # TestCase is safe to use with clone_test_with_new_id.
+ self.__details = None
self.__RunTest = kwargs.get('runTest', RunTest)
self.__exception_handlers = []
self.exception_handlers = [
@@ -117,6 +119,8 @@
:param content_object: The content object for this detail. See
testtools.content for more detail.
"""
+ if self.__details is None:
+ self.__details = {}
self.__details[name] = content_object
def getDetails(self):
@@ -124,6 +128,8 @@
For more details see pydoc testtools.TestResult.
"""
+ if self.__details is None:
+ self.__details = {}
return self.__details
def patch(self, obj, attribute, value):
=== modified file 'testtools/tests/test_testtools.py'
--- testtools/tests/test_testtools.py 2010-09-12 00:36:10 +0000
+++ testtools/tests/test_testtools.py 2010-09-17 06:36:52 +0000
@@ -770,6 +770,18 @@
self.assertEqual(oldName, test.id(),
"the original test instance should be unchanged.")
+ def test_cloned_testcase_does_not_share_details(self):
+ """A cloned TestCase does not share the details dict."""
+ class Test(TestCase):
+ def test_foo(self):
+ self.addDetail(
+ 'foo', content.Content('text/plain', lambda: 'foo'))
+ orig_test = Test('test_foo')
+ cloned_test = clone_test_with_new_id(orig_test, self.getUniqueString())
+ orig_test.run(unittest.TestResult())
+ self.assertEqual('foo', orig_test.getDetails()['foo'].iter_bytes())
+ self.assertEqual(None, cloned_test.getDetails().get('foo'))
+
class TestDetailsProvided(TestWithDetails):
Follow ups