Merge lp:~jelmer/brz/more-foreign-fixes into lp:brz

[Jelmer Vernooij]

Fix several more foreign tests.

Fix several more foreign tests.

* Make sure directories have contents when testing with them (e.g. in test_remove)
* Skip some workingtree tests that require right handside ghost parents for formats that don't support them
* skip text revision parent tests for non-inventory trees

[Martin Packman (gz)]

Thanks.

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz-dev/69/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz-dev/72/

=== modified file 'breezy/tests/per_workingtree/test_get_parent_ids.py'
--- breezy/tests/per_workingtree/test_get_parent_ids.py	2017-06-10 00:17:06 +0000
+++ breezy/tests/per_workingtree/test_get_parent_ids.py	2018-03-24 13:48:13 +0000
@@ -17,6 +17,7 @@
 
 """Tests for interface conformance of 'workingtree.get_parent_ids'"""
 
+from breezy.tests import TestNotApplicable
 from breezy.tests.per_workingtree import TestCaseWithWorkingTree
 
 
@@ -39,6 +40,10 @@
         """Test the correspondence between set pending merges and get_parent_ids."""
         wt = self.make_branch_and_tree('.')
         self.assertEqual([], wt.get_parent_ids())
+        if not wt._format. supports_righthand_parent_id_as_ghost:
+            raise TestNotApplicable(
+                    'format does not support right hand side parents '
+                    'that are ghosts')
         # the first pending merge replaces the 'last revision' because
         # 'last revision' is shorthand for 'left most parent'
         wt.add_pending_merge('foo@azkhazan-123123-abcabc')

=== modified file 'breezy/tests/per_workingtree/test_parents.py'
--- breezy/tests/per_workingtree/test_parents.py	2018-02-25 15:31:42 +0000
+++ breezy/tests/per_workingtree/test_parents.py	2018-03-24 13:48:13 +0000
@@ -28,10 +28,14 @@
     InventoryDirectory,
     InventoryLink,
     )
-from ...bzr.inventorytree import InventoryRevisionTree
+from ...bzr.inventorytree import (
+    InventoryRevisionTree,
+    InventoryTree,
+    )
 from ...sixish import (
     BytesIO,
     )
