The parsing of change descriptions had a lot of overlap and inconsistencies between gcl and git-cl. In particular, we weren't handling TBR= consistently, or probably a few other things.

This change moves most of the code into presubmit_support and gclient_utils and just leaves the formatting differences for the messages between the two tools.

Review URL: http://codereview.chromium.org/6719004

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

gcl.py

@@ -290,9 +290,7 @@ class ChangeInfo(object):
     self.name = name
     self.issue = int(issue)
     self.patchset = int(patchset)
-    self._description = None
-    self._subject = None
-    self._reviewers = None
+    self._change_desc = None
     self._set_description(description)
     if files is None:
       files = []
@@ -306,42 +304,21 @@ class ChangeInfo(object):
       self.rietveld = GetCodeReviewSetting('CODE_REVIEW_SERVER')
 
   def _get_description(self):
-    return self._description
+    return self._change_desc.description
 
   def _set_description(self, description):
-    # TODO(dpranke): Cloned from git_cl.py. These should be shared.
-    if not description:
-      self._description = description
-      return
-
-    parsed_lines = []
-    reviewers_re = re.compile(REVIEWERS_REGEX)
-    reviewers = ''
-    subject = ''
-    for l in description.splitlines():
-      if not subject:
-        subject = l
-      matched_reviewers = reviewers_re.match(l)
-      if matched_reviewers:
-        reviewers = matched_reviewers.group(1).split(',')
-      parsed_lines.append(l)
-
-    if len(subject) > 100:
-      subject = subject[:97] + '...'
-
-    self._subject = subject
-    self._reviewers = reviewers
-    self._description = '\n'.join(parsed_lines)
+    self._change_desc = presubmit_support.ChangeDescription(
+        description=description)
 
   description = property(_get_description, _set_description)
 
   @property
   def reviewers(self):
-    return self._reviewers
+    return self._change_desc.reviewers
 
   @property
   def subject(self):
-    return self._subject
+    return self._change_desc.subject
 
   def NeedsUpload(self):
     return self.needs_upload
@@ -378,7 +355,7 @@ class ChangeInfo(object):
           'patchset': self.patchset,
           'needs_upload': self.NeedsUpload(),
           'files': self.GetFiles(),
-          'description': self.description,
+          'description': self._change_desc.description,
           'rietveld': self.rietveld,
         }, sort_keys=True, indent=2)
     gclient_utils.FileWrite(GetChangelistInfoFile(self.name), data)
@@ -739,20 +716,6 @@ def ListFiles(show_unknown_files):
   return 0
 
 
-def GetEditor():
-  editor = os.environ.get("SVN_EDITOR")
-  if not editor:
-    editor = os.environ.get("EDITOR")
-
-  if not editor:
-    if sys.platform.startswith("win"):
-      editor = "notepad"
-    else:
-      editor = "vi"
-
-  return editor
-
-
 def GenerateDiff(files, root=None):
   return SVN.GenerateDiff(files, root=root)
 
@@ -1098,48 +1061,38 @@ def CMDchange(args):
   affected_files = [x for x in other_files if file_re.match(x[0])]
   unaffected_files = [x for x in other_files if not file_re.match(x[0])]
 
-  if not change_info.reviewers:
+  reviewers = change_info.reviewers
+  if not reviewers:
     files_for_review = affected_files[:]
     files_for_review.extend(change_info.GetFiles())
-    suggested_reviewers = suggest_reviewers(change_info, files_for_review)
-    if suggested_reviewers:
-      reviewers_re = re.compile(REVIEWERS_REGEX)
-      if not any(reviewers_re.match(l) for l in description.splitlines()):
-        description += '\n\nR=' + ','.join(suggested_reviewers)
-
-  description = description.rstrip() + '\n'
+    reviewers = suggest_reviewers(change_info, files_for_review)
 
   separator1 = ("\n---All lines above this line become the description.\n"
                 "---Repository Root: " + change_info.GetLocalRoot() + "\n"
                 "---Paths in this changelist (" + change_info.name + "):\n")
   separator2 = "\n\n---Paths modified but not in any changelist:\n\n"
-  text = (description + separator1 + '\n' +
-          '\n'.join([f[0] + f[1] for f in change_info.GetFiles()]))
+
+  footer = (separator1 + '\n' +
+            '\n'.join([f[0] + f[1] for f in change_info.GetFiles()]))
 
   if change_info.Exists():
