Rev 5372: Pull out the test-case changes that I understand. in http://bazaar.launchpad.net/~jameinel/bzr/2.4-613247-test-cases

John Arbash Meinel john at arbash-meinel.com
Thu May 19 18:59:44 UTC 2011


At http://bazaar.launchpad.net/~jameinel/bzr/2.4-613247-test-cases

------------------------------------------------------------
revno: 5372
revision-id: john at arbash-meinel.com-20110519185931-9xcxj2ow2v1y4xvv
parent: gzlist at googlemail.com-20110518185816-3szkam8ga7d3b0u4
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.4-613247-test-cases
timestamp: Thu 2011-05-19 20:59:31 +0200
message:
  Pull out the test-case changes that I understand.
  
  Stuff like avoiding closures over self by using a local variable,
  and then assigning that local variable to an attribute of self.
-------------- next part --------------
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py	2011-05-18 18:49:43 +0000
+++ b/bzrlib/branch.py	2011-05-19 18:59:31 +0000
@@ -3191,10 +3191,7 @@
         try:
             target.unlock()
         finally:
-            try:
-                raise exc_info[0], exc_info[1], exc_info[2]
-            finally:
-                del exc_info
+            raise exc_info[0], exc_info[1], exc_info[2]
     else:
         target.unlock()
         return result

=== modified file 'bzrlib/cleanup.py'
--- a/bzrlib/cleanup.py	2010-08-14 20:51:06 +0000
+++ b/bzrlib/cleanup.py	2011-05-19 18:59:31 +0000
@@ -188,10 +188,7 @@
                 # but don't propagate them.
                 _run_cleanup(cleanup, *c_args, **kwargs)
         if exc_info is not None:
-            try:
-                raise exc_info[0], exc_info[1], exc_info[2]
-            finally:
-                del exc_info
+            raise exc_info[0], exc_info[1], exc_info[2]
         # No error, so we can return the result
         return result
 

=== modified file 'bzrlib/decorators.py'
--- a/bzrlib/decorators.py	2011-05-18 18:49:43 +0000
+++ b/bzrlib/decorators.py	2011-05-19 18:59:31 +0000
@@ -109,10 +109,7 @@
         try:
             self.unlock()
         finally:
-            try:
-                raise exc_info[0], exc_info[1], exc_info[2]
-            finally:
-                del exc_info
+            raise exc_info[0], exc_info[1], exc_info[2]
     else:
         self.unlock()
         return result
@@ -158,10 +155,7 @@
             try:
                 self.unlock()
             finally:
-                try:
-                    raise exc_info[0], exc_info[1], exc_info[2]
-                finally:
-                    del exc_info
+                raise exc_info[0], exc_info[1], exc_info[2]
         else:
             self.unlock()
             return result
@@ -183,10 +177,7 @@
         try:
             self.unlock()
         finally:
-            try:
-                raise exc_info[0], exc_info[1], exc_info[2]
-            finally:
-                del exc_info
+            raise exc_info[0], exc_info[1], exc_info[2]
     else:
         self.unlock()
         return result
@@ -220,10 +211,7 @@
             try:
                 self.unlock()
             finally:
-                try:
-                    raise exc_info[0], exc_info[1], exc_info[2]
-                finally:
-                    del exc_info
+                raise exc_info[0], exc_info[1], exc_info[2]
         else:
             self.unlock()
             return result

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2011-05-18 18:49:43 +0000
+++ b/bzrlib/errors.py	2011-05-19 18:59:31 +0000
@@ -1659,7 +1659,6 @@
 
     def __init__(self, exc_info):
         import traceback
