Rev 3555: Add expected failures for cases where we should be looking at more than in http://bzr.arbash-meinel.com/branches/bzr/1.7-dev/merge_lca_multi

John Arbash Meinel john at arbash-meinel.com
Thu Jul 31 20:34:35 BST 2008


At http://bzr.arbash-meinel.com/branches/bzr/1.7-dev/merge_lca_multi

------------------------------------------------------------
revno: 3555
revision-id: john at arbash-meinel.com-20080731193427-94lq166eeytf8tts
parent: john at arbash-meinel.com-20080731191259-81qy04c3h4bxir0s
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: merge_lca_multi
timestamp: Thu 2008-07-31 14:34:27 -0500
message:
  Add expected failures for cases where we should be looking at more than
  just BASE + LCAs, such as intermediate LCAs, or the whole per-file/attribute graph.
  
  They are fairly unlikely, but it is good to know that we don't handle them yet.
-------------- next part --------------
=== modified file 'bzrlib/tests/test_merge.py'
--- a/bzrlib/tests/test_merge.py	2008-07-31 19:12:59 +0000
+++ b/bzrlib/tests/test_merge.py	2008-07-31 19:34:27 +0000
@@ -1238,44 +1238,6 @@
         self.assertIsInstance(merge_obj, UnsupportedLCATreesMerger)
         self.assertFalse('lca_trees' in merge_obj.kwargs)
 
-# XXX: Thing left to test:
-#       Double-criss-cross merge, the ultimate base value is different from the
-#       intermediate.
-#         A    value 'foo'
-#         |\
-#         B C  B value 'bar', C = 'foo'
-#         |X|
-#         D E  D = 'bar', E supersedes to 'bing'
-#         |X|
-#         F G  F = 'bing', G supersedes to 'barry'
-#
-#       In this case, we technically should not care about the value 'bar' for
-#       D, because it was clearly superseded by E's 'bing'. The
-#       per-file/attribute graph would actually look like:
-#         A
-#         |
-#         B
-#         |
-#         E
-#         |
-#         G
-#
-#       Because the other side of the merge never modifies the value, it just
-#       takes the value from the merge.
-#
-#       ATM I expect this to fail because we will prune 'foo' from the LCAs,
-#       and not 'bar'. I don't know if this is strictly a problem, as it is a
-#       distinctly edge case.
-#
-#       Another incorrect resolution from the same basic flaw:
-#         A    value 'foo'
-#         |\
-#         B C  B value 'bar', C = 'foo'
-#         |X|
-#         D E  D = 'bar', E reverts to 'foo'
-#         |X|
-#         F G  F = 'bar', G reverts to 'foo'
-
 
 class TestMergerEntriesLCA(TestMergerBase):
 
@@ -1517,6 +1479,93 @@
 
         self.assertEqual([], list(merge_obj._entries_lca()))
 
