Add checks for RELEASE_NOTE and cos-patch trailer.
This CL does the following:
- Add check for 'RELEASE_NOTE' (disabled by default)
- Add check for cos-patch trailer (disabled by default)
BUG=b/173034990
TEST=manual test on some changelists.
cos-patch: none
Change-Id: If4065e74f645cf586fc43ac98b36223ee3ada644
Reviewed-on: https://cos-review.googlesource.com/c/cos/repohooks/+/8560
Reviewed-by: Dexter Rivera <riverade@google.com>
Reviewed-by: Robert Kolchmeyer <rkolchmeyer@google.com>
Tested-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
diff --git a/pre-upload.py b/pre-upload.py
index 24e9b49..11a9b44 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -118,6 +118,10 @@
TEST_FIELD_RE = r'\nTEST=\S+'
+RELEASE_NOTE_FIELD_RE = r'\nRELEASE_NOTE=\S+'
+COS_PATCH_RE = (
+ r'\ncos-patch: (security-(critical|high|medium|low)|bug|lts-refresh)'
+)
# Exceptions
@@ -760,6 +764,50 @@
return None
+def _check_change_has_release_note_field(_project, commit):
+ """Check for a non-empty 'RELEASE_NOTE=' field in the commit message."""
+ desc = _get_commit_desc(commit)
+ if not re.search(RELEASE_NOTE_FIELD_RE, desc):
+ msg = ('Changelist description needs RELEASE_NOTE field '
+ '(after first line)\n'
+ 'Examples:\n'
+ 'RELEASE_NOTE=None\n'
+ 'RELEASE_NOTE=New feature\n'
+ 'RELEASE_NOTE=Bugfix\n'
+ 'Please set RELEASE_NOTE=None for '
+ 'non-lakitu boards.\n')
+ return HookFailure(msg)
+
+ RELEASE_NOTE_BEFORE_TEST_RE = RELEASE_NOTE_FIELD_RE + r'.*' + TEST_FIELD_RE
+
+ if re.search(RELEASE_NOTE_BEFORE_TEST_RE, desc, re.DOTALL):
+ msg = ('The TEST field must come before the RELEASE_NOTE field.\n')
+ return HookFailure(msg)
+ return None
+
+
+def _check_change_has_cos_patch_trailer(_project, commit):
+ """Check for a non-empty 'cos-patch' trailer in the commit message."""
+ desc = _get_commit_desc(commit)
+ if not re.search(COS_PATCH_RE, desc):
+ msg = ('Changelist description needs cos-patch trailer\n'
+ 'Examples:\n'
+ 'cos-patch: security-critical\n'
+ 'cos-patch: bug\n'
+ 'cos-patch: lts-refresh\n'
+ 'For valid values, please check go/cos-lts-policy.')
+ return HookFailure(msg)
+
+ # Check that 'cos-patch' is in the same paragraph as Change-Id.
+ msg = 'cos-patch is not in the same paragraph as Change-Id.'
+ paragraphs = desc.split('\n\n')
+ for paragraph in paragraphs:
+ if (re.search(r'^cos-patch:', paragraph, re.M) and not
+ re.search('^Change-Id:', paragraph, re.M)):
+ return HookFailure(msg)
+ return None
+
+
def _check_change_has_valid_cq_depend(_project, commit):
"""Check for a correctly formatted Cq-Depend field in the commit message."""
desc = _get_commit_desc(commit)
@@ -2137,8 +2185,6 @@
# A dictionary of project-specific hooks(callbacks), indexed by project name.
# dict[project] = [callback1, callback2]
_PROJECT_SPECIFIC_HOOKS = {
- 'chromiumos/third_party/kernel': [_kernel_configcheck],
- 'chromiumos/third_party/kernel-next': [_kernel_configcheck],
}
@@ -2162,6 +2208,8 @@
'signoff_check': _check_change_has_signoff_field,
'bug_field_check': _check_change_has_bug_field,
'test_field_check': _check_change_has_test_field,
+ 'release_note_field_check': _check_change_has_release_note_field,
+ 'cos_patch_trailer_check': _check_change_has_cos_patch_trailer,
'manifest_check': _check_manifests,
'contribution_check': _check_change_is_contribution,
'project_prefix_check': _check_project_prefix,
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index 34ce829..968340e 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -1583,6 +1583,76 @@
self.assertMessageRejected('\ntest=none\n')
+class CheckCommitMessageReleaseNote(CommitMessageTestCase):
+ """Tests for _check_change_has_release_note_field."""
+
+ @staticmethod
+ def CheckMessage(project, commit):
+ return pre_upload._check_change_has_release_note_field(project, commit)
+
+ def testNormal(self):
+ """Accept a commit message w/a valid RELEASE_NOTE."""
+ self.assertMessageAccepted('\nRELEASE_NOTE=i did it\n')
+
+ def testNone(self):
+ """Accept RELEASE_NOTE=None."""
+ self.assertMessageAccepted('\nRELEASE_NOTE=None\n')
+ self.assertMessageAccepted('\nRELEASE_NOTE=none\n')
+
+ def testBlank(self):
+ """Reject blank values."""
+ self.assertMessageRejected('\nRELEASE_NOTE=\n')
+ self.assertMessageRejected('\nRELEASE_NOTE= \n')
+
+ def testNotFirstLine(self):
+ """Reject the first line."""
+ self.assertMessageRejected('RELEASE_NOTE=None\n\n\n')
+
+ def testNotInline(self):
+ """Reject not at the start of line."""
+ self.assertMessageRejected('\n RELEASE_NOTE=None\n')
+ self.assertMessageRejected('\n\tRELEASE_NOTE=None\n')
+
+ def testMissing(self):
+ """Reject commit messages w/no RELEASE_NOTE line."""
+ self.assertMessageRejected('foo\n')
+
+ def testCase(self):
+ """Reject bug lines that are not RELEASE_NOTE."""
+ self.assertMessageRejected('\nrelease_note=none\n')
+
+
+class CheckCommitMessageCosPatch(CommitMessageTestCase):
+ """Tests for _check_change_has_cos_patch_trailer."""
+
+ @staticmethod
+ def CheckMessage(project, commit):
+ return pre_upload._check_change_has_cos_patch_trailer(project, commit)
+
+ def testNormal(self):
+ """Accept valid cos-patch line."""
+ self.assertMessageAccepted('\ncos-patch: security-critical\n'
+ 'Change-Id: I123')
+ self.assertMessageAccepted('\ncos-patch: security-high\n'
+ 'Change-Id: I123')
+ self.assertMessageAccepted('\ncos-patch: security-medium\n'
+ 'Change-Id: I123')
+ self.assertMessageAccepted('\ncos-patch: security-low\n'
+ 'Change-Id: I123')
+ self.assertMessageAccepted('\ncos-patch: bug\n'
+ 'Change-Id: I123')
+ self.assertMessageAccepted('\ncos-patch: lts-refresh\n'
+ 'Change-Id: I123')
+
+ def testInvalid(self):
+ """Reject invalid cos-patch lines."""
+ self.assertMessageRejected('\ncos-patch: none\nChange-Id: I123')
+ self.assertMessageRejected('\ncos-patch: security-123\nChange-Id: I123')
+ self.assertMessageRejected('\ncos-patch: security-high\n')
+ self.assertMessageRejected('\ncos-patch: bug\n\nChange-Id: I123')
+ self.assertMessageRejected('\nCOS-PATCH: lts-refresh\n')
+
+
class CheckCommitMessageChangeId(CommitMessageTestCase):
"""Tests for _check_change_has_proper_changeid."""
