Factor out SVN-specific methods out of AffectedFile.

TEST=New unit tests.
BUG=None

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

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

presubmit_support.py

@@ -324,15 +324,18 @@ class AffectedFile(object):
 
   def __init__(self, path, action, repository_root=''):
     self.path = path
-    self.action = action.strip()
+    self.action = action
     self.repository_root = repository_root
+    self.server_path = None
+    self.is_directory = None
+    self.properties = {}
 
   def ServerPath(self):
     """Returns a path string that identifies the file in the SCM system.
 
     Returns the empty string if the file does not exist in SCM.
     """
-    return gclient.CaptureSVNInfo(self.AbsoluteLocalPath()).get('URL', '')
+    return ""
 
   def LocalPath(self):
     """Returns the path of this file on the local disk relative to client root.
@@ -347,24 +350,23 @@ class AffectedFile(object):
   def IsDirectory(self):
     """Returns true if this object is a directory."""
     path = self.AbsoluteLocalPath()
-    if os.path.exists(path):
+    if self.is_directory is None:
-      # Retrieve directly from the file system; it is much faster than querying
+      self.is_directory = (os.path.exists(path) and
-      # subversion, especially on Windows.
+                           os.path.isdir(path))
-      return os.path.isdir(path)
+    return self.is_directory
-    else:
-      return gclient.CaptureSVNInfo(path).get('Node Kind') in ('dir',
-                                                               'directory')
-
-  def SvnProperty(self, property_name):
-    """Returns the specified SVN property of this file, or the empty string
-    if no such property.
-    """
-    return gcl.GetSVNFileProperty(self.AbsoluteLocalPath(), property_name)
 
   def Action(self):
     """Returns the action on this opened file, e.g. A, M, D, etc."""
+    # TODO(maruel): Somewhat crappy, Could be "A" or "A  +" for svn but
+    # different for other SCM.
     return self.action
 
+  def Property(self, property_name):
+    """Returns the specified SCM property of this file, or None if no such
+    property.
+    """
+    return self.properties.get(property_name, None)
+
   def NewContents(self):
     """Returns an iterator over the lines in the new version of file.
 
@@ -395,6 +397,34 @@ class AffectedFile(object):
     raise NotImplementedError()  # Implement if/when needed.
 
 
+class SvnAffectedFile(AffectedFile):
+  """Representation of a file in a change out of a Subversion checkout."""
+
+  def ServerPath(self):
+    if self.server_path is None:
+      self.server_path = gclient.CaptureSVNInfo(
+          self.AbsoluteLocalPath()).get('URL', '')
+    return self.server_path
+
+  def IsDirectory(self):
+    path = self.AbsoluteLocalPath()
+    if self.is_directory is None:
+      if os.path.exists(path):
+        # Retrieve directly from the file system; it is much faster than
+        # querying subversion, especially on Windows.
+        self.is_directory = os.path.isdir(path)
+      else:
+        self.is_directory = gclient.CaptureSVNInfo(
+            path).get('Node Kind') in ('dir', 'directory')
+    return self.is_directory
+
+  def Property(self, property_name):
+    if not property_name in self.properties:
+      self.properties[property_name] = gcl.GetSVNFileProperty(
+          self.AbsoluteLocalPath(), property_name)
+    return self.properties[property_name]
+
+
 class GclChange(object):
   """A gcl change. See gcl.ChangeInfo for more info."""
 
@@ -418,8 +448,10 @@ class GclChange(object):
     self.description_without_tags = '\n'.join(self.description_without_tags)
     self.description_without_tags = self.description_without_tags.rstrip()
 
-    self.affected_files = [AffectedFile(info[1], info[0], repository_root) for
+    self.affected_files = [
-                           info in change_info.files]
+        SvnAffectedFile(info[1], info[0].strip(), repository_root)
+        for info in change_info.files
+    ]
 
   def Change(self):
     """Returns the change name."""
@@ -551,7 +583,6 @@ def ListRelevantPresubmitFiles(files):
 
 
 class PresubmitExecuter(object):
-
   def __init__(self, change_info, committing):
     """
     Args:

tests/presubmit_unittest.py

@@ -3,7 +3,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""Unit tests for presubmit.py and presubmit_canned_checks.py."""
+"""Unit tests for presubmit_support.py and presubmit_canned_checks.py."""
 
 import os
 import StringIO
