Skip to content

Commit

Permalink
ash: Adapt a presubmit check
Browse files Browse the repository at this point in the history
The check prompted to add an assert(is_chromeos_ash) if not present.
Since we are going to remove is_chromeos_ash in favor of is_chromeos
(due to Lacros removal), change the check to allow both flags and
change the error message so that it mentions is_chromeos.

Also fix an issue in the tests. They were using mock files of the same
name, so only the first one was actually used.

Bug: 373971535
Change-Id: I0510a23df5fc61ac8d4327df1835f9c84bf64392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5951185
Commit-Queue: Georg Neis <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1371890}
  • Loading branch information
GeorgNeis authored and Chromium LUCI CQ committed Oct 22, 2024
1 parent ef0a2ba commit 94f87f0
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 7 deletions.
11 changes: 6 additions & 5 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -7272,7 +7272,8 @@ def CheckConsistentGrdChanges(input_api, output_api):

def CheckAssertAshOnlyCode(input_api, output_api):
"""Errors if a BUILD.gn file in an ash/ directory doesn't include
assert(is_chromeos_ash).
assert(is_chromeos).
For a transition period, assert(is_chromeos_ash) is also accepted.
"""

def FileFilter(affected_file):
Expand All @@ -7285,16 +7286,16 @@ def FileFilter(affected_file):
files_to_skip=(input_api.DEFAULT_FILES_TO_SKIP))

errors = []
pattern = input_api.re.compile(r'assert\(is_chromeos_ash')
pattern = input_api.re.compile(r'assert\(is_chromeos(_ash)?\b')
for f in input_api.AffectedFiles(include_deletes=False,
file_filter=FileFilter):
if (not pattern.search(input_api.ReadFile(f))):
errors.append(
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 '
'Please add assert(is_chromeos) to %s. If that\'s not '
'possible, please create an issue and add a comment such '
'as:\n # TODO(crbug.com/XXX): add '
'assert(is_chromeos_ash) when ...' % f.LocalPath()))
'assert(is_chromeos) when ...' % f.LocalPath()))
return errors


Expand Down
10 changes: 8 additions & 2 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4913,7 +4913,7 @@ def testErrorsOnlyOnAshDirectories(self):
]
other_files = [
MockFile('chrome/browser/BUILD.gn', []),
MockFile('chrome/browser/BUILD.gn', ['assert(is_chromeos_ash)']),
MockFile('chrome/browser/foo/BUILD.gn', ['assert(is_chromeos_ash)']),
]
input_api = MockInputApi()
input_api.files = files_in_ash
Expand Down Expand Up @@ -4949,7 +4949,13 @@ def testDoesNotErrorWithAssertion(self):
MockFile('ash/BUILD.gn', ['assert(is_chromeos_ash)']),
MockFile('chrome/browser/ash/BUILD.gn',
['assert(is_chromeos_ash)']),
MockFile('chrome/browser/ash/BUILD.gn',
MockFile('chrome/browser/ash/1/BUILD.gn',
['assert(is_chromeos)']),
MockFile('chrome/browser/ash/2/BUILD.gn',
['assert(is_chromeos_ash)']),
MockFile('chrome/browser/ash/3/BUILD.gn',
['assert(is_chromeos, "test")']),
MockFile('chrome/browser/ash/4/BUILD.gn',
['assert(is_chromeos_ash, "test")']),
]
errors = PRESUBMIT.CheckAssertAshOnlyCode(input_api, MockOutputApi())
Expand Down

0 comments on commit 94f87f0

Please sign in to comment.