Switch ChangeDescription usage to be stricter.

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 12 matching lines @@
   def __init__(self, *args, **kwargs):
     self.reviewers = []
   @staticmethod
   def should_continue():
     return True
 
 
 class RietveldMock(object):
   def __init__(self, *args, **kwargs):
     pass
+
   @staticmethod
   def get_description(issue):
     return 'Issue: %d' % issue
 
+  @staticmethod
+  def get_issue_properties(_issue, _messages):
+    return {
+      'reviewers': ['joe@chromium.org', 'john@chromium.org'],
+      'messages': [
+        {
+          'approval': True,
+          'sender': 'john@chromium.org',
+        },
+      ],
+    }
+
 
 class WatchlistsMock(object):
   def __init__(self, _):
     pass
   @staticmethod
   def GetWatchersForPaths(_):
     return ['joe@example.com']
 
 
 class CodereviewSettingsFileMock(object):
@@ skipping 217 matching lines @@
       ((['git', 'show-ref', '--quiet', '--verify',
          'refs/heads/git-cl-commit'],),
        (('', None), 0)),
       ((['git', 'branch', '-D', 'git-cl-commit'],), ''),
       ((['git', 'show-ref', '--quiet', '--verify',
          'refs/heads/git-cl-cherry-pick'],), ''),
       ((['git', 'rev-parse', '--show-cdup'],), '\n'),
       ((['git', 'checkout', '-q', '-b', 'git-cl-commit'],), ''),
       ((['git', 'reset', '--soft', 'fake_ancestor_sha'],), ''),
       ((['git', 'commit', '-m',
-         'Issue: 12345\n\nReview URL: https://codereview.example.com/12345'],),
+         'Issue: 12345\n\n'
+         'Review URL: https://codereview.example.com/12345'],),
        ''),
       ((['git', 'svn', 'dcommit', '-C50', '--no-rebase', '--rmdir'],),
        (('', None), 0)),
       ((['git', 'checkout', '-q', 'working'],), ''),
       ((['git', 'branch', '-D', 'git-cl-commit'],), ''),
   ]
 
   @staticmethod
   def _cmd_line(description, args, similarity, find_copies):
     """Returns the upload command line passed to upload.RealMain()."""
@@ skipping 42 matching lines @@
       cmd_line = self._cmd_line(final_description, reviewers, similarity,
                                 find_copies)
       self.assertEquals(cmd_line, args)
       return 1, 2
     self.mock(git_cl.upload, 'RealMain', check_upload)
     git_cl.main(['upload'] + upload_args)
 
   def test_no_reviewer(self):
     self._run_reviewer_test(
         [],
-        'desc\n\nBUG=\n',
-        '# Blah blah comment.\ndesc\n\nBUG=\n',
-        'desc\n\nBUG=\n',
+        'desc\n\nBUG=',
+        '# Blah blah comment.\ndesc\n\nBUG=',
+        'desc\n\nBUG=',
         [])
 
   def test_keep_similarity(self):
     self._run_reviewer_test(
         ['--similarity', '70'],
-        'desc\n\nBUG=\n',
-        '# Blah blah comment.\ndesc\n\nBUG=\n',
-        'desc\n\nBUG=\n',
+        'desc\n\nBUG=',
+        '# Blah blah comment.\ndesc\n\nBUG=',
+        'desc\n\nBUG=',
         [])
 
   def test_keep_find_copies(self):
     self._run_reviewer_test(
         ['--no-find-copies'],
-        'desc\n\nBUG=\n',
+        'desc\n\nBUG=',
         '# Blah blah comment.\ndesc\n\nBUG=\n',
-        'desc\n\nBUG=\n',
+        'desc\n\nBUG=',
         [])
 
   def test_reviewers_cmd_line(self):
     # Reviewer is passed as-is
