Improve and refactor push-to-trunk script.

tools/push-to-trunk/test_scripts.py

 #!/usr/bin/env python
 # Copyright 2013 the V8 project authors. All rights reserved.
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 #       notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
 #       copyright notice, this list of conditions and the following
@@ skipping 53 matching lines @@
     self.assertEquals("    Line 1\n    Line 3\n",
         StripComments("    Line 1\n#   Line 2\n    Line 3\n#\n"))
     self.assertEquals("\nLine 2 ### Test\n #",
         StripComments("###\n# \n\n#  Line 1\nLine 2 ### Test\n #"))
 
   def testMakeChangeLogBodySimple(self):
     commits = [
           ["Title text 1",
            "Title text 1\n\nBUG=\n",
            "author1@chromium.org"],
-          ["Title text 2",
+          ["Title text 2.",
            "Title text 2\n\nBUG=1234\n",
            "author2@chromium.org"],
         ]
-    self.assertEquals("        Title text 1\n"
+    self.assertEquals("        Title text 1.\n"
                       "        (author1@chromium.org)\n\n"
-                      "        Title text 2\n"
-                      "        (Chromium issue 1234)\n"
+                      "        Title text 2 (Chromium issue 1234).\n"
                       "        (author2@chromium.org)\n\n",
                       MakeChangeLogBody(commits))
 
   def testMakeChangeLogBodyEmpty(self):
     self.assertEquals("", MakeChangeLogBody([]))
 
   def testMakeChangeLogBodyAutoFormat(self):
     commits = [
-          ["Title text 1",
+          ["Title text 1!",
            "Title text 1\nLOG=y\nBUG=\n",
            "author1@chromium.org"],
           ["Title text 2",
            "Title text 2\n\nBUG=1234\n",
            "author2@chromium.org"],
           ["Title text 3",
            "Title text 3\n\nBUG=1234\nLOG = Yes\n",
            "author3@chromium.org"],
           ["Title text 3",
            "Title text 4\n\nBUG=1234\nLOG=\n",
            "author4@chromium.org"],
         ]
-    self.assertEquals("        Title text 1\n\n"
-                      "        Title text 3\n"
-                      "        (Chromium issue 1234)\n\n",
+    self.assertEquals("        Title text 1.\n\n"
+                      "        Title text 3 (Chromium issue 1234).\n\n",
                       MakeChangeLogBody(commits, True))
 
   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",
+    self.assertEquals("(issue 987654)",
                       MakeChangeLogBugReference("BUG = v8:987654"))
-    self.assertEquals("        (Chromium issue 987654)\n",
+    self.assertEquals("(Chromium issue 987654)",
                       MakeChangeLogBugReference("BUG=987654 "))
 
   def testMakeChangeLogBugReferenceFromBody(self):
-    self.assertEquals("        (Chromium issue 1234567)\n",
+    self.assertEquals("(Chromium issue 1234567)",
                       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",
+    self.assertEquals("(issues 123, 234, Chromium issue 345)",
                       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",
+    self.assertEquals("(Chromium issues 123, 234)",
                       MakeChangeLogBugReference("Title\n\n"
                                                 "BUG=234,,chromium:123 \n"
                                                 "R=somebody\n"))
-    self.assertEquals("        (Chromium issues 123, 234)\n",
+    self.assertEquals("(Chromium issues 123, 234)",
                       MakeChangeLogBugReference("Title\n\n"
                                                 "BUG=chromium:234, , 123\n"
                                                 "R=somebody\n"))
-    self.assertEquals("        (issues 345, 456)\n",
+    self.assertEquals("(issues 345, 456)",
                       MakeChangeLogBugReference("Title\n\n"
                                                 "\t\tBUG=v8:345,v8:456\n"
                                                 "R=somebody\n"))
-    self.assertEquals("        (issue 123, Chromium issues 345, 456)\n",
+    self.assertEquals("(issue 123, Chromium issues 345, 456)",
                       MakeChangeLogBugReference("Title\n\n"
                                                 "BUG=chromium:456\n"
                                                 "BUG = none\n"
                                                 "R=somebody\n"
                                                 "BUG=456,v8:123, 345"))
 
+  # TODO(machenbach): These test don't make much sense when the formatting is
+  # done later.
   def testMakeChangeLogBugReferenceLong(self):
     # -----------------00--------10--------20--------30--------
-    self.assertEquals("        (issues 234, 1234567890, 1234567"
-                      "8901234567890, Chromium issues 12345678,\n"
-                      "        123456789)\n",
+    self.assertEquals("(issues 234, 1234567890, 1234567"
+                      "8901234567890, Chromium issues 12345678,"
+                      " 123456789)",
                       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",
+    self.assertEquals("(issues 234, 1234567890, 1234567"
+                      "8901234567890, Chromium issues"
+                      " 123456789, 1234567890)",
                       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",
+    self.assertEquals("(Chromium issues 234, 1234567890"
+                      ", 12345678901234567, "
+                      "1234567890123456789)",
                       MakeChangeLogBugReference("BUG=234\n"
                                                 "BUG=12345678901234567\n"
                                                 "BUG=1234567890123456789\n"
                                                 "BUG=1234567890\n"))
 
 
 class SimpleMock(object):
   def __init__(self, name):
     self._name = name
     self._recipe = []
     self._index = -1
 
   def Expect(self, recipe):
     self._recipe = recipe
 
   def Call(self, *args):
     self._index += 1
     try:
       expected_call = self._recipe[self._index]
     except IndexError:
-      raise Exception("Calling %s %s" % (name, " ".join(args)))
+      raise Exception("Calling %s %s" % (self._name, " ".join(args)))
 
     # Pack expectations without arguments into a list.
     if not isinstance(expected_call, list):
       expected_call = [expected_call]
 
     # The number of arguments in the expectation must match the actual
     # arguments.
     if len(args) > len(expected_call):
       raise Exception("When calling %s with arguments, the expectations "
                       "must consist of at least as many arguments.")
@@ skipping 182 matching lines @@
     self.assertEqual("//\n#define BUILD_NUMBER  3\n",
                      MSub(r"(?<=#define BUILD_NUMBER)(?P<space>\s+)\d*$",
                           r"\g<space>3",
                           "//\n#define BUILD_NUMBER  321\n"))
 
   def testPrepareChangeLog(self):
     TEST_CONFIG[VERSION_FILE] = self.MakeTempVersionFile()
     TEST_CONFIG[CHANGELOG_ENTRY_FILE] = self.MakeEmptyTempFile()
 
     self.ExpectGit([
-      ["log 1234..HEAD --format=%H", "rev1\nrev2\nrev3"],
+      ["log 1234..HEAD --format=%H", "rev1\nrev2\nrev3\nrev4"],
       ["log -1 rev1 --format=\"%s\"", "Title text 1"],
       ["log -1 rev1 --format=\"%B\"", "Title\n\nBUG=\nLOG=y\n"],
       ["log -1 rev1 --format=\"%an\"", "author1@chromium.org"],
-      ["log -1 rev2 --format=\"%s\"", "Title text 2"],
+      ["log -1 rev2 --format=\"%s\"", "Title text 2."],
       ["log -1 rev2 --format=\"%B\"", "Title\n\nBUG=123\nLOG= \n"],
       ["log -1 rev2 --format=\"%an\"", "author2@chromium.org"],
       ["log -1 rev3 --format=\"%s\"", "Title text 3"],
       ["log -1 rev3 --format=\"%B\"", "Title\n\nBUG=321\nLOG=true\n"],
       ["log -1 rev3 --format=\"%an\"", "author3@chromium.org"],
+      ["log -1 rev4 --format=\"%s\"", "Title text 4"],
+      ["log -1 rev4 --format=\"%B\"",
+       ("Title\n\nBUG=456\nLOG=Y\n\n"
+        "Review URL: https://codereview.chromium.org/9876543210\n")],
+      ["log -1 rev4 --format=\"%an\"", "author4@chromium.org"],
+    ])
+
+    # The cl for rev4 on rietveld has an updated LOG flag.
+    self.ExpectReadURL([
+      ["https://codereview.chromium.org/9876543210/description",
+       "Title\n\nBUG=456\nLOG=N\n\n"],
     ])
 
     self.MakeStep().Persist("last_push", "1234")
     self.MakeStep(PrepareChangeLog).Run()
 
     actual_cl = FileToText(TEST_CONFIG[CHANGELOG_ENTRY_FILE])
 
     # TODO(machenbach): Mock out call to date() in order to make a fixed
     # comparison here instead of a regexp match.
     expected_cl = """\\d+\\-\\d+\\-\\d+: Version 3\\.22\\.5
 
-        Title text 1
+        Title text 1.
 
-        Title text 3
-        \\(Chromium issue 321\\)
+        Title text 3 \\(Chromium issue 321\\).
 
         Performance and stability improvements on all platforms\\.
 #
 # The change log above is auto-generated\\. Please review if all relevant
 # commit messages from the list below are included\\.
 # All lines starting with # will be stripped\\.
 #
-#       Title text 1
+#       Title text 1.
 #       \\(author1@chromium\\.org\\)
 #
-#       Title text 2
-#       \\(Chromium issue 123\\)
+#       Title text 2 \\(Chromium issue 123\\).
 #       \\(author2@chromium\\.org\\)
 #
-#       Title text 3
-#       \\(Chromium issue 321\\)
+#       Title text 3 \\(Chromium issue 321\\).
 #       \\(author3@chromium\\.org\\)
 #
+#       Title text 4 \\(Chromium issue 456\\).
+#       \\(author4@chromium\\.org\\)
+#
 #"""
 
     self.assertTrue(re.match(expected_cl, actual_cl))
     self.assertEquals("3", self.MakeStep().Restore("major"))
     self.assertEquals("22", self.MakeStep().Restore("minor"))
     self.assertEquals("5", self.MakeStep().Restore("build"))
     self.assertEquals("0", self.MakeStep().Restore("patch"))
 
   def testEditChangeLog(self):
     TEST_CONFIG[CHANGELOG_ENTRY_FILE] = self.MakeEmptyTempFile()
     TEST_CONFIG[CHANGELOG_FILE] = self.MakeEmptyTempFile()
     TextToFile("        Original CL", TEST_CONFIG[CHANGELOG_FILE])
     TextToFile("  New  \n\tLines  \n", TEST_CONFIG[CHANGELOG_ENTRY_FILE])
     os.environ["EDITOR"] = "vi"
 
     self.ExpectReadline([
       "",  # Open editor.
     ])
 
     self.MakeStep(EditChangeLog).Run()
 
-    self.assertEquals("        New\n        Lines\n\n\n        Original CL",
+    self.assertEquals("New\n        Lines\n\n\n        Original CL",
                       FileToText(TEST_CONFIG[CHANGELOG_FILE]))
 
   def testIncrementVersion(self):
     TEST_CONFIG[VERSION_FILE] = self.MakeTempVersionFile()
     self.MakeStep().Persist("build", "5")
 
     self.ExpectReadline([
       "Y",  # Increment build number.
     ])
 
@@ skipping 57 matching lines @@
     if not os.path.exists(TEST_CONFIG[CHROMIUM]):
       os.makedirs(TEST_CONFIG[CHROMIUM])
     TextToFile("1999-04-05: Version 3.22.4", TEST_CONFIG[CHANGELOG_FILE])
     TextToFile("Some line\n   \"v8_revision\": \"123444\",\n  some line",
                  TEST_CONFIG[DEPS_FILE])
     os.environ["EDITOR"] = "vi"
 
     def CheckPreparePush():
       cl = FileToText(TEST_CONFIG[CHANGELOG_FILE])
       self.assertTrue(re.search(r"Version 3.22.5", cl))
-      self.assertTrue(re.search(r"        Log text 1", cl))
-      self.assertTrue(re.search(r"        \(issue 321\)", cl))
+      self.assertTrue(re.search(r"        Log text 1 \(issue 321\).", cl))
       self.assertFalse(re.search(r"        \(author1@chromium\.org\)", cl))
 
       # Make sure all comments got stripped.
       self.assertFalse(re.search(r"^#", cl, flags=re.M))
 
       version = FileToText(TEST_CONFIG[VERSION_FILE])
       self.assertTrue(re.search(r"#define BUILD_NUMBER\s+6", version))
 
     def CheckUpload():
       cl = FileToText(TEST_CONFIG[CHANGELOG_FILE])
 
     def CheckSVNCommit():
       commit = FileToText(TEST_CONFIG[COMMITMSG_FILE])
       self.assertTrue(re.search(r"Version 3.22.5", commit))
-      self.assertTrue(re.search(r"Log text 1. \(issue 321\)", commit))
+      self.assertTrue(re.search(r"Log text 1 \(issue 321\).", commit))
       version = FileToText(TEST_CONFIG[VERSION_FILE])
       self.assertTrue(re.search(r"#define MINOR_VERSION\s+22", version))
       self.assertTrue(re.search(r"#define BUILD_NUMBER\s+5", version))
       self.assertFalse(re.search(r"#define BUILD_NUMBER\s+6", version))
       self.assertTrue(re.search(r"#define PATCH_LEVEL\s+0", version))
       self.assertTrue(re.search(r"#define IS_CANDIDATE_VERSION\s+0", version))
 
     force_flag = " -f" if force else ""
     self.ExpectGit([
       ["status -s -uno", ""],
@@ skipping 67 matching lines @@
     options.l = None
     options.f = force
     options.r = "reviewer@chromium.org" if force else None
     options.c = TEST_CONFIG[CHROMIUM]
     RunPushToTrunk(TEST_CONFIG, options, self)
 
     deps = FileToText(TEST_CONFIG[DEPS_FILE])
     self.assertTrue(re.search("\"v8_revision\": \"123456\"", deps))
 
     cl = FileToText(TEST_CONFIG[CHANGELOG_FILE])
-    self.assertTrue(re.search(r"\d\d\d\d\-\d+\-\d+: Version 3\.22\.5", cl))
-    self.assertTrue(re.search(r"        Log text 1", cl))
-    self.assertTrue(re.search(r"        \(issue 321\)", cl))
+    self.assertTrue(re.search(r"^\d\d\d\d\-\d+\-\d+: Version 3\.22\.5", cl))
+    self.assertTrue(re.search(r"        Log text 1 \(issue 321\).", cl))
     self.assertTrue(re.search(r"1999\-04\-05: Version 3\.22\.4", cl))
 
     # Note: The version file is on build number 5 again in the end of this test
     # since the git command that merges to the bleeding edge branch is mocked
     # out.
 
   def testPushToTrunk(self):
     self._PushToTrunk()
 
   def testPushToTrunkForced(self):
@@ skipping 20 matching lines @@
     class Options( object ):
       pass
 
     options = Options()
     options.s = 0
 
     auto_roll.RunAutoRoll(TEST_CONFIG, options, self)
 
     self.assertEquals("100", self.MakeStep().Restore("lkgr"))
     self.assertEquals("101", self.MakeStep().Restore("latest"))
+
+
+class SystemTest(unittest.TestCase):
+  def testReload(self):
+    step = MakeStep(step_class=PrepareChangeLog, number=0, state={}, config={},
+                    options=None,
+                    side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER)
+    body = step.Reload(
+"""------------------------------------------------------------------------
+r17997 | machenbach@chromium.org | 2013-11-22 11:04:04 +0100 (...) | 6 lines
+
+Prepare push to trunk.  Now working on version 3.23.11.
+
+R=danno@chromium.org
+
+Review URL: https://codereview.chromium.org/83173002
+
+------------------------------------------------------------------------""")
+    self.assertEquals(
+"""Prepare push to trunk.  Now working on version 3.23.11.
+
+R=danno@chromium.org
+
+Committed: https://code.google.com/p/v8/source/detail?r=17997""", body)