Allow to skip EnsureAuthenticated for Gerrit.

Adding this line to codereview.settings should do the trick:
  GERRIT_SKIP_ENSURE_AUTHENTICATED: true

And I also cleaned up tests so they don't spill too much to stdout.

BUG=603378
R=machenbach@chromium.org,andybons@chromium.org
CC=shinyak@chromium.org

Review URL: https://codereview.chromium.org/1884173003

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

git_cl.py

@@ -589,6 +589,7 @@ class Settings(object):
     self.updated = False
     self.is_gerrit = None
     self.squash_gerrit_uploads = None
+    self.gerrit_skip_ensure_authenticated = None
     self.git_editor = None
     self.project = None
     self.force_https_commit_url = None
@@ -771,6 +772,15 @@ class Settings(object):
                  error_ok=True).strip() == 'true')
     return self.squash_gerrit_uploads
 
+  def GetGerritSkipEnsureAuthenticated(self):
+    """Return True if EnsureAuthenticated should not be done for Gerrit
+    uploads."""
+    if self.gerrit_skip_ensure_authenticated is None:
+      self.gerrit_skip_ensure_authenticated = (
+          RunGit(['config', '--bool', 'gerrit.skip_ensure_authenticated'],
+                 error_ok=True).strip() == 'true')
+    return self.gerrit_skip_ensure_authenticated
+
   def GetGitEditor(self):
     """Return the editor specified in the git config, or None if none is."""
     if self.git_editor is None:
@@ -2022,6 +2032,10 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
 
   def EnsureAuthenticated(self, force):
     """Best effort check that user is authenticated with Gerrit server."""
+    if settings.GetGerritSkipEnsureAuthenticated():
+      # For projects with unusual authentication schemes.
+      # See http://crbug.com/603378.
+      return
     # Lazy-loader to identify Gerrit and Git hosts.
     if gerrit_util.GceAuthenticator.is_gce():
       return
@@ -2717,6 +2731,10 @@ def LoadCodereviewSettingsFromFile(fileobj):
     RunGit(['config', 'gerrit.squash-uploads',
             keyvals['GERRIT_SQUASH_UPLOADS']])
 
+  if 'GERRIT_SKIP_ENSURE_AUTHENTICATED' in keyvals:
+    RunGit(['config', 'gerrit.skip_ensure_authenticated',
+            keyvals['GERRIT_SKIP_ENSURE_AUTHENTICATED']])
+
   if 'PUSH_URL_CONFIG' in keyvals and 'ORIGIN_URL_CONFIG' in keyvals:
     #should be of the form
     #PUSH_URL_CONFIG: url.ssh://gitrw.chromium.org.pushinsteadof

tests/git_cl_test.py

@@ -527,6 +527,7 @@ class TestGitCl(TestCase):
       reviewers,
       private=False):
     """Generic reviewer test framework."""
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     try:
       similarity = upload_args[upload_args.index('--similarity')+1]
     except ValueError:
@@ -660,6 +661,7 @@ class TestGitCl(TestCase):
           'Must specify reviewers to send email.\n', stderr.getvalue())
 
   def test_dcommit(self):
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     self.calls = (
         self._dcommit_calls_1() +
         self._git_sanity_checks('fake_ancestor_sha', 'working') +
@@ -668,6 +670,7 @@ class TestGitCl(TestCase):
     git_cl.main(['dcommit'])
 
   def test_dcommit_bypass_hooks(self):
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     self.calls = (
         self._dcommit_calls_1() +
         self._dcommit_calls_bypassed() +
@@ -676,8 +679,12 @@ class TestGitCl(TestCase):
 
 
   @classmethod
-  def _gerrit_ensure_auth_calls(cls, issue=None):
-    calls = []
+  def _gerrit_ensure_auth_calls(cls, issue=None, skip_auth_check=False):
+    cmd = ['git', 'config', '--bool', 'gerrit.skip_ensure_authenticated']
+    if skip_auth_check:
+      return [((cmd, ), 'true')]
+
+    calls = [((cmd, ), '', subprocess2.CalledProcessError(1, '', '', '', ''))]
     if issue:
       calls.extend([
           ((['git', 'config', 'branch.master.gerritserver'],), ''),
@@ -849,6 +856,7 @@ class TestGitCl(TestCase):
       issue=None):
     """Generic gerrit upload test framework."""
     reviewers = reviewers or []
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     self.mock(git_cl.gerrit_util, "CookiesAuthenticator",
               CookiesAuthenticatorMockFactory(same_cookie='same_cred'))
     self.calls = self._gerrit_base_calls(issue=issue)
@@ -926,6 +934,7 @@ class TestGitCl(TestCase):
         issue=123456)
 
   def test_upload_branch_deps(self):
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     def mock_run_git(*args, **_kwargs):
       if args[0] == ['for-each-ref',
                        '--format=%(refname:short) %(upstream:short)',
@@ -1094,6 +1103,7 @@ class TestGitCl(TestCase):
     self.assertNotEqual(git_cl.main(['diff']), 0)
 
   def _patch_common(self, is_gerrit=False, force_codereview=False):
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset',
               lambda x: '60001')
     self.mock(git_cl._RietveldChangelistImpl, 'GetPatchSetDiff',
@@ -1265,11 +1275,13 @@ class TestGitCl(TestCase):
 
   def test_checkout_not_found(self):
     """Tests git cl checkout <issue>."""
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     self.calls = self._checkout_calls()
     self.assertEqual(1, git_cl.main(['checkout', '99999']))
 
   def test_checkout_no_branch_issues(self):
     """Tests git cl checkout <issue>."""
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     self.calls = [
         ((['git', 'config', '--local', '--get-regexp',
            'branch\\..*\\.rietveldissue'], ), '',
@@ -1281,17 +1293,18 @@ class TestGitCl(TestCase):
     ]
     self.assertEqual(1, git_cl.main(['checkout', '99999']))
 
-  def _test_gerrit_ensure_authenticated_common(self, auth):
+  def _test_gerrit_ensure_authenticated_common(self, auth,
+                                               skip_auth_check=False):
     self.mock(git_cl.gerrit_util, 'CookiesAuthenticator',
               CookiesAuthenticatorMockFactory(hosts_with_creds=auth))
     self.mock(git_cl, 'DieWithError',
               lambda msg: self._mocked_call(['DieWithError', msg]))
     self.mock(git_cl, 'ask_for_data',
               lambda msg: self._mocked_call(['ask_for_data', msg]))
-    self.calls = [
-        ((['git', 'symbolic-ref', 'HEAD'],), 'master')
-    ] + self._gerrit_ensure_auth_calls()
+    self.calls = self._gerrit_ensure_auth_calls(skip_auth_check=skip_auth_check)
     cl = git_cl.Changelist(codereview='gerrit')
+    cl.branch = 'master'
+    cl.branchref = 'refs/heads/master'
     cl.lookedup_issue = True
     return cl
 
@@ -1309,6 +1322,7 @@ class TestGitCl(TestCase):
     self.assertIsNone(cl.EnsureAuthenticated(force=False))
 
   def test_gerrit_ensure_authenticated_conflict(self):
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     cl = self._test_gerrit_ensure_authenticated_common(auth={
       'chromium.googlesource.com': 'one',
       'chromium-review.googlesource.com': 'other',
@@ -1325,6 +1339,11 @@ class TestGitCl(TestCase):
     })
     self.assertIsNone(cl.EnsureAuthenticated(force=False))
 
+  def test_gerrit_ensure_authenticated_skipped(self):
+    cl = self._test_gerrit_ensure_authenticated_common(
+        auth={}, skip_auth_check=True)
+    self.assertIsNone(cl.EnsureAuthenticated(force=False))
+
   def test_cmd_set_commit_rietveld(self):
     self.mock(git_cl._RietveldChangelistImpl, 'SetFlag',
               lambda _, f, v: self._mocked_call(['SetFlag', f, v]))