Rev 4354: (vila) Fix unbalanced locks in tests, in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Tue May 12 10:05:35 BST 2009


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

------------------------------------------------------------
revno: 4354
revision-id: pqm at pqm.ubuntu.com-20090512090530-v9355ohetp61ltc1
parent: pqm at pqm.ubuntu.com-20090512045404-3gjp1vrdmrjpwjgh
parent: v.ladeuil+lp at free.fr-20090512080854-jrius5e8zdapzin9
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2009-05-12 10:05:30 +0100
message:
  (vila) Fix unbalanced locks in tests,
  	the check is now fatal (use -Dlock to turn it into a warning).
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/branch_implementations/test_locking.py test_locking.py-20060707151933-tav3o2hpibwi53u4-4
  bzrlib/tests/per_repository/test_repository.py test_repository.py-20060131092128-ad07f494f5c9d26c
  bzrlib/tests/per_repository/test_write_group.py test_write_group.py-20070716105516-89n34xtogq5frn0m-1
  bzrlib/tests/test_lockable_files.py test_lockable_files.py-20051225183927-365c7fd99591caf1
  bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
  bzrlib/tests/test_smart.py     test_smart.py-20061122024551-ol0l0o0oofsu9b3t-2
    ------------------------------------------------------------
    revno: 4350.1.1
    revision-id: v.ladeuil+lp at free.fr-20090512080854-jrius5e8zdapzin9
    parent: pqm at pqm.ubuntu.com-20090511073632-ti658145fo07a4vw
    parent: v.ladeuil+lp at free.fr-20090512080737-itf02crgp0kxir0p
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: integration
    timestamp: Tue 2009-05-12 10:08:54 +0200
    message:
      Fix unbalanced locks in tests, the check is now fatal (use -Dlock to turn it into a warning).
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
      bzrlib/tests/branch_implementations/test_locking.py test_locking.py-20060707151933-tav3o2hpibwi53u4-4
      bzrlib/tests/per_repository/test_repository.py test_repository.py-20060131092128-ad07f494f5c9d26c
      bzrlib/tests/per_repository/test_write_group.py test_write_group.py-20070716105516-89n34xtogq5frn0m-1
      bzrlib/tests/test_lockable_files.py test_lockable_files.py-20051225183927-365c7fd99591caf1
      bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
      bzrlib/tests/test_smart.py     test_smart.py-20061122024551-ol0l0o0oofsu9b3t-2
    ------------------------------------------------------------
    revno: 4327.1.12
    revision-id: v.ladeuil+lp at free.fr-20090512080737-itf02crgp0kxir0p
    parent: v.ladeuil+lp at free.fr-20090511103757-x030vepytcneozqa
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Tue 2009-05-12 10:07:37 +0200
    message:
      Fixed as per Robert's review.
      
      * tests/__init__.py:
      (TestCase._track_locks): Make unbalanced locks fatal.
      
      * tests/test_selftest.py:
      (TestTestCaseWithMemoryTransport.test_dangling_locks_cause_failures):
      The failure is now unconditional.
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
      bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
    ------------------------------------------------------------
    revno: 4327.1.11
    revision-id: v.ladeuil+lp at free.fr-20090511103757-x030vepytcneozqa
    parent: v.ladeuil+lp at free.fr-20090511102532-5ios60jawjyhf8ur
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Mon 2009-05-11 12:37:57 +0200
    message:
      Fix the last 6 lock-related failures.
      
      * tests/branch_implementations/test_locking.py:
      (TestBranchLocking.test_leave_lock_in_place): Clean locks.
    modified:
      bzrlib/tests/branch_implementations/test_locking.py test_locking.py-20060707151933-tav3o2hpibwi53u4-4
    ------------------------------------------------------------
    revno: 4327.1.10
    revision-id: v.ladeuil+lp at free.fr-20090511102532-5ios60jawjyhf8ur
    parent: v.ladeuil+lp at free.fr-20090511074936-smhc6y3kvl28zu5m
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Mon 2009-05-11 12:25:32 +0200
    message:
      Fix 10 more lock-related test failures.
      
      * tests/test_smart.py:
      Add correct cleanups for locks.
    modified:
      bzrlib/tests/test_smart.py     test_smart.py-20061122024551-ol0l0o0oofsu9b3t-2
    ------------------------------------------------------------
    revno: 4327.1.9
    revision-id: v.ladeuil+lp at free.fr-20090511074936-smhc6y3kvl28zu5m
    parent: v.ladeuil+lp at free.fr-20090511071830-jsb2f0v9ozp3fpxb
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Mon 2009-05-11 09:49:36 +0200
    message:
      Fix 4 more lock-related test failures.
      
      * tests/per_repository/test_write_group.py:
      (TestWriteGroup.test_abort_write_group_does_not_raise_when_suppressed):
      Better cleanup.
    modified:
      bzrlib/tests/per_repository/test_write_group.py test_write_group.py-20070716105516-89n34xtogq5frn0m-1
    ------------------------------------------------------------
    revno: 4327.1.8
    revision-id: v.ladeuil+lp at free.fr-20090511071830-jsb2f0v9ozp3fpxb
    parent: v.ladeuil+lp at free.fr-20090511071458-bjghkrcdxbv87193
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Mon 2009-05-11 09:18:30 +0200
    message:
      Fix 4 more lock-related test failures.
      
      * tests/per_repository/test_repository.py:
      (TestRepositoryLocking.test_leave_lock_in_place): Restore clearing
      lock behavior.
    modified:
      bzrlib/tests/per_repository/test_repository.py test_repository.py-20060131092128-ad07f494f5c9d26c
    ------------------------------------------------------------
    revno: 4327.1.7
    revision-id: v.ladeuil+lp at free.fr-20090511071458-bjghkrcdxbv87193
    parent: v.ladeuil+lp at free.fr-20090508163409-83gvsoy3fuy58k2y
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: failing-lock-tests
    timestamp: Mon 2009-05-11 09:14:58 +0200
    message:
      Fix 2 more lock-related test failures.
      
      * tests/test_lockable_files.py:
      Replace some try/finally with an addCleanup().
      (_TestLockableFiles_mixin.test_leave_in_place): Restore clearing
      lock behavior.
    modified:
      bzrlib/tests/test_lockable_files.py test_lockable_files.py-20051225183927-365c7fd99591caf1
