Make git cl upload correctly track upstream refs when uploading to Gerrit

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
@@ skipping 557 matching lines @@
            'log', '--pretty=format:%s%n%n%b', 'fake_ancestor_sha...'],),
          'foo'),
         ((['git', 'config', 'user.email'],), 'me@example.com'),
         ((['git',
            'diff', '--no-ext-diff', '--stat', '--find-copies-harder',
            '-l100000', '-C50', 'fake_ancestor_sha', 'HEAD'],),
          '+dat'),
         ]
 
   @staticmethod
-  def _gerrit_upload_calls(description, reviewers, squash):
+  def _gerrit_upload_calls(description, reviewers, squash,
+                           expected_upstream_ref='origin/refs/heads/master'):

[luqui, 2015/04/28 23:56:24]

Yes yes this is awful.  My alternatives:

1. Dig into git_common and get it not to shorten this under some conditions.
2. Manually shorten the upstream ref under some conditions
3. Remove upload --squash coverage
4. Refactor tests to allow "equivalence classes" of tests

4 is the best but way too much work for this.  I worry about evil corner cases
of 1 and 2. 

[Mike Wittman, 2015/05/01 01:36:29]

I went the same way attempting to get the GetTargetRef functionality tested in
the style of these tests before giving up under the complexity of managing the
various equivalent branch names across all the various git calls.

This is much more limited in scope though and doesn't look too horrible to me.

     calls = [
         ((['git', 'log', '--pretty=format:%s\n\n%b',
            'fake_ancestor_sha..HEAD'],),
          description)
         ]
     if git_cl.CHANGE_ID not in description:
       calls += [
           ((['git', 'log', '--pretty=format:%s\n\n%b',
              'fake_ancestor_sha..HEAD'],),
            description),
@@ skipping 18 matching lines @@
           ((['git', 'rev-parse', 'HEAD:'],),
            '0123456789abcdef'),
           ((['git', 'commit-tree', '0123456789abcdef', '-p',
              'origin/master', '-m', 'd'],),
            ref_to_push),
           ]
     else:
       ref_to_push = 'HEAD'
 
     calls += [
-        ((['git', 'rev-list', 'origin/master..' + ref_to_push],), ''),
+        ((['git', 'rev-list', expected_upstream_ref + '..' + ref_to_push],), ''),
         ((['git', 'config', 'rietveld.cc'],), '')
         ]
     receive_pack = '--receive-pack=git receive-pack '
     receive_pack += '--cc=joe@example.com'  # from watch list
     if reviewers:
       receive_pack += ' '
       receive_pack += ' '.join(
           '--reviewer=' + email for email in sorted(reviewers))
     receive_pack += ''
     calls += [
         ((['git',
-           'push', receive_pack, 'origin', ref_to_push + ':refs/for/master'],),
+           'push', receive_pack, 'origin',
+           ref_to_push + ':refs/for/refs/heads/master'],),
          '')
         ]
     if squash:
       calls += [
           ((['git', 'rev-parse', 'HEAD'],), 'abcdef0123456789'),
           ((['git', 'update-ref', '-m', 'Uploaded abcdef0123456789',
             'refs/heads/git_cl_uploads/master', 'abcdef0123456789'],),
            '')
           ]
 
     return calls
 
   def _run_gerrit_upload_test(
       self,
       upload_args,
       description,
       reviewers,
-      squash=False):
+      squash=False,
+      expected_upstream_ref='origin/refs/heads/master'):
     """Generic gerrit upload test framework."""
     self.calls = self._gerrit_base_calls()
-    self.calls += self._gerrit_upload_calls(description, reviewers, squash)
+    self.calls += self._gerrit_upload_calls(
+        description, reviewers, squash,
+        expected_upstream_ref=expected_upstream_ref)
     git_cl.main(['upload'] + upload_args)
 
   def test_gerrit_upload_without_change_id(self):
     self._run_gerrit_upload_test(
         [],
         'desc\n\nBUG=\n',
         [])
 
   def test_gerrit_no_reviewer(self):
     self._run_gerrit_upload_test(
@@ skipping 12 matching lines @@
         [],
         'desc\nTBR=reviewer@example.com\nBUG=\nR=another@example.com\n'
         'Change-Id:123456789\n',
         ['reviewer@example.com', 'another@example.com'])
 
   def test_gerrit_upload_squash(self):
     self._run_gerrit_upload_test(
         ['--squash'],
         'desc\n\nBUG=\nChange-Id:123456789\n',
         [],
-        squash=True)
+        squash=True,
+        expected_upstream_ref='origin/master')
 
   def test_config_gerrit_download_hook(self):
     self.mock(git_cl, 'FindCodereviewSettingsFile', CodereviewSettingsFileMock)
     def ParseCodereviewSettingsContent(content):
       keyvals = {}
       keyvals['CODE_REVIEW_SERVER'] = 'gerrit.chromium.org'
       keyvals['GERRIT_HOST'] = 'gerrit.chromium.org'
       keyvals['GERRIT_PORT'] = '29418'
       return keyvals
     self.mock(git_cl.gclient_utils, 'ParseCodereviewSettingsContent',
@@ skipping 156 matching lines @@
     # Check target refs for pending prefix.
     self.assertEqual('prefix/heads/master',
                      git_cl.GetTargetRef('origin', 'refs/remotes/origin/master',
                                          None, 'prefix/'))
 
 
 if __name__ == '__main__':
   git_cl.logging.basicConfig(
       level=git_cl.logging.DEBUG if '-v' in sys.argv else git_cl.logging.ERROR)
   unittest.main()