Rev 4639: Some cleanup. in file:///home/vila/src/bzr/bugs/531967-unify-name-conflicts/

Vincent Ladeuil v.ladeuil+lp at free.fr
Wed Mar 10 10:31:45 GMT 2010


At file:///home/vila/src/bzr/bugs/531967-unify-name-conflicts/

------------------------------------------------------------
revno: 4639
revision-id: v.ladeuil+lp at free.fr-20100310103145-7qyepzvdoeooezx2
parent: v.ladeuil+lp at free.fr-20100310100646-cxdu0h3y6pokm5je
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 531967-unify-name-conflicts
timestamp: Wed 2010-03-10 11:31:45 +0100
message:
  Some cleanup.
  
  * bzrlib/tests/test_conflicts.py:
  (TestResolveContentsConflict, TestResolvePathConflict): Cleanup
  scenarios and sme method names.
  
  * bzrlib/tests/test_conflicts.py:
  (TestParametrizedResolveConflicts): Cleanup common parameter which
  are really test class attributes.
  
  * bzrlib/conflicts.py:
  (PathConflict._infer_file_id): Cleaned up.
-------------- next part --------------
=== modified file 'bzrlib/conflicts.py'
--- a/bzrlib/conflicts.py	2010-03-10 10:06:46 +0000
+++ b/bzrlib/conflicts.py	2010-03-10 10:31:45 +0000
@@ -506,11 +506,7 @@
     def _revision_tree(self, tree, revid):
         return tree.branch.repository.revision_tree(revid)
 
-    def _get_or_infer_file_id(self, tree):
-        # FIXME: Needs cleanup
-        if self.file_id is not None:
-            return self.file_id
-
+    def _infer_file_id(self, tree):
         # Prior to bug #531967, file_id wasn't always set, there may still be
         # conflict files in the wild so we need to cope with them
         # Establish which path we should use to find back the file-id
@@ -518,9 +514,6 @@
         for p in (self.path, self.conflict_path):
             if p == '<deleted>':
                 # special hard-coded path 
-
-                # FIXME: this forbids that path to the user. That's bad but we
-                # are in recovery mode here anyway -- vila 20100305
                 continue
             if p is not None:
                 possible_paths.append(p)
@@ -531,12 +524,6 @@
             for p in possible_paths:
                 file_id = revtree.path2id(p)
                 if file_id is not None:
-                    # Now we need to add the item as it was in the revtree to
-                    # the current tree
-
-                    # and I have no idea about how to do that for all possible
-                    # cases (well, I have some but all sound far too
-                    # complicated)
                     return revtree, file_id
         return None, None
 
@@ -547,7 +534,7 @@
         else:
             # Prior to bug #531967 we need to find back the file_id and restore
             # the content from there
-            revtree, file_id = self._get_or_infer_file_id(tree)
+            revtree, file_id = self._infer_file_id(tree)
             tree.revert([revtree.id2path(file_id)],
                         old_tree=revtree, backups=False)
 
@@ -559,7 +546,7 @@
         else:
             # Prior to bug #531967 we need to find back the file_id and restore
             # the content from there
-            revtree, file_id = self._get_or_infer_file_id(tree)
+            revtree, file_id = self._infer_file_id(tree)
             tree.revert([revtree.id2path(file_id)],
                         old_tree=revtree, backups=False)
 

=== modified file 'bzrlib/tests/test_conflicts.py'
--- a/bzrlib/tests/test_conflicts.py	2010-03-09 15:24:27 +0000
+++ b/bzrlib/tests/test_conflicts.py	2010-03-10 10:31:45 +0000
@@ -36,7 +36,7 @@
         standard_tests, tests.condition_isinstance((
                 TestParametrizedResolveConflicts,
                 )))
-    # Each test class define its own scenarios. This is needed for
+    # Each test class defines its own scenarios. This is needed for
     # TestResolvePathConflictBefore531967 that verifies that the same tests as
     # TestResolvePathConflict still pass.
     for test in tests.iter_suite_tests(sp_tests):
@@ -219,11 +219,14 @@
 # TestResolveConflicts -- vila 20100308
 class TestParametrizedResolveConflicts(tests.TestCaseWithTransport):
 
+    # Set by daughter classes
+    _conflict_type = None
+    _assert_conflict = None
+
     # Set by load_tests
     _base_actions = None
     _this_actions = None
     _other_actions = None
