Rev 4347: (vila) Fix some lock-related test failures in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Fri May 8 20:51:56 BST 2009


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

------------------------------------------------------------
revno: 4347
revision-id: pqm at pqm.ubuntu.com-20090508195148-cw1mw95i0qo39ggg
parent: pqm at pqm.ubuntu.com-20090508182630-mijdi970vhbii7lq
parent: v.ladeuil+lp at free.fr-20090508172933-b9855z2ru1ul9gwh
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2009-05-08 20:51:48 +0100
message:
  (vila) Fix some lock-related test failures
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/lock.py                 lock.py-20050527050856-ec090bb51bc03349
  bzrlib/lockdir.py              lockdir.py-20060220222025-98258adf27fbdda3
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_knit.py      test_knit.py-20051212171302-95d4c00dd5f11f2b
  bzrlib/tests/test_lockdir.py   test_lockdir.py-20060220222025-33d4221569a3d600
  bzrlib/tests/test_rename_map.py test_rename_map.py-20090312140439-xexkkmjlg2enbohc-2
    ------------------------------------------------------------
    revno: 4345.1.1
    revision-id: v.ladeuil+lp at free.fr-20090508172933-b9855z2ru1ul9gwh
    parent: pqm at pqm.ubuntu.com-20090508170256-u5k6lfa0n93h9obf
    parent: v.ladeuil+lp at free.fr-20090508163409-83gvsoy3fuy58k2y
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: integration
    timestamp: Fri 2009-05-08 19:29:33 +0200
    message:
      Fix some lock-related test failures
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/lock.py                 lock.py-20050527050856-ec090bb51bc03349
      bzrlib/lockdir.py              lockdir.py-20060220222025-98258adf27fbdda3
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
      bzrlib/tests/test_knit.py      test_knit.py-20051212171302-95d4c00dd5f11f2b
      bzrlib/tests/test_lockdir.py   test_lockdir.py-20060220222025-33d4221569a3d600
      bzrlib/tests/test_rename_map.py test_rename_map.py-20090312140439-xexkkmjlg2enbohc-2
    ------------------------------------------------------------
    revno: 4327.1.6
    revision-id: v.ladeuil+lp at free.fr-20090508163409-83gvsoy3fuy58k2y
    parent: v.ladeuil+lp at free.fr-20090508161726-gwbl943242sh3lxb
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Fri 2009-05-08 18:34:09 +0200
    message:
      Fix 1 more lock-related test failure.
      
      * tests/test_rename_map.py:
      (TestRenameMap.test_guess_renames_preserves_children): Add the
      forgotten unlock.
    modified:
      bzrlib/tests/test_rename_map.py test_rename_map.py-20090312140439-xexkkmjlg2enbohc-2
    ------------------------------------------------------------
    revno: 4327.1.5
    revision-id: v.ladeuil+lp at free.fr-20090508161726-gwbl943242sh3lxb
    parent: v.ladeuil+lp at free.fr-20090508155800-svg2sw0dp40udwq5
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Fri 2009-05-08 18:17:26 +0200
    message:
      Fix 4 more lock-related test failures.
      
      * tests/test_knit.py:
      (TestPackKnitAccess.make_vf_for_retrying): The tree should be
      unlocked, not only the repository.
    modified:
      bzrlib/tests/test_knit.py      test_knit.py-20051212171302-95d4c00dd5f11f2b
    ------------------------------------------------------------
    revno: 4327.1.4
    revision-id: v.ladeuil+lp at free.fr-20090508155800-svg2sw0dp40udwq5
    parent: v.ladeuil+lp at free.fr-20090508154027-r450tion5jpua558
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Fri 2009-05-08 17:58:00 +0200
    message:
      Fix lock test failures by taking lock breaking into account.
      
      * tests/test_lockdir.py:
      (TestLockDir.test_43_break): Release the lock after breaking and
      acquiring it.
      
      * tests/__init__.py:
      (TestCase._check_locks): Consider lock breaks as releases.
      (TestCase._track_locks, TestCase._lock_broken): Also track broken locks.
    modified:
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
      bzrlib/tests/test_lockdir.py   test_lockdir.py-20060220222025-33d4221569a3d600
    ------------------------------------------------------------
    revno: 4327.1.3
    revision-id: v.ladeuil+lp at free.fr-20090508154027-r450tion5jpua558
    parent: v.ladeuil+lp at free.fr-20090508153911-pkdvtiyj4567fgqq
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Fri 2009-05-08 17:40:27 +0200
    message:
      Delete useless setup.
      
      * tests/test_lockdir.py:
      (TestLockDirHooks.setUp): Simplified, the test suite already
      restore hooks.
    modified:
      bzrlib/tests/test_lockdir.py   test_lockdir.py-20060220222025-33d4221569a3d600
    ------------------------------------------------------------
    revno: 4327.1.2
    revision-id: v.ladeuil+lp at free.fr-20090508153911-pkdvtiyj4567fgqq
    parent: v.ladeuil+lp at free.fr-20090505112801-6ijinkqgaymoofxq
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Fri 2009-05-08 17:39:11 +0200
    message:
      Introduce a new lock_broken hook.
      
      * tests/test_lockdir.py:
      (TestLockDirHooks): Separate hook related tests from other lock
      tests.
      (TestLockDirHooks.test_LockDir_broken_success,
      TestLockDirHooks.test_LockDir_broken_failure): Add tests for the
      new lock_broken hook.
      
      * lockdir.py:
      (LockDir.force_break): Fire lock_broken hook.
      
      * lock.py:
      (LockHooks.__init__): Add lock_broken hook.
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/lock.py                 lock.py-20050527050856-ec090bb51bc03349
      bzrlib/lockdir.py              lockdir.py-20060220222025-98258adf27fbdda3
      bzrlib/tests/test_lockdir.py   test_lockdir.py-20060220222025-33d4221569a3d600
    ------------------------------------------------------------
    revno: 4327.1.1
    revision-id: v.ladeuil+lp at free.fr-20090505112801-6ijinkqgaymoofxq
    parent: pqm at pqm.ubuntu.com-20090505094032-o2kvwmfl4dqaf1jr
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Tue 2009-05-05 13:28:01 +0200
    message:
      Start addressing test failing when run with -Dlock.
      
      * tests/test_lockdir.py:
      Fix 7 out 10 failures when using -Dlock.
      
      * lock.py:
      (LockResult.__repr__): For debug and to make 'Broken test' traces
      less obscure.
    modified:
      bzrlib/lock.py                 lock.py-20050527050856-ec090bb51bc03349
      bzrlib/tests/test_lockdir.py   test_lockdir.py-20060220222025-33d4221569a3d600
=== modified file 'NEWS'
--- a/NEWS	2009-05-07 16:21:20 +0000
+++ b/NEWS	2009-05-08 17:29:33 +0000
@@ -140,6 +140,10 @@
   via ~/.bazaar/authentication.conf fails. (Jelmer Vernooij, 
   Vincent Ladeuil, #321918)
 
+* New hook ``Lock.lock_broken`` which runs when a lock is
+  broken. This is mainly for testing that lock/unlock are
+  balanced in tests. (Vincent Ladeuil)
+
 * New smart server verb ``BzrDir.initialize_ex`` which implements a
   refactoring to the core of clone allowing less round trips on new
   branches. (Robert Collins)

=== modified file 'bzrlib/lock.py'
--- a/bzrlib/lock.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/lock.py	2009-05-08 15:39:11 +0000
@@ -55,6 +55,9 @@
         self.create_hook(HookPoint('lock_released',
             "Called with a bzrlib.lock.LockResult when a physical lock is "
             "released.", (1, 8), None))
+        self.create_hook(HookPoint('lock_broken',
+            "Called with a bzrlib.lock.LockResult when a physical lock is "
+            "broken.", (1, 15), None))
 
 
 class Lock(object):
@@ -77,6 +80,10 @@
     def __eq__(self, other):
         return self.lock_url == other.lock_url and self.details == other.details
 
+    def __repr__(self):
+        return '%s(%s%s)' % (self.__class__.__name__,
+                             self.lock_url, self.details)
+
 
 try:
     import fcntl

=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/lockdir.py	2009-05-08 15:39:11 +0000
@@ -325,7 +325,7 @@
             self._trace("... unlock succeeded after %dms",
                     (time.time() - start_time) * 1000)
             result = lock.LockResult(self.transport.abspath(self.path),
-                old_nonce)
+                                     old_nonce)
             for hook in self.hooks['lock_released']:
                 hook(result)
 
@@ -379,6 +379,10 @@
             raise LockBreakMismatch(self, broken_info, dead_holder_info)
         self.transport.delete(broken_info_path)
         self.transport.rmdir(tmpname)
+        result = lock.LockResult(self.transport.abspath(self.path),
+                                 current_info.get('nonce'))
+        for hook in self.hooks['lock_broken']:
+            hook(result)
 
     def _check_not_locked(self):
         """If the lock is held by this instance, raise an error."""

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2009-05-08 13:40:19 +0000
+++ b/bzrlib/tests/__init__.py	2009-05-08 17:29:33 +0000
@@ -862,15 +862,19 @@
         # matching has occured with -Dlock.
         # unhook:
         acquired_locks = [lock for action, lock in self._lock_actions
-            if action == 'acquired']
+                          if action == 'acquired']
         released_locks = [lock for action, lock in self._lock_actions
-            if action == 'released']
+                          if action == 'released']
+        broken_locks = [lock for action, lock in self._lock_actions
+                        if action == 'broken']
         # 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))
