Clean up revert.py and fixes an issue when only one file is reverted in a subdirectory.

Add a basic unit test.

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

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

revert.py

@@ -24,32 +24,6 @@ class OutsideOfCheckout(exceptions.Exception):
   pass
 
 
-def getTexts(nodelist):
-  """Return a list of texts in the children of a list of DOM nodes."""
-  rc = []
-  for node in nodelist:
-    if node.nodeType == node.TEXT_NODE:
-      rc.append(node.data)
-    else:
-      rc.extend(getTexts(node.childNodes))
-  return rc
-
-
-def RunShellXML(command, print_output=False, keys=None):
-  output = gcl.RunShell(command, print_output)
-  try:
-    dom = xml.dom.minidom.parseString(output)
-    if not keys:
-      return dom
-    result = {}
-    for key in keys:
-      result[key] = getTexts(dom.getElementsByTagName(key))
-  except xml.parsers.expat.ExpatError:
-    print "Failed to parse output:\n%s" % output
-    raise
-  return result
-
-
 def UniqueFast(list):
   list = [item for item in set(list)]
   list.sort()
@@ -58,9 +32,9 @@ def UniqueFast(list):
 
 def GetRepoBase():
   """Returns the repository base of the root local checkout."""
-  xml_data = RunShellXML(['svn', 'info', '.', '--xml'], keys=['root', 'url'])
+  info = gclient.CaptureSVNInfo('.')
-  root = xml_data['root'][0]
+  root = info['Repository Root']
-  url = xml_data['url'][0]
+  url = info['URL']
   if not root or not url:
     raise exceptions.Exception("I'm confused by your checkout")
   if not url.startswith(root):
@@ -68,6 +42,38 @@ def GetRepoBase():
   return url[len(root):] + '/'
 
 
+def CaptureSVNLog(args):
+  command = ['log', '--xml']
+  if args:
+    command += args
+  output = gclient.CaptureSVN(command)
+  dom = gclient.ParseXML(output)
+  entries = []
+  if dom:
+    # /log/logentry/
+    #   @revision
+    #   author|date
+    #   paths/
+    #     path (@kind&@action)
+    for node in dom.getElementsByTagName('logentry'):
+      paths = []
+      for path in node.getElementsByTagName('path'):
+        item = {
+          'kind': path.getAttribute('kind'),
+          'action': path.getAttribute('action'),
+          'path': path.firstChild.nodeValue,
+        }
+        paths.append(item)
+      entry = {
+        'revision': int(node.getAttribute('revision')),
+        'author': gclient.GetNamedNodeText(node, 'author'),
+        'date': gclient.GetNamedNodeText(node, 'date'),
+        'paths': paths,
+      }
+      entries.append(entry)
+  return entries
+
+
 def Revert(revisions, force=False, commit=True, send_email=True, message=None,
            reviewers=None):
   """Reverts many revisions in one change list.
@@ -91,20 +97,23 @@ def Revert(revisions, force=False, commit=True, send_email=True, message=None,
   revisions_string = ",".join([str(rev) for rev in revisions])
   revisions_string_rev = ",".join([str(-rev) for rev in revisions])
 
-  repo_base = GetRepoBase()
-  files = []
-  blames = []
   # Get all the modified files by the revision. We'll use this list to optimize
   # the svn merge.
+  logs = []
   for revision in revisions:
-    log = RunShellXML(["svn", "log", "-r", str(revision), "-v", "--xml"],
+    logs.extend(CaptureSVNLog(["-r", str(revision), "-v"]))
-                        keys=['path', 'author'])
+
-    for file in log['path']:
+  files = []
+  blames = []
+  repo_base = GetRepoBase()
+  for log in logs:
+    for file in log['paths']:
+      file_name = file['path']
       # Remove the /trunk/src/ part. The + 1 is for the last slash.
-      if not file.startswith(repo_base):
+      if not file_name.startswith(repo_base):
-        raise OutsideOfCheckout(file)
+        raise OutsideOfCheckout(file_name)
-      files.append(file[len(repo_base):])
+      files.append(file_name[len(repo_base):])
-    blames.extend(log['author'])
+    blames.append(log['author'])
 
   # On Windows, we need to fix the slashes once they got the url part removed.
   if sys.platform == 'win32':
@@ -178,7 +187,6 @@ def Revert(revisions, force=False, commit=True, send_email=True, message=None,
         print 'svn up . -N failed in %s/.' % root
         return retcode
 
-    # TODO(maruel): BUG WITH ONLY ONE FILE.
     command = ["svn", "merge", "-c", revisions_string_rev]
     command.extend(file_list)
     (output, retcode) = gcl.RunShellWithReturnCode(command, print_output=True)

tests/revert_unittest.py

@@ -0,0 +1,50 @@
+#!/usr/bin/python
+# Copyright (c) 2009 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Unit tests for revert.py."""
+
+import os
+import unittest
+
+# Local imports
+import revert
+
+
+class RevertTestsBase(unittest.TestCase):
+  """Setups and tear downs the mocks but doesn't test anything as-is."""
+  def setUp(self):
+    pass
+
+  def tearDown(self):
+    pass
+
+  def compareMembers(self, object, members):
+    """If you add a member, be sure to add the relevant test!"""
+    # Skip over members starting with '_' since they are usually not meant to
+    # be for public use.
+    actual_members = [x for x in sorted(dir(object))
+                      if not x.startswith('_')]
+    expected_members = sorted(members)
+    if actual_members != expected_members:
+      diff = ([i for i in actual_members if i not in expected_members] +
+              [i for i in expected_members if i not in actual_members])
+      print diff
+    self.assertEqual(actual_members, expected_members)
+
+
+class RevertUnittest(RevertTestsBase):
+  """General revert.py tests."""
+  def testMembersChanged(self):
+    members = [
+      'CaptureSVNLog', 'GetRepoBase', 'Main', 'ModifiedFile', 'NoBlameList',
+      'NoModifiedFile', 'OutsideOfCheckout', 'Revert', 'UniqueFast',
+      'exceptions', 'gcl', 'gclient', 'optparse', 'os', 'sys', 'xml'
+    ]
+    # If this test fails, you should add the relevant test.
+    self.compareMembers(revert, members)
+
+
+if __name__ == '__main__':
+  unittest.main()