Skip to content

Commit

Permalink
Adding check for team and component tags in owners files.
Browse files Browse the repository at this point in the history
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.

R=stgao@chromium.org,dpranke@chromium.org,phajdan.jr@chromium.org
BUG=667954

Review-Url: https://codereview.chromium.org/2601773004
Cr-Commit-Position: refs/heads/master@{#441420}
  • Loading branch information
ro-berto authored and Commit bot committed Jan 4, 2017
1 parent bcf1404 commit 832f599
Show file tree
Hide file tree
Showing 6 changed files with 346 additions and 0 deletions.
21 changes: 21 additions & 0 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 2 additions & 0 deletions tools/checkteamtags/OWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
robertocn@chromium.org
stgao@chromium.org
46 changes: 46 additions & 0 deletions tools/checkteamtags/PRESUBMIT.py
Original file line number Diff line number Diff line change
@@ -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)
117 changes: 117 additions & 0 deletions tools/checkteamtags/checkteamtags.py
Original file line number Diff line number Diff line change
@@ -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())
119 changes: 119 additions & 0 deletions tools/checkteamtags/checkteamtags_test.py
Original file line number Diff line number Diff line change
@@ -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())
41 changes: 41 additions & 0 deletions tools/checkteamtags/run_tests
Original file line number Diff line number Diff line change
@@ -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())

0 comments on commit 832f599

Please sign in to comment.