Rev 6209: (gz) Bug #613247, cleanup test cases when they are finished running, in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

Patch Queue Manager pqm at pqm.ubuntu.com
Wed Oct 12 16:00:16 UTC 2011


At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6209 [merge]
revision-id: pqm at pqm.ubuntu.com-20111012160013-wkxrrcfib74vqm7m
parent: pqm at pqm.ubuntu.com-20111012130609-iqfvfq1pghcs1zz2
parent: martin.packman at canonical.com-20111005144609-jm3uas5kvxswncep
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2011-10-12 16:00:13 +0000
message:
  (gz) Bug #613247, cleanup test cases when they are finished running,
   lowering peak memory during selftest. (Martin Packman)
modified:
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/tests/TestUtil.py       TestUtil.py-20050824080200-5f70140a2d938694
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
  doc/en/release-notes/bzr-2.5.txt bzr2.5.txt-20110708125756-587p0hpw7oke4h05-1
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2011-09-15 15:06:59 +0000
+++ b/bzrlib/errors.py	2011-10-12 16:00:13 +0000
@@ -1666,6 +1666,7 @@
 
     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/tests/TestUtil.py'
--- a/bzrlib/tests/TestUtil.py	2011-05-26 08:05:45 +0000
+++ b/bzrlib/tests/TestUtil.py	2011-10-04 16:39:38 +0000
@@ -19,6 +19,7 @@
 import sys
 import logging
 import unittest
+import weakref
 
 from bzrlib import pyutils
 
@@ -66,6 +67,24 @@
                     test, test.__class__)
 
 
+class FailedCollectionCase(unittest.TestCase):
+    """Pseudo-test to run and report failure if given case was uncollected"""
+
+    def __init__(self, case):
+        super(FailedCollectionCase, self).__init__("fail_uncollected")
+        # GZ 2011-09-16: Maybe catch errors from id() method as cases may be
+        #                in a bit of a funny state by now.
+        self._problem_case_id = case.id()
+
+    def id(self):
+        if self._problem_case_id[-1:] == ")":
+            return self._problem_case_id[:-1] + ",uncollected)"
+        return self._problem_case_id + "(uncollected)"
+
+    def fail_uncollected(self):
+        self.fail("Uncollected test case: " + self._problem_case_id)
+
+
 class TestSuite(unittest.TestSuite):
     """I am an extended TestSuite with a visitor interface.
     This is primarily to allow filtering of tests - and suites or
@@ -82,14 +101,34 @@
         tests = list(self)
         tests.reverse()
         self._tests = []
+        stored_count = 0
+        count_stored_tests = getattr(result, "_count_stored_tests", int)
+        from bzrlib.tests import selftest_debug_flags
+        notify = "uncollected_cases" in selftest_debug_flags
         while tests:
             if result.shouldStop:
                 self._tests = reversed(tests)
                 break
-            tests.pop().run(result)
+            case = _run_and_collect_case(tests.pop(), result)()
+            new_stored_count = count_stored_tests()
+            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
+                    FailedCollectionCase(case).run(result)
+                    # Adding a new failure so need to reupdate the count
+                    new_stored_count = count_stored_tests()
+                # GZ 2011-09-16: Previously zombied the case at this point by
+                #                clearing the dict as fallback, skip for now.
+            stored_count = new_stored_count
         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-09-30 14:04:22 +0000
+++ b/bzrlib/tests/__init__.py	2011-10-12 16:00:13 +0000
@@ -212,6 +212,27 @@
         osutils.set_or_unset_env(var, value)
 
 
+def _clear__type_equality_funcs(test):
+    """Cleanup bound methods stored on TestCase instances
+
+    Clear the dict breaking a few (mostly) harmless cycles in the affected
+    unittests released with Python 2.6 and initial Python 2.7 versions.
+
+    For a few revisions between Python 2.7.1 and Python 2.7.2 that annoyingly
+    shipped in Oneiric, an object with no clear method was used, hence the
+    extra complications, see bug 809048 for details.
+    """
+    type_equality_funcs = getattr(test, "_type_equality_funcs", None)
+    if type_equality_funcs is not None:
+        tef_clear = getattr(type_equality_funcs, "clear", None)
+        if tef_clear is None:
+            tef_instance_dict = getattr(type_equality_funcs, "__dict__", None)
+            if tef_instance_dict is not None:
+                tef_clear = tef_instance_dict.clear
+        if tef_clear is not None:
+            tef_clear()
+
+
 class ExtendedTestResult(testtools.TextTestResult):
     """Accepts, reports and accumulates the results of running tests.
 
@@ -385,18 +406,7 @@
         getDetails = getattr(test, "getDetails", None)
         if getDetails is not None:
             getDetails().clear()
