commit b707c52be93ed52d3e965e7c344adac291a2bdee
author Cheng Yueh <cyueh@google.com> Thu Jan 02 14:06:59 2020 +0800
committer Commit Bot <commit-bot@chromium.org> Tue Feb 04 12:53:56 2020 +0000
tree 8617ed5d5bf9606afe02e290207855fcc96aa437
parent 380592f72e3057aea62c2448b966ac2de33b2318

pre-upload: Check no BUG line after TEST line.

In the Chromium OS Contributing Guide, we state that BUG line must be
before TEST line. This CL checks this in pre-upload.py.

BUG=None
TEST=pre-upload_unittest.py

Change-Id: I03a5669b3ff2749bc905fd113d1bc66555217251
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/repohooks/+/1985506
Commit-Queue: Cheng Yueh <cyueh@chromium.org>
Tested-by: Cheng Yueh <cyueh@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

pre-upload.py

diff --git a/pre-upload.py b/pre-upload.py
index 46380e7..4220d14 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -111,6 +111,9 @@
 # excluded from presubmit checks. Lines beginning with '#' are ignored.
 _IGNORE_FILE = '.presubmitignore'
 
+
+TEST_FIELD_RE = r'\nTEST=\S+'
+
 # Exceptions
 
 
@@ -635,9 +638,8 @@
 def _check_change_has_test_field(_project, commit):
   """Check for a non-empty 'TEST=' field in the commit message."""
   SEE_ALSO = 'Please review the documentation:\n%s' % (DOC_COMMIT_MSG_URL,)
-  TEST_RE = r'\nTEST=\S+'
 
-  if not re.search(TEST_RE, _get_commit_desc(commit)):
+  if not re.search(TEST_FIELD_RE, _get_commit_desc(commit)):
     msg = ('Changelist description needs TEST field (after first line)\n%s' %
            (SEE_ALSO,))
     return HookFailure(msg)
@@ -715,6 +717,13 @@
              'BUG=None\n%s' % (SEE_ALSO,))
       return HookFailure(msg)
 
+  TEST_BEFORE_BUG_RE = TEST_FIELD_RE + r'.*' + BUG_RE
+
+  if re.search(TEST_BEFORE_BUG_RE, _get_commit_desc(commit), re.DOTALL):
+    msg = ('The BUG field must come before the TEST field.\n%s' %
+           (SEE_ALSO,))
+    return HookFailure(msg)
+
   return None
 
 

pre-upload_unittest.py

diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index 13135d7..40245cb 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -1132,6 +1132,17 @@
     """Reject bug lines that are not BUG."""
     self.assertMessageRejected('\nbug=none\n')
 
+  def testNotAfterTest(self):
+    """Reject any TEST line before any BUG line."""
+    test_field = 'TEST=i did not do it\n'
+    middle_field = 'A random between line\n'
+    for project, bug_field in ((self.AOSP_PROJECT, 'Bug:none\n'),
+                               (self.CROS_PROJECT, 'BUG=None\n')):
+      self.assertMessageRejected(
+          '\n' + test_field + middle_field + bug_field, project)
+      self.assertMessageRejected(
+          '\n' + test_field + bug_field, project)
+
 
 class CheckCommitMessageCqDepend(CommitMessageTestCase):
   """Tests for _check_change_has_valid_cq_depend."""