Fix addition of CQ_INCLUDE_TRYBOTS on Gerrit.

BUG=710547

Change-Id: I6558ba08683b8e010ac5051dabfd82d9a5b9c551
Reviewed-on: https://chromium-review.googlesource.com/486083
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>

presubmit_support.py

@@ -299,7 +299,26 @@ class OutputApi(object):
     if m:
       new_description = include_re.sub(new_include_trybots, description)
     else:
-      new_description = description + '\n' + new_include_trybots + '\n'
+      # If we're adding a new CQ_INCLUDE_TRYBOTS line then make
+      # absolutely sure to add it before any Change-Id: line, to avoid
+      # breaking Gerrit.
+      #
+      # The use of \n outside the capture group causes the last
+      # newline before Change-Id and any extra newlines after it to be
+      # consumed. They are re-added during the join operation.
+      #
+      # The filter operation drops the trailing empty string after the
+      # original string is split.
+      split_desc = filter(
+        None, re.split('\n(Change-Id: \w*)\n*', description, 1, re.M))
+      # Make sure to insert this before the last entry. For backward
+      # compatibility, ensure the CL description ends in a newline.
+      if len(split_desc) == 1:
+        insert_idx = 1
+      else:
+        insert_idx = len(split_desc) - 1
+      split_desc.insert(insert_idx, new_include_trybots)
+      new_description = '\n'.join(split_desc) + '\n'
     cl.UpdateDescription(new_description, force=True)
     return [self.PresubmitNotifyResult(message)]
 

tests/presubmit_unittest.py

@@ -1459,6 +1459,53 @@ CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserve
       ],
       """A change to GPU-related code.
 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
+""")
+
+    # Starting without any CQ_INCLUDE_TRYBOTS line, but with a
+    # Change-Id: line (with no trailing newline), which must continue
+    # to be the last line.
+    self._testIncludingCQTrybots(
+      """A change to GPU-related code.
+Change-Id: Idaeacea9cdbe912c24c8388147a8a767c7baa5f2""",
+      [
+        'master.tryserver.chromium.linux:linux_optional_gpu_tests_rel',
+        'master.tryserver.chromium.mac:mac_optional_gpu_tests_rel',
+      ],
+      """A change to GPU-related code.
+CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
+Change-Id: Idaeacea9cdbe912c24c8388147a8a767c7baa5f2
+""")
+
+    # Starting without any CQ_INCLUDE_TRYBOTS line, but with a
+    # Change-Id: line (with a trailing newline), which must continue
+    # to be the last line.
+    self._testIncludingCQTrybots(
+      """A change to GPU-related code.
+Change-Id: Idaeacea9cdbe912c24c8388147a8a767c7baa5f2
+""",
+      [
+        'master.tryserver.chromium.mac:mac_optional_gpu_tests_rel',
+        'master.tryserver.chromium.win:win_optional_gpu_tests_rel',
+      ],
+      """A change to GPU-related code.
+CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
+Change-Id: Idaeacea9cdbe912c24c8388147a8a767c7baa5f2
+""")
+
+    # Starting with a CQ_INCLUDE_TRYBOTS line and a Change-Id: line,
+    # the latter of which must continue to be the last line.
+    self._testIncludingCQTrybots(
+      """A change to GPU-related code.
+CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel
+Change-Id: Idaeacea9cdbe912c24c8388147a8a767c7baa5f2
+""",
+      [
+        'master.tryserver.chromium.linux:linux_optional_gpu_tests_rel',
+        'master.tryserver.chromium.win:win_optional_gpu_tests_rel',
+      ],
+      """A change to GPU-related code.
+CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
+Change-Id: Idaeacea9cdbe912c24c8388147a8a767c7baa5f2
 """)
 
     # All pre-existing bots are already in output set.