-        # Clear _type_equality_funcs to try to stop TestCase instances
-        # from wasting memory. 'clear' is not available in all Python
-        # versions (bug 809048)
-        type_equality_funcs = getattr(test, "_type_equality_funcs", None)
-        if type_equality_funcs is not None:
-            tef_clear = getattr(type_equality_funcs, "clear", None)
-            if tef_clear is None:
-                tef_instance_dict = getattr(type_equality_funcs, "__dict__", None)
-                if tef_instance_dict is not None:
-                    tef_clear = tef_instance_dict.clear
-            if tef_clear is not None:
-                tef_clear()
+        _clear__type_equality_funcs(test)
         self._traceback_from_test = None
 
     def startTests(self):
@@ -503,6 +513,10 @@
         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):
@@ -3251,6 +3265,9 @@
                             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),
@@ -3354,34 +3371,17 @@
 
 class TestDecorator(TestUtil.TestSuite):
     """A decorator for TestCase/TestSuite objects.
-    
-    Usually, subclasses should override __iter__(used when flattening test
-    suites), which we do to filter, reorder, parallelise and so on, run() and
-    debug().
+
+    Contains rather than flattening suite passed on construction
     """
 
-    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
+    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
 
 
 class CountingDecorator(TestDecorator):
@@ -3398,90 +3398,50 @@
     """A decorator which excludes test matching an exclude pattern."""
 
     def __init__(self, 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)
+        super(ExcludeDecorator, self).__init__(
+            exclude_tests_by_re(suite, exclude_pattern))
 
 
 class FilterTestsDecorator(TestDecorator):
     """A decorator which filters tests to those matching a pattern."""
 
     def __init__(self, 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)
+        super(FilterTestsDecorator, self).__init__(
+            filter_suite_by_re(suite, pattern))
 
 
 class RandomDecorator(TestDecorator):
     """A decorator which randomises the order of its tests."""
 
     def __init__(self, suite, random_seed, stream):
-        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()))
+        random_seed = self.actual_seed(random_seed)
+        stream.write("Randomizing test order using seed %s\n\n" %
+            (random_seed,))
         # Initialise the random number generator.
-        random.seed(self.actual_seed())
-        suite = randomize_suite(self)
-        del self._tests[:]
-        self.addTests(suite)
-        return iter(self._tests)
+        random.seed(random_seed)
+        super(RandomDecorator, self).__init__(randomize_suite(suite))
 
-    def actual_seed(self):
-        if self.random_seed == "now":
+    @staticmethod
+    def actual_seed(seed):
+        if seed == "now":
             # We convert the seed to a long to make it reuseable across
             # invocations (because the user can reenter it).
-            self.random_seed = long(time.time())
+            return long(time.time())
         else:
             # Convert the seed to a long if we can
             try:
-                self.random_seed = long(self.random_seed)
-            except:
+                return long(seed)
+            except (TypeError, ValueError):
                 pass
-        return self.random_seed
+        return seed
 
 
 class TestFirstDecorator(TestDecorator):
     """A decorator which moves named tests to the front."""
 
     def __init__(self, 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)
+        super(TestFirstDecorator, self).__init__()
+        self.addTests(split_suite_by_re(suite, pattern))
 
 
 def partition_tests(suite, count):