-    text += (separator2 +
-            '\n'.join([f[0] + f[1] for f in affected_files]) + '\n')
+    footer += (separator2 +
+              '\n'.join([f[0] + f[1] for f in affected_files]) + '\n')
   else:
-    text += ('\n'.join([f[0] + f[1] for f in affected_files]) + '\n' +
-            separator2)
-  text += '\n'.join([f[0] + f[1] for f in unaffected_files]) + '\n'
+    footer += ('\n'.join([f[0] + f[1] for f in affected_files]) + '\n' +
+              separator2)
+  footer += '\n'.join([f[0] + f[1] for f in unaffected_files]) + '\n'
 
-  handle, filename = tempfile.mkstemp(text=True)
-  os.write(handle, text)
-  os.close(handle)
+  change_desc = presubmit_support.ChangeDescription(description=description,
+      reviewers=reviewers)
 
-  # Open up the default editor in the system to get the CL description.
-  try:
-    if not silent:
-      cmd = '%s %s' % (GetEditor(), filename)
-      if sys.platform == 'win32' and os.environ.get('TERM') == 'msys':
-        # Msysgit requires the usage of 'env' to be present.
-        cmd = 'env ' + cmd
-      # shell=True to allow the shell to handle all forms of quotes in $EDITOR.
-      subprocess.check_call(cmd, shell=True)
-    result = gclient_utils.FileRead(filename, 'r')
-  finally:
-    os.remove(filename)
+  # These next few lines are equivalent to change_desc.UserUpdate(). We
+  # call them individually to avoid passing a lot of state back and forth.
+  original_description = change_desc.description
+
+  result = change_desc.EditableDescription() + footer
+  if not silent:
+    result = change_desc.editor(result)
 
   if not result:
     return 0
@@ -1151,8 +1104,8 @@ def CMDchange(args):
   # Update the CL description if it has changed.
   new_description = split_result[0]
   cl_files_text = split_result[1]
-  if new_description != description or override_description:
-    change_info.description = new_description
+  change_desc.Parse(new_description)
+  if change_desc.description != original_description or override_description:
     change_info.needs_upload = True
 
   new_cl_files = []
@@ -1166,7 +1119,7 @@ def CMDchange(args):
     new_cl_files.append((status, filename))
 
   if (not len(change_info.GetFiles()) and not change_info.issue and
-      not len(new_description) and not new_cl_files):
+      not len(change_desc.description) and not new_cl_files):
     ErrorExit("Empty changelist not saved")
 
   change_info._files = new_cl_files

gclient_utils.py

@@ -12,6 +12,7 @@ import re
 import stat
 import subprocess
 import sys
+import tempfile
 import threading
 import time
 import xml.dom.minidom
@@ -710,3 +711,37 @@ class ExecutionQueue(object):
         work_queue.ready_cond.notifyAll()
       finally:
         work_queue.ready_cond.release()
+
+
+def GetEditor():
+  editor = os.environ.get("SVN_EDITOR")
+  if not editor:
+    editor = os.environ.get("EDITOR")
+
+  if not editor:
+    if sys.platform.startswith("win"):
+      editor = "notepad"
+    else:
+      editor = "vi"
+
+  return editor
+
+
+def UserEdit(text):
+  """Open an editor, edit the text, and return the result."""
+  (file_handle, filename) = tempfile.mkstemp()
+  fileobj = os.fdopen(file_handle, 'w')
+  fileobj.write(text)
+  fileobj.close()
+
+  # Open up the default editor in the system to get the CL description.
+  try:
+    cmd = '%s %s' % (GetEditor(), filename)
+    if sys.platform == 'win32' and os.environ.get('TERM') == 'msys':
+      # Msysgit requires the usage of 'env' to be present.
+      cmd = 'env ' + cmd
+    # shell=True to allow the shell to handle all forms of quotes in $EDITOR.
+    subprocess.check_call(cmd, shell=True)
+    return FileRead(filename, 'r')
+  finally:
+    os.remove(filename)

git_cl/git_cl.py

@@ -9,7 +9,6 @@ import os
 import re
 import subprocess
 import sys
-import tempfile
 import textwrap
 import urlparse
 import urllib2
@@ -21,17 +20,17 @@ except ImportError:
 
 # TODO(dpranke): don't use relative import.
 import upload  # pylint: disable=W0403