=== modified file 'NEWS'
--- a/NEWS	2009-05-12 03:47:18 +0000
+++ b/NEWS	2009-05-12 09:05:30 +0000
@@ -167,6 +167,12 @@
 
 * Updated the bundled ``ConfigObj`` library to 4.6.0 (Matt Nordhoff)
 
+Testing
+*******
+
+* ``bzr selftest`` will now fail if lock/unlock are not correctly balanced in
+  tests. Using ``-Dlock`` will turn the related failures into warnings.
+  (Vincent Ladeuil, Robert Collins)
 
 bzr 1.14
 ###########

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2009-05-11 06:44:30 +0000
+++ b/bzrlib/tests/__init__.py	2009-05-12 08:08:54 +0000
@@ -884,7 +884,7 @@
     def _track_locks(self):
         """Track lock activity during tests."""
         self._lock_actions = []
-        self._lock_check_thorough = 'lock' in debug.debug_flags
+        self._lock_check_thorough = 'lock' not in debug.debug_flags
         self.addCleanup(self._check_locks)
         _mod_lock.Lock.hooks.install_named_hook('lock_acquired',
                                                 self._lock_acquired, None)

=== modified file 'bzrlib/tests/branch_implementations/test_locking.py'
--- a/bzrlib/tests/branch_implementations/test_locking.py	2009-04-15 05:39:38 +0000
+++ b/bzrlib/tests/branch_implementations/test_locking.py	2009-05-11 10:37:57 +0000
@@ -432,6 +432,10 @@
             branch.unlock()
         # We should be unable to relock the repo.
         self.assertRaises(errors.LockContention, branch.lock_write)
+        # Cleanup
+        branch.lock_write(token)
+        branch.dont_leave_lock_in_place()
+        branch.unlock()
 
     def test_dont_leave_lock_in_place(self):
         branch = self.make_branch('b')

=== modified file 'bzrlib/tests/per_repository/test_repository.py'
--- a/bzrlib/tests/per_repository/test_repository.py	2009-04-09 20:23:07 +0000
+++ b/bzrlib/tests/per_repository/test_repository.py	2009-05-11 07:18:30 +0000
@@ -1046,6 +1046,10 @@
             repo.unlock()
         # We should be unable to relock the repo.
         self.assertRaises(errors.LockContention, repo.lock_write)
+        # Cleanup
+        repo.lock_write(token)
+        repo.dont_leave_lock_in_place()
+        repo.unlock()
 
     def test_dont_leave_lock_in_place(self):
         repo = self.make_repository('r')

