Increase coverage to 91%; Much stricter about header parsing.

tests/patch_test.py

 #!/usr/bin/env python
 # Copyright (c) 2011 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 patch.py."""
 
+import logging
 import os
 import sys
 import unittest
 
 ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
 sys.path.insert(0, os.path.join(ROOT_DIR, '..'))
 
 import patch
 
 
@@ skipping 100 matching lines @@
 GIT_NEW = (
     'diff --git a/foo b/foo\n'
     'new file mode 100644\n'
     'index 0000000..5716ca5\n'
     '--- /dev/null\n'
     '+++ b/foo\n'
     '@@ -0,0 +1 @@\n'
     '+bar\n')
 
 
+NEW = (
+    '--- /dev/null\n'
+    '+++ foo\n'
+    '@@ -0,0 +1 @@\n'
+    '+bar\n')
+
+
 class PatchTest(unittest.TestCase):
   def testFilePatchDelete(self):
     c = patch.FilePatchDelete('foo', False)
+    self.assertEquals(c.filename, 'foo')
+    self.assertEquals(c.is_binary, False)
     self.assertEquals(c.is_delete, True)
-    self.assertEquals(c.is_binary, False)
-    self.assertEquals(c.filename, 'foo')
+    self.assertEquals(c.is_new, False)
     try:
       c.get()
       self.fail()
     except NotImplementedError:
       pass
     c = patch.FilePatchDelete('foo', True)
+    self.assertEquals(c.filename, 'foo')
+    self.assertEquals(c.is_binary, True)
     self.assertEquals(c.is_delete, True)
-    self.assertEquals(c.is_binary, True)
-    self.assertEquals(c.filename, 'foo')
+    self.assertEquals(c.is_new, False)
     try:
       c.get()
       self.fail()
     except NotImplementedError:
       pass
 
   def testFilePatchBinary(self):
-    c = patch.FilePatchBinary('foo', 'data', [])
+    c = patch.FilePatchBinary('foo', 'data', [], False)

[Dirk Pranke, 2011/06/03 19:17:27]

Nit: I'd normally name the parameter here as well (and below).

[M-A Ruel, 2011/06/03 19:42:31]

done everywhere

+    self.assertEquals(c.filename, 'foo')
+    self.assertEquals(c.is_binary, True)
     self.assertEquals(c.is_delete, False)
+    self.assertEquals(c.is_new, False)
+    self.assertEquals(c.get(), 'data')
+
+  def testFilePatchBinaryNew(self):
+    c = patch.FilePatchBinary('foo', 'data', [], True)
+    self.assertEquals(c.filename, 'foo')
     self.assertEquals(c.is_binary, True)
-    self.assertEquals(c.filename, 'foo')
+    self.assertEquals(c.is_delete, False)
+    self.assertEquals(c.is_new, True)
     self.assertEquals(c.get(), 'data')
 
   def testFilePatchDiff(self):
     c = patch.FilePatchDiff('chrome/file.cc', SVN_PATCH, [])
+    self.assertEquals(c.filename, 'chrome/file.cc')
+    self.assertEquals(c.is_binary, False)
     self.assertEquals(c.is_delete, False)
-    self.assertEquals(c.is_binary, False)
-    self.assertEquals(c.filename, 'chrome/file.cc')
     self.assertEquals(c.is_git_diff, False)
+    self.assertEquals(c.is_new, False)
     self.assertEquals(c.patchlevel, 0)
     self.assertEquals(c.get(), SVN_PATCH)
+
+  def testFilePatchDiffHeaderMode(self):
     diff = (
         'diff --git a/git_cl/git-cl b/git_cl/git-cl\n'
         'old mode 100644\n'
         'new mode 100755\n')
     c = patch.FilePatchDiff('git_cl/git-cl', diff, [])
+    self.assertEquals(c.filename, 'git_cl/git-cl')
+    self.assertEquals(c.is_binary, False)
     self.assertEquals(c.is_delete, False)
-    self.assertEquals(c.is_binary, False)
-    self.assertEquals(c.filename, 'git_cl/git-cl')
     self.assertEquals(c.is_git_diff, True)
