Commit e5d0a56e authored by Dominic Battre's avatar Dominic Battre Committed by LUCI CQ

Warn user in case they use commit messages with "Bug=" instead of "Bug:"

Chrome uses the syntax "Bug: xyz" in commit messages to attribute CLs
to bugs while Critique uses "BUG=xyz". As per
```
git log --grep '\(Bug\|Fixed\)=' --since=2021-01-01 --format=oneline \
| wc -l
```
we've had 27 CLs this year using the wrong syntax which led to crbug.com
not learning about the CLs <-> Bug attribution. Yours truly caused one
of them and wants to prevent the problem in the future.

We might relax the requirments in crbug.com or
chromium-review.googlesource.com but then anyone grepping through the
commit logs would need to deal with the ambiguity. Therefore, we can
just enforce the right syntax here.

https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/contributing.md
contains the correct syntax.

Change-Id: I60ee579deac50a74e1a014ceb1d9744cbc10ab41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3141567Reviewed-by: 's avatarJosip Sokcevic <sokcevic@google.com>
Commit-Queue: Dominic Battré <battre@chromium.org>
parent b7ac872c
......@@ -109,6 +109,22 @@ def CheckChangeWasUploaded(input_api, output_api):
return []
def CheckDescriptionUsesColonInsteadOfEquals(input_api, output_api):
"""Checks that the CL description uses a colon after 'Bug' and 'Fixed' tags
instead of equals.
crbug.com only interprets the lines "Bug: xyz" and "Fixed: xyz" but not
"Bug=xyz" or "Fixed=xyz".
"""
text = input_api.change.DescriptionText()
if input_api.re.search(r'^(Bug|Fixed)=',
text,
flags=input_api.re.IGNORECASE
| input_api.re.MULTILINE):
return [output_api.PresubmitError('Use Bug:/Fixed: instead of Bug=/Fixed=')]
return []
### Content checks
......
......@@ -9,6 +9,7 @@ import unittest
from presubmit_canned_checks_test_mocks import MockFile, MockAffectedFile
from presubmit_canned_checks_test_mocks import MockInputApi, MockOutputApi
from presubmit_canned_checks_test_mocks import MockChange
import presubmit_canned_checks
......@@ -200,5 +201,45 @@ class InclusiveLanguageCheckTest(unittest.TestCase):
self.assertEqual([], errors)
class DescriptionChecksTest(unittest.TestCase):
def testCheckDescriptionUsesColonInsteadOfEquals(self):
input_api = MockInputApi()
input_api.change.RepositoryRoot = lambda: ''
input_api.presubmit_local_path = ''
# Verify error in case of the attempt to use "Bug=".
input_api.change = MockChange([], 'Broken description\nBug=123')
errors = presubmit_canned_checks.CheckDescriptionUsesColonInsteadOfEquals(
input_api, MockOutputApi())
self.assertEqual(1, len(errors))
self.assertTrue('Bug=' in errors[0].message)
# Verify error in case of the attempt to use "Fixed=".
input_api.change = MockChange([], 'Broken description\nFixed=123')
errors = presubmit_canned_checks.CheckDescriptionUsesColonInsteadOfEquals(
input_api, MockOutputApi())
self.assertEqual(1, len(errors))
self.assertTrue('Fixed=' in errors[0].message)
# Verify error in case of the attempt to use the lower case "bug=".
input_api.change = MockChange([], 'Broken description lowercase\nbug=123')
errors = presubmit_canned_checks.CheckDescriptionUsesColonInsteadOfEquals(
input_api, MockOutputApi())
self.assertEqual(1, len(errors))
self.assertTrue('Bug=' in errors[0].message)
# Verify no error in case of "Bug:"
input_api.change = MockChange([], 'Correct description\nBug: 123')
errors = presubmit_canned_checks.CheckDescriptionUsesColonInsteadOfEquals(
input_api, MockOutputApi())
self.assertEqual(0, len(errors))
# Verify no error in case of "Fixed:"
input_api.change = MockChange([], 'Correct description\nFixed: 123')
errors = presubmit_canned_checks.CheckDescriptionUsesColonInsteadOfEquals(
input_api, MockOutputApi())
self.assertEqual(0, len(errors))
if __name__ == '__main__':
unittest.main()
......@@ -119,7 +119,7 @@ class MockInputApi(object):
def ReadFile(self, filename, mode='rU'):
if hasattr(filename, 'AbsoluteLocalPath'):
filename = filename.AbsoluteLocalPath()
filename = filename.AbsoluteLocalPath()
for file_ in self.files:
if file_.LocalPath() == filename:
return '\n'.join(file_.NewContents())
......@@ -243,9 +243,10 @@ class MockChange(object):
current change.
"""
def __init__(self, changed_files):
def __init__(self, changed_files, description=''):
self._changed_files = changed_files
self.footers = defaultdict(list)
self._description = description
def LocalPaths(self):
return self._changed_files
......@@ -257,3 +258,5 @@ class MockChange(object):
def GitFootersFromDescription(self):
return self.footers
def DescriptionText(self):
return self._description
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment