Reland of Gerrit git cl: implement git cl patch.

tests/git_cl_test.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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 git_cl.py."""
 
 import os
 import StringIO
 import stat
 import sys
 import unittest
+import urlparse
 
 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
 from testing_support.auto_stub import TestCase
 
 import git_cl
 import git_common
 import git_footers
 import subprocess2
 
@@ skipping 43 matching lines @@
             "GERRIT_HOST: True\n")
 
 
 class AuthenticatorMock(object):
   def __init__(self, *_args):
     pass
   def has_cached_credentials(self):
     return True
 
 
+class TestGitClBasic(unittest.TestCase):
+  def _test_ParseIssueUrl(self, func, url, issue, patchset, hostname, fail):
+    parsed = urlparse.urlparse(url)
+    result = func(parsed)
+    if fail:
+      self.assertIsNone(result)
+      return None
+    self.assertIsNotNone(result)
+    self.assertEqual(result.issue, issue)
+    self.assertEqual(result.patchset, patchset)
+    self.assertEqual(result.hostname, hostname)
+    return result
+
+  def test_ParseIssueURL_rietveld(self):
+    def test(url, issue=None, patchset=None, hostname=None, patch_url=None,
+             fail=None):
+      result = self._test_ParseIssueUrl(
+          git_cl._RietveldChangelistImpl.ParseIssueURL,
+          url, issue, patchset, hostname, fail)
+      if not fail:
+        self.assertEqual(result.patch_url, patch_url)
+
+    test('http://codereview.chromium.org/123',
+         123, None, 'codereview.chromium.org')
+    test('https://codereview.chromium.org/123',
+         123, None, 'codereview.chromium.org')
+    test('https://codereview.chromium.org/123/',
+         123, None, 'codereview.chromium.org')
+    test('https://codereview.chromium.org/123/whatever',
+         123, None, 'codereview.chromium.org')
+    test('http://codereview.chromium.org/download/issue123_4.diff',
+         123, 4, 'codereview.chromium.org',
+         patch_url='https://codereview.chromium.org/download/issue123_4.diff')
+    # This looks like bad Gerrit, but is actually valid Rietveld.
+    test('https://chrome-review.source.com/123/4/',
+         123, None, 'chrome-review.source.com')
+
+    test('https://codereview.chromium.org/deadbeaf', fail=True)
+    test('https://codereview.chromium.org/api/123', fail=True)
+    test('bad://codereview.chromium.org/123', fail=True)
+    test('http://codereview.chromium.org/download/issue123_4.diffff', fail=True)
+
+  def test_ParseIssueURL_gerrit(self):
+    def test(url, issue=None, patchset=None, hostname=None, fail=None):
+      self._test_ParseIssueUrl(
+          git_cl._GerritChangelistImpl.ParseIssueURL,
+          url, issue, patchset, hostname, fail)
+
+    test('http://chrome-review.source.com/c/123',
+         123, None, 'chrome-review.source.com')
+    test('https://chrome-review.source.com/c/123/',
+         123, None, 'chrome-review.source.com')
+    test('https://chrome-review.source.com/c/123/4',
+         123, 4, 'chrome-review.source.com')
+    test('https://chrome-review.source.com/#/c/123/4',
+         123, 4, 'chrome-review.source.com')
+    test('https://chrome-review.source.com/c/123/4',
+         123, 4, 'chrome-review.source.com')
+    test('https://chrome-review.source.com/123',
+         123, None, 'chrome-review.source.com')
+    test('https://chrome-review.source.com/123/4',
+         123, 4, 'chrome-review.source.com')
+
+    test('https://chrome-review.source.com/c/123/1/whatisthis', fail=True)
+    test('https://chrome-review.source.com/c/abc/', fail=True)
+    test('ssh://chrome-review.source.com/c/123/1/', fail=True)
+
+  def test_ParseIssueNumberArgument(self):
+    def test(arg, issue=None, patchset=None, hostname=None, fail=False):
+      result = git_cl.ParseIssueNumberArgument(arg)
+      self.assertIsNotNone(result)
+      if fail:
+        self.assertFalse(result.valid)
+      else:
+        self.assertEqual(result.issue, issue)
+        self.assertEqual(result.patchset, patchset)
+        self.assertEqual(result.hostname, hostname)
+
+    test('123', 123)
+    test('', fail=True)
+    test('abc', fail=True)
+    test('123/1', fail=True)
+    test('123a', fail=True)
+    test('ssh://chrome-review.source.com/#/c/123/4/', fail=True)
+    # Rietveld.
+    test('https://codereview.source.com/123',
+         123, None, 'codereview.source.com')
+    test('https://codereview.source.com/www123', fail=True)
+    # Gerrrit.
+    test('https://chrome-review.source.com/c/123/4',
+         123, 4, 'chrome-review.source.com')
+    test('https://chrome-review.source.com/bad/123/4', fail=True)
+
+
 class TestGitCl(TestCase):
   def setUp(self):
     super(TestGitCl, self).setUp()
     self.calls = []
     self._calls_done = 0
     self.mock(subprocess2, 'call', self._mocked_call)
     self.mock(subprocess2, 'check_call', self._mocked_call)
     self.mock(subprocess2, 'check_output', self._mocked_call)
     self.mock(subprocess2, 'communicate', self._mocked_call)
     self.mock(git_cl.gclient_utils, 'CheckCallAndFilter', self._mocked_call)
     self.mock(git_common, 'is_dirty_git_tree', lambda x: False)
     self.mock(git_common, 'get_or_create_merge_base',
               lambda *a: (
                   self._mocked_call(['get_or_create_merge_base']+list(a))))
     self.mock(git_cl, 'BranchExists', lambda _: True)
     self.mock(git_cl, 'FindCodereviewSettingsFile', lambda: '')
     self.mock(git_cl, 'ask_for_data', self._mocked_call)
     self.mock(git_cl.presubmit_support, 'DoPresubmitChecks', PresubmitMock)
     self.mock(git_cl.rietveld, 'Rietveld', RietveldMock)
     self.mock(git_cl.rietveld, 'CachingRietveld', RietveldMock)
     self.mock(git_cl.upload, 'RealMain', self.fail)
     self.mock(git_cl.watchlists, 'Watchlists', WatchlistsMock)
     self.mock(git_cl.auth, 'get_authenticator_for_host', AuthenticatorMock)
     # It's important to reset settings to not have inter-tests interference.
     git_cl.settings = None
 
+
   def tearDown(self):
     try:
+      # Note: has_failed returns True if at least 1 test ran so far, current
+      # included, has failed. That means current test may have actually ran
+      # fine, and the check for no leftover calls would be skipped.
       if not self.has_failed():
         self.assertEquals([], self.calls)
     finally:
       super(TestGitCl, self).tearDown()
 
   def _mocked_call(self, *args, **_kwargs):
     self.assertTrue(
         self.calls,
         '@%d  Expected: <Missing>   Actual: %r' % (self._calls_done, args))
     top = self.calls.pop(0)
@@ skipping 795 matching lines @@
                                            branch, None))
 
     # Check target refs for pending prefix.
     self.assertEqual('prefix/heads/master',
                      git_cl.GetTargetRef('origin', 'refs/remotes/origin/master',
                                          None, 'prefix/'))
 
   def test_patch_when_dirty(self):
     # Patch when local tree is dirty
     self.mock(git_common, 'is_dirty_git_tree', lambda x: True)
+    self.calls = [
+      ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
+      ((['git', 'config', 'branch.master.rietveldissue'],), ''),
+      ((['git', 'config', 'branch.master.gerritissue'],), ''),
+      ((['git', 'config', 'rietveld.autoupdate'],), ''),
+      ((['git', 'config', 'gerrit.host'],), ''),
+      ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'),
+    ]
     self.assertNotEqual(git_cl.main(['patch', '123456']), 0)
 
   def test_diff_when_dirty(self):
     # Do 'git cl diff' when local tree is dirty
     self.mock(git_common, 'is_dirty_git_tree', lambda x: True)
     self.assertNotEqual(git_cl.main(['diff']), 0)
 
-  def _patch_common(self):
+  def _patch_common(self, is_gerrit=False):
     self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset',
               lambda x: '60001')
     self.mock(git_cl._RietveldChangelistImpl, 'GetPatchSetDiff',
               lambda *args: None)
+    self.mock(git_cl._GerritChangelistImpl, '_GetChangeDetail',
+              lambda *args: {
+                'current_revision': '7777777777',
+                'revisions': {
+                  '1111111111': {
+                    '_number': 1,
+                    'fetch': {'http': {
+                      'url': 'https://chromium.googlesource.com/my/repo',
+                      'ref': 'refs/changes/56/123456/1',
+                    }},
+                  },
+                  '7777777777': {
+                    '_number': 7,
+                    'fetch': {'http': {
+                      'url': 'https://chromium.googlesource.com/my/repo',
+                      'ref': 'refs/changes/56/123456/7',
+                    }},
+                  },
+                },
+              })
     self.mock(git_cl.Changelist, 'GetDescription',
               lambda *args: 'Description')
-    self.mock(git_cl.Changelist, 'SetIssue', lambda *args: None)
-    self.mock(git_cl.Changelist, 'SetPatchset', lambda *args: None)
     self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True)
 
     self.calls = [
+      ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
+      ((['git', 'config', 'branch.master.rietveldissue'],), ''),
+      ((['git', 'config', 'branch.master.gerritissue'],), ''),
       ((['git', 'config', 'rietveld.autoupdate'],), ''),
-      ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'),
-      ((['git', 'rev-parse', '--show-cdup'],), ''),
-      ((['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],), ''),
     ]
+    if is_gerrit:
+      self.calls += [
+        ((['git', 'config', 'gerrit.host'],), 'true'),
+      ]
+    else:
+      self.calls += [
+        ((['git', 'config', 'gerrit.host'],), ''),
+        ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'),
+        ((['git', 'rev-parse', '--show-cdup'],), ''),
+        ((['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],), ''),
+      ]
 
   def test_patch_successful(self):
     self._patch_common()
     self.calls += [
       ((['git', 'apply', '--index', '-p0', '--3way'],), ''),
       ((['git', 'commit', '-m',
          'Description\n\n' +
          'patch from issue 123456 at patchset 60001 ' +
          '(http://crrev.com/123456#ps60001)'],), ''),
-      ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
+      ((['git', 'config', 'branch.master.rietveldissue', '123456'],), ''),
       ((['git', 'config', 'branch.master.rietveldserver'],), ''),
+      ((['git', 'config', 'branch.master.rietveldserver',
+         'https://codereview.example.com'],), ''),
+      ((['git', 'config', 'branch.master.rietveldpatchset', '60001'],), ''),
     ]
     self.assertEqual(git_cl.main(['patch', '123456']), 0)
 
   def test_patch_conflict(self):
     self._patch_common()
     self.calls += [
       ((['git', 'apply', '--index', '-p0', '--3way'],), '',
        subprocess2.CalledProcessError(1, '', '', '', '')),
     ]
     self.assertNotEqual(git_cl.main(['patch', '123456']), 0)
 
+  def test_gerrit_patch_successful(self):
+    self._patch_common(is_gerrit=True)
+    self.calls += [
+      ((['git', 'fetch', 'https://chromium.googlesource.com/my/repo',
+         'refs/changes/56/123456/7'],), ''),
+      ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
+      ((['git', 'config', 'branch.master.gerritissue', '123456'],), ''),
+      ((['git', 'config', 'branch.master.gerritserver'],), ''),
+      ((['git', 'config', 'branch.master.merge'],), 'master'),
+      ((['git', 'config', 'branch.master.remote'],), 'origin'),
+      ((['git', 'config', 'remote.origin.url'],),
+       'https://chromium.googlesource.com/my/repo'),
+      ((['git', 'config', 'branch.master.gerritserver',
+        'https://chromium-review.googlesource.com'],), ''),
+      ((['git', 'config', 'branch.master.gerritpatchset', '7'],), ''),
+    ]
+    self.assertEqual(git_cl.main(['patch', '123456']), 0)
+
+  def test_gerrit_patch_url_successful(self):
+    self._patch_common(is_gerrit=True)
+    self.calls += [
+      ((['git', 'fetch', 'https://chromium.googlesource.com/my/repo',
+         'refs/changes/56/123456/1'],), ''),
+      ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
+      ((['git', 'config', 'branch.master.gerritissue', '123456'],), ''),
+      ((['git', 'config', 'branch.master.gerritserver',
+        'https://chromium-review.googlesource.com'],), ''),
+      ((['git', 'config', 'branch.master.gerritpatchset', '1'],), ''),
+    ]
+    self.assertEqual(git_cl.main(
+      ['patch', 'https://chromium-review.googlesource.com/#/c/123456/1']), 0)
+
+  def test_gerrit_patch_conflict(self):
+    self._patch_common(is_gerrit=True)
+    self.mock(git_cl, 'DieWithError',
+              lambda msg: self._mocked_call(['DieWithError', msg]))
+    class SystemExitMock(Exception):
+      pass
+    self.calls += [
+      ((['git', 'fetch', 'https://chromium.googlesource.com/my/repo',
+         'refs/changes/56/123456/1'],), ''),
+      ((['git', 'cherry-pick', 'FETCH_HEAD'],),
+        '', subprocess2.CalledProcessError(1, '', '', '', '')),
+      ((['DieWithError', 'git cherry-pick FETCH_HEAD" failed.\n'],),
+        '', SystemExitMock()),
+    ]
+    with self.assertRaises(SystemExitMock):
+      git_cl.main(['patch',
+                   'https://chromium-review.googlesource.com/#/c/123456/1'])
+
+
 if __name__ == '__main__':
   git_cl.logging.basicConfig(
       level=git_cl.logging.DEBUG if '-v' in sys.argv else git_cl.logging.ERROR)
   unittest.main()