+    def test_one_lca_supersedes_path(self):
+        # Double-criss-cross merge, the ultimate base value is different from
+        # the intermediate.
+        #   A    value 'foo'
+        #   |\
+        #   B C  B value 'bar', C = 'foo'
+        #   |X|
+        #   D E  D = 'bar', E supersedes to 'bing'
+        #   |X|
+        #   F G  F = 'bing', G supersedes to 'barry'
+        #
+        # In this case, we technically should not care about the value 'bar' for
+        # D, because it was clearly superseded by E's 'bing'. The
+        # per-file/attribute graph would actually look like:
+        #   A
+        #   |
+        #   B
+        #   |
+        #   E
+        #   |
+        #   G
+        #
+        # Because the other side of the merge never modifies the value, it just
+        # takes the value from the merge.
+        #
+        # ATM this fails because we will prune 'foo' from the LCAs, but we
+        # won't prune 'bar'. This is getting far off into edge-case land, so we
+        # aren't supporting it yet.
+        #
+        builder = self.get_builder()
+        builder.build_snapshot('A-id', None,
+            [('add', (u'', 'a-root-id', 'directory', None)),
+             ('add', (u'foo', 'foo-id', 'file', 'A content\n'))])
+        builder.build_snapshot('C-id', ['A-id'], [])
+        builder.build_snapshot('B-id', ['A-id'],
+            [('rename', ('foo', 'bar'))])
+        builder.build_snapshot('D-id', ['B-id', 'C-id'], [])
+        builder.build_snapshot('E-id', ['C-id', 'B-id'],
+            [('rename', ('foo', 'bing'))]) # override to bing
+        builder.build_snapshot('G-id', ['E-id', 'D-id'],
+            [('rename', ('bing', 'barry'))]) # override to barry
+        builder.build_snapshot('F-id', ['D-id', 'E-id'],
+            [('rename', ('bar', 'bing'))]) # Merge in E's change
+        merge_obj = self.make_merge_obj(builder, 'G-id')
+
+        self.expectFailure("We don't do an actual heads() check on lca values,"
+            " or use the per-attribute graph",
+            self.assertEqual, [], list(merge_obj._entries_lca()))
+
+    def test_one_lca_accidentally_pruned(self):
+        # Another incorrect resolution from the same basic flaw:
+        #   A    value 'foo'
+        #   |\
+        #   B C  B value 'bar', C = 'foo'
+        #   |X|
+        #   D E  D = 'bar', E reverts to 'foo'
+        #   |X|
+        #   F G  F = 'bing', G switches to 'bar'
+        #
+        # 'bar' will not be seen as an interesting change, because 'foo' will
+        # be pruned from the LCAs, even though it was newly introduced by E
+        # (superseding B).
+        builder = self.get_builder()
+        builder.build_snapshot('A-id', None,
+            [('add', (u'', 'a-root-id', 'directory', None)),
+             ('add', (u'foo', 'foo-id', 'file', 'A content\n'))])
+        builder.build_snapshot('C-id', ['A-id'], [])
+        builder.build_snapshot('B-id', ['A-id'],
+            [('rename', ('foo', 'bar'))])
+        builder.build_snapshot('D-id', ['B-id', 'C-id'], [])
+        builder.build_snapshot('E-id', ['C-id', 'B-id'], [])
+        builder.build_snapshot('G-id', ['E-id', 'D-id'],
+            [('rename', ('foo', 'bar'))])
+        builder.build_snapshot('F-id', ['D-id', 'E-id'],
+            [('rename', ('bar', 'bing'))]) # should end up conflicting
+        merge_obj = self.make_merge_obj(builder, 'G-id')
+
+        entries = list(merge_obj._entries_lca())
+        root_id = 'a-root-id'
+        self.expectFailure("We prune values from BASE even when relevant.",
+            self.assertEqual,
+                [('foo-id', False,
+                  ((root_id, [root_id, root_id]), root_id, root_id),
+                  ((u'foo', [u'bar', u'foo']), u'bar', u'bing'),
+                  ((False, [False, False]), False, False)),
+                ], entries)
+
     def test_both_sides_revert(self):
         # Both sides of a criss-cross revert the text to the lca
         #       A    base, introduces 'foo'
@@ -1789,10 +1838,10 @@
         """Get a real WorkingTree from the builder."""
         the_branch = builder.get_branch()
         wt = the_branch.bzrdir.create_workingtree()
-        # XXX: This is a little bit ugly, but we are holding the branch
-        #      write-locked as part of the build process, and we would like to
-        #      maintain that. So we just force the WT to re-use the same branch
-        #      object.
+        # Note: This is a little bit ugly, but we are holding the branch
+        #       write-locked as part of the build process, and we would like to
+        #       maintain that. So we just force the WT to re-use the same
+        #       branch object.
         wt._branch = the_branch
         wt.lock_write()
         self.addCleanup(wt.unlock)



More information about the bazaar-commits mailing list