-        # GZ 2010-08-10: Cycle with exc_tb/exc_info affects at least one test
         self.exc_type, self.exc_value, self.exc_tb = exc_info
         self.exc_info = exc_info
         traceback_strings = traceback.format_exception(

=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py	2011-05-18 18:49:43 +0000
+++ b/bzrlib/osutils.py	2011-05-19 18:59:31 +0000
@@ -261,10 +261,7 @@
             else:
                 rename_func(tmp_name, new)
     if failure_exc is not None:
-        try:
-            raise failure_exc[0], failure_exc[1], failure_exc[2]
-        finally:
-            del failure_exc
+        raise failure_exc[0], failure_exc[1], failure_exc[2]
 
 
 # In Python 2.4.2 and older, os.path.abspath and os.path.realpath

=== modified file 'bzrlib/tests/TestUtil.py'
--- a/bzrlib/tests/TestUtil.py	2011-05-18 18:55:55 +0000
+++ b/bzrlib/tests/TestUtil.py	2011-05-19 18:59:31 +0000
@@ -19,7 +19,6 @@
 import sys
 import logging
 import unittest
-import weakref
 
 from bzrlib import pyutils
 
@@ -83,36 +82,14 @@
         tests = list(self)
         tests.reverse()
         self._tests = []
-        stream = getattr(result, "stream", None)
-        # With subunit, not only is stream underscored, but the actual result
-        # object is hidden inside a wrapper decorator, get out the real stream
-        if stream is None:
-            stream = result.decorated._stream
-        stored_count = 0
-        from bzrlib.tests import selftest_debug_flags
-        notify = "collection" in selftest_debug_flags
         while tests:
             if result.shouldStop:
                 self._tests = reversed(tests)
                 break
-            case = _run_and_collect_case(tests.pop(), result)()
-            new_stored_count = getattr(result, "_count_stored_tests", int)()
-            if case is not None and isinstance(case, unittest.TestCase):
-                if stored_count == new_stored_count and notify:
-                    # Testcase didn't fail, but somehow is still alive
-                    stream.write("Uncollected test case: %s\n" % (case.id(),))
-                # Zombie the testcase but leave a working stub id method
-                case.__dict__ = {"id": lambda _id=case.id(): _id}
-            stored_count = new_stored_count
+            tests.pop().run(result)
         return result
 
 
-def _run_and_collect_case(case, res):
-    """Run test case against result and use weakref to drop the refcount"""
-    case.run(res)
-    return weakref.ref(case)
-
-
 class TestLoader(unittest.TestLoader):
     """Custom TestLoader to extend the stock python one."""
 

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2011-05-18 18:58:16 +0000
+++ b/bzrlib/tests/__init__.py	2011-05-19 18:59:31 +0000
@@ -377,21 +377,13 @@
         if isinstance(test, TestCase):
             test.addCleanup(self._check_leaked_threads, test)
 
-    def stopTest(self, test):
-        super(ExtendedTestResult, self).stopTest(test)
-        # Manually break cycles, means touching various private things but hey
-        getDetails = getattr(test, "getDetails", None)
-        if getDetails is not None:
-            getDetails().clear()
-        type_equality_funcs = getattr(test, "_type_equality_funcs", None)
-        if type_equality_funcs is not None:
-            type_equality_funcs.clear()
-        self._traceback_from_test = None
-
     def startTests(self):
         self.report_tests_starting()
         self._active_threads = threading.enumerate()
 
+    def stopTest(self, test):
+        self._traceback_from_test = None
+
     def _check_leaked_threads(self, test):
         """See if any threads have leaked since last call
 
@@ -492,10 +484,6 @@
         self.not_applicable_count += 1
         self.report_not_applicable(test, reason)
 
-    def _count_stored_tests(self):
-        """Count of tests instances kept alive due to not succeeding"""
-        return self.error_count + self.failure_count + self.known_failure_count
-
     def _post_mortem(self, tb=None):
         """Start a PDB post mortem session."""
         if os.environ.get('BZR_TEST_PDB', None):
@@ -975,6 +963,10 @@
         super(TestCase, self).setUp()
         for feature in getattr(self, '_test_needs_features', []):
             self.requireFeature(feature)
+        self._log_contents = None
+        self.addDetail("log", content.Content(content.ContentType("text",
+            "plain", {"charset": "utf8"}),
+            lambda:[self._get_log(keep_log_file=True)]))
         self._cleanEnvironment()
         self._silenceUI()
         self._startLogFile()
@@ -1331,13 +1323,13 @@
         try:
             def capture():
                 orig_log_exception_quietly()
-                captured.append(sys.exc_info()[1])
+                captured.append(sys.exc_info())
             trace.log_exception_quietly = capture
             func(*args, **kwargs)
         finally:
             trace.log_exception_quietly = orig_log_exception_quietly
         self.assertLength(1, captured)
-        err = captured[0]
+        err = captured[0][1]
         self.assertIsInstance(err, exception_class)
         return err
 
@@ -1638,14 +1630,7 @@
 
         The file is removed as the test is torn down.
         """
-        pseudo_log_file = StringIO()
-        def _get_log_contents_for_weird_testtools_api():
-            return [pseudo_log_file.getvalue().decode(
-                "utf-8", "replace").encode("utf-8")]          
-        self.addDetail("log", content.Content(content.ContentType("text",
-            "plain", {"charset": "utf8"}),
-            _get_log_contents_for_weird_testtools_api))
-        self._log_file = pseudo_log_file
+        self._log_file = StringIO()
         self._log_memento = trace.push_log_file(self._log_file)
         self.addCleanup(self._finishLogFile)
 
@@ -1658,6 +1643,8 @@
             # flush the log file, to get all content
             trace._trace_file.flush()
         trace.pop_log_file(self._log_memento)
+        # Cache the log result and delete the file on disk
+        self._get_log(False)
 
     def thisFailsStrictLockCheck(self):
         """It is known that this test would fail with -Dstrict_locks.
@@ -1712,9 +1699,7 @@
     def _restoreHooks(self):
         for klass, (name, hooks) in self._preserved_hooks.items():
             setattr(klass, name, hooks)
-        self._preserved_hooks.clear()
-        bzrlib.hooks._lazy_hooks = self._preserved_lazy_hooks
-        self._preserved_lazy_hooks.clear()
+        hooks._lazy_hooks = self._preserved_lazy_hooks
 
     def knownFailure(self, reason):
         """This test has failed for some known reason."""
@@ -1812,6 +1797,41 @@
     def log(self, *args):
         trace.mutter(*args)
 
+    def _get_log(self, keep_log_file=False):
+        """Internal helper to get the log from bzrlib.trace for this test.
+
+        Please use self.getDetails, or self.get_log to access this in test case
+        code.
+
+        :param keep_log_file: When True, if the log is still a file on disk
+            leave it as a file on disk. When False, if the log is still a file
+            on disk, the log file is deleted and the log preserved as
+            self._log_contents.
+        :return: A string containing the log.
+        """
+        if self._log_contents is not None:
+            try:
+                self._log_contents.decode('utf8')
+            except UnicodeDecodeError:
+                unicodestr = self._log_contents.decode('utf8', 'replace')
+                self._log_contents = unicodestr.encode('utf8')
+            return self._log_contents
+        if self._log_file is not None:
+            log_contents = self._log_file.getvalue()
+            try:
+                log_contents.decode('utf8')
+            except UnicodeDecodeError:
+                unicodestr = log_contents.decode('utf8', 'replace')
+                log_contents = unicodestr.encode('utf8')
+            if not keep_log_file:
+                self._log_file = None
+                # Permit multiple calls to get_log until we clean it up in
+                # finishLogFile
+                self._log_contents = log_contents
+            return log_contents
+        else:
+            return "No log file content."
+
     def get_log(self):
         """Get a unicode string containing the log from bzrlib.trace.
 
@@ -3064,9 +3084,6 @@
                             result_decorators=result_decorators,
                             )
     runner.stop_on_failure=stop_on_failure