+    self.assertEquals(c.is_new, False)
     self.assertEquals(c.patchlevel, 1)
     self.assertEquals(c.get(), diff)
+
+  def testFilePatchDiffHeaderModeIndex(self):
     diff = (
         'Index: Junk\n'
         'diff --git a/git_cl/git-cl b/git_cl/git-cl\n'
         'old mode 100644\n'
         'new mode 100755\n')
     c = patch.FilePatchDiff('git_cl/git-cl', diff, [])
+    self.assertEquals(c.filename, 'git_cl/git-cl')
+    self.assertEquals(c.is_binary, False)
     self.assertEquals(c.is_delete, False)
-    self.assertEquals(c.is_binary, False)
-    self.assertEquals(c.filename, 'git_cl/git-cl')
     self.assertEquals(c.is_git_diff, True)
+    self.assertEquals(c.is_new, False)
     self.assertEquals(c.patchlevel, 1)
     self.assertEquals(c.get(), diff)
 
-  def testFilePatchBadDiff(self):
+  def testFilePatchDiffSvnNew(self):
+    # The code path is different for git and svn.
+    c = patch.FilePatchDiff('foo', NEW, [])
+    self.assertEquals(c.filename, 'foo')
+    self.assertEquals(c.is_binary, False)
+    self.assertEquals(c.is_delete, False)
+    self.assertEquals(c.is_git_diff, False)
+    self.assertEquals(c.is_new, True)
+    self.assertEquals(c.patchlevel, 0)
+    self.assertEquals(c.get(), NEW)
+
+  def testFilePatchDiffGitNew(self):
+    # The code path is different for git and svn.
+    c = patch.FilePatchDiff('foo', GIT_NEW, [])
+    self.assertEquals(c.filename, 'foo')
+    self.assertEquals(c.is_binary, False)
+    self.assertEquals(c.is_delete, False)
+    self.assertEquals(c.is_git_diff, True)
+    self.assertEquals(c.is_new, True)
+    self.assertEquals(c.patchlevel, 1)
+    self.assertEquals(c.get(), GIT_NEW)
+
+  def testFilePatchDiffBad(self):
     try:
       patch.FilePatchDiff('foo', 'data', [])
       self.fail()
     except patch.UnsupportedPatchFormat:
       pass
 
-  def testFilePatchNoDiff(self):
+  def testFilePatchDiffEmpty(self):
     try:
       patch.FilePatchDiff('foo', '', [])
       self.fail()
     except patch.UnsupportedPatchFormat:
       pass
 
-  def testFilePatchNoneDiff(self):
+  def testFilePatchDiffNone(self):
     try:
       patch.FilePatchDiff('foo', None, [])
       self.fail()
     except patch.UnsupportedPatchFormat:
       pass
 
   def testFilePatchBadDiffName(self):
     try:
       patch.FilePatchDiff('foo', SVN_PATCH, [])
       self.fail()
+    except patch.UnsupportedPatchFormat, e:
+      self.assertEquals(
+          "Can't process patch for file foo.\nUnexpected diff: chrome/file.cc.",
+          str(e))
+
+  def testFilePatchDiffBadHeader(self):
+    try:
+      diff = (
+        '+++ b/foo\n'
+        '@@ -0,0 +1 @@\n'
+        '+bar\n')
+      patch.FilePatchDiff('foo', diff, [])
+      self.fail()
     except patch.UnsupportedPatchFormat:
       pass
 
