Gerrit git cl upload: warn and offer to remove Gerrit commit-msg hook.

tests/git_cl_test.py

Index: tests/git_cl_test.py
diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index 42ffba4f5eaf4ca8711384386a601637f298114f..e70cd24a379a5108f39302deecbb35fc35d8c55b 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -878,8 +878,10 @@ class TestGitCl(TestCase):
     """Generic gerrit upload test framework."""
     reviewers = reviewers or []
     self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
-    self.mock(git_cl.gerrit_util, "CookiesAuthenticator",
+    self.mock(git_cl.gerrit_util, 'CookiesAuthenticator',
               CookiesAuthenticatorMockFactory(same_cookie='same_cred'))
+    self.mock(git_cl._GerritChangelistImpl, '_GerritCommitMsgHookCheck',
+              lambda _, offer_removal: '')

[Sergiy Byelozyorov, 2016/06/07 16:33:21]

not that it matters, but why return '' and not None?

[tandrii(chromium), 2016/06/07 16:40:51]

Typo, though '' is shorter than None.

[Sergiy Byelozyorov, 2016/06/07 17:29:11]

Acknowledged.

     self.calls = self._gerrit_base_calls(issue=issue)
     self.calls += self._gerrit_upload_calls(
         description, reviewers, squash,
@@ -1524,6 +1526,50 @@ class TestGitCl(TestCase):
     ]
     self.assertEqual(0, git_cl.main(['issue', '0']))
 
+  def _common_GerritCommitMsgHookCheck(self):
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
+    self.mock(git_cl.os.path, 'abspath',
+              lambda path: self._mocked_call(['abspath', path]))

[Sergiy Byelozyorov, 2016/06/07 16:33:21]

where is path defined? also didn't quite figure out how self._mocked_call works
here... but it's used like that in other places, so i guess it's fine

[tandrii(chromium), 2016/06/07 16:40:51]

it basically pops next item from self.calls, ensures that func args match what
has been popped, and returns whatever popped items contains as result.

[Sergiy Byelozyorov, 2016/06/07 17:29:11]

Oh. Nevermind about path. It's lambda argument.
Ack.

+    self.mock(git_cl.os.path, 'exists',
+              lambda path: self._mocked_call(['exists', path]))
+    self.mock(git_cl.gclient_utils, 'FileRead',
+              lambda path: self._mocked_call(['FileRead', path]))
+    self.mock(git_cl.gclient_utils, 'rm_file_or_tree',
+              lambda path: self._mocked_call(['rm_file_or_tree', path]))
+    self.calls = [
+        ((['git', 'rev-parse', '--show-cdup'],), '../'),
+        ((['abspath', '../'],), '/abs/git_repo_root'),
+    ]
+    return git_cl.Changelist(codereview='gerrit', issue=123)
+
+  def test_GerritCommitMsgHookCheck_custom_hook(self):
+    cl = self._common_GerritCommitMsgHookCheck()
+    self.calls += [
+        ((['exists', '/abs/git_repo_root/.git/hooks/commit-msg'],), True),
+        ((['FileRead', '/abs/git_repo_root/.git/hooks/commit-msg'],),
+         '#!/bin/sh\necho "custom hook"')
+    ]
+    cl._codereview_impl._GerritCommitMsgHookCheck(offer_removal=True)
+
+  def test_GerritCommitMsgHookCheck_not_exists(self):
+    cl = self._common_GerritCommitMsgHookCheck()
+    self.calls += [
+        ((['exists', '/abs/git_repo_root/.git/hooks/commit-msg'],), False),
+    ]
+    cl._codereview_impl._GerritCommitMsgHookCheck(offer_removal=True)
+
+  def test_GerritCommitMsgHookCheck(self):
+    cl = self._common_GerritCommitMsgHookCheck()
+    self.calls += [
+        ((['exists', '/abs/git_repo_root/.git/hooks/commit-msg'],), True),
+        ((['FileRead', '/abs/git_repo_root/.git/hooks/commit-msg'],),
+         '...\n# From Gerrit Code Review\n...\nadd_ChangeId()\n'),
+        (('Do you want to remove it now? [Yes/No]',), 'Yes'),

[Sergiy Byelozyorov, 2016/06/07 16:33:22]

is there a function name missing here? ask_for_data?

[tandrii(chromium), 2016/06/07 16:40:51]

yeah, that's how ask_for_data is mocked for this whole test class.

[Sergiy Byelozyorov, 2016/06/07 17:29:11]

Highly intuitive... :-)

+        ((['rm_file_or_tree', '/abs/git_repo_root/.git/hooks/commit-msg'],),
+         ''),
+    ]
+    cl._codereview_impl._GerritCommitMsgHookCheck(offer_removal=True)
+
 
 if __name__ == '__main__':
   git_cl.logging.basicConfig(