+from ...tests import TestNotApplicable
 from ..per_workingtree import TestCaseWithWorkingTree
 from .. import (
     features,
@@ -392,11 +396,8 @@
 
     def assertDeltaApplicationResultsInExpectedBasis(self, tree, revid, delta,
         expected_inventory):
-        tree.lock_write()
-        try:
+        with tree.lock_write():
             tree.update_basis_by_delta(revid, delta)
-        finally:
-            tree.unlock()
         # check the last revision was adjusted to rev_id
         self.assertEqual(revid, tree.last_revision())
         # check the parents are what we expect
@@ -404,11 +405,8 @@
         # check that the basis tree has the inventory we expect from applying
         # the delta.
         result_basis = tree.basis_tree()
-        result_basis.lock_read()
-        try:
+        with result_basis.lock_read():
             self.assertEqual(expected_inventory, result_basis.root_inventory)
-        finally:
-            result_basis.unlock()
 
     def make_inv_delta(self, old, new):
         """Make an inventory delta from two inventories."""
@@ -430,6 +428,9 @@
 
     def fake_up_revision(self, tree, revid, shape):
 
+        if not isinstance(tree, InventoryTree):
+            raise TestNotApplicable("test requires inventory tree")
+
         class ShapeTree(InventoryRevisionTree):
 
             def __init__(self, shape):

=== modified file 'breezy/tests/per_workingtree/test_remove.py'
--- breezy/tests/per_workingtree/test_remove.py	2018-01-12 08:52:43 +0000
+++ breezy/tests/per_workingtree/test_remove.py	2018-03-24 13:48:13 +0000
@@ -36,7 +36,12 @@
         tree = self.get_tree(files)
         tree.add(files)
         tree.commit(message)
-        self.assertInWorkingTree(files)
+        if not tree.has_versioned_directories():
+            self.assertInWorkingTree(
+                [f for f in files if not f.endswith("/")])
+            self.assertPathExists(files)
+        else:
+            self.assertInWorkingTree(files)
         return tree
 
     def assertRemovedAndDeleted(self, files):
@@ -51,7 +56,6 @@
         """Check that files and directories are unversioned but not deleted."""
         tree = self.get_tree(TestRemove.files)
         tree.add(TestRemove.files)
-        self.assertInWorkingTree(TestRemove.files)
 
         tree.remove(TestRemove.files)
         self.assertRemovedAndNotDeleted(TestRemove.files)
@@ -76,7 +80,6 @@
         """Removal of newly added files must back them up."""
         tree = self.get_tree(TestRemove.files)
         tree.add(TestRemove.files)
-        self.assertInWorkingTree(TestRemove.files)
         tree.remove(TestRemove.files, keep_files=False)
         self.assertNotInWorkingTree(TestRemove.files)
         self.assertPathExists(TestRemove.backup_files)
@@ -97,7 +100,6 @@
         tree = self.get_committed_tree(TestRemove.files)
         for f in TestRemove.rfiles:
             osutils.delete_any(f)
-        self.assertInWorkingTree(TestRemove.files)
         self.assertPathDoesNotExist(TestRemove.files)
         tree.remove(TestRemove.files, keep_files=False)
         self.assertRemovedAndDeleted(TestRemove.files)
@@ -110,7 +112,6 @@
         for f in TestRemove.rfiles:
             tree.rename_one(f, f+'x')
         rfilesx = ['bx/cx', 'bx', 'ax', 'dx']
-        self.assertInWorkingTree(rfilesx)
         self.assertPathExists(rfilesx)
 
         tree.remove(rfilesx, keep_files=False)
@@ -126,20 +127,21 @@
         rfilesx = ['bx/cx', 'bx', 'ax', 'dx']
         self.build_tree_contents([('ax', 'changed and renamed!'),
                                   ('bx/cx', 'changed and renamed!')])
-        self.assertInWorkingTree(rfilesx)
         self.assertPathExists(rfilesx)
 
         tree.remove(rfilesx, keep_files=False)
         self.assertNotInWorkingTree(rfilesx)
         self.assertPathExists(['bx.~1~/cx.~1~', 'bx.~1~', 'ax.~1~'])
-        self.assertPathDoesNotExist('dx.~1~') # unchanged file
+        if tree.supports_rename_tracking():
+            self.assertPathDoesNotExist('dx.~1~') # unchanged file
+        else:
+            self.assertPathExists('dx.~1~') # renamed, so appears changed
         tree._validate()
 
     def test_force_remove_changed_files(self):
         """Check that changed files are removed and deleted when forced."""
         tree = self.get_tree(TestRemove.files)
         tree.add(TestRemove.files)
-        self.assertInWorkingTree(TestRemove.files)
 
         tree.remove(TestRemove.files, keep_files=False, force=True)
         self.assertRemovedAndDeleted(TestRemove.files)
@@ -201,7 +203,6 @@
         tree = self.get_tree(files)
         tree.add(files)
         ignores.add_runtime_ignores(["*ignored*"])
-        self.assertInWorkingTree(files)
         self.assertNotEqual(None, tree.is_ignored(files[0]))
 
         tree.remove(files, keep_files=False)
@@ -211,7 +212,7 @@
 
     def test_dont_remove_directory_with_unknowns(self):
         """Directories with unknowns should be backed up."""
-        directories = ['a/', 'b/', 'c/', 'c/c/']
+        directories = ['a/', 'b/', 'c/', 'c/c/', 'c/blah']
         tree = self.get_committed_tree(directories)
 
         self.build_tree(['a/unknown_file'])
@@ -288,7 +289,7 @@
     def test_remove_directory_with_renames(self):
         """Delete directory with renames in or out."""
 
-        files = ['a/', 'a/file', 'a/directory/', 'b/']
+        files = ['a/', 'a/file', 'a/directory/', 'a/directory/stuff', 'b/']
         files_to_move = ['a/file', 'a/directory/']
 
         tree = self.get_committed_tree(files)