=== modified file 'bzrlib/tests/per_repository/test_write_group.py'
--- a/bzrlib/tests/per_repository/test_write_group.py	2009-05-12 01:36:08 +0000
+++ b/bzrlib/tests/per_repository/test_write_group.py	2009-05-12 09:05:30 +0000
@@ -112,13 +112,13 @@
         self.addCleanup(repo.unlock)
         repo.start_write_group()
         # Damage the repository on the filesystem
-        self.get_transport('').rename('repo', 'foo')
+        t = self.get_transport('')
+        t.rename('repo', 'foo')
+        self.addCleanup(t.rename, 'foo', 'repo')
         # abort_write_group will not raise an error, because either an
         # exception was not generated, or the exception was caught and
         # suppressed.  See also test_pack_repository's test of the same name.
         self.assertEqual(None, repo.abort_write_group(suppress_errors=True))
-        if token is not None:
-            repo.leave_lock_in_place()
 
 class TestGetMissingParentInventories(TestCaseWithRepository):
 

=== modified file 'bzrlib/tests/test_lockable_files.py'
--- a/bzrlib/tests/test_lockable_files.py	2009-04-04 02:50:01 +0000
+++ b/bzrlib/tests/test_lockable_files.py	2009-05-11 07:14:58 +0000
@@ -56,72 +56,68 @@
             deprecated_in((1, 5, 0)),
             self.lockable.get_utf8, 'foo')
         self.lockable.lock_write()
-        try:
-            unicode_string = u'bar\u1234'
-            self.assertEqual(4, len(unicode_string))
-            byte_string = unicode_string.encode('utf-8')
-            self.assertEqual(6, len(byte_string))
-            self.assertRaises(UnicodeEncodeError,
-                self.applyDeprecated,
-                deprecated_in((1, 6, 0)),
-                self.lockable.put, 'foo',
-                StringIO(unicode_string))
-            self.applyDeprecated(
-                deprecated_in((1, 6, 0)),
-                self.lockable.put,
-                'foo', StringIO(byte_string))
-            byte_stream = self.applyDeprecated(
-                deprecated_in((1, 5, 0)),
-                self.lockable.get,
-                'foo')
-            self.assertEqual(byte_string, byte_stream.read())
-            unicode_stream = self.applyDeprecated(
-                deprecated_in((1, 5, 0)),
-                self.lockable.get_utf8,
-                'foo')
-            self.assertEqual(unicode_string,
-                unicode_stream.read())
-            self.assertRaises(BzrBadParameterNotString,
-                self.applyDeprecated,
-                deprecated_in((1, 6, 0)),
-                self.lockable.put_utf8,
-                'bar',
-                StringIO(unicode_string))
-            self.applyDeprecated(
-                deprecated_in((1, 6, 0)),
-                self.lockable.put_utf8,
-                'bar',
-                unicode_string)
-            unicode_stream = self.applyDeprecated(
-                deprecated_in((1, 5, 0)),
-                self.lockable.get_utf8,
-                'bar')
-            self.assertEqual(unicode_string,
-                unicode_stream.read())
-            byte_stream = self.applyDeprecated(
-                deprecated_in((1, 5, 0)),
-                self.lockable.get,
-                'bar')
-            self.assertEqual(byte_string, byte_stream.read())
-            self.applyDeprecated(
-                deprecated_in((1, 6, 0)),
-                self.lockable.put_bytes,
-                'raw', 'raw\xffbytes')
-            byte_stream = self.applyDeprecated(
-                deprecated_in((1, 5, 0)),
-                self.lockable.get,
-                'raw')
-            self.assertEqual('raw\xffbytes', byte_stream.read())
-        finally:
-            self.lockable.unlock()
+        self.addCleanup(self.lockable.unlock)
+        unicode_string = u'bar\u1234'
+        self.assertEqual(4, len(unicode_string))
+        byte_string = unicode_string.encode('utf-8')
+        self.assertEqual(6, len(byte_string))
+        self.assertRaises(UnicodeEncodeError,
+            self.applyDeprecated,
+            deprecated_in((1, 6, 0)),
+            self.lockable.put, 'foo',
+            StringIO(unicode_string))
+        self.applyDeprecated(
+            deprecated_in((1, 6, 0)),
+            self.lockable.put,
+            'foo', StringIO(byte_string))
+        byte_stream = self.applyDeprecated(
+            deprecated_in((1, 5, 0)),
+            self.lockable.get,
+            'foo')
+        self.assertEqual(byte_string, byte_stream.read())
+        unicode_stream = self.applyDeprecated(
+            deprecated_in((1, 5, 0)),
+            self.lockable.get_utf8,
+            'foo')
+        self.assertEqual(unicode_string,
+            unicode_stream.read())
+        self.assertRaises(BzrBadParameterNotString,
+            self.applyDeprecated,
+            deprecated_in((1, 6, 0)),
+            self.lockable.put_utf8,
+            'bar',
+            StringIO(unicode_string))
+        self.applyDeprecated(
+            deprecated_in((1, 6, 0)),
+            self.lockable.put_utf8,
+            'bar',
+            unicode_string)
+        unicode_stream = self.applyDeprecated(
+            deprecated_in((1, 5, 0)),
+            self.lockable.get_utf8,
+            'bar')
+        self.assertEqual(unicode_string,
+            unicode_stream.read())
+        byte_stream = self.applyDeprecated(
+            deprecated_in((1, 5, 0)),
+            self.lockable.get,
+            'bar')
+        self.assertEqual(byte_string, byte_stream.read())
+        self.applyDeprecated(
+            deprecated_in((1, 6, 0)),
+            self.lockable.put_bytes,
+            'raw', 'raw\xffbytes')
+        byte_stream = self.applyDeprecated(
+            deprecated_in((1, 5, 0)),
+            self.lockable.get,
+            'raw')
+        self.assertEqual('raw\xffbytes', byte_stream.read())
 
     def test_locks(self):
         self.lockable.lock_read()
-        try:
-            self.assertRaises(ReadOnlyError, self.lockable.put, 'foo',
-                              StringIO('bar\u1234'))
-        finally:
-            self.lockable.unlock()
+        self.addCleanup(self.lockable.unlock)
+        self.assertRaises(ReadOnlyError, self.lockable.put, 'foo',
+                          StringIO('bar\u1234'))
 
     def test_transactions(self):
         self.assertIs(self.lockable.get_transaction().__class__,
@@ -176,92 +172,80 @@
 
     def test_lock_write_returns_None_refuses_token(self):
         token = self.lockable.lock_write()
-        try:
-            if token is not None:
-                # This test does not apply, because this lockable supports
-                # tokens.
-                raise TestNotApplicable("%r uses tokens" % (self.lockable,))
-            self.assertRaises(errors.TokenLockingNotSupported,
-                              self.lockable.lock_write, token='token')
-        finally:
-            self.lockable.unlock()
+        self.addCleanup(self.lockable.unlock)
+        if token is not None:
+            # This test does not apply, because this lockable supports
+            # tokens.
+            raise TestNotApplicable("%r uses tokens" % (self.lockable,))
+        self.assertRaises(errors.TokenLockingNotSupported,
+                          self.lockable.lock_write, token='token')
 
     def test_lock_write_returns_token_when_given_token(self):
         token = self.lockable.lock_write()
-        try:
-            if token is None:
-                # This test does not apply, because this lockable refuses
-                # tokens.
-                return
-            new_lockable = self.get_lockable()
-            token_from_new_lockable = new_lockable.lock_write(token=token)
-            try:
-                self.assertEqual(token, token_from_new_lockable)
-            finally:
-                new_lockable.unlock()
-        finally:
-            self.lockable.unlock()
+        self.addCleanup(self.lockable.unlock)
+        if token is None:
+            # This test does not apply, because this lockable refuses
+            # tokens.
+            return
+        new_lockable = self.get_lockable()
+        token_from_new_lockable = new_lockable.lock_write(token=token)
+        self.addCleanup(new_lockable.unlock)
+        self.assertEqual(token, token_from_new_lockable)
 
     def test_lock_write_raises_on_token_mismatch(self):
         token = self.lockable.lock_write()
-        try:
-            if token is None:
-                # This test does not apply, because this lockable refuses
-                # tokens.
-                return
-            different_token = token + 'xxx'
-            # Re-using the same lockable instance with a different token will
-            # raise TokenMismatch.
-            self.assertRaises(errors.TokenMismatch,
-                              self.lockable.lock_write, token=different_token)
-            # A separate instance for the same lockable will also raise
-            # TokenMismatch.
-            # This detects the case where a caller claims to have a lock (via
-            # the token) for an external resource, but doesn't (the token is
-            # different).  Clients need a separate lock object to make sure the
-            # external resource is probed, whereas the existing lock object
-            # might cache.
-            new_lockable = self.get_lockable()
-            self.assertRaises(errors.TokenMismatch,
-                              new_lockable.lock_write, token=different_token)
-        finally:
-            self.lockable.unlock()
+        self.addCleanup(self.lockable.unlock)
+        if token is None:
+            # This test does not apply, because this lockable refuses
+            # tokens.
+            return
+        different_token = token + 'xxx'
+        # Re-using the same lockable instance with a different token will
+        # raise TokenMismatch.
+        self.assertRaises(errors.TokenMismatch,
+                          self.lockable.lock_write, token=different_token)
+        # A separate instance for the same lockable will also raise
+        # TokenMismatch.
+        # This detects the case where a caller claims to have a lock (via
+        # the token) for an external resource, but doesn't (the token is
+        # different).  Clients need a separate lock object to make sure the
+        # external resource is probed, whereas the existing lock object
+        # might cache.
+        new_lockable = self.get_lockable()
+        self.assertRaises(errors.TokenMismatch,
+                          new_lockable.lock_write, token=different_token)
 
     def test_lock_write_with_matching_token(self):
         # If the token matches, so no exception is raised by lock_write.
         token = self.lockable.lock_write()
-        try:
-            if token is None:
-                # This test does not apply, because this lockable refuses
-                # tokens.
-                return
-            # The same instance will accept a second lock_write if the specified
-            # token matches.
-            self.lockable.lock_write(token=token)
-            self.lockable.unlock()
-            # Calling lock_write on a new instance for the same lockable will
-            # also succeed.
-            new_lockable = self.get_lockable()
-            new_lockable.lock_write(token=token)
-            new_lockable.unlock()
-        finally:
-            self.lockable.unlock()
+        self.addCleanup(self.lockable.unlock)
+        if token is None:
+            # This test does not apply, because this lockable refuses
+            # tokens.
+            return
+        # The same instance will accept a second lock_write if the specified
+        # token matches.
+        self.lockable.lock_write(token=token)
+        self.lockable.unlock()
+        # Calling lock_write on a new instance for the same lockable will
+        # also succeed.
+        new_lockable = self.get_lockable()
+        new_lockable.lock_write(token=token)
+        new_lockable.unlock()
 
     def test_unlock_after_lock_write_with_token(self):
         # If lock_write did not physically acquire the lock (because it was
         # passed a token), then unlock should not physically release it.
         token = self.lockable.lock_write()
-        try:
-            if token is None:
-                # This test does not apply, because this lockable refuses
-                # tokens.
-                return
-            new_lockable = self.get_lockable()
-            new_lockable.lock_write(token=token)
-            new_lockable.unlock()
-            self.assertTrue(self.lockable.get_physical_lock_status())
-        finally:
-            self.lockable.unlock()
+        self.addCleanup(self.lockable.unlock)
+        if token is None:
+            # This test does not apply, because this lockable refuses
+            # tokens.
+            return
+        new_lockable = self.get_lockable()
+        new_lockable.lock_write(token=token)
+        new_lockable.unlock()
+        self.assertTrue(self.lockable.get_physical_lock_status())
 
     def test_lock_write_with_token_fails_when_unlocked(self):
         # Lock and unlock to get a superficially valid token.  This mimics a
@@ -332,6 +316,11 @@
         # But should be relockable with a token.
         self.lockable.lock_write(token=token)
         self.lockable.unlock()
+        # Cleanup: we should still be able to get the lock, but we restore the
+        # behavior to clearing the lock when unlocking.
+        self.lockable.lock_write(token=token)
+        self.lockable.dont_leave_in_place()
+        self.lockable.unlock()
 
     def test_dont_leave_in_place(self):
         token = self.lockable.lock_write()

=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py	2009-04-30 06:16:30 +0000
+++ b/bzrlib/tests/test_selftest.py	2009-05-12 08:07:37 +0000
@@ -622,9 +622,6 @@
         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('.')

=== modified file 'bzrlib/tests/test_smart.py'
--- a/bzrlib/tests/test_smart.py	2009-04-28 03:55:56 +0000
+++ b/bzrlib/tests/test_smart.py	2009-05-11 10:25:32 +0000
@@ -586,6 +586,8 @@
             '')
         self.assertEqual(SuccessfulSmartServerResponse(()), result)
         self.assertEqual('bar', config.get_option('foo'))