@@ -3519,7 +3479,7 @@
     """
     concurrency = osutils.local_concurrency()
     result = []
-    from subunit import TestProtocolClient, ProtocolTestCase
+    from subunit import ProtocolTestCase
     from subunit.test_results import AutoTimingTestResultDecorator
     class TestInOtherProcess(ProtocolTestCase):
         # Should be in subunit, I think. RBC.
@@ -3534,9 +3494,12 @@
                 os.waitpid(self.pid, 0)
 
     test_blocks = partition_tests(suite, concurrency)
+    # Clear the tests from the original suite so it doesn't keep them alive
+    suite._tests[:] = []
     for process_tests in test_blocks:
-        process_suite = TestUtil.TestSuite()
-        process_suite.addTests(process_tests)
+        process_suite = TestUtil.TestSuite(process_tests)
+        # Also clear each split list so new suite has only reference
+        process_tests[:] = []
         c2pread, c2pwrite = os.pipe()
         pid = os.fork()
         if pid == 0:
@@ -3548,12 +3511,15 @@
                 # read from stdin (otherwise its a roulette to see what
                 # child actually gets keystrokes for pdb etc).
                 sys.stdin.close()
-                sys.stdin = None
+                # GZ 2011-06-16: Why set stdin to None? Breaks multi fork.
+                #sys.stdin = None
                 stream = os.fdopen(c2pwrite, 'wb', 1)
                 subunit_result = AutoTimingTestResultDecorator(
-                    TestProtocolClient(stream))
+                    SubUnitBzrProtocolClient(stream))
                 process_suite.run(subunit_result)
             finally:
+                # GZ 2011-06-16: Is always exiting with silent success
+                #                really the right thing? Hurts debugging.
                 os._exit(0)
         else:
             os.close(c2pwrite)
@@ -3666,7 +3632,8 @@
 #                           with proper exclusion rules.
 #   -Ethreads               Will display thread ident at creation/join time to
 #                           help track thread leaks
-
+#   -Euncollected_cases     Display the identity of any test cases that weren't
+#                           deallocated after being completed.
 #   -Econfig_stats          Will collect statistics using addDetail
 selftest_debug_flags = set()
 
@@ -4470,6 +4437,10 @@
     from subunit.test_results import AutoTimingTestResultDecorator
     class SubUnitBzrProtocolClient(TestProtocolClient):
 
+        def stopTest(self, test):
+            super(SubUnitBzrProtocolClient, self).stopTest(test)
+            _clear__type_equality_funcs(test)
+
         def addSuccess(self, test, details=None):
             # The subunit client always includes the details in the subunit
             # stream, but we don't want to include it in ours.

=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py	2011-08-19 22:34:02 +0000
+++ b/bzrlib/tests/test_selftest.py	2011-10-05 14:46:09 +0000
@@ -17,6 +17,7 @@
 """Tests for the test framework."""
 
 from cStringIO import StringIO
+import gc
 import doctest
 import os
 import signal
@@ -3314,6 +3315,113 @@
         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 _inject_stream_into_subunit(self, stream):
+        """To be overridden by subclasses that run tests out of process"""
+
+    def _run_selftest_with_suite(self, **kwargs):
+        sio = StringIO()
+        self._inject_stream_into_subunit(sio)
+        old_flags = tests.selftest_debug_flags
+        tests.selftest_debug_flags = old_flags.union(["uncollected_cases"])
+        gc_on = gc.isenabled()
+        if gc_on:
+            gc.disable()
+        try:
+            tests.selftest(test_suite_factory=self._get_suite, stream=sio,
+                stop_on_failure=False, **kwargs)
+        finally:
+            if gc_on:
+                gc.enable()
+            tests.selftest_debug_flags = old_flags
+        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 TestUncollectedWarningsForking(TestUncollectedWarnings):
+    """Check warnings from tests staying alive are emitted when forking"""
+
+    _test_needs_features = [features.subunit]
+
+    def _inject_stream_into_subunit(self, stream):
+        """Monkey-patch subunit so the extra output goes to stream not stdout
+
+        Some APIs need rewriting so this kind of bogus hackery can be replaced
+        by passing the stream param from run_tests down into ProtocolTestCase.
+        """
+        from subunit import ProtocolTestCase
+        _original_init = ProtocolTestCase.__init__
+        def _init_with_passthrough(self, *args, **kwargs):
+            _original_init(self, *args, **kwargs)
+            self._passthrough = stream
+        self.overrideAttr(ProtocolTestCase, "__init__", _init_with_passthrough)
+
+    def _run_selftest_with_suite(self, **kwargs):
+        # GZ 2011-05-26: Add a PosixSystem feature so this check can go away
+        if getattr(os, "fork", None) is None:
+            raise tests.TestNotApplicable("Platform doesn't support forking")
+        # Make sure the fork code is actually invoked by claiming two cores
+        self.overrideAttr(osutils, "local_concurrency", lambda: 2)
+        kwargs.setdefault("suite_decorators", []).append(tests.fork_decorator)
+        return TestUncollectedWarnings._run_selftest_with_suite(self, **kwargs)
+
+
 class TestEnvironHandling(tests.TestCase):
 
     def test_overrideEnv_None_called_twice_doesnt_leak(self):

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- a/doc/en/release-notes/bzr-2.5.txt	2011-10-09 13:52:06 +0000
+++ b/doc/en/release-notes/bzr-2.5.txt	2011-10-12 16:00:13 +0000
@@ -70,6 +70,11 @@
    suite.  This can include new facilities for writing tests, fixes to 
    spurious test failures and changes to the way things should be tested.
 
+*  Ensure TestCase instances are deallocated immediately after running where
+   possible. This greatly reduces the peak resource needs of a full test suite
+   run. The new ``-Euncollected_cases`` selftest flag will add failures if any
+   case which persists pasts its expected lifetime. (Martin Packman, #613247)
+
 
 bzr 2.5b2
 #########




More information about the bazaar-commits mailing list