-    _conflict_type = None
     _item_path = None
     _item_id = None
 
@@ -234,7 +237,7 @@
     _other_id = None
 
     @classmethod
-    def mirror_scenarios(klass, base_scenarios, common_params):
+    def mirror_scenarios(klass, common_params, base_scenarios):
         scenarios = []
         def adapt(d, side):
             """Modify dict to apply to the given side.
@@ -303,8 +306,7 @@
         self.assertLength(1, confs)
         c = confs[0]
         self.assertIsInstance(c, self._conflict_type)
-        _assert_conflict = getattr(self, self._assert_conflict)
-        _assert_conflict(c)
+        self._assert_conflict(c)
 
     def check_resolved(self, wt, path, action):
         conflicts.resolve(wt, [path], action=action)
@@ -327,7 +329,6 @@
         self.assertFileEqual('trunk content\nmore content\n', 'branch/file')
 
     def do_delete_file(self):
-        # None or <deleted> ?
         return ('file', 'file-id', [('unversion', 'file-id')])
 
     def check_file_doesnt_exist(self):
@@ -348,8 +349,6 @@
         self.failUnlessExists('branch/new-dir2')
 
     def do_delete_dir(self):
-        # None or <deleted> ?
-        # bug #531967 also mess up the paths
         return ('<deleted>', 'dir-id', [('unversion', 'dir-id')])
 
     def check_dir_doesnt_exist(self):
@@ -378,46 +377,48 @@
 
 class TestResolveContentsConflict(TestParametrizedResolveConflicts):
 
+    _conflict_type = conflicts.ContentsConflict,
     @classmethod
     def scenarios(klass):
+        common = dict(_actions_base='create_file',
+                      _item_path='file', item_id='file-id',
+                      )
         base_scenarios = [
             (('file_modified', dict(actions='modify_file',
                                    check='file_has_more_content')),
              ('file_deleted', dict(actions='delete_file',
                                    check='file_doesnt_exist'))),
             ]
-        common = dict(_conflict_type=conflicts.ContentsConflict,
-                      _actions_base='create_file',
-                      _assert_conflict='assertContentsConflict',
-                      _item_path='file', item_id='file-id',
-                      )
-        return klass.mirror_scenarios(base_scenarios, common)
+        return klass.mirror_scenarios(common, base_scenarios)
 
     def assertContentsConflict(self, c):
         self.assertEqual(self._other_id, c.file_id)
         self.assertEqual(self._other_path, c.path)
+    _assert_conflict = assertContentsConflict
+
 
 
 class TestResolvePathConflict(TestParametrizedResolveConflicts):
 
+    _conflict_type = conflicts.PathConflict,
+
     @classmethod
     def scenarios(klass):
+        common = dict(_actions_base='create_dir',
+                      _item_path='new-dir', _item_id='dir-id',)
         base_scenarios = [
         (('dir_renamed', dict(actions='rename_dir', check='dir_renamed')),
          ('dir_deleted', dict(actions='delete_dir', check='dir_doesnt_exist'))),
         (('dir_renamed', dict(actions='rename_dir', check='dir_renamed')),
          ('dir_renamed2', dict(actions='rename_dir2', check='dir_renamed2'))),
             ]
-        common = dict(_conflict_type=conflicts.PathConflict,
-                      _assert_conflict='assert_PathConflict',
-                      _actions_base='create_dir',
-                      _item_path='new-dir', _item_id='dir-id',)
-        return klass.mirror_scenarios(base_scenarios, common)
+        return klass.mirror_scenarios(common, base_scenarios)
 
-    def assert_PathConflict(self, c):
+    def assertPathConflict(self, c):
         self.assertEqual(self._item_id, c.file_id)
         self.assertEqual(self._this_path, c.path)
         self.assertEqual(self._other_path, c.conflict_path)
+    _assert_conflict = assertPathConflict
 
 
 class TestResolvePathConflictBefore531967(TestParametrizedResolveConflicts):
@@ -427,7 +428,7 @@
     # FIXME: Now that bug #531697 is fixed, we need to inject a conflict object
     # as it existed before the fix.
 
-    def assert_PathConflict(self, c):
+    def assertPathConflict(self, c):
         # bug #531967 is about file_id not being set in some cases
         self.assertIs(None, c.file_id)
         # Whatever this and other are saying, the same paths are used



More information about the bazaar-commits mailing list