-    description = 'desc\n\nR=foo@example.com\nBUG=\n'
+    description = 'desc\n\nR=foo@example.com\nBUG='
     self._run_reviewer_test(
         ['-r' 'foo@example.com'],
         description,
         '\n%s\n' % description,
         description,
-        ['--reviewers', 'foo@example.com'])
+        ['--reviewers=foo@example.com'])
 
   def test_reviewer_tbr_overriden(self):
     # Reviewer is overriden with TBR
     # Also verifies the regexp work without a trailing LF
-    description = 'Foo Bar\nTBR=reviewer@example.com\n'
+    description = 'Foo Bar\n\nTBR=reviewer@example.com'
     self._run_reviewer_test(
         ['-r' 'foo@example.com'],
-        'desc\n\nR=foo@example.com\nBUG=\n',
+        'desc\n\nR=foo@example.com\nBUG=',
         description.strip('\n'),
         description,
-        ['--reviewers', 'reviewer@example.com'])
+        ['--reviewers=reviewer@example.com'])
 
   def test_reviewer_multiple(self):
     # Handles multiple R= or TBR= lines.
     description = (
-        'Foo Bar\nTBR=reviewer@example.com\nBUG=\nR=another@example.com\n')
+        'Foo Bar\nTBR=reviewer@example.com\nBUG=\nR=another@example.com')
     self._run_reviewer_test(
         [],
-        'desc\n\nBUG=\n',
+        'desc\n\nBUG=',
         description,
         description,
-        ['--reviewers', 'reviewer@example.com,another@example.com'])
+        ['--reviewers=another@example.com,reviewer@example.com'])
 
   def test_reviewer_send_mail(self):
     # --send-mail can be used without -r if R= is used
-    description = 'Foo Bar\nR=reviewer@example.com\n'
+    description = 'Foo Bar\nR=reviewer@example.com'
     self._run_reviewer_test(
         ['--send-mail'],
-        'desc\n\nBUG=\n',
+        'desc\n\nBUG=',
         description.strip('\n'),
         description,
-        ['--reviewers', 'reviewer@example.com', '--send_mail'])
+        ['--reviewers=reviewer@example.com', '--send_mail'])
 
   def test_reviewer_send_mail_no_rev(self):
     # Fails without a reviewer.
     class FileMock(object):
       buf = StringIO.StringIO()
       def write(self, content):
         self.buf.write(content)
 
     mock = FileMock()
     try:
@@ skipping 74 matching lines @@
              'fake_ancestor_sha..HEAD'],),
            description)
           ]
     calls += [
         ((['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 reviewers])
+      receive_pack += ' '.join(
+          '--reviewer=' + email for email in sorted(reviewers))
     receive_pack += ''
     calls += [
         ((['git', 'push', receive_pack, 'origin', 'HEAD:refs/for/master'],),
          '')
         ]
     return calls
 
   def _run_gerrit_upload_test(
       self,
       upload_args,
@@ skipping 79 matching lines @@
         (('CC list:',), ''),
         ((['git', 'config', 'rietveld.tree-status-url'],), ''),
         (('Tree status URL:',), ''),
         ((['git', 'config', 'rietveld.viewvc-url'],), ''),
         (('ViewVC URL:',), ''),
         # DownloadHooks(True)
         ((commit_msg_path, os.X_OK,), True),
         ]
     git_cl.main(['config'])
 
+  def test_update_reviewers(self):
+    data = [
+      ('foo', [], 'foo'),
+      ('foo', ['a@c'], 'foo\n\nR=a@c'),
+      ('foo\nBUG=', ['a@c'], 'foo\nBUG=\nR=a@c'),
+      ('foo\nR=xx\nTBR=yy\nR=bar', ['a@c'], 'foo\nTBR=a@c'),
+      ('foo', ['a@c', 'b@c'], 'foo\n\nR=a@c, b@c'),
+    ]
+    for orig, reviewers, expected in data:
+      obj = git_cl.ChangeDescription(orig)
+      obj.update_reviewers(reviewers)
+      self.assertEqual(expected, obj.description)
+
 
 if __name__ == '__main__':
+  git_cl.logging.basicConfig(
+      level=git_cl.logging.DEBUG if '-v' in sys.argv else git_cl.logging.ERROR)
   unittest.main()