Fix CheckNoAbbreviationInPngFileName checks path components.
Modify CheckNoAbbreviationInPngFileName presubmit rule and add unittest for it to ensure it only checks the file name, not every path component ahead. Change-Id: I8743dcec9966c1b8ecdb6dd8d8fe6836dbed1793 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4054771 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Yuanqing Zhu <yuanqingzhu@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1079007}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
8389d9df6b
commit
9eef0283e2
@ -2586,23 +2586,25 @@ def CheckChromeOsSyncedPrefRegistration(input_api, output_api):
|
||||
return results
|
||||
|
||||
|
||||
# TODO: add unit tests.
|
||||
def CheckNoAbbreviationInPngFileName(input_api, output_api):
|
||||
"""Makes sure there are no abbreviations in the name of PNG files.
|
||||
The native_client_sdk directory is excluded because it has auto-generated PNG
|
||||
files for documentation.
|
||||
"""
|
||||
errors = []
|
||||
files_to_check = [r'.*_[a-z]_.*\.png$|.*_[a-z]\.png$']
|
||||
files_to_check = [r'.*\.png$']
|
||||
files_to_skip = [r'^native_client_sdk/',
|
||||
r'^services/test/',
|
||||
r'^third_party/blink/web_tests/',
|
||||
]
|
||||
file_filter = lambda f: input_api.FilterSourceFile(
|
||||
f, files_to_check=files_to_check, files_to_skip=files_to_skip)
|
||||
abbreviation = input_api.re.compile('.+_[a-z]\.png|.+_[a-z]_.*\.png')
|
||||
for f in input_api.AffectedFiles(include_deletes=False,
|
||||
file_filter=file_filter):
|
||||
errors.append(' %s' % f.LocalPath())
|
||||
file_name = input_api.os_path.split(f.LocalPath())[1]
|
||||
if abbreviation.search(file_name):
|
||||
errors.append(' %s' % f.LocalPath())
|
||||
|
||||
results = []
|
||||
if errors:
|
||||
|
@ -4859,5 +4859,42 @@ class AssertNoJsInIosTest(unittest.TestCase):
|
||||
self.assertEqual('warning', results[0].type)
|
||||
self.assertEqual(1, len(results[0].items))
|
||||
|
||||
class CheckNoAbbreviationInPngFileNameTest(unittest.TestCase):
|
||||
def testHasAbbreviation(self):
|
||||
"""test png file names with abbreviation that fails the check"""
|
||||
input_api = MockInputApi()
|
||||
input_api.files = [
|
||||
MockFile('image_a.png', [], action='A'),
|
||||
MockFile('image_a_.png', [], action='A'),
|
||||
MockFile('image_a_name.png', [], action='A'),
|
||||
MockFile('chrome/ui/feature_name/resources/image_a.png', [], action='A'),
|
||||
MockFile('chrome/ui/feature_name/resources/image_a_.png', [], action='A'),
|
||||
MockFile('chrome/ui/feature_name/resources/image_a_name.png', [], action='A'),
|
||||
]
|
||||
results = PRESUBMIT.CheckNoAbbreviationInPngFileName(input_api, MockOutputApi())
|
||||
self.assertEqual(1, len(results))
|
||||
self.assertEqual('error', results[0].type)
|
||||
self.assertEqual(len(input_api.files), len(results[0].items))
|
||||
|
||||
def testNoAbbreviation(self):
|
||||
"""test png file names without abbreviation that passes the check"""
|
||||
input_api = MockInputApi()
|
||||
input_api.files = [
|
||||
MockFile('a.png', [], action='A'),
|
||||
MockFile('_a.png', [], action='A'),
|
||||
MockFile('image.png', [], action='A'),
|
||||
MockFile('image_ab_.png', [], action='A'),
|
||||
MockFile('image_ab_name.png', [], action='A'),
|
||||
# These paths used to fail because `feature_a_name` matched the regex by mistake.
|
||||
# They should pass now because the path components ahead of the file name are ignored in the check.
|
||||
MockFile('chrome/ui/feature_a_name/resources/a.png', [], action='A'),
|
||||
MockFile('chrome/ui/feature_a_name/resources/_a.png', [], action='A'),
|
||||
MockFile('chrome/ui/feature_a_name/resources/image.png', [], action='A'),
|
||||
MockFile('chrome/ui/feature_a_name/resources/image_ab_.png', [], action='A'),
|
||||
MockFile('chrome/ui/feature_a_name/resources/image_ab_name.png', [], action='A'),
|
||||
]
|
||||
results = PRESUBMIT.CheckNoAbbreviationInPngFileName(input_api, MockOutputApi())
|
||||
self.assertEqual(0, len(results))
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
Reference in New Issue
Block a user