Add OutputApi.AppendCC to allow presubmit checks to add CCs

Bug: 786386
Change-Id: If29acf287355d150e0f89f45b2f7aa87a08dcc57
Reviewed-on: https://chromium-review.googlesource.com/776645
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>

git_cl.py

@@ -1155,7 +1155,7 @@ class Changelist(object):
     self.lookedup_patchset = False
     self.patchset = None
     self.cc = None
-    self.watchers = ()
+    self.more_cc = []
     self._remote = None
 
     self._codereview_impl = None
@@ -1203,21 +1203,19 @@ class Changelist(object):
     """
     if self.cc is None:
       base_cc = settings.GetDefaultCCList()
-      more_cc = ','.join(self.watchers)
+      more_cc = ','.join(self.more_cc)
       self.cc = ','.join(filter(None, (base_cc, more_cc))) or ''
     return self.cc
 
   def GetCCListWithoutDefault(self):
     """Return the users cc'd on this CL excluding default ones."""
     if self.cc is None:
-      self.cc = ','.join(self.watchers)
+      self.cc = ','.join(self.more_cc)
     return self.cc
 
-  def SetWatchers(self, watchers):
+  def ExtendCC(self, more_cc):
-    """Sets the list of email addresses that should be cc'd based on the changed
+    """Extends the list of users to cc on this CL based on the changed files."""
-    files in this CL.
+    self.more_cc.extend(more_cc)
-    """
-    self.watchers = watchers
 
   def GetBranch(self):
     """Returns the short branch name, e.g. 'master'."""
@@ -1627,7 +1625,7 @@ class Changelist(object):
     watchlist = watchlists.Watchlists(change.RepositoryRoot())
     files = [f.LocalPath() for f in change.AffectedFiles()]
     if not options.bypass_watchlists:
-      self.SetWatchers(watchlist.GetWatchersForPaths(files))
+      self.ExtendCC(watchlist.GetWatchersForPaths(files))
 
     if not options.bypass_hooks:
       if options.reviewers or options.tbrs or options.add_owners_to:
@@ -1646,6 +1644,7 @@ class Changelist(object):
         return 1
       if not options.reviewers and hook_results.reviewers:
         options.reviewers = hook_results.reviewers.split(',')
+      self.ExtendCC(hook_results.more_cc)
 
     # TODO(tandrii): Checking local patchset against remote patchset is only
     # supported for Rietveld. Extend it to Gerrit or remove it completely.

presubmit_support.py

@@ -94,6 +94,7 @@ class PresubmitOutput(object):
     self.input_stream = input_stream
     self.output_stream = output_stream
     self.reviewers = []
+    self.more_cc = []
     self.written_output = []
     self.error_count = 0
 
@@ -275,6 +276,11 @@ class OutputApi(object):
 
   def __init__(self, is_committing):
     self.is_committing = is_committing
+    self.more_cc = []
+
+  def AppendCC(self, cc):
+    """Appends a user to cc for this change."""
+    self.more_cc.append(cc)
 
   def PresubmitPromptOrNotify(self, *args, **kwargs):
     """Warn the user when uploading, but only notify if committing."""
@@ -1271,6 +1277,7 @@ class PresubmitExecuter(object):
     self.gerrit = gerrit_obj
     self.verbose = verbose
     self.dry_run = dry_run