-try:
-  # TODO(dpranke): We wrap this in a try block for a limited form of
-  # backwards-compatibility with older versions of git-cl that weren't
-  # dependent on depot_tools. This version should still work outside of
-  # depot_tools as long as --bypass-hooks is used. We should remove this
-  # once this has baked for a while and things seem safe.
-  depot_tools_path = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
-  sys.path.append(depot_tools_path)
-  import breakpad  # pylint: disable=W0611
-except ImportError:
-  pass
+
+# TODO(dpranke): move this file up a directory so we don't need this.
+depot_tools_path = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+sys.path.append(depot_tools_path)
+
+import breakpad  # pylint: disable=W0611
+
+import presubmit_support
+import scm
+import watchlists
+
 
 DEFAULT_SERVER = 'http://codereview.appspot.com'
 POSTUPSTREAM_HOOK_PATTERN = '.git/hooks/post-cl-%s'
@@ -333,6 +332,7 @@ class Changelist(object):
     self.description = None
     self.has_patchset = False
     self.patchset = None
+    self.tbr = False
 
   def GetBranch(self):
     """Returns the short branch name, e.g. 'master'."""
@@ -535,53 +535,6 @@ def GetCodereviewSettingsInteractively():
   # svn-based hackery.
 
 
-class ChangeDescription(object):
-  """Contains a parsed form of the change description."""
-  def __init__(self, subject, log_desc, reviewers):
-    self.subject = subject
-    self.log_desc = log_desc
-    self.reviewers = reviewers
-    self.description = self.log_desc
-
-  def Update(self):
-    initial_text = """# Enter a description of the change.
-# This will displayed on the codereview site.
-# The first line will also be used as the subject of the review.
-"""
-    initial_text += self.description
-    if 'R=' not in self.description and self.reviewers:
-      initial_text += '\nR=' + self.reviewers
-    if 'BUG=' not in self.description:
-      initial_text += '\nBUG='
-    if 'TEST=' not in self.description:
-      initial_text += '\nTEST='
-    self._ParseDescription(UserEditedLog(initial_text))
-
-  def _ParseDescription(self, description):
-    if not description:
-      self.description = description
-      return
-
-    parsed_lines = []
-    reviewers_regexp = re.compile('\s*R=(.+)')
-    reviewers = ''
-    subject = ''
-    for l in description.splitlines():
-      if not subject:
-        subject = l
-      matched_reviewers = reviewers_regexp.match(l)
-      if matched_reviewers:
-        reviewers = matched_reviewers.group(1)
-      parsed_lines.append(l)
-
-    self.description = '\n'.join(parsed_lines) + '\n'
-    self.subject = subject
-    self.reviewers = reviewers
-
-  def IsEmpty(self):
-    return not self.description
-
-
 def FindCodereviewSettingsFile(filename='codereview.settings'):
   """Finds the given file starting in the cwd and going up.
 
@@ -731,36 +684,6 @@ def CreateDescriptionFromLog(args):
   return RunGit(['log', '--pretty=format:%s\n\n%b'] + log_args)
 
 
-def UserEditedLog(starting_text):
-  """Given some starting text, let the user edit it and return the result."""
-  editor = os.getenv('EDITOR', 'vi')
-
-  (file_handle, filename) = tempfile.mkstemp()
-  fileobj = os.fdopen(file_handle, 'w')
-  fileobj.write(starting_text)
-  fileobj.close()
-
-  # Open up the default editor in the system to get the CL description.
-  try:
-    cmd = '%s %s' % (editor, filename)
-    if sys.platform == 'win32' and os.environ.get('TERM') == 'msys':
-      # Msysgit requires the usage of 'env' to be present.
-      cmd = 'env ' + cmd
-    # shell=True to allow the shell to handle all forms of quotes in $EDITOR.
-    subprocess.check_call(cmd, shell=True)
-    fileobj = open(filename)
-    text = fileobj.read()
-    fileobj.close()
-  finally:
-    os.remove(filename)
-
-  if not text:
-    return
-
-  stripcomment_re = re.compile(r'^#.*$', re.MULTILINE)
-  return stripcomment_re.sub('', text).strip()
-
-
 def ConvertToInteger(inputval):
   """Convert a string to integer, but returns either an int or None."""
   try:
@@ -769,12 +692,20 @@ def ConvertToInteger(inputval):
     return None
 
 
+class GitChangeDescription(presubmit_support.ChangeDescription):
+  def UserEdit(self):
+    header = (
+        "# Enter a description of the change.\n"
+        "# This will displayed on the codereview site.\n"
+        "# The first line will also be used as the subject of the review.\n"
+        "\n")
+    edited_text = self.editor(header + self.EditableDescription())
+    stripcomment_re = re.compile(r'^#.*$', re.MULTILINE)
+    self.Parse(stripcomment_re.sub('', edited_text).strip())
+
+
 def RunHook(committing, upstream_branch, rietveld_server, tbr, may_prompt):
   """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
