Improve SVN.Revert() to catch more corner cases.

Add a unit test to catch potential regressions. Remove an old test new code broke.

TEST=unit tests
BUG=none

Review URL: http://codereview.chromium.org/6613025

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@76805 0039d316-1c4b-4281-b951-d872f2087c98

scm.py

@@ -878,21 +878,26 @@ class SVN(object):
 
       # svn revert is really stupid. It fails on inconsistent line-endings,
       # on switched directories, etc. So take no chance and delete everything!
-      if file_status[0][0] == 'D':
-        # Deleted file requires manual intervention and require calling
+      if file_status[0][0] in ('D', 'A') or file_status[0][2] != ' ':
+        # Added, deleted file requires manual intervention and require calling
         # revert, like for properties.
-        gclient_utils.CheckCall(
-            ['svn', 'revert', file_status[1]], cwd=repo_root)
+        try:
+          SVN.Capture(['revert', file_status[1]], cwd=repo_root)
+        except gclient_utils.CheckCallError:
+          if not os.path.exists(file_path):
+            continue
+          raise
+
+      if not os.path.exists(file_path):
+        continue
+
+      if os.path.isfile(file_path) or os.path.islink(file_path):
+        logging.info('os.remove(%s)' % file_path)
+        os.remove(file_path)
+      elif os.path.isdir(file_path):
+        logging.info('gclient_utils.RemoveDirectory(%s)' % file_path)
+        gclient_utils.RemoveDirectory(file_path)
       else:
-        if not os.path.exists(file_path):
-          pass
-        elif os.path.isfile(file_path) or os.path.islink(file_path):
-          logging.info('os.remove(%s)' % file_path)
-          os.remove(file_path)
-        elif os.path.isdir(file_path):
-          logging.info('gclient_utils.RemoveDirectory(%s)' % file_path)
-          gclient_utils.RemoveDirectory(file_path)
-        else:
-          logging.critical(
-            ('No idea what is %s.\nYou just found a bug in gclient'
-             ', please ping maruel@chromium.org ASAP!') % file_path)
+        logging.critical(
+          ('No idea what is %s.\nYou just found a bug in gclient'
+            ', please ping maruel@chromium.org ASAP!') % file_path)

tests/gclient_scm_test.py

@@ -168,37 +168,6 @@ class SVNWrapperTestCase(BaseTestCase):
     file_list = []
     scm.revert(options, self.args, file_list)
 
-  def testRevert2Files(self):
-    options = self.Options(verbose=True)
-    gclient_scm.os.path.isdir(self.base_path).AndReturn(True)
-    items = [
-      ('M      ', 'a'),
-      ('A      ', 'b'),
-    ]
-    file_path1 = join(self.base_path, 'a')
-    file_path2 = join(self.base_path, 'b')
-    gclient_scm.scm.SVN.CaptureStatus(self.base_path).AndReturn(items)
-    gclient_scm.os.path.exists(file_path1).AndReturn(True)
-    gclient_scm.os.path.isfile(file_path1).AndReturn(True)
-    gclient_scm.os.remove(file_path1)
-    gclient_scm.os.path.exists(file_path2).AndReturn(True)
-    gclient_scm.os.path.isfile(file_path2).AndReturn(True)
-    gclient_scm.os.remove(file_path2)
-    gclient_scm.scm.SVN.RunAndGetFileList(
-        options.verbose,
-        ['update', '--revision', 'BASE', '--ignore-externals'],
-        cwd=self.base_path,
-        file_list=mox.IgnoreArg())
-
-    self.mox.ReplayAll()
-    scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir,
-                            relpath=self.relpath)
-    file_list = []
-    scm.revert(options, self.args, file_list)
-    self.checkstdout(
-        ('%s\n%s\n' % (join(self.base_path, 'a'),
-            join(self.base_path, 'b'))))
-
   def testRevertDirectory(self):
     options = self.Options(verbose=True)
     gclient_scm.os.path.isdir(self.base_path).AndReturn(True)

tests/scm_unittest.py

@@ -10,6 +10,7 @@
 # Fixes include path.
 from super_mox import SuperMoxTestBase
 
+import fake_repos
 import scm
 
 
@@ -240,6 +241,54 @@ class SVNTestCase(BaseSCMTestCase):
     self.assertEquals(info, [])
 
 
+class RealSvnTest(fake_repos.FakeReposTestBase):
+  # Tests that work with a checkout.
+  def setUp(self):
+    super(RealSvnTest, self).setUp()
+    self.FAKE_REPOS.setUpSVN()
+    self.svn_root = scm.os.path.join(self.root_dir, 'base')
+    scm.SVN.Capture(
+        ['checkout', self.svn_base + 'trunk/third_party', 'base'],
+        cwd=self.root_dir)
+    self.tree = self.mangle_svn_tree(('trunk/third_party@-1', ''),)
+
+  def _capture(self, cmd, **kwargs):
+    kwargs.setdefault('cwd', self.svn_root)
+    return scm.SVN.Capture(cmd, **kwargs)
+
+  def testCheckout(self):
+    # Checkout and verify the tree.
+    self.assertTree(self.tree, self.svn_root)
+
+  def testRevert(self):
+    # Mess around and make sure revert works for all corner cases.
+    # - svn add a file
+    # - svn add a file and delete it
+    # - Delete a file
+    # - svn delete a file
+    # - svn move a directory and svn rename files in it
+    self._capture(['move', 'foo', 'foo2'])
+    self._capture(
+        ['move',
+         scm.os.path.join('foo2', 'origin'),
+         scm.os.path.join('foo2', 'o')])
+    scm.os.remove(scm.os.path.join(self.svn_root, 'origin'))
+    self._capture(
+        ['propset', 'foo', 'bar',
+         scm.os.path.join(self.svn_root, 'prout', 'origin')])
+    fake_repos.rmtree(scm.os.path.join(self.svn_root, 'prout'))
+    with open(scm.os.path.join(self.svn_root, 'faa'), 'w') as f:
+      f.write('eh')
+    with open(scm.os.path.join(self.svn_root, 'faala'), 'w') as f:
+      f.write('oh')
+    self._capture(['add', scm.os.path.join(self.svn_root, 'faala')])
+
+    scm.SVN.Revert(self.svn_root)
+    self._capture(['update', '--revision', 'base'])
+
+    self.assertTree(self.tree, self.svn_root)
+
+
 if __name__ == '__main__':
   import unittest
   unittest.main()