+    self.more_cc = []
 
   def ExecPresubmitScript(self, script_text, presubmit_path):
     """Executes a single presubmit script.
@@ -1292,6 +1299,7 @@ class PresubmitExecuter(object):
     input_api = InputApi(self.change, presubmit_path, self.committing,
                          self.rietveld, self.verbose,
                          gerrit_obj=self.gerrit, dry_run=self.dry_run)
+    output_api = OutputApi(self.committing)
     context = {}
     try:
       exec script_text in context
@@ -1306,10 +1314,11 @@ class PresubmitExecuter(object):
       function_name = 'CheckChangeOnUpload'
     if function_name in context:
       try:
-        context['__args'] = (input_api, OutputApi(self.committing))
+        context['__args'] = (input_api, output_api)
         logging.debug('Running %s in %s', function_name, presubmit_path)
         result = eval(function_name + '(*__args)', context)
         logging.debug('Running %s done.', function_name)
+        self.more_cc = output_api.more_cc
       finally:
         map(os.remove, input_api._named_temporary_files)
       if not (isinstance(result, types.TupleType) or
@@ -1402,6 +1411,7 @@ def DoPresubmitChecks(change,
       presubmit_script = gclient_utils.FileRead(filename, 'rU')
       results += executer.ExecPresubmitScript(presubmit_script, filename)
 
+    output.more_cc.extend(executer.more_cc)
     errors = []
     notifications = []
     warnings = []

tests/git_cl_test.py

@@ -71,6 +71,8 @@ class ChangelistMock(object):
 class PresubmitMock(object):
   def __init__(self, *args, **kwargs):
     self.reviewers = []
+    self.more_cc = ['chromium-reviews+test-more-cc@chromium.org']
+
   @staticmethod
   def should_continue():
     return True
@@ -889,16 +891,17 @@ class TestGitCl(TestCase):
   def _cmd_line(description, args, similarity, find_copies, private, cc):
     """Returns the upload command line passed to upload.RealMain()."""
     return [
-        'upload', '--assume_yes', '--server',
+        'upload', '--assume_yes', '--server', 'https://codereview.example.com',
-        'https://codereview.example.com',
         '--message', description
     ] + args + [
-        '--cc', ','.join(['joe@example.com'] + cc),
+        '--cc',
-    ] + (['--private'] if private else []) + [
+        ','.join(
-        '--git_similarity', similarity or '50'
+            ['joe@example.com', 'chromium-reviews+test-more-cc@chromium.org'] +
-    ] + (['--git_no_find_copies'] if find_copies is False else []) + [
+            cc),
-        'fake_ancestor_sha', 'HEAD'
+    ] + (['--private']
-    ]
+         if private else []) + ['--git_similarity', similarity or '50'] + (
+             ['--git_no_find_copies']
+             if find_copies is False else []) + ['fake_ancestor_sha', 'HEAD']
 
   def _run_reviewer_test(
       self,
@@ -1627,9 +1630,10 @@ class TestGitCl(TestCase):
       ]
     calls += [
         ((['git', 'config', 'rietveld.cc'],), ''),
-        (('AddReviewers', 'chromium-review.googlesource.com',
+        (('AddReviewers', 'chromium-review.googlesource.com', 123456
-           123456 if squash else None, sorted(reviewers),
+          if squash else None, sorted(reviewers),
-          ['joe@example.com'] + cc, notify), ''),
+          ['joe@example.com', 'chromium-reviews+test-more-cc@chromium.org'] +
+          cc, notify), ''),
     ]
     if tbr:
       calls += [

tests/presubmit_unittest.py

@@ -1403,9 +1403,16 @@ class OutputApiUnittest(PresubmitTestsBase):
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
-      'MailTextResult', 'PresubmitError', 'PresubmitNotifyResult',
+        'AppendCC',
-      'PresubmitPromptWarning', 'PresubmitPromptOrNotify', 'PresubmitResult',
+        'MailTextResult',
-      'is_committing', 'EnsureCQIncludeTrybotsAreAdded',
+        'PresubmitError',
+        'PresubmitNotifyResult',
+        'PresubmitPromptWarning',
+        'PresubmitPromptOrNotify',
+        'PresubmitResult',
+        'is_committing',
+        'more_cc',
+        'EnsureCQIncludeTrybotsAreAdded',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(presubmit.OutputApi(False), members)
@@ -1424,6 +1431,12 @@ class OutputApiUnittest(PresubmitTestsBase):
 
     # TODO(joi) Test MailTextResult once implemented.
 
+  def testAppendCC(self):
+    output_api = presubmit.OutputApi(False)
+    output_api.AppendCC('chromium-reviews@chromium.org')
+    self.assertEquals(['chromium-reviews@chromium.org'], output_api.more_cc)
+
+
   def testOutputApiHandling(self):
     self.mox.ReplayAll()