-  import presubmit_support
-  import scm
-  import watchlists
-
   root = RunCommand(['git', 'rev-parse', '--show-cdup']).strip()
   if not root:
     root = '.'
@@ -843,13 +774,13 @@ def CMDpresubmit(parser, args):
   if options.upload:
     print '*** Presubmit checks for UPLOAD would report: ***'
     RunHook(committing=False, upstream_branch=base_branch,
-            rietveld_server=cl.GetRietveldServer(), tbr=False,
+            rietveld_server=cl.GetRietveldServer(), tbr=cl.tbr,
             may_prompt=False)
     return 0
   else:
     print '*** Presubmit checks for DCOMMIT would report: ***'
     RunHook(committing=True, upstream_branch=base_branch,
-            rietveld_server=cl.GetRietveldServer, tbr=False,
+            rietveld_server=cl.GetRietveldServer, tbr=cl.tbr,
             may_prompt=False)
     return 0
 
@@ -893,10 +824,10 @@ def CMDupload(parser, args):
 
   if not options.bypass_hooks and not options.force:
     hook_results = RunHook(committing=False, upstream_branch=base_branch,
-                           rietveld_server=cl.GetRietveldServer(), tbr=False,
+                           rietveld_server=cl.GetRietveldServer(), tbr=cl.tbr,
                            may_prompt=True)
     if not options.reviewers and hook_results.reviewers:
-      options.reviewers = hook_results.reviewers
+      options.reviewers = ','.join(hook_results.reviewers)
 
 
   # --no-ext-diff is broken in some versions of Git, so try to work around
@@ -930,10 +861,10 @@ def CMDupload(parser, args):
            "Adding patch to that issue." % cl.GetIssue())
   else:
     log_desc = CreateDescriptionFromLog(args)
-    change_desc = ChangeDescription(options.message, log_desc,
-                                    options.reviewers)
-    if not options.from_logs:
-      change_desc.Update()
+    change_desc = GitChangeDescription(subject=options.message,
+        description=log_desc, reviewers=options.reviewers, tbr=cl.tbr)
+    if not options.from_logs and (not options.force):
+      change_desc.UserEdit()
 
     if change_desc.IsEmpty():
       print "Description is empty; aborting."
@@ -1044,7 +975,7 @@ def SendUpstream(parser, args, cmd):
 
   if not options.bypass_hooks and not options.force:
     RunHook(committing=True, upstream_branch=base_branch,
-            rietveld_server=cl.GetRietveldServer(), tbr=options.tbr,
+            rietveld_server=cl.GetRietveldServer(), tbr=(cl.tbr or options.tbr),
             may_prompt=True)
 
     if cmd == 'dcommit':
@@ -1083,17 +1014,15 @@ def SendUpstream(parser, args, cmd):
       # create a template description. Eitherway, give the user a chance to edit
       # it to fill in the TBR= field.
       if cl.GetIssue():
-        description = cl.GetDescription()
+        change_desc = GitChangeDescription(description=cl.GetDescription())
 
-      # TODO(dpranke): Update to use ChangeDescription object.
       if not description:
-        description = """# Enter a description of the change.
-# This will be used as the change log for the commit.
+        log_desc = CreateDescriptionFromLog(args)
+        change_desc = GitChangeDescription(description=log_desc, tbr=True)
 
-"""
-        description += CreateDescriptionFromLog(args)
-
-      description = UserEditedLog(description + '\nTBR=')
+      if not options.force:
+        change_desc.UserEdit()
+      description = change_desc.description
 
     if not description:
       print "Description empty; aborting."

presubmit_support.py

@@ -812,6 +812,96 @@ class GitChange(Change):
     self.scm = 'git'
 
 