-    if isinstance(suite, unittest.TestSuite):
-        # Empty out _tests list of passed suite and populate new TestSuite
-        suite._tests[:], suite = [], TestSuite(suite)
     # built in decorator factories:
     decorators = [
         random_order(random_seed, runner),
@@ -3170,17 +3187,34 @@
 
 class TestDecorator(TestUtil.TestSuite):
     """A decorator for TestCase/TestSuite objects.
-
-    Contains rather than flattening suite passed on construction
+    
+    Usually, subclasses should override __iter__(used when flattening test
+    suites), which we do to filter, reorder, parallelise and so on, run() and
+    debug().
     """
 
-    def __init__(self, suite=None):
-        super(TestDecorator, self).__init__()
-        if suite is not None:
-            self.addTest(suite)
-
-    # Don't need subclass run method with suite emptying
-    run = unittest.TestSuite.run
+    def __init__(self, suite):
+        TestUtil.TestSuite.__init__(self)
+        self.addTest(suite)
+
+    def countTestCases(self):
+        cases = 0
+        for test in self:
+            cases += test.countTestCases()
+        return cases
+
+    def debug(self):
+        for test in self:
+            test.debug()
+
+    def run(self, result):
+        # Use iteration on self, not self._tests, to allow subclasses to hook
+        # into __iter__.
+        for test in self:
+            if result.shouldStop:
+                break
+            test.run(result)
+        return result
 
 
 class CountingDecorator(TestDecorator):
@@ -3197,50 +3231,90 @@
     """A decorator which excludes test matching an exclude pattern."""
 
     def __init__(self, suite, exclude_pattern):
-        super(ExcludeDecorator, self).__init__(
-            exclude_tests_by_re(suite, exclude_pattern))
+        TestDecorator.__init__(self, suite)
+        self.exclude_pattern = exclude_pattern
+        self.excluded = False
+
+    def __iter__(self):
+        if self.excluded:
+            return iter(self._tests)
+        self.excluded = True
+        suite = exclude_tests_by_re(self, self.exclude_pattern)
+        del self._tests[:]
+        self.addTests(suite)
+        return iter(self._tests)
 
 
 class FilterTestsDecorator(TestDecorator):
     """A decorator which filters tests to those matching a pattern."""
 
     def __init__(self, suite, pattern):
-        super(FilterTestsDecorator, self).__init__(
-            filter_suite_by_re(suite, pattern))
+        TestDecorator.__init__(self, suite)
+        self.pattern = pattern
+        self.filtered = False
+
+    def __iter__(self):
+        if self.filtered:
+            return iter(self._tests)
+        self.filtered = True
+        suite = filter_suite_by_re(self, self.pattern)
+        del self._tests[:]
+        self.addTests(suite)
+        return iter(self._tests)
 
 
 class RandomDecorator(TestDecorator):
     """A decorator which randomises the order of its tests."""
 
     def __init__(self, suite, random_seed, stream):
-        random_seed = self.actual_seed(random_seed)
-        stream.write("Randomizing test order using seed %s\n\n" %
-            (random_seed,))
+        TestDecorator.__init__(self, suite)
+        self.random_seed = random_seed
+        self.randomised = False
+        self.stream = stream
+
+    def __iter__(self):
+        if self.randomised:
+            return iter(self._tests)
+        self.randomised = True
+        self.stream.write("Randomizing test order using seed %s\n\n" %
+            (self.actual_seed()))
         # Initialise the random number generator.
-        random.seed(random_seed)
-        super(RandomDecorator, self).__init__(randomize_suite(suite))
+        random.seed(self.actual_seed())
+        suite = randomize_suite(self)
+        del self._tests[:]
+        self.addTests(suite)
+        return iter(self._tests)
 
-    @staticmethod
-    def actual_seed(seed):
-        if seed == "now":
+    def actual_seed(self):
+        if self.random_seed == "now":
             # We convert the seed to a long to make it reuseable across
             # invocations (because the user can reenter it).
-            return long(time.time())
+            self.random_seed = long(time.time())
         else:
             # Convert the seed to a long if we can
             try:
-                return long(seed)
-            except (TypeError, ValueError):
+                self.random_seed = long(self.random_seed)
+            except:
                 pass
-        return seed
+        return self.random_seed
 
 
 class TestFirstDecorator(TestDecorator):
     """A decorator which moves named tests to the front."""
 
     def __init__(self, suite, pattern):
-        super(TestFirstDecorator, self).__init__()
-        self.addTests(split_suite_by_re(suite, pattern))
+        TestDecorator.__init__(self, suite)
+        self.pattern = pattern
+        self.filtered = False
+
+    def __iter__(self):
+        if self.filtered:
+            return iter(self._tests)
+        self.filtered = True
+        suites = split_suite_by_re(self, self.pattern)
+        del self._tests[:]
+        self.addTests(suites)
+        return iter(self._tests)
 
 
 def partition_tests(suite, count):
@@ -3293,10 +3367,9 @@
                 os.waitpid(self.pid, 0)
 
     test_blocks = partition_tests(suite, concurrency)
-    suite._tests[:] = []
     for process_tests in test_blocks:
-        process_suite = TestUtil.TestSuite(process_tests)
-        process_tests[:] = []
+        process_suite = TestUtil.TestSuite()
+        process_suite.addTests(process_tests)
         c2pread, c2pwrite = os.pipe()
         pid = os.fork()
         if pid == 0:
@@ -3426,8 +3499,6 @@
 #                           with proper exclusion rules.
 #   -Ethreads               Will display thread ident at creation/join time to
 #                           help track thread leaks
-#   -Ecollection            Display the identity of any test cases that weren't
-#                           deallocated after being completed.
 selftest_debug_flags = set()
 
 

=== modified file 'bzrlib/tests/test_errors.py'
--- a/bzrlib/tests/test_errors.py	2011-05-18 18:49:43 +0000
+++ b/bzrlib/tests/test_errors.py	2011-05-19 18:59:31 +0000
@@ -577,17 +577,12 @@
         try:
             1/0
         except ZeroDivisionError:
-            err = errors.HookFailed('hook stage', 'hook name', sys.exc_info(),
-                warn=False)
-        # GZ 2010-11-08: Should not store exc_info in exception instances, but
-        #                HookFailed is deprecated anyway and could maybe go.
-        try:
-            self.assertStartsWith(
-                str(err), 'Hook \'hook name\' during hook stage failed:\n')
-            self.assertEndsWith(
-                str(err), 'integer division or modulo by zero')
-        finally:
-            del err
+            exc_info = sys.exc_info()
+        err = errors.HookFailed('hook stage', 'hook name', exc_info, warn=False)
+        self.assertStartsWith(
+            str(err), 'Hook \'hook name\' during hook stage failed:\n')
+        self.assertEndsWith(
+            str(err), 'integer division or modulo by zero')
 
     def test_tip_change_rejected(self):
         err = errors.TipChangeRejected(u'Unicode message\N{INTERROBANG}')
@@ -616,14 +611,11 @@
         try:
             raise Exception("example error")
         except Exception:
-            err = errors.SmartMessageHandlerError(sys.exc_info())
-        # GZ 2010-11-08: Should not store exc_info in exception instances.
-        try:
-            self.assertStartsWith(
-                str(err), "The message handler raised an exception:\n")
-            self.assertEndsWith(str(err), "Exception: example error\n")
-        finally:
-            del err
+            exc_info = sys.exc_info()
+        err = errors.SmartMessageHandlerError(exc_info)
+        self.assertStartsWith(
+            str(err), "The message handler raised an exception:\n")
+        self.assertEndsWith(str(err), "Exception: example error\n")
 
     def test_must_have_working_tree(self):
         err = errors.MustHaveWorkingTree('foo', 'bar')

=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py	2011-05-18 18:49:43 +0000
+++ b/bzrlib/tests/test_knit.py	2011-05-19 18:59:31 +0000
@@ -444,7 +444,6 @@
         except _TestException, e:
             retry_exc = errors.RetryWithNewPacks(None, reload_occurred=False,
                                                  exc_info=sys.exc_info())
-        # GZ 2010-08-10: Cycle with exc_info affects 3 tests
         return retry_exc
 
     def test_read_from_several_packs(self):

=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py	2011-05-18 18:49:43 +0000
+++ b/bzrlib/tests/test_selftest.py	2011-05-19 18:59:31 +0000
@@ -17,7 +17,6 @@
 """Tests for the test framework."""
 
 from cStringIO import StringIO
-import gc
 import doctest
 import os
 import signal
@@ -3380,78 +3379,6 @@
         self.assertLength(1, calls)
 
 
-class TestUncollectedWarnings(tests.TestCase):
-    """Check a test case still alive after being run emits a warning"""
-
-    class Test(tests.TestCase):
-        def test_pass(self):
-            pass
-        def test_self_ref(self):
-            self.also_self = self.test_self_ref
-        def test_skip(self):
-            self.skip("Don't need")
-
-    def _get_suite(self):
-        return TestUtil.TestSuite([
-            self.Test("test_pass"),
-            self.Test("test_self_ref"),
-            self.Test("test_skip"),
-            ])
-
-    def _run_selftest_with_suite(self, **kwargs):
-        sio = StringIO()
-        gc_on = gc.isenabled()
-        if gc_on:
-            gc.disable()
-        try:
-            tests.selftest(test_suite_factory=self._get_suite, stream=sio,
-                **kwargs)
-        finally:
-            if gc_on:
-                gc.enable()
-        output = sio.getvalue()
-        self.assertNotContainsRe(output, "Uncollected test case.*test_pass")
-        self.assertContainsRe(output, "Uncollected test case.*test_self_ref")
-        return output
-
-    def test_testsuite(self):
-        self._run_selftest_with_suite()
-
-    def test_pattern(self):
-        out = self._run_selftest_with_suite(pattern="test_(?:pass|self_ref)$")
-        self.assertNotContainsRe(out, "test_skip")
-
-    def test_exclude_pattern(self):
-        out = self._run_selftest_with_suite(exclude_pattern="test_skip$")
-        self.assertNotContainsRe(out, "test_skip")
-
-    def test_random_seed(self):
-        self._run_selftest_with_suite(random_seed="now")
-
-    def test_matching_tests_first(self):
-        self._run_selftest_with_suite(matching_tests_first=True,
-            pattern="test_self_ref$")
-
-    def test_starting_with_and_exclude(self):
-        out = self._run_selftest_with_suite(starting_with=["bt."],
-            exclude_pattern="test_skip$")
-        self.assertNotContainsRe(out, "test_skip")
-
-    def test_additonal_decorator(self):
-        out = self._run_selftest_with_suite(
-            suite_decorators=[tests.TestDecorator])
-
-
-class TestUncollectedWarningsSubunit(TestUncollectedWarnings):
-    """Check warnings from tests staying alive are emitted with subunit"""
-
-    _test_needs_features = [features.subunit]
-
-    def _run_selftest_with_suite(self, **kwargs):
-        return TestUncollectedWarnings._run_selftest_with_suite(self,
-            runner_class=tests.SubUnitBzrRunner, **kwargs)
-
-
 class TestEnvironHandling(tests.TestCase):
 
     def test_overrideEnv_None_called_twice_doesnt_leak(self):



More information about the bazaar-commits mailing list