-  def testInvalidFilePatchDiffGit(self):
+  def testFilePatchDiffBadGitHeader(self):
+    try:
+      diff = (
+        'diff --git a/foo b/foo\n'
+        '+++ b/foo\n'
+        '@@ -0,0 +1 @@\n'
+        '+bar\n')
+      patch.FilePatchDiff('foo', diff, [])
+      self.fail()
+    except patch.UnsupportedPatchFormat:
+      pass
+
+  def testFilePatchDiffBadHeaderReversed(self):
+    try:
+      diff = (
+        '+++ b/foo\n'
+        '--- b/foo\n'
+        '@@ -0,0 +1 @@\n'
+        '+bar\n')
+      patch.FilePatchDiff('foo', diff, [])
+      self.fail()
+    except patch.UnsupportedPatchFormat:
+      pass
+
+  def testFilePatchDiffGitBadHeaderReversed(self):
+    try:
+      diff = (
+        'diff --git a/foo b/foo\n'
+        '+++ b/foo\n'
+        '--- b/foo\n'
+        '@@ -0,0 +1 @@\n'
+        '+bar\n')
+      patch.FilePatchDiff('foo', diff, [])
+      self.fail()
+    except patch.UnsupportedPatchFormat:
+      pass
+
+  def testFilePatchDiffInvalidGit(self):
     try:
       patch.FilePatchDiff('svn_utils_test.txt', (
         'diff --git a/tests/svn_utils_test_data/svn_utils_test.txt '
         'b/tests/svn_utils_test_data/svn_utils_test.txt\n'
         'index 0e4de76..8320059 100644\n'
         '--- a/svn_utils_test.txt\n'
         '+++ b/svn_utils_test.txt\n'
         '@@ -3,6 +3,7 @@ bb\n'
         'ccc\n'
         'dd\n'
@@ skipping 57 matching lines @@
         patch.FilePatchDiff('chrome/file.cc', SVN_PATCH, []),
         patch.FilePatchDiff(
             'tools\\clang_check/README.chromium', GIT_DELETE, []),
         patch.FilePatchDiff('tools/run_local_server.sh', GIT_RENAME, []),
         patch.FilePatchDiff(
             'chrome\\browser/chromeos/views/webui_menu_widget.h',
             GIT_RENAME_PARTIAL, []),
         patch.FilePatchDiff('pp', GIT_COPY, []),
         patch.FilePatchDiff('foo', GIT_NEW, []),
         patch.FilePatchDelete('other/place/foo', True),
-        patch.FilePatchBinary('bar', 'data', []),
+        patch.FilePatchBinary('bar', 'data', [], False),
     ])
     expected = [
         'chrome/file.cc', 'tools/clang_check/README.chromium',
         'tools/run_local_server.sh',
         'chrome/browser/chromeos/views/webui_menu_widget.h', 'pp', 'foo',
         'other/place/foo', 'bar']
     self.assertEquals(expected, patches.filenames)
     orig_name = patches.patches[0].filename
     patches.set_relpath(os.path.join('a', 'bb'))
     expected = [os.path.join('a', 'bb', x) for x in expected]
@@ skipping 14 matching lines @@
     patches = patch.PatchSet([
         patch.FilePatchDiff('chrome\\file.cc', SVN_PATCH, []),
         patch.FilePatchDelete('other\\place\\foo', True),
     ])
     try:
       patches.set_relpath('..')
       self.fail()
     except patch.UnsupportedPatchFormat:
       pass
 
+  def testRelPathEmpty(self):
+    patches = patch.PatchSet([
+        patch.FilePatchDiff('chrome\\file.cc', SVN_PATCH, []),
+        patch.FilePatchDelete('other\\place\\foo', True),
+    ])
+    patches.set_relpath('')
+    self.assertEquals(
+        ['chrome/file.cc', 'other/place/foo'],
+        [f.filename for f in patches])
+
   def testBackSlash(self):
     mangled_patch = SVN_PATCH.replace('chrome/', 'chrome\\')
     patches = patch.PatchSet([
         patch.FilePatchDiff('chrome\\file.cc', mangled_patch, []),
         patch.FilePatchDelete('other\\place\\foo', True),
     ])
     expected = ['chrome/file.cc', 'other/place/foo']
     self.assertEquals(expected, patches.filenames)
     self.assertEquals(SVN_PATCH, patches.patches[0].get())
 
@@ skipping 61 matching lines @@
         '--- /dev/null\n'
         '+++ b/natsort_test.py\n'
         '@@ -0,0 +1,1 @@\n'
         '+#!/usr/bin/env python\n')
     self.assertEquals(
         [('svn:executable', '*')],
         patch.FilePatchDiff('natsort_test.py', diff, []).svn_properties)
 
 
 if __name__ == '__main__':
+  logging.basicConfig(level=
+      [logging.WARNING, logging.INFO, logging.DEBUG][
+        min(2, sys.argv.count('-v'))])
   unittest.main()