+        # Cleanup
+        branch.unlock()
 
     def test_value_name_section(self):
         branch = self.make_branch('.')
@@ -597,6 +599,8 @@
             'gam')
         self.assertEqual(SuccessfulSmartServerResponse(()), result)
         self.assertEqual('bar', config.get_option('foo', 'gam'))
+        # Cleanup
+        branch.unlock()
 
 
 class SetLastRevisionTestBase(TestLockedBranch):
@@ -915,17 +919,24 @@
         # with a new branch object.
         new_branch = repository.bzrdir.open_branch()
         self.assertRaises(errors.LockContention, new_branch.lock_write)
+        # Cleanup
+        request = smart.branch.SmartServerBranchRequestUnlock(backing)
+        response = request.execute('', branch_nonce, repository_nonce)
 
     def test_lock_write_on_locked_branch(self):
         backing = self.get_transport()
         request = smart.branch.SmartServerBranchRequestLockWrite(backing)
         branch = self.make_branch('.')
-        branch.lock_write()
+        branch_token = branch.lock_write()
         branch.leave_lock_in_place()
         branch.unlock()
         response = request.execute('')
         self.assertEqual(
             SmartServerResponse(('LockContention',)), response)
+        # Cleanup
+        branch.lock_write(branch_token)
+        branch.dont_leave_lock_in_place()
+        branch.unlock()
 
     def test_lock_write_with_tokens_on_locked_branch(self):
         backing = self.get_transport()
