Rev 5859: (broken) Expand the test coverage for cases we care about. in http://bazaar.launchpad.net/~jameinel/bzr/2.4-set-parent-trees-delta-282941
John Arbash Meinel
john at arbash-meinel.com
Wed May 18 15:05:33 UTC 2011
At http://bazaar.launchpad.net/~jameinel/bzr/2.4-set-parent-trees-delta-282941
------------------------------------------------------------
revno: 5859
revision-id: john at arbash-meinel.com-20110518150527-s0xt8srys3lhbn17
parent: john at arbash-meinel.com-20110518104503-d571kyyo148b364b
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.4-set-parent-trees-delta-282941
timestamp: Wed 2011-05-18 17:05:27 +0200
message:
(broken) Expand the test coverage for cases we care about.
There are now a fair number of more edge cases if tree0 can diverge from
tree1 when calling update_basis_by_delta. The biggest thing is that we
now have to handle renames correctly.
Just snapshotting progress, since it will take a bit more to actually
get the tests passing.
-------------- next part --------------
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py 2011-05-18 10:45:03 +0000
+++ b/bzrlib/dirstate.py 2011-05-18 15:05:27 +0000
@@ -371,7 +371,7 @@
# A pack_stat (the x's) that is just noise and will never match the output
# of base64 encode.
NULLSTAT = 'x' * 32
- NULL_PARENT_DETAILS = ('a', '', 0, False, '')
+ NULL_PARENT_DETAILS = static_tuple.StaticTuple('a', '', 0, False, '')
HEADER_FORMAT_2 = '#bazaar dirstate flat format 2\n'
HEADER_FORMAT_3 = '#bazaar dirstate flat format 3\n'
@@ -1285,8 +1285,7 @@
except IndexError:
pass
entry_index = bisect.bisect_left(block, (key, []))
- present = (entry_index < len_block and
- block[entry_index][0] == key)
+ present = (entry_index < len_block and block[entry_index][0] == key)
self._last_entry_index = entry_index
return entry_index, present
@@ -1525,7 +1524,9 @@
if inv_entry is not None and file_id != inv_entry.file_id:
raise errors.InconsistentDelta(new_path, file_id,
"mismatched entry file_id %r" % inv_entry)
- if new_path is not None:
+ if new_path is None:
+ new_path_utf8 = None
+ else:
if inv_entry is None:
raise errors.InconsistentDelta(new_path, file_id,
"new_path with no entry")
@@ -1535,12 +1536,24 @@
if basename_utf8:
parents.add((dirname_utf8, inv_entry.parent_id))
if old_path is None:
- adds.append((None, encode(new_path), file_id,
+ old_path_utf8 = None
+ else:
+ old_path_utf8 = encode(old_path)
+ if old_path is None:
+ adds.append((None, new_path_utf8, file_id,
inv_to_entry(inv_entry), True))
new_ids.add(file_id)
elif new_path is None:
- deletes.append((encode(old_path), None, file_id, None, True))
- elif (old_path, new_path) != root_only:
+ deletes.append((old_path_utf8, None, file_id, None, True))
+ elif (old_path, new_path) == root_only:
+ # changes to just the root should not require remove/insertion
+ # of everything.
+ changes.append((old_path_utf8, new_path_utf8, file_id,
+ inv_to_entry(inv_entry)))
+ else:
+ # TODO: if old_path == new_path, I think we can get away with
+ # treating this entry as a simple 'changes' entry, rather
+ # than a delete + add. JAM 2011-05-18
# Renames:
# Because renames must preserve their children we must have
# processed all relocations and removes before hand. The sort
@@ -1556,36 +1569,31 @@
self._update_basis_apply_deletes(deletes)
deletes = []
# Split into an add/delete pair recursively.
- adds.append((None, new_path_utf8, file_id,
- inv_to_entry(inv_entry), False))
+ adds.append((old_path_utf8, new_path_utf8, file_id,
+ inv_to_entry(inv_entry), False))
# Expunge deletes that we've seen so that deleted/renamed
# children of a rename directory are handled correctly.
- new_deletes = reversed(list(self._iter_child_entries(1,
- encode(old_path))))
+ new_deletes = reversed(list(
+ self._iter_child_entries(1, old_path_utf8)))
# Remove the current contents of the tree at orig_path, and
# reinsert at the correct new path.
for entry in new_deletes:
- if entry[0][0]:
- source_path = entry[0][0] + '/' + entry[0][1]
+ child_dirname, child_basename, child_file_id = entry[0]
+ if child_dirname:
+ source_path = child_dirname + '/' + child_basename
else:
- source_path = entry[0][1]
+ source_path = child_basename
if new_path_utf8:
target_path = new_path_utf8 + source_path[len(old_path):]
else:
if old_path == '':
raise AssertionError("cannot rename directory to"
- " itself")
+ " itself")
target_path = source_path[len(old_path) + 1:]
adds.append((None, target_path, entry[0][2], entry[1][1], False))
deletes.append(
(source_path, target_path, entry[0][2], None, False))
- deletes.append(
- (encode(old_path), new_path, file_id, None, False))
- else:
- # changes to just the root should not require remove/insertion
- # of everything.
- changes.append((encode(old_path), encode(new_path), file_id,
- inv_to_entry(inv_entry)))
+ deletes.append((old_path_utf8, new_path, file_id, None, False))
self._check_delta_ids_absent(new_ids, delta, 1)
try:
# Finish expunging deletes/first half of renames.
@@ -1646,12 +1654,26 @@
"""
# Adds are accumulated partly from renames, so can be in any input
# order - sort it.
- adds.sort()
+ # TODO: we may want to sort in dirblocks order. That way each entry
+ # will end up in the same directory, allowing the _get_entry
+ # fast-path for looking up 2 items in the same dir work.
+ adds.sort(key=lambda x: x[1])
# adds is now in lexographic order, which places all parents before
# their children, so we can process it linearly.
absent = 'ar'
st = static_tuple.StaticTuple
for old_path, new_path, file_id, new_details, real_add in adds:
+ if real_add and old_path is not None:
+ raise errors.InconsistentDelta(new_path, file_id,
+ 'considered a real add but still had old_path at %s'
+ % (old_path,))
+ # This change may or may not be reflected in tree0. When adding a
+ # record, we have to watch out for
+ # 1) Simple case, the file exists in tree0, and old_path =
+ # new_path, just add the details for tree1
+ # 2) The file doesn't exist in tree0 at all
+ # 3) Something else exists in tree0 at the same path
+ # 4) The file exists in tree0, but at a different path
entry = self._get_entry(1, file_id, new_path)
if entry[0] is None:
# This entry doesn't exist in tree 1, but it might just be an
=== modified file 'bzrlib/tests/test_inv.py'
--- a/bzrlib/tests/test_inv.py 2011-05-18 09:52:44 +0000
+++ b/bzrlib/tests/test_inv.py 2011-05-18 15:05:27 +0000
@@ -85,9 +85,9 @@
repo.texts.add_lines((ie.file_id, ie.revision), [], lines)
-def apply_inventory_Inventory(self, basis, delta):
+def apply_inventory_Inventory(self, basis, delta, expect_fail=True):
"""Apply delta to basis and return the result.
-
+
:param basis: An inventory to be used as the basis.
:param delta: The inventory delta to apply:
:return: An inventory resulting from the application.
@@ -96,11 +96,11 @@
return basis
-def apply_inventory_WT(self, basis, delta):
+def apply_inventory_WT(self, basis, delta, expect_fail=True):
"""Apply delta to basis and return the result.
This sets the tree state to be basis, and then calls apply_inventory_delta.
-
+
:param basis: An inventory to be used as the basis.
:param delta: The inventory delta to apply:
:return: An inventory resulting from the application.
@@ -125,14 +125,48 @@
tree = tree.bzrdir.open_workingtree()
tree.lock_read()
self.addCleanup(tree.unlock)
- # One could add 'tree._validate' here but that would cause 'early' failues
- # as far as higher level code is concerned. Possibly adding an
- # expect_fail parameter to this function and if that is False then do a
- # validate call.
+ if not expect_fail:
+ tree._validate()
return tree.inventory
-def apply_inventory_WT_basis(self, basis, delta):
+def _create_repo_revisions(repo, basis, delta, expect_fail):
+ repo.start_write_group()
+ try:
+ rev = revision.Revision('basis', timestamp=0, timezone=None,
+ message="", committer="foo at example.com")
+ basis.revision_id = 'basis'
+ create_texts_for_inv(repo, basis)
+ repo.add_revision('basis', rev, basis)
+ if expect_fail:
+ # We don't want to apply the delta to the basis, because we expect
+ # the delta is invalid.
+ result_inv = basis
+ result_inv.revision_id = 'result'
+ target_entries = None
+ else:
+ result_inv = basis.create_by_apply_delta(delta, 'result')
+ create_texts_for_inv(repo, result_inv)
+ target_entries = list(result_inv.iter_entries_by_dir())
+ rev = revision.Revision('result', timestamp=0, timezone=None,
+ message="", committer="foo at example.com")
+ repo.add_revision('result', rev, result_inv)
+ repo.commit_write_group()
+ except:
+ repo.abort_write_group()
+ raise
+ return target_entries
+
+
+def _get_basis_entries(tree):
+ basis_tree = tree.basis_tree()
+ basis_tree.lock_read()
+ basis_tree_entries = list(basis_tree.inventory.iter_entries_by_dir())
+ basis_tree.unlock()
+ return basis_tree_entries
+
+
+def apply_inventory_WT_basis(test, basis, delta, expect_fail=True):
"""Apply delta to basis and return the result.
This sets the parent and then calls update_basis_by_delta.
@@ -140,37 +174,47 @@
allow safety checks made by the WT to succeed, and finally ensures that all
items in the delta with a new path are present in the WT before calling
update_basis_by_delta.
-
+
:param basis: An inventory to be used as the basis.
:param delta: The inventory delta to apply:
:return: An inventory resulting from the application.
"""
- control = self.make_bzrdir('tree', format=self.format._matchingbzrdir)
+ control = test.make_bzrdir('tree', format=test.format._matchingbzrdir)
control.create_repository()
control.create_branch()
- tree = self.format.initialize(control)
+ tree = test.format.initialize(control)
tree.lock_write()
try:
- repo = tree.branch.repository
- repo.start_write_group()
- try:
- rev = revision.Revision('basis', timestamp=0, timezone=None,
- message="", committer="foo at example.com")
- basis.revision_id = 'basis'
- create_texts_for_inv(tree.branch.repository, basis)
- repo.add_revision('basis', rev, basis)
- # Add a revision for the result, with the basis content -
- # update_basis_by_delta doesn't check that the delta results in
- # result, and we want inconsistent deltas to get called on the
- # tree, or else the code isn't actually checked.
- rev = revision.Revision('result', timestamp=0, timezone=None,
- message="", committer="foo at example.com")
- basis.revision_id = 'result'
- repo.add_revision('result', rev, basis)
- repo.commit_write_group()
- except:
- repo.abort_write_group()
- raise
+ target_entries = _create_repo_revisions(tree.branch.repository, basis,
+ delta, expect_fail)
+ # For successful deltas, we try 2 different permutations
+ # 1) active WT has no state
+ # 2) active WT is at target, basis tree is updated from basis to target
+ # 3) active WT is at basis, basis tree is updated to new target
+ # For expect_fail = True, we only do the last one.
+ if not expect_fail:
+ tree1 = tree.bzrdir.sprout('tree1').open_workingtree()
+ tree1.branch.repository.fetch(tree.branch.repository)
+ tree1.set_parent_ids(['basis'])
+ tree1.lock_write()
+ try:
+ tree1.update_basis_by_delta('result', delta)
+ tree1._validate()
+ finally:
+ tree1.unlock()
+ test.assertEqual(target_entries, _get_basis_entries(tree1))
+ tree2 = tree.bzrdir.sprout('tree2').open_workingtree()
+ tree2.branch.repository.fetch(tree.branch.repository)
+ tree2.lock_write()
+ try:
+ tree2._write_inventory(basis)
+ tree2.set_parent_ids(['basis'])
+ tree2.apply_inventory_delta(delta)
+ tree2.update_basis_by_delta('result', delta)
+ tree2._validate()
+ finally:
+ tree2.unlock()
+ test.assertEqual(target_entries, _get_basis_entries(tree1))
# Set the basis state as the trees current state
tree._write_inventory(basis)
# This reads basis from the repo and puts it into the tree's local
@@ -182,20 +226,24 @@
tree.lock_write()
try:
tree.update_basis_by_delta('result', delta)
+ if not expect_fail:
+ tree._validate()
finally:
tree.unlock()
# reload tree - ensure we get what was written.
tree = tree.bzrdir.open_workingtree()
basis_tree = tree.basis_tree()
basis_tree.lock_read()
- self.addCleanup(basis_tree.unlock)
- # Note, that if the tree does not have a local cache, the trick above of
- # setting the result as the basis, will come back to bite us. That said,
- # all the implementations in bzr do have a local cache.
- return basis_tree.inventory
-
-
-def apply_inventory_Repository_add_inventory_by_delta(self, basis, delta):
+ test.addCleanup(basis_tree.unlock)
+ basis_inv = basis_tree.inventory
+ if target_entries:
+ basis_entries = list(basis_inv.iter_entries_by_dir())
+ test.assertEqual(target_entries, basis_entries)
+ return basis_inv
+
+
+def apply_inventory_Repository_add_inventory_by_delta(self, basis, delta,
+ expect_fail=True):
"""Apply delta to basis and return the result.
This inserts basis as a whole inventory and then uses
@@ -570,7 +618,7 @@
file1.text_size = 0
file1.text_sha1 = ''
delta = [(None, u'path', 'file-id', file1)]
- res_inv = self.apply_delta(self, inv, delta)
+ res_inv = self.apply_delta(self, inv, delta, expect_fail=False)
self.assertEqual('file-id', res_inv['file-id'].file_id)
def test_remove_file(self):
@@ -581,7 +629,7 @@
file1.text_sha1 = ''
inv.add(file1)
delta = [(u'path', None, 'file-id', None)]
- res_inv = self.apply_delta(self, inv, delta)
+ res_inv = self.apply_delta(self, inv, delta, expect_fail=False)
self.assertEqual(None, res_inv.path2id('path'))
self.assertRaises(errors.NoSuchId, res_inv.id2path, 'file-id')
@@ -591,7 +639,7 @@
inv.add(file1)
file2 = self.make_file_ie(name='path2', parent_id=inv.root.file_id)
delta = [(u'path', 'path2', 'file-id', file2)]
- res_inv = self.apply_delta(self, inv, delta)
+ res_inv = self.apply_delta(self, inv, delta, expect_fail=False)
self.assertEqual(None, res_inv.path2id('path'))
self.assertEqual('file-id', res_inv.path2id('path2'))
@@ -602,7 +650,7 @@
file2 = self.make_file_ie(file_id='id2', parent_id=inv.root.file_id)
delta = [(u'name', None, 'id1', None),
(None, u'name', 'id2', file2)]
- res_inv = self.apply_delta(self, inv, delta)
+ res_inv = self.apply_delta(self, inv, delta, expect_fail=False)
self.assertEqual('id2', res_inv.path2id('name'))
def test_is_root(self):
More information about the bazaar-commits
mailing list