@@ -111,13 +111,13 @@ def CheckChangeOnUpload(input_api, output_api):
 
 
 class PresubmitUnittest(PresubmitTestsBase):
-  """General presubmit.py tests (excluding InputApi and OutputApi)."""
+  """General presubmit_support.py tests (excluding InputApi and OutputApi)."""
   def testMembersChanged(self):
     members = [
       'AffectedFile', 'DoPresubmitChecks', 'GclChange', 'InputApi',
       'ListRelevantPresubmitFiles', 'Main', 'NotImplementedException',
       'OutputApi', 'ParseFiles', 'PresubmitExecuter', 'SPECIAL_KEYS',
-      'ScanSubDirs', 'cPickle', 'cStringIO', 'exceptions',
+      'ScanSubDirs', 'SvnAffectedFile', 'cPickle', 'cStringIO', 'exceptions',
       'fnmatch', 'gcl', 'gclient', 'glob', 'marshal', 'normpath', 'optparse',
       'os', 'pickle', 'presubmit_canned_checks', 're', 'subprocess', 'sys',
       'tempfile', 'types', 'urllib2',
@@ -216,17 +216,6 @@ class PresubmitUnittest(PresubmitTestsBase):
     self.failUnless(rhs_lines[3][1] == 2)
     self.failUnless(rhs_lines[3][2] == 'two:%s' % files[3][1])
 
-  def testAffectedFile(self):
-    af = presubmit.AffectedFile('foo/blat.cc', 'M')
-    self.failUnless(af.ServerPath() == 'svn:/foo/foo/blat.cc')
-    self.failUnless(af.LocalPath() == presubmit.normpath('foo/blat.cc'))
-    self.failUnless(af.Action() == 'M')
-    self.failUnless(af.NewContents() == ['one:%s' % af.LocalPath(),
-                                         'two:%s' % af.LocalPath()])
-
-    af = presubmit.AffectedFile('notfound.cc', 'A')
-    self.failUnless(af.ServerPath() == '')
-
   def testExecPresubmitScript(self):
     description_lines = ('Hello there',
                          'this is a change',
@@ -405,11 +394,6 @@ def CheckChangeOnUpload(input_api, output_api):
     affected_files_and_dirs = change.AffectedFiles(include_dirs=True)
     self.failUnless(len(affected_files_and_dirs) == 2)
 
-  def testSvnProperty(self):
-    affected_file = presubmit.AffectedFile('foo.cc', 'A')
-    self.failUnless(affected_file.SvnProperty('svn:secret-property') ==
-                    'secret-property-value')
-
 
 class InputApiUnittest(PresubmitTestsBase):
   """Tests presubmit.InputApi."""
@@ -597,6 +581,34 @@ class OuputApiUnittest(PresubmitTestsBase):
     self.failUnless(output.getvalue().count('???'))
 
 
+class AffectedFileUnittest(PresubmitTestsBase):
+  def testMembersChanged(self):
+    members = [
+      'AbsoluteLocalPath', 'Action', 'IsDirectory', 'LocalPath', 'NewContents',
+      'OldContents', 'OldFileTempPath', 'Property', 'ServerPath', 'action',
+      'is_directory', 'path', 'properties', 'repository_root', 'server_path',
+    ]
+    # If this test fails, you should add the relevant test.
+    self.compareMembers(presubmit.AffectedFile('a', 'b'), members)
+    self.compareMembers(presubmit.SvnAffectedFile('a', 'b'), members)
+
+  def testAffectedFile(self):
+    af = presubmit.SvnAffectedFile('foo/blat.cc', 'M')
+    self.failUnless(af.ServerPath() == 'svn:/foo/foo/blat.cc')
+    self.failUnless(af.LocalPath() == presubmit.normpath('foo/blat.cc'))
+    self.failUnless(af.Action() == 'M')
+    self.failUnless(af.NewContents() == ['one:%s' % af.LocalPath(),
+                                         'two:%s' % af.LocalPath()])
+
+    af = presubmit.AffectedFile('notfound.cc', 'A')
+    self.failUnless(af.ServerPath() == '')
+
+  def testProperty(self):
+    affected_file = presubmit.SvnAffectedFile('foo.cc', 'A')
+    self.failUnless(affected_file.Property('svn:secret-property') ==
+                    'secret-property-value')
+
+
 class CannedChecksUnittest(PresubmitTestsBase):
   """Tests presubmit_canned_checks.py."""
   class MockInputApi(object):