@@ -941,6 +952,13 @@
                                    branch_token, repo_token)
         self.assertEqual(
             SmartServerResponse(('ok', branch_token, repo_token)), response)
+        # Cleanup
+        branch.repository.lock_write(repo_token)
+        branch.repository.dont_leave_lock_in_place()
+        branch.repository.unlock()
+        branch.lock_write(branch_token)
+        branch.dont_leave_lock_in_place()
+        branch.unlock()
 
     def test_lock_write_with_mismatched_tokens_on_locked_branch(self):
         backing = self.get_transport()
@@ -956,17 +974,29 @@
                                    branch_token+'xxx', repo_token)
         self.assertEqual(
             SmartServerResponse(('TokenMismatch',)), response)
+        # Cleanup
+        branch.repository.lock_write(repo_token)
+        branch.repository.dont_leave_lock_in_place()
+        branch.repository.unlock()
+        branch.lock_write(branch_token)
+        branch.dont_leave_lock_in_place()
+        branch.unlock()
 
     def test_lock_write_on_locked_repo(self):
         backing = self.get_transport()
         request = smart.branch.SmartServerBranchRequestLockWrite(backing)
         branch = self.make_branch('.', format='knit')
-        branch.repository.lock_write()
-        branch.repository.leave_lock_in_place()
-        branch.repository.unlock()
+        repo = branch.repository
+        repo_token = repo.lock_write()
+        repo.leave_lock_in_place()
+        repo.unlock()
         response = request.execute('')
         self.assertEqual(
             SmartServerResponse(('LockContention',)), response)
+        # Cleanup
+        repo.lock_write(repo_token)
+        repo.dont_leave_lock_in_place()
+        repo.unlock()
 
     def test_lock_write_on_readonly_transport(self):
         backing = self.get_readonly_transport()
@@ -1031,6 +1061,10 @@
             '', 'branch token', repo_token)
         self.assertEqual(
             SmartServerResponse(('TokenMismatch',)), response)
+        # Cleanup
+        branch.repository.lock_write(repo_token)
+        branch.repository.dont_leave_lock_in_place()
+        branch.repository.unlock()
 
 
 class TestSmartServerRepositoryRequest(tests.TestCaseWithMemoryTransport):
@@ -1282,17 +1316,24 @@
         # object.
         new_repo = repository.bzrdir.open_repository()
         self.assertRaises(errors.LockContention, new_repo.lock_write)
+        # Cleanup
+        request = smart.repository.SmartServerRepositoryUnlock(backing)
+        response = request.execute('', nonce)
 
     def test_lock_write_on_locked_repo(self):
         backing = self.get_transport()
         request = smart.repository.SmartServerRepositoryLockWrite(backing)
         repository = self.make_repository('.', format='knit')
-        repository.lock_write()
+        repo_token = repository.lock_write()
         repository.leave_lock_in_place()
         repository.unlock()
         response = request.execute('')
         self.assertEqual(
             SmartServerResponse(('LockContention',)), response)
+        # Cleanup
+        repository.lock_write(repo_token)
+        repository.dont_leave_lock_in_place()
+        repository.unlock()
 
     def test_lock_write_on_readonly_transport(self):
         backing = self.get_readonly_transport()




More information about the bazaar-commits mailing list