Adding check for team and component tags in owners files.
As a part of presubmit, if an OWNERS file is modified or added the
following checks will be performed:
- Ensure that at most one line starts with `# COMPONENT: ` exists in the
file.
- Ensure that at most one line starts with `# TEAM: ` exists in the
file, and that it contains exactly one email address.
[email protected],[email protected],[email protected]
BUG=667954
Review-Url: https://codereview.chromium.org/2601773004
Cr-Commit-Position: refs/heads/master@{#441420}
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 32a16e14..2677a4e4 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -751,6 +751,26 @@
long_text=error.output)]
+def _CheckTeamTags(input_api, output_api):
+ """Checks that OWNERS files have consistent TEAM and COMPONENT tags."""
+ checkteamtags_tool = input_api.os_path.join(
+ input_api.PresubmitLocalPath(),
+ 'tools', 'checkteamtags', 'checkteamtags.py')
+ args = [input_api.python_executable, checkteamtags_tool,
+ '--root', input_api.change.RepositoryRoot()]
+ files = [f.LocalPath() for f in input_api.AffectedFiles()
+ if input_api.os_path.basename(f.AbsoluteLocalPath()).upper() ==
+ 'OWNERS']
+ try:
+ if files:
+ input_api.subprocess.check_output(args + files)
+ return []
+ except input_api.subprocess.CalledProcessError as error:
+ return [output_api.PresubmitError(
+ 'checkteamtags.py failed:',
+ long_text=error.output)]
+
+
def _CheckNoAuraWindowPropertyHInHeaders(input_api, output_api):
"""Makes sure we don't include ui/aura/window_property.h
in header files.
@@ -2045,6 +2065,7 @@
results.extend(_CheckNoTrinaryTrueFalse(input_api, output_api))
results.extend(_CheckUnwantedDependencies(input_api, output_api))
results.extend(_CheckFilePermissions(input_api, output_api))
+ results.extend(_CheckTeamTags(input_api, output_api))
results.extend(_CheckNoAuraWindowPropertyHInHeaders(input_api, output_api))
results.extend(_CheckIncludeOrder(input_api, output_api))
results.extend(_CheckForVersionControlConflicts(input_api, output_api))
diff --git a/tools/checkteamtags/OWNERS b/tools/checkteamtags/OWNERS
new file mode 100644
index 0000000..8b2c6bf2
--- /dev/null
+++ b/tools/checkteamtags/OWNERS
@@ -0,0 +1,2 @@
[email protected]
[email protected]
diff --git a/tools/checkteamtags/PRESUBMIT.py b/tools/checkteamtags/PRESUBMIT.py
new file mode 100644
index 0000000..542eb1f
--- /dev/null
+++ b/tools/checkteamtags/PRESUBMIT.py
@@ -0,0 +1,46 @@
+# Copyright (c) 2017 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Top-level presubmit script for checkteamtags
+
+See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts for
+details on the presubmit API.
+"""
+
+import subprocess
+
+
+def CheckChangeOnUpload(input_api, output_api):
+ return _CommonChecks(input_api, output_api)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+ return _CommonChecks(input_api, output_api)
+
+
+def _CommonChecks(input_api, output_api):
+ """Does all presubmit checks for chekteamtags."""
+ results = []
+ results.extend(_RunUnitTests(input_api, output_api))
+ results.extend(_RunPyLint(input_api, output_api))
+ return results
+
+def _RunUnitTests(input_api, output_api):
+ """Runs unit tests for checkteamtags."""
+ repo_root = input_api.change.RepositoryRoot()
+ checkteamtags_dir = input_api.os_path.join(repo_root, 'tools',
+ 'checkteamtags')
+ test_runner = input_api.os_path.join(checkteamtags_dir, 'run_tests')
+ return_code = subprocess.call(['python', test_runner])
+ if return_code:
+ message = 'Checkteamtags unit tests did not all pass.'
+ return [output_api.PresubmitError(message)]
+ return []
+
+
+def _RunPyLint(input_api, output_api):
+ """Runs unit tests for checkteamtags."""
+ tests = input_api.canned_checks.GetPylint(
+ input_api, output_api)
+ return input_api.RunTests(tests)
diff --git a/tools/checkteamtags/checkteamtags.py b/tools/checkteamtags/checkteamtags.py
new file mode 100755
index 0000000..cfad8d1a
--- /dev/null
+++ b/tools/checkteamtags/checkteamtags.py
@@ -0,0 +1,117 @@
+#!/usr/bin/env python
+# Copyright (c) 2017 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Makes sure OWNERS files have consistent TEAM and COMPONENT tags."""
+
+
+import json
+import logging
+import optparse
+import os
+import sys
+
+
+def check_owners(root, owners_path):
+ """Component and Team check in OWNERS files. crbug.com/667954"""
+ if root:
+ full_path = os.path.join(root, owners_path)
+ rel_path = owners_path
+ else:
+ full_path = os.path.abspath(owners_path)
+ rel_path = os.path.relpath(owners_path)
+
+ def result_dict(error):
+ return {
+ 'error': error,
+ 'full_path': full_path,
+ 'rel_path': rel_path,
+ }
+
+ with open(full_path) as f:
+ owners_file_lines = f.readlines()
+
+ component_entries = [l for l in owners_file_lines if l.split()[:2] ==
+ ['#', 'COMPONENT:']]
+ team_entries = [l for l in owners_file_lines if l.split()[:2] ==
+ ['#', 'TEAM:']]
+ if len(component_entries) > 1:
+ return result_dict('Contains more than one component per directory')
+ if len(team_entries) > 1:
+ return result_dict('Contains more than one team per directory')
+
+ if not component_entries and not team_entries:
+ return
+
+ if component_entries:
+ component = component_entries[0].split(':')[1]
+ if not component:
+ return result_dict('Has COMPONENT line but no component name')
+ # Check for either of the following formats:
+ # component1, component2, ...
+ # component1,component2,...
+ # component1 component2 ...
+ component_count = max(
+ len(component.strip().split()),
+ len(component.strip().split(',')))
+ if component_count > 1:
+ return result_dict('Has more than one component name')
+ # TODO(robertocn): Check against a static list of valid components,
+ # perhaps obtained from monorail at the beginning of presubmit.
+
+ if team_entries:
+ team_entry_parts = team_entries[0].split('@')
+ if len(team_entry_parts) != 2:
+ return result_dict('Has TEAM line, but not exactly 1 team email')
+ # TODO(robertocn): Raise a warning if only one of (COMPONENT, TEAM) is
+ # present.
+
+
+def main():
+ usage = """Usage: python %prog [--root <dir>] <owners_file1> <owners_file2>...
+ owners_fileX specifies the path to the file to check, these are expected
+ to be relative to the root directory if --root is used.
+
+Examples:
+ python %prog --root /home/<user>/chromium/src/ tools/OWNERS v8/OWNERS
+ python %prog /home/<user>/chromium/src/tools/OWNERS
+ python %prog ./OWNERS
+ """
+
+ parser = optparse.OptionParser(usage=usage)
+ parser.add_option(
+ '--root', help='Specifies the repository root.')
+ parser.add_option(
+ '-v', '--verbose', action='count', default=0, help='Print debug logging')
+ parser.add_option(
+ '--bare',
+ action='store_true',
+ default=False,
+ help='Prints the bare filename triggering the checks')
+ parser.add_option('--json', help='Path to JSON output file')
+ options, args = parser.parse_args()
+
+ levels = [logging.ERROR, logging.INFO, logging.DEBUG]
+ logging.basicConfig(level=levels[min(len(levels) - 1, options.verbose)])
+
+ errors = filter(None, [check_owners(options.root, f) for f in args])
+
+ if options.json:
+ with open(options.json, 'w') as f:
+ json.dump(errors, f)
+
+ if errors:
+ if options.bare:
+ print '\n'.join(e['full_path'] for e in errors)
+ else:
+ print '\nFAILED\n'
+ print '\n'.join('%s: %s' % (e['full_path'], e['error']) for e in errors)
+ return 1
+ if not options.bare:
+ print '\nSUCCESS\n'
+ return 0
+
+
+if '__main__' == __name__:
+ sys.exit(main())
diff --git a/tools/checkteamtags/checkteamtags_test.py b/tools/checkteamtags/checkteamtags_test.py
new file mode 100644
index 0000000..b99f664
--- /dev/null
+++ b/tools/checkteamtags/checkteamtags_test.py
@@ -0,0 +1,119 @@
+# Copyright 2017 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import os
+import sys
+import unittest
+
+import checkteamtags
+
+SRC = os.path.join(os.path.dirname(__file__), os.path.pardir, os.path.pardir)
+sys.path.append(os.path.join(SRC, 'third_party', 'pymock'))
+
+import mock
+
+
+def mock_file(lines):
+ inner_mock = mock.MagicMock()
+ inner_attrs = {'readlines.return_value': lines}
+ inner_mock.configure_mock(**inner_attrs)
+
+ return_val = mock.MagicMock()
+ attrs = {'__enter__.return_value': inner_mock}
+ return_val.configure_mock(**attrs)
+ return return_val
+
+NO_TAGS = """
[email protected]
+""".splitlines()
+
+MULTIPLE_COMPONENT_TAGS = """
[email protected]
+
+# COMPONENT: Blink>mock_component
+# COMPONENT: V8>mock_component
+""".splitlines()
+
+MULTIPLE_COMPONENTS_IN_TAG = """
[email protected]
+
+# COMPONENT: Blink>mock_component, V8>mock_component
+""".splitlines()
+
+MISSING_COMPONENT = """
[email protected]
+
+# COMPONENT:
+""".splitlines()
+
+MULTIPLE_TEAM_TAGS = """
[email protected]
+
+# TEAM: [email protected]
+# TEAM: [email protected]
+""".splitlines()
+
+MULTIPLE_TEAMS_IN_TAG = """
[email protected]
+
+# TEAM: [email protected] [email protected]
+""".splitlines()
+
+MISSING_TEAM = """
[email protected]
+
+# TEAM:
+""".splitlines()
+
+BASIC = """
[email protected]
+
+# TEAM: [email protected]
+# COMPONENT: V8>mock_component
+""".splitlines()
+
+open_name = 'checkteamtags.open'
+
[email protected]('sys.argv', ['checkteamtags', '--bare' ,'OWNERS'])
[email protected]('sys.stdout', mock.MagicMock())
+class CheckTeamTagsTest(unittest.TestCase):
+ def testNoTags(self):
+ with mock.patch(open_name, create=True) as mock_open:
+ mock_open.return_value = mock_file(NO_TAGS)
+ self.assertEqual(0, checkteamtags.main())
+
+ def testMultipleComponentTags(self):
+ with mock.patch(open_name, create=True) as mock_open:
+ mock_open.return_value = mock_file(MULTIPLE_COMPONENT_TAGS)
+ self.assertEqual(1, checkteamtags.main())
+
+ def testMultipleComponentsInTag(self):
+ with mock.patch(open_name, create=True) as mock_open:
+ mock_open.return_value = mock_file(MULTIPLE_COMPONENTS_IN_TAG)
+ self.assertEqual(1, checkteamtags.main())
+
+ def testMissingComponent(self):
+ with mock.patch(open_name, create=True) as mock_open:
+ mock_open.return_value = mock_file(MISSING_COMPONENT)
+ self.assertEqual(1, checkteamtags.main())
+
+ def testMultipleTeamTags(self):
+ with mock.patch(open_name, create=True) as mock_open:
+ mock_open.return_value = mock_file(MULTIPLE_TEAM_TAGS)
+ self.assertEqual(1, checkteamtags.main())
+
+ def testMultipleTeamsInTag(self):
+ with mock.patch(open_name, create=True) as mock_open:
+ mock_open.return_value = mock_file(MULTIPLE_TEAMS_IN_TAG)
+ self.assertEqual(1, checkteamtags.main())
+
+ def testMissingTeam(self):
+ with mock.patch(open_name, create=True) as mock_open:
+ mock_open.return_value = mock_file(MISSING_TEAM)
+ self.assertEqual(1, checkteamtags.main())
+
+ def testBasic(self):
+ with mock.patch(open_name, create=True) as mock_open:
+ mock_open.return_value = mock_file(BASIC)
+ self.assertEqual(0, checkteamtags.main())
diff --git a/tools/checkteamtags/run_tests b/tools/checkteamtags/run_tests
new file mode 100755
index 0000000..62040289
--- /dev/null
+++ b/tools/checkteamtags/run_tests
@@ -0,0 +1,41 @@
+#!/usr/bin/env python
+# Copyright 2017 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Runs all tests in all unit test modules in this directory."""
+
+import os
+import sys
+import unittest
+import logging
+
+SRC = os.path.join(os.path.dirname(__file__), os.path.pardir, os.path.pardir)
+
+
+def main():
+ if 'full-log' in sys.argv:
+ # Configure logging to show line numbers and logging level
+ fmt = '%(module)s:%(lineno)d - %(levelname)s: %(message)s'
+ logging.basicConfig(level=logging.DEBUG, stream=sys.stdout, format=fmt)
+ elif 'no-log' in sys.argv:
+ # Only WARN and above are shown, to standard error. (This is the logging
+ # module default config, hence we do nothing here)
+ pass
+ else:
+ # Behave as before. Make logging.info mimic print behavior
+ fmt = '%(message)s'
+ logging.basicConfig(level=logging.INFO, stream=sys.stdout, format=fmt)
+
+ suite = unittest.TestSuite()
+ loader = unittest.TestLoader()
+ script_dir = os.path.dirname(__file__)
+ suite.addTests(loader.discover(start_dir=script_dir, pattern='*_test.py'))
+
+ print 'Running unit tests in %s...' % os.path.abspath(script_dir)
+ result = unittest.TextTestRunner(verbosity=1).run(suite)
+ return 0 if result.wasSuccessful() else 1
+
+
+if __name__ == '__main__':
+ sys.exit(main())