+        # have as many in each list, it should be ok. Some lock tests also
+        # break some locks on purpose and should be taken into account by
+        # considering that breaking a lock is just a dirty way of releasing it.
+        if len(acquired_locks) != (len(released_locks) + len(broken_locks)):
+            message = ('Different number of acquired and '
+                       'released or broken locks. (%s, %s + %s)' %
+                       (acquired_locks, released_locks, broken_locks))
             if not self._lock_check_thorough:
                 # Rather than fail, just warn
                 print "Broken test %s: %s" % (self, message)
@@ -882,8 +886,12 @@
         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)
+        _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)
+        _mod_lock.Lock.hooks.install_named_hook('lock_broken',
+                                                self._lock_broken, None)
 
     def _lock_acquired(self, result):
         self._lock_actions.append(('acquired', result))
@@ -891,6 +899,9 @@
     def _lock_released(self, result):
         self._lock_actions.append(('released', result))
 
+    def _lock_broken(self, result):
+        self._lock_actions.append(('broken', result))
+
     def _ndiff_strings(self, a, b):
         """Return ndiff between two strings containing lines.
 

=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py	2009-04-22 07:00:51 +0000
+++ b/bzrlib/tests/test_knit.py	2009-05-08 16:17:26 +0000
@@ -368,7 +368,7 @@
         """
         tree = self.make_branch_and_memory_tree('tree')
         tree.lock_write()
-        self.addCleanup(tree.branch.repository.unlock)
+        self.addCleanup(tree.unlock)
         tree.add([''], ['root-id'])
         tree.commit('one', rev_id='rev-1')
         tree.commit('two', rev_id='rev-2')
@@ -385,7 +385,6 @@
         collection.reset()
         repo.refresh_data()
         vf = tree.branch.repository.revisions
-        del tree
         # Set up a reload() function that switches to using the new pack file
         new_index = new_pack.revision_index
         access_tuple = new_pack.access_tuple()

=== modified file 'bzrlib/tests/test_lockdir.py'
--- a/bzrlib/tests/test_lockdir.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_lockdir.py	2009-05-08 15:58:00 +0000
@@ -142,6 +142,7 @@
         lf1 = LockDir(t, 'test_lock')
         lf1.create()
         lf1.attempt_lock()
+        self.addCleanup(lf1.unlock)
         # lock is held, should get some info on it
         info1 = lf1.peek()
         self.assertEqual(set(info1.keys()),
@@ -161,6 +162,7 @@
         lf2 = LockDir(self.get_readonly_transport(), 'test_lock')
         self.assertEqual(lf2.peek(), None)
         lf1.attempt_lock()
+        self.addCleanup(lf1.unlock)
         info2 = lf2.peek()
         self.assertTrue(info2)
         self.assertEqual(info2['nonce'], lf1.nonce)
@@ -451,6 +453,7 @@
         lf1 = LockDir(t, 'test_lock')
         lf1.create()
         lf1.attempt_lock()
+        self.addCleanup(lf1.unlock)
         lf1.confirm()
 
     def test_41_confirm_not_held(self):
@@ -468,6 +471,9 @@
         lf1.attempt_lock()
         t.move('test_lock', 'lock_gone_now')
         self.assertRaises(LockBroken, lf1.confirm)
+        # Clean up
+        t.move('lock_gone_now', 'test_lock')
+        lf1.unlock()
 
     def test_43_break(self):
         """Break a lock whose caller has forgotten it"""
@@ -484,6 +490,7 @@
         lf2.force_break(holder_info)
         # now we should be able to take it
         lf2.attempt_lock()
+        self.addCleanup(lf2.unlock)
         lf2.confirm()
 
     def test_44_break_already_released(self):
@@ -501,6 +508,7 @@
         lf2.force_break(holder_info)
         # now we should be able to take it
         lf2.attempt_lock()
+        self.addCleanup(lf2.unlock)
         lf2.confirm()
 
     def test_45_break_mismatch(self):
@@ -621,9 +629,11 @@
     def test_lock_by_token(self):
         ld1 = self.get_lock()
         token = ld1.lock_write()
+        self.addCleanup(ld1.unlock)
         self.assertNotEqual(None, token)
         ld2 = self.get_lock()
         t2 = ld2.lock_write(token)
+        self.addCleanup(ld2.unlock)
         self.assertEqual(token, t2)
 
     def test_lock_with_buggy_rename(self):
@@ -654,29 +664,30 @@
         check_dir([])
         # when held, that's all we see
         ld1.attempt_lock()
+        self.addCleanup(ld1.unlock)
         check_dir(['held'])
         # second guy should fail
         self.assertRaises(errors.LockContention, ld2.attempt_lock)
         # no kibble
         check_dir(['held'])
 
+
+class TestLockDirHooks(TestCaseWithTransport):
+
+    def setUp(self):
+        super(TestLockDirHooks, self).setUp()
+        self._calls = []
+
+    def get_lock(self):
+        return LockDir(self.get_transport(), 'test_lock')
+
     def record_hook(self, result):
         self._calls.append(result)
 
-    def reset_hooks(self):
-        self._old_hooks = lock.Lock.hooks
-        self.addCleanup(self.restore_hooks)
-        lock.Lock.hooks = lock.LockHooks()
-
-    def restore_hooks(self):
-        lock.Lock.hooks = self._old_hooks
-
     def test_LockDir_acquired_success(self):
         # the LockDir.lock_acquired hook fires when a lock is acquired.
-        self._calls = []
-        self.reset_hooks()
         LockDir.hooks.install_named_hook('lock_acquired',
-            self.record_hook, 'record_hook')
+                                         self.record_hook, 'record_hook')
         ld = self.get_lock()
         ld.create()
         self.assertEqual([], self._calls)
