From d79114a2ed63efaef78e2e2b5e8c7d200836e89b Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Wed, 9 Dec 2020 23:28:27 +0000 Subject: [PATCH] [presubmit] Add check to prevent new metadata in OWNERS files. Bug: 1113033 Change-Id: I000cd307fea8862b9ef9cab907aafd357ee761bc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2582566 Commit-Queue: Edward Lesmes Auto-Submit: Edward Lesmes Reviewed-by: Josip Sokcevic --- presubmit_canned_checks.py | 27 +++++++++++++++++++++++++++ tests/presubmit_unittest.py | 25 ++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 0ab1c51e5b..c7ee5a0cef 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -1069,6 +1069,33 @@ def CheckDirMetadataFormat(input_api, output_api, dirmd_bin=None): name, cmd, kwargs, output_api.PresubmitError)] +def CheckNoNewMetadataInOwners(input_api, output_api): + """Check that no metadata is added to OWNERS files.""" + _METADATA_LINE_RE = input_api.re.compile( + r'^#\s*(TEAM|COMPONENT|OS|WPT-NOTIFY)+\s*:\s*\S+$', + input_api.re.MULTILINE | input_api.re.IGNORECASE) + affected_files = input_api.change.AffectedFiles( + include_deletes=False, + file_filter=lambda f: input_api.basename(f.LocalPath()) == 'OWNERS') + + errors = [] + for f in affected_files: + for _, line in f.ChangedContents(): + if _METADATA_LINE_RE.search(line): + errors.append(f.AbsoluteLocalPath()) + break + + if not errors: + return [] + + return [output_api.PresubmitError( + 'New metadata was added to the following OWNERS files, but should ' + 'have been added to DIR_METADATA files instead:\n' + + '\n'.join(errors) + '\n' + + 'See https://source.chromium.org/chromium/infra/infra/+/HEAD:' + 'go/src/infra/tools/dirmd/proto/dir_metadata.proto for details.')] + + def CheckOwnersDirMetadataExclusive(input_api, output_api): """Check that metadata in OWNERS files and DIR_METADATA files are mutually exclusive. diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 2bfc809919..4cab06be5d 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -2572,11 +2572,14 @@ the current line as well! change.AffectedFiles = lambda *a, **kw: ( presubmit.Change.AffectedFiles(change, *a, **kw)) change._affected_files = [] - for path, (action, _) in files.items(): + for path, (action, contents) in files.items(): affected_file = mock.MagicMock(presubmit.GitAffectedFile) affected_file.AbsoluteLocalPath.return_value = path affected_file.LocalPath.return_value = path affected_file.Action.return_value = action + affected_file.ChangedContents.return_value = [ + (1, contents or ''), + ] change._affected_files.append(affected_file) input_api = self.MockInputApi(None, False) @@ -2608,6 +2611,26 @@ the current line as well! self.assertEqual(1, len(commands)) self.assertEqual(expected_cmd, commands[0].cmd) + def testCheckNoNewMetadataInOwners(self): + input_api = self.GetInputApiWithFiles({ + 'no-new-metadata/OWNERS': ('M', '# WARNING: Blah'), + 'added-no-new-metadata/OWNERS': ('A', '# WARNING: Bleh'), + 'deleted/OWNERS': ('D', None), + }) + self.assertEqual( + [], + presubmit_canned_checks.CheckNoNewMetadataInOwners( + input_api, presubmit.OutputApi)) + + def testCheckNoNewMetadataInOwnersFails(self): + input_api = self.GetInputApiWithFiles({ + 'new-metadata/OWNERS': ('M', '# CoMpOnEnT: Monorail>Component'), + }) + results = presubmit_canned_checks.CheckNoNewMetadataInOwners( + input_api, presubmit.OutputApi) + self.assertEqual(1, len(results)) + self.assertIsInstance(results[0], presubmit.OutputApi.PresubmitError) + def testCheckOwnersDirMetadataExclusiveWorks(self): input_api = self.GetInputApiWithFiles({ 'only-owners/OWNERS': ('M', '# COMPONENT: Monorail>Component'),