+class ChangeDescription(object):
+  """Contains a parsed form of the change description."""
+  MAX_SUBJECT_LENGTH = 100
+
+  def __init__(self, subject=None, description=None, reviewers=None, tbr=False,
+               editor=None):
+    self.subject = (subject or '').strip()
+    self.description = (description or '').strip()
+    self.reviewers = reviewers or []
+    self.tbr = tbr
+    self.editor = editor or gclient_utils.UserEdit
+
+    if self.description:
+      if not self.description.startswith(self.subject):
+        self.description = self.subject + '\n\n' + self.description
+    elif self.subject:
+      self.description = self.subject
+    self.Parse(self.EditableDescription())
+
+  def EditableDescription(self):
+    text = self.description.strip()
+    if text:
+      text += '\n'
+
+    tbr_present = False
+    r_present = False
+    bug_present = False
+    test_present = False
+    for l in text.splitlines():
+      l = l.strip()
+      r_present = r_present or l.startswith('R=')
+      tbr_present = tbr_present or l.startswith('TBR=')
+
+    if text and not (r_present or tbr_present):
+      text += '\n'
+
+    if not tbr_present and not r_present:
+      if self.tbr:
+        text += 'TBR=' + ','.join(self.reviewers) + '\n'
+      else:
+        text += 'R=' + ','.join(self.reviewers) + '\n'
+    if not bug_present:
+      text += 'BUG=\n'
+    if not test_present:
+      text += 'TEST=\n'
+
+    return text
+
+  def UserEdit(self):
+    """Allows the user to update the description.
+
+    Uses the editor callback passed to the constructor."""
+    self.Parse(self.editor(self.EditableDescription()))
+
+  def Parse(self, text):
+    """Parse the text returned from UserEdit() and update our state."""
+    parsed_lines = []
+    reviewers_regexp = re.compile('\s*(TBR|R)=(.+)')
+    reviewers = []
+    subject = ''
+    tbr = False
+    for l in text.splitlines():
+      l = l.strip()
+
+      # Throw away empty BUG=, TEST=, and R= lines. We leave in TBR= lines
+      # to indicate that this change was meant to be "unreviewed".
+      if l in ('BUG=', 'TEST=', 'R='):
+        continue
+
+      if not subject:
+        subject = l
+      matched_reviewers = reviewers_regexp.match(l)
+      if matched_reviewers:
+        tbr = (matched_reviewers.group(1) == 'TBR')
+        reviewers.extend(matched_reviewers.group(2).split(','))
+      parsed_lines.append(l)
+
+    if len(subject) > self.MAX_SUBJECT_LENGTH:
+      subject = subject[:self.MAX_SUBJECT_LENGTH - 3] + '...'
+
+    self.description = '\n'.join(parsed_lines).strip()
+    self.subject = subject
+    self.reviewers = reviewers
+    self.tbr = tbr
+
+  def IsEmpty(self):
+    return not self.description
+
+
+
 def ListRelevantPresubmitFiles(files, root):
   """Finds all presubmit files that apply to a given set of source files.
 

tests/gcl_unittest.py

@@ -84,7 +84,7 @@ class GclUnittest(GclTestsBase):
         'ErrorExit', 'FILES_CACHE', 'FilterFlag', 'GenUsage',
         'GenerateChangeName', 'GenerateDiff', 'GetCLs', 'GetCacheDir',
         'GetCachedFile', 'GetChangelistInfoFile', 'GetChangesDir',
-        'GetCodeReviewSetting', 'GetEditor', 'GetFilesNotInCL', 'GetInfoDir',
+        'GetCodeReviewSetting', 'GetFilesNotInCL', 'GetInfoDir',
         'GetModifiedFiles', 'GetRepositoryRoot', 'ListFiles',
         'LoadChangelistInfoForMultiple', 'MISSING_TEST_MSG',
         'OptionallyDoPresubmitChecks', 'REPOSITORY_ROOT', 'REVIEWERS_REGEX',

tests/gclient_utils_test.py

@@ -28,13 +28,13 @@ class GclientUtilsUnittest(GclientUtilBase):
         'CheckCall', 'CheckCallError', 'CheckCallAndFilter',
         'CheckCallAndFilterAndHeader', 'Error', 'ExecutionQueue', 'FileRead',
         'FileWrite', 'FindFileUpwards', 'FindGclientRoot',
-        'GetGClientRootAndEntries', 'GetNamedNodeText', 'MakeFileAutoFlush',
-        'GetNodeNamedAttributeText', 'MakeFileAnnotated', 'PathDifference',
-        'ParseXML', 'Popen',
+        'GetGClientRootAndEntries', 'GetEditor', 'GetNamedNodeText',
+        'MakeFileAutoFlush', 'GetNodeNamedAttributeText', 'MakeFileAnnotated',
+        'PathDifference', 'ParseXML', 'Popen',
         'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision',
-        'SyntaxErrorToError', 'WorkItem',
+        'SyntaxErrorToError', 'UserEdit', 'WorkItem',
         'errno', 'hack_subprocess', 'logging', 'os', 'Queue', 're', 'rmtree',
-        'stat', 'subprocess', 'sys','threading', 'time', 'xml',
+        'stat', 'subprocess', 'sys', 'tempfile', 'threading', 'time', 'xml',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(gclient_utils, members)

tests/presubmit_unittest.py

@@ -9,6 +9,7 @@
 # pylint: disable=E1101,E1103,W0212,W0403
 
 import StringIO
+import unittest
 
 # Fixes include path.
 from super_mox import mox, SuperMoxTestBase
@@ -135,8 +136,8 @@ class PresubmitUnittest(PresubmitTestsBase):
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
-      'AffectedFile', 'Change', 'DoGetTrySlaves', 'DoPresubmitChecks',
-      'GetTrySlavesExecuter', 'GitAffectedFile',
+      'AffectedFile', 'Change', 'ChangeDescription', 'DoGetTrySlaves',
+      'DoPresubmitChecks', 'GetTrySlavesExecuter', 'GitAffectedFile',
       'GitChange', 'InputApi', 'ListRelevantPresubmitFiles', 'Main',
       'NotImplementedException', 'OutputApi', 'ParseFiles',
       'PresubmitExecuter', 'PresubmitOutput', 'ScanSubDirs',
@@ -1971,6 +1972,81 @@ mac|success|blew
         uncovered_files=set(), host_url='https://localhost')
 
 
+def change_desc(editor=None, **kwargs):
+  if editor is None:
+    editor = lambda x: x
+  return presubmit.ChangeDescription(editor=editor, **kwargs)
+
+
+class ChangeDescriptionTests(unittest.TestCase):
+  def setUp(self):
+    self.editor_input = None
+    self.editor_output = None
+
+  def tearDown(self):
+    self.editor_input = None
+    self.editor_output = None
+
+  def editor(self, text):
+    if self.editor_input:
+      self.assertTrue(self.editor_input in text)
+    if self.editor_output is not None:
+      return self.editor_output
+    return text
+
+  def test_empty(self):
+    desc = change_desc()
+    self.assertTrue(desc.IsEmpty())
+    desc.UserEdit()
+    self.assertTrue(desc.IsEmpty())
+
+  def test_basic(self):
+    desc = change_desc(subject='foo', description='desc',
+                       reviewers=['joe@example.com'])
+    desc.UserEdit()
+    self.assertFalse(desc.IsEmpty())
+    self.assertEqual(desc.subject, 'foo')
+    self.assertEquals(desc.description,
+       'foo\n'
+       '\n'
+       'desc\n'
+       '\n'
+       'R=joe@example.com')
+    self.assertEquals(desc.reviewers, ['joe@example.com'])
+    self.assertFalse(desc.tbr)
+
+  def test_subject_only(self):
+    self.editor_input = 'foo\n\nR=\nBUG=\nTEST=\n'
+    desc = change_desc(subject='foo', editor=self.editor)
+    desc.UserEdit()
+    self.assertEquals(desc.description, 'foo')
+
+  def test_tbr_with_reviewer(self):
+    self.editor_input = 'TBR=\nBUG=\nTEST=\n'
+    self.editor_output = 'foo\n\nTBR=joe@example.com'
+    desc = change_desc(tbr=True, editor=self.editor)
+    self.assertFalse(desc.tbr)
+    self.assertEquals(desc.reviewers, [])
+    desc.UserEdit()
+    self.assertTrue(desc.tbr)
+    self.assertEquals(desc.reviewers, ['joe@example.com'])
+    self.assertEquals(desc.description,
+        'foo\n'
+        '\n'
+        'TBR=joe@example.com')
+
+  def test_tbr_without_reviewer(self):
+    desc = change_desc(subject='foo', tbr=True)
+    desc.UserEdit()
+    self.assertEquals(desc.description, 'foo\n\nTBR=')
+
+  def test_really_long_subject(self):
+    subject = 'foo' * 40
+    desc = change_desc(subject=subject)
+    self.assertEquals(desc.description, subject)
+    self.assertEquals(desc.subject, subject[:97] + '...')
+
+
 if __name__ == '__main__':
   import unittest
   unittest.main()