@@ -688,15 +699,13 @@
 
     def test_LockDir_acquired_fail(self):
         # the LockDir.lock_acquired hook does not fire on failure.
-        self._calls = []
-        self.reset_hooks()
         ld = self.get_lock()
         ld.create()
         ld2 = self.get_lock()
         ld2.attempt_lock()
         # install a lock hook now, when the disk lock is locked
         LockDir.hooks.install_named_hook('lock_acquired',
-            self.record_hook, 'record_hook')
+                                         self.record_hook, 'record_hook')
         self.assertRaises(errors.LockContention, ld.attempt_lock)
         self.assertEqual([], self._calls)
         ld2.unlock()
@@ -704,10 +713,8 @@
 
     def test_LockDir_released_success(self):
         # the LockDir.lock_released hook fires when a lock is acquired.
-        self._calls = []
-        self.reset_hooks()
         LockDir.hooks.install_named_hook('lock_released',
-            self.record_hook, 'record_hook')
+                                         self.record_hook, 'record_hook')
         ld = self.get_lock()
         ld.create()
         self.assertEqual([], self._calls)
@@ -719,14 +726,39 @@
 
     def test_LockDir_released_fail(self):
         # the LockDir.lock_released hook does not fire on failure.
-        self._calls = []
-        self.reset_hooks()
         ld = self.get_lock()
         ld.create()
         ld2 = self.get_lock()
         ld.attempt_lock()
         ld2.force_break(ld2.peek())
         LockDir.hooks.install_named_hook('lock_released',
-            self.record_hook, 'record_hook')
+                                         self.record_hook, 'record_hook')
         self.assertRaises(LockBroken, ld.unlock)
         self.assertEqual([], self._calls)
+
+    def test_LockDir_broken_success(self):
+        # the LockDir.lock_broken hook fires when a lock is broken.
+        ld = self.get_lock()
+        ld.create()
+        ld2 = self.get_lock()
+        result = ld.attempt_lock()
+        LockDir.hooks.install_named_hook('lock_broken',
+                                         self.record_hook, 'record_hook')
+        ld2.force_break(ld2.peek())
+        lock_path = ld.transport.abspath(ld.path)
+        self.assertEqual([lock.LockResult(lock_path, result)], self._calls)
+
+    def test_LockDir_broken_failure(self):
+        # the LockDir.lock_broken hook does not fires when a lock is already
+        # released.
+        ld = self.get_lock()
+        ld.create()
+        ld2 = self.get_lock()
+        result = ld.attempt_lock()
+        holder_info = ld2.peek()
+        ld.unlock()
+        LockDir.hooks.install_named_hook('lock_broken',
+                                         self.record_hook, 'record_hook')
+        ld2.force_break(holder_info)
+        lock_path = ld.transport.abspath(ld.path)
+        self.assertEqual([], self._calls)

=== modified file 'bzrlib/tests/test_rename_map.py'
--- a/bzrlib/tests/test_rename_map.py	2009-03-30 20:59:42 +0000
+++ b/bzrlib/tests/test_rename_map.py	2009-05-08 16:34:09 +0000
@@ -150,6 +150,7 @@
         """When a directory has been moved, its children are preserved."""
         tree = self.make_branch_and_tree('tree')
         tree.lock_write()
+        self.addCleanup(tree.unlock)
         self.build_tree_contents([('tree/foo/', ''),
                                   ('tree/foo/bar', 'bar'),
                                   ('tree/foo/empty', '')])




More information about the bazaar-commits mailing list