Skip to content

Commit

Permalink
Add presubmit to ensure only updated bug IDs are added to the codebase
Browse files Browse the repository at this point in the history
I'm migrating all TODOs referencing bug IDs to the new issue tracker
ID numbers. To avoid backsliding, I'm adding this presubmit. This LSC
is documented at go/crbug-todo-migration.

Bug: 321899722
Change-Id: Ib2fdd299796bf89124d5419cf0908c7cf380fe29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5455308
Commit-Queue: Alison Gale <[email protected]>
Reviewed-by: Nico Weber <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1288678}
  • Loading branch information
Alison Gale authored and Chromium LUCI CQ committed Apr 17, 2024
1 parent 3d65a40 commit d6b25fe
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 2 deletions.
35 changes: 33 additions & 2 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -6829,7 +6829,7 @@ def FileFilter(affected_file):
output_api.PresubmitError(
'Please add assert(is_chromeos_ash) to %s. If that\'s not '
'possible, please create and issue and add a comment such '
'as:\n # TODO(https://crbug.com/XXX): add '
'as:\n # TODO(crbug.com/XXX): add '
'assert(is_chromeos_ash) when ...' % f.LocalPath()))
return errors

Expand All @@ -6855,7 +6855,7 @@ def _IsMiraclePtrDisallowed(input_api, affected_file):
# We assume that everything else may be used outside of Renderer processes.
return False

# TODO(https://crbug.com/1273182): Remove these checks, once they are replaced
# TODO(crbug.com/40206238): Remove these checks, once they are replaced
# by the Chromium Clang Plugin (which will be preferable because it will
# 1) report errors earlier - at compile-time and 2) cover more rules).
def CheckRawPtrUsage(input_api, output_api):
Expand Down Expand Up @@ -7298,3 +7298,34 @@ def CheckInlineConstexprDefinitionsInHeaders(input_api, output_api):
]
else:
return []

def CheckTodoBugReferences(input_api, output_api):
"""Checks that bugs in TODOs use updated issue tracker IDs."""

files_to_skip = ['PRESUBMIT_test.py']

def _FilterFile(affected_file):
return input_api.FilterSourceFile(
affected_file,
files_to_skip=files_to_skip)

# Monorail bug IDs are all less than or equal to 1524553 so check that all
# bugs in TODOs are greater than that value.
pattern = input_api.re.compile(r'.*TODO\([^\)0-9]*([0-9]+)\).*')
problems = []
for f in input_api.AffectedSourceFiles(_FilterFile):
for line_number, line in f.ChangedContents():
match = pattern.match(line)
if match and int(match.group(1)) <= 1524553:
problems.append(
f"{f.LocalPath()}: {line_number}\n {line}")

if problems:
return [
output_api.PresubmitPromptWarning(
'TODOs should use the new Chromium Issue Tracker IDs. '
'See https://crbug.com/321899722 for more details.',
problems)
]
else:
return []
18 changes: 18 additions & 0 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -5310,5 +5310,23 @@ def testNoExplicitInlineConstexprInsideClassInHeaderFile(self):
warnings = PRESUBMIT.CheckInlineConstexprDefinitionsInHeaders(input_api, MockOutputApi())
self.assertEqual(0, len(warnings))

def testTodoBugReferencesWithOldBugId(self):
"""Tests that an old monorail bug ID in a TODO fails."""
input_api = MockInputApi()
input_api.files = [
MockAffectedFile('src/helpers.h', ['// TODO(crbug.com/12345)'])
]
warnings = PRESUBMIT.CheckTodoBugReferences(input_api, MockOutputApi())
self.assertEqual(1, len(warnings))

def testTodoBugReferencesWithUpdatedBugId(self):
"""Tests that a new issue tracker bug ID in a TODO passes."""
input_api = MockInputApi()
input_api.files = [
MockAffectedFile('src/helpers.h', ['// TODO(crbug.com/40781525)'])
]
warnings = PRESUBMIT.CheckTodoBugReferences(input_api, MockOutputApi())
self.assertEqual(0, len(warnings))

if __name__ == '__main__':
unittest.main()

0 comments on commit d6b25fe

Please sign in to comment.