Refactor and improve bug line generation for push-to-trunk.

BUG=
R=jkummerow@chromium.org

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@17892 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/tools/push-to-trunk/common_includes.py b/tools/push-to-trunk/common_includes.py
index 06b7ebe..9ab2b14 100644 (file)
--- a/tools/push-to-trunk/common_includes.py
+++ b/tools/push-to-trunk/common_includes.py
@@ -30,6 +30,7 @@ import os
 import re
 import subprocess
 import sys
+import textwrap
 
 PERSISTFILE_BASENAME = "PERSISTFILE_BASENAME"
 TEMP_BRANCH = "TEMP_BRANCH"
@@ -67,6 +68,11 @@ def MSub(rexp, replacement, text):
   return re.sub(rexp, replacement, text, flags=re.MULTILINE)
 
 
+def Fill80(line):
+  return textwrap.fill(line, width=80, initial_indent="        ",
+                       subsequent_indent="        ")
+
+
 def GetLastChangeLogEntries(change_log_file):
   result = []
   for line in LinesInFile(change_log_file):
@@ -81,28 +87,56 @@ def MakeChangeLogBody(commit_generator):
     # Add the commit's title line.
     result += "%s\n" % title.rstrip()
 
-    # Grep for "BUG=xxxx" lines in the commit message and convert them to
-    # "(issue xxxx)".
-    out = body.splitlines()
-    out = filter(lambda x: re.search(r"^BUG=", x), out)
-    out = filter(lambda x: not re.search(r"BUG=$", x), out)
-    out = filter(lambda x: not re.search(r"BUG=none$", x), out)
-
-    # TODO(machenbach): Handle multiple entries (e.g. BUG=123, 234).
-    def FormatIssue(text):
-      text = re.sub(r"BUG=v8:(.*)$", r"(issue \1)", text)
-      text = re.sub(r"BUG=chromium:(.*)$", r"(Chromium issue \1)", text)
-      text = re.sub(r"BUG=(.*)$", r"(Chromium issue \1)", text)
-      return "        %s\n" % text
-
-    for line in map(FormatIssue, out):
-      result += line
+    # Add bug references.
+    result += MakeChangeLogBugReference(body)
 
     # Append the commit's author for reference.
     result += "%s\n\n" % author.rstrip()
   return result
 
 
+def MakeChangeLogBugReference(body):
+  """Grep for "BUG=xxxx" lines in the commit message and convert them to
+  "(issue xxxx)".
+  """
+  crbugs = []
+  v8bugs = []
+
+  def AddIssues(text):
+    ref = re.match(r"^BUG[ \t]*=[ \t]*(.+)$", text.strip())
+    if not ref:
+      return
+    for bug in ref.group(1).split(","):
+      bug = bug.strip()
+      match = re.match(r"^v8:(\d+)$", bug)
+      if match: v8bugs.append(int(match.group(1)))
+      else:
+        match = re.match(r"^(?:chromium:)?(\d+)$", bug)
+        if match: crbugs.append(int(match.group(1)))
+
+  # Add issues to crbugs and v8bugs.
+  map(AddIssues, body.splitlines())
+
+  # Filter duplicates, sort, stringify.
+  crbugs = map(str, sorted(set(crbugs)))
+  v8bugs = map(str, sorted(set(v8bugs)))
+
+  bug_groups = []
+  def FormatIssues(prefix, bugs):
+    if len(bugs) > 0:
+      plural = "s" if len(bugs) > 1 else ""
+      bug_groups.append("%sissue%s %s" % (prefix, plural, ", ".join(bugs)))
+
+  FormatIssues("", v8bugs)
+  FormatIssues("Chromium ", crbugs)
+
+  if len(bug_groups) > 0:
+    # Format with 8 characters indentation and max 80 character lines.
+    return "%s\n" % Fill80("(%s)" % ", ".join(bug_groups))
+  else:
+    return ""
+
+
 # Some commands don't like the pipe, e.g. calling vi from within the script or
 # from subscripts like git cl upload.
 def Command(cmd, args="", prefix="", pipe=True):

diff --git a/tools/push-to-trunk/test_scripts.py b/tools/push-to-trunk/test_scripts.py
index b9d762d..b44c70b 100644 (file)
--- a/tools/push-to-trunk/test_scripts.py
+++ b/tools/push-to-trunk/test_scripts.py
@@ -52,6 +52,101 @@ TEST_CONFIG = {
 }
 
 
+class ToplevelTest(unittest.TestCase):
+  def testMakeChangeLogBodySimple(self):
+    commits = lambda: [
+          ["        Title text 1",
+           "Title text 1\n\nBUG=\n",
+           "        author1@chromium.org"],
+          ["        Title text 2",
+           "Title text 2\n\nBUG=1234\n",
+           "        author2@chromium.org"],
+        ]
+    self.assertEquals("        Title text 1\n"
+                      "        author1@chromium.org\n\n"
+                      "        Title text 2\n"
+                      "        (Chromium issue 1234)\n"
+                      "        author2@chromium.org\n\n",
+                      MakeChangeLogBody(commits))
+
+  def testMakeChangeLogBodyEmpty(self):
+    commits = lambda: []
+    self.assertEquals("", MakeChangeLogBody(commits))
+
+  def testMakeChangeLogBugReferenceEmpty(self):
+    self.assertEquals("", MakeChangeLogBugReference(""))
+    self.assertEquals("", MakeChangeLogBugReference("LOG="))
+    self.assertEquals("", MakeChangeLogBugReference(" BUG ="))
+    self.assertEquals("", MakeChangeLogBugReference("BUG=none\t"))
+
+  def testMakeChangeLogBugReferenceSimple(self):
+    self.assertEquals("        (issue 987654)\n",
+                      MakeChangeLogBugReference("BUG = v8:987654"))
+    self.assertEquals("        (Chromium issue 987654)\n",
+                      MakeChangeLogBugReference("BUG=987654 "))
+
+  def testMakeChangeLogBugReferenceFromBody(self):
+    self.assertEquals("        (Chromium issue 1234567)\n",
+                      MakeChangeLogBugReference("Title\n\nTBR=\nBUG=\n"
+                                                " BUG=\tchromium:1234567\t\n"
+                                                "R=somebody\n"))
+
+  def testMakeChangeLogBugReferenceMultiple(self):
+    # All issues should be sorted and grouped. Multiple references to the same
+    # issue should be filtered.
+    self.assertEquals("        (issues 123, 234, Chromium issue 345)\n",
+                      MakeChangeLogBugReference("Title\n\n"
+                                                "BUG=v8:234\n"
+                                                "  BUG\t= 345, \tv8:234,\n"
+                                                "BUG=v8:123\n"
+                                                "R=somebody\n"))
+    self.assertEquals("        (Chromium issues 123, 234)\n",
+                      MakeChangeLogBugReference("Title\n\n"
+                                                "BUG=234,,chromium:123 \n"
+                                                "R=somebody\n"))
+    self.assertEquals("        (Chromium issues 123, 234)\n",
+                      MakeChangeLogBugReference("Title\n\n"
+                                                "BUG=chromium:234, , 123\n"
+                                                "R=somebody\n"))
+    self.assertEquals("        (issues 345, 456)\n",
+                      MakeChangeLogBugReference("Title\n\n"
+                                                "\t\tBUG=v8:345,v8:456\n"
+                                                "R=somebody\n"))
+    self.assertEquals("        (issue 123, Chromium issues 345, 456)\n",
+                      MakeChangeLogBugReference("Title\n\n"
+                                                "BUG=chromium:456\n"
+                                                "BUG = none\n"
+                                                "R=somebody\n"
+                                                "BUG=456,v8:123, 345"))
+
+  def testMakeChangeLogBugReferenceLong(self):
+    # -----------------00--------10--------20--------30--------
+    self.assertEquals("        (issues 234, 1234567890, 1234567"
+                      "8901234567890, Chromium issues 12345678,\n"
+                      "        123456789)\n",
+                      MakeChangeLogBugReference("BUG=v8:234\n"
+                                                "BUG=v8:1234567890\n"
+                                                "BUG=v8:12345678901234567890\n"
+                                                "BUG=123456789\n"
+                                                "BUG=12345678\n"))
+    # -----------------00--------10--------20--------30--------
+    self.assertEquals("        (issues 234, 1234567890, 1234567"
+                      "8901234567890, Chromium issues\n"
+                      "        123456789, 1234567890)\n",
+                      MakeChangeLogBugReference("BUG=v8:234\n"
+                                                "BUG=v8:12345678901234567890\n"
+                                                "BUG=v8:1234567890\n"
+                                                "BUG=123456789\n"
+                                                "BUG=1234567890\n"))
+    # -----------------00--------10--------20--------30--------
+    self.assertEquals("        (Chromium issues 234, 1234567890"
+                      ", 12345678901234567,\n"
+                      "        1234567890123456789)\n",
+                      MakeChangeLogBugReference("BUG=234\n"
+                                                "BUG=12345678901234567\n"
+                                                "BUG=1234567890123456789\n"
+                                                "BUG=1234567890\n"))
+
 class ScriptTest(unittest.TestCase):
   def MakeEmptyTempFile(self):
     handle, name = tempfile.mkstemp()