diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 32a16e1471d4f4..2677a4e487a095 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -751,6 +751,26 @@ def _CheckFilePermissions(input_api, output_api): 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 @@ def _CommonChecks(input_api, output_api): 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 00000000000000..8b2c6bf2b37643 --- /dev/null +++ b/tools/checkteamtags/OWNERS @@ -0,0 +1,2 @@ +robertocn@chromium.org +stgao@chromium.org diff --git a/tools/checkteamtags/PRESUBMIT.py b/tools/checkteamtags/PRESUBMIT.py new file mode 100644 index 00000000000000..542eb1f622feaa --- /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 00000000000000..cfad8d1a934708 --- /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 ] ... + 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//chromium/src/ tools/OWNERS v8/OWNERS + python %prog /home//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 00000000000000..b99f66430edf35 --- /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 = """ +mock@chromium.org +""".splitlines() + +MULTIPLE_COMPONENT_TAGS = """ +mock@chromium.org + +# COMPONENT: Blink>mock_component +# COMPONENT: V8>mock_component +""".splitlines() + +MULTIPLE_COMPONENTS_IN_TAG = """ +mock@chromium.org + +# COMPONENT: Blink>mock_component, V8>mock_component +""".splitlines() + +MISSING_COMPONENT = """ +mock@chromium.org + +# COMPONENT: +""".splitlines() + +MULTIPLE_TEAM_TAGS = """ +mock@chromium.org + +# TEAM: some-team@chromium.org +# TEAM: some-other-team@chromium.org +""".splitlines() + +MULTIPLE_TEAMS_IN_TAG = """ +mock@chromium.org + +# TEAM: some-team@chromium.org some-other-team@chromium.org +""".splitlines() + +MISSING_TEAM = """ +mock@chromium.org + +# TEAM: +""".splitlines() + +BASIC = """ +mock@chromium.org + +# TEAM: some-team@chromium.org +# COMPONENT: V8>mock_component +""".splitlines() + +open_name = 'checkteamtags.open' + +@mock.patch('sys.argv', ['checkteamtags', '--bare' ,'OWNERS']) +@mock.patch('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 00000000000000..62040289ecdec1 --- /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())