Rev 4317: (robertc) Add debugging of lock activity during tests. (Robert in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Fri May 1 05:42:08 BST 2009


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4317
revision-id: pqm at pqm.ubuntu.com-20090501044204-tvp4oeoj89zr9fby
parent: pqm at pqm.ubuntu.com-20090430150023-1cw4lwqf312vpuu8
parent: robertc at robertcollins.net-20090501035047-j32bh821vsr7x8pl
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2009-05-01 05:42:04 +0100
message:
  (robertc) Add debugging of lock activity during tests. (Robert
  	Collins)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
    ------------------------------------------------------------
    revno: 4314.2.3
    revision-id: robertc at robertcollins.net-20090501035047-j32bh821vsr7x8pl
    parent: robertc at robertcollins.net-20090501011803-hirv7ms95xrhwk26
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: integration
    timestamp: Fri 2009-05-01 13:50:47 +1000
    message:
      Scatter a few _runCleanups in  TestCase.run
    modified:
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
    ------------------------------------------------------------
    revno: 4314.2.2
    revision-id: robertc at robertcollins.net-20090501011803-hirv7ms95xrhwk26
    parent: robertc at robertcollins.net-20090430061630-fj6xih0prt4cw3s8
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: tests.lock-checks
    timestamp: Fri 2009-05-01 11:18:03 +1000
    message:
      Review feedback - add a comment.
    modified:
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
    ------------------------------------------------------------
    revno: 4314.2.1
    revision-id: robertc at robertcollins.net-20090430061630-fj6xih0prt4cw3s8
    parent: pqm at pqm.ubuntu.com-20090430024621-xa6yjq3700c5vke7
    parent: robertc at robertcollins.net-20080404035733-3d7jzhcfa0fiid2v
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: tests.lock-checks
    timestamp: Thu 2009-04-30 16:16:30 +1000
    message:
      Update lock debugging support patch.
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
      bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
    ------------------------------------------------------------
    revno: 3331.4.1
    revision-id: robertc at robertcollins.net-20080404035733-3d7jzhcfa0fiid2v
    parent: robertc at robertcollins.net-20080404035337-ldligjwfp2bzqi47
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: test-suite.lock_checking
    timestamp: Fri 2008-04-04 14:57:33 +1100
    message:
       * -Dlock when passed to the selftest (e.g. ``bzr -Dlock selftest``) will
         cause mismatched physical locks to cause test errors rather than just
         reporting to the screen. (Robert Collins)
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
      bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
=== modified file 'NEWS'
--- a/NEWS	2009-04-30 01:55:03 +0000
+++ b/NEWS	2009-04-30 06:16:30 +0000
@@ -88,6 +88,10 @@
   called before the first test is started, ``done`` called after the last
   test completes, and a new parameter ``strict``. (Robert Collins)
 
+* -Dlock when passed to the selftest (e.g. ``bzr -Dlock selftest``) will
+  cause mismatched physical locks to cause test errors rather than just
+  reporting to the screen. (Robert Collins)
+
 * Fallback ``CredentialStore`` instances registered with ``fallback=True``
   are now be able to provide credentials if obtaining credentials 
   via ~/.bazaar/authentication.conf fails. (Jelmer Vernooij, 

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2009-04-20 08:37:32 +0000
+++ b/bzrlib/tests/__init__.py	2009-05-01 03:50:47 +0000
@@ -55,6 +55,7 @@
     debug,
     errors,
     hooks,
+    lock as _mod_lock,
     memorytree,
     osutils,
     progress,
@@ -798,6 +799,8 @@
         self._benchcalls = []
         self._benchtime = None
         self._clear_hooks()
+        # Track locks - needs to be called before _clear_debug_flags.
+        self._track_locks()
         self._clear_debug_flags()
         TestCase._active_threads = threading.activeCount()
         self.addCleanup(self._check_leaked_threads)
@@ -850,6 +853,44 @@
         ui.ui_factory = ui.SilentUIFactory()
         self.addCleanup(_restore)
 
+    def _check_locks(self):
+        """Check that all lock take/release actions have been paired."""
+        # once we have fixed all the current lock problems, we can change the
+        # following code to always check for mismatched locks, but only do
+        # traceback showing with -Dlock (self._lock_check_thorough is True).
+        # For now, because the test suite will fail, we only assert that lock
+        # matching has occured with -Dlock.
+        # unhook:
+        acquired_locks = [lock for action, lock in self._lock_actions
+            if action == 'acquired']
+        released_locks = [lock for action, lock in self._lock_actions
+            if action == 'released']
+        # trivially, given the tests for lock acquistion and release, if we
+        # have as many in each list, it should be ok.
+        if len(acquired_locks) != len(released_locks):
+            message = \
+                ("Different number of acquired and released locks. (%s, %s)" %
+                (acquired_locks, released_locks))
+            if not self._lock_check_thorough:
+                # Rather than fail, just warn
+                print "Broken test %s: %s" % (self, message)
+                return
+            self.fail(message)
+
+    def _track_locks(self):
+        """Track lock activity during tests."""
+        self._lock_actions = []
+        self._lock_check_thorough = 'lock' in debug.debug_flags
+        self.addCleanup(self._check_locks)
+        _mod_lock.Lock.hooks.install_named_hook('lock_acquired', self._lock_acquired, None)
+        _mod_lock.Lock.hooks.install_named_hook('lock_released', self._lock_released, None)
+
+    def _lock_acquired(self, result):
+        self._lock_actions.append(('acquired', result))
+
+    def _lock_released(self, result):
+        self._lock_actions.append(('released', result))
+
     def _ndiff_strings(self, a, b):
         """Return ndiff between two strings containing lines.
 
@@ -1331,7 +1372,7 @@
                 else:
                     result.addSuccess(self)
                 result.stopTest(self)
-                return
+                return result
         try:
             try:
                 result.startTest(self)
@@ -1350,14 +1391,16 @@
                                 "test setUp did not invoke "
                                 "bzrlib.tests.TestCase's setUp")
                     except KeyboardInterrupt:
+                        self._runCleanups()
                         raise
                     except TestSkipped, e:
                         self._do_skip(result, e.args[0])
                         self.tearDown()
-                        return
+                        return result
                     except:
                         result.addError(self, sys.exc_info())
-                        return
+                        self._runCleanups()
+                        return result
 
                     ok = False
                     try:
@@ -1372,6 +1415,7 @@
                             reason = e.args[0]
                         self._do_skip(result, reason)
                     except KeyboardInterrupt:
+                        self._runCleanups()
                         raise
                     except:
                         result.addError(self, sys.exc_info())
@@ -1383,18 +1427,22 @@
                                 "test tearDown did not invoke "
                                 "bzrlib.tests.TestCase's tearDown")
                     except KeyboardInterrupt:
+                        self._runCleanups()
                         raise
                     except:
                         result.addError(self, sys.exc_info())
+                        self._runCleanups()
                         ok = False
                     if ok: result.addSuccess(self)
                 finally:
                     result.stopTest(self)
-                return
+                return result
             except TestNotApplicable:
                 # Not moved from the result [yet].
+                self._runCleanups()
                 raise
             except KeyboardInterrupt:
+                self._runCleanups()
                 raise
         finally:
             saved_attrs = {}
@@ -1406,9 +1454,9 @@
             self.__dict__ = saved_attrs
 
     def tearDown(self):
-        self._bzr_test_tearDown_run = True
         self._runCleanups()
         self._log_contents = ''
+        self._bzr_test_tearDown_run = True
         unittest.TestCase.tearDown(self)
 
     def time(self, callable, *args, **kwargs):

=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py	2009-04-09 06:48:33 +0000
+++ b/bzrlib/tests/test_selftest.py	2009-04-30 06:16:30 +0000
@@ -28,7 +28,9 @@
 from bzrlib import (
     branchbuilder,
     bzrdir,
+    debug,
     errors,
+    lockdir,
     memorytree,
     osutils,
     remote,
@@ -619,6 +621,20 @@
         # But we have a safety net in place.
         self.assertRaises(AssertionError, self._check_safety_net)
 
+    def test_dangling_locks_cause_failures(self):
+        # This is currently only enabled during debug runs, so turn debugging
+        # on.
+        debug.debug_flags.add('lock')
+        class TestDanglingLock(TestCaseWithMemoryTransport):
+            def test_function(self):
+                t = self.get_transport('.')
+                l = lockdir.LockDir(t, 'lock')
+                l.create()
+                l.attempt_lock()
+        test = TestDanglingLock('test_function')
+        result = test.run()
+        self.assertEqual(1, len(result.errors))
+
 
 class TestTestCaseWithTransport(TestCaseWithTransport):
     """Tests for the convenience functions TestCaseWithTransport introduces."""




More information about the bazaar-commits mailing list