Allow include/exclude options in several checks.
check_no_long_lines, check_no_stray_whitespace, check_no_tabs,
check_tabbed_indents will accept options to include/exclude
specific files from checking.
We can often run better checks (e.g. pylint) than those naive
checks. In such cases, it is a good idea to exclude files from
checks partially (e.g. *.py only), rather than totally opting
out.
BUG=None
TEST=pre-upload_unittest.py
Change-Id: Ie3015e1d30064e3335d677657b7613f0315e4df0
Reviewed-on: https://chromium-review.googlesource.com/564925
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Shuhei Takahashi <nya@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index a8c4240..c061f4f 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -423,10 +423,28 @@
return HookFailure(error_description, errors)
+def _parse_common_inclusion_options(options):
+ """Parses common hook options for including/excluding files.
+
+ Args:
+ options: Option string list.
+
+ Returns:
+ (included, excluded) where each one is a list of regex strings.
+ """
+ parser = argparse.ArgumentParser()
+ parser.add_argument('--exclude_regex', action='append')
+ parser.add_argument('--include_regex', action='append')
+ opts = parser.parse_args(options)
+ included = opts.include_regex or []
+ excluded = opts.exclude_regex or []
+ return included, excluded
+
+
# Common Hooks
-def _check_no_long_lines(_project, commit):
+def _check_no_long_lines(_project, commit, options=()):
"""Checks there are no lines longer than MAX_LEN in any of the text files."""
MAX_LEN = 80
@@ -434,11 +452,12 @@
r'https?://',
r'^#\s*(define|include|import|pragma|if|endif)\b']))
- errors = []
+ included, excluded = _parse_common_inclusion_options(options)
files = _filter_files(_get_affected_files(commit),
- COMMON_INCLUDED_PATHS,
- COMMON_EXCLUDED_PATHS)
+ included + COMMON_INCLUDED_PATHS,
+ excluded + COMMON_EXCLUDED_PATHS)
+ errors = []
for afile in files:
for line_num, line in _get_file_diff(afile, commit):
# Allow certain lines to exceed the maxlen rule.
@@ -454,17 +473,18 @@
return HookFailure(msg, errors)
-def _check_no_stray_whitespace(_project, commit):
+def _check_no_stray_whitespace(_project, commit, options=()):
"""Checks that there is no stray whitespace at source lines end."""
+ included, excluded = _parse_common_inclusion_options(options)
files = _filter_files(_get_affected_files(commit),
- COMMON_INCLUDED_PATHS,
- COMMON_EXCLUDED_PATHS)
+ included + COMMON_INCLUDED_PATHS,
+ excluded + COMMON_EXCLUDED_PATHS)
return _check_lines_in_diff(commit, files,
lambda line: line.rstrip() != line,
'Found line ending with white space in:')
-def _check_no_tabs(_project, commit):
+def _check_no_tabs(_project, commit, options=()):
"""Checks there are no unexpanded tabs."""
TAB_OK_PATHS = [
r"/src/third_party/u-boot/",
@@ -475,15 +495,16 @@
r".*\.mk$"
]
+ included, excluded = _parse_common_inclusion_options(options)
files = _filter_files(_get_affected_files(commit),
- COMMON_INCLUDED_PATHS,
- COMMON_EXCLUDED_PATHS + TAB_OK_PATHS)
+ included + COMMON_INCLUDED_PATHS,
+ excluded + COMMON_EXCLUDED_PATHS + TAB_OK_PATHS)
return _check_lines_in_diff(commit, files,
lambda line: '\t' in line,
'Found a tab character in:')
-def _check_tabbed_indents(_project, commit):
+def _check_tabbed_indents(_project, commit, options=()):
"""Checks that indents use tabs only."""
TABS_REQUIRED_PATHS = [
r".*\.ebuild$",
@@ -491,9 +512,10 @@
]
LEADING_SPACE_RE = re.compile('[\t]* ')
+ included, excluded = _parse_common_inclusion_options(options)
files = _filter_files(_get_affected_files(commit),
- TABS_REQUIRED_PATHS,
- COMMON_EXCLUDED_PATHS)
+ included + TABS_REQUIRED_PATHS,
+ excluded + COMMON_EXCLUDED_PATHS)
return _check_lines_in_diff(
commit, files,
lambda line: LEADING_SPACE_RE.match(line) is not None,
@@ -1009,12 +1031,7 @@
)
copyright_re = re.compile(COPYRIGHT_LINE)
- parser = argparse.ArgumentParser()
- parser.add_argument('--exclude_regex', action='append')
- parser.add_argument('--include_regex', action='append')
- opts = parser.parse_args(options)
- included = opts.include_regex or []
- excluded = opts.exclude_regex or []
+ included, excluded = _parse_common_inclusion_options(options)
bad_files = []
bad_copyright_files = []
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index bc03c27..6e517d2 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -54,11 +54,10 @@
"""Tests for _check_no_long_lines."""
def setUp(self):
- self.PatchObject(pre_upload, '_get_affected_files', return_value=['x.py'])
- self.PatchObject(pre_upload, '_filter_files', return_value=['x.py'])
self.diff_mock = self.PatchObject(pre_upload, '_get_file_diff')
- def runTest(self):
+ def testCheck(self):
+ self.PatchObject(pre_upload, '_get_affected_files', return_value=['x.py'])
self.diff_mock.return_value = [
(1, u"x" * 80), # OK
(2, "\x80" * 80), # OK
@@ -77,12 +76,33 @@
for line in [3, 4, 8]],
failure.items)
+ def testIncludeOptions(self):
+ self.PatchObject(pre_upload,
+ '_get_affected_files',
+ return_value=['foo.txt'])
+ self.diff_mock.return_value = [(1, u"x" * 81)]
+ self.assertFalse(pre_upload._check_no_long_lines(
+ ProjectNamed('PROJECT'), 'COMMIT'))
+ self.assertTrue(pre_upload._check_no_long_lines(
+ ProjectNamed('PROJECT'), 'COMMIT', options=['--include_regex=foo']))
+
+ def testExcludeOptions(self):
+ self.PatchObject(pre_upload,
+ '_get_affected_files',
+ return_value=['foo.py'])
+ self.diff_mock.return_value = [(1, u"x" * 81)]
+ self.assertTrue(pre_upload._check_no_long_lines(
+ ProjectNamed('PROJECT'), 'COMMIT'))
+ self.assertFalse(pre_upload._check_no_long_lines(
+ ProjectNamed('PROJECT'), 'COMMIT', options=['--exclude_regex=foo']))
+
class CheckTabbedIndentsTest(cros_test_lib.MockTestCase):
"""Tests for _check_tabbed_indents."""
def setUp(self):
- self.PatchObject(pre_upload, '_get_affected_files', return_value=['x.py'])
- self.PatchObject(pre_upload, '_filter_files', return_value=['x.py'])
+ self.PatchObject(pre_upload,
+ '_get_affected_files',
+ return_value=['x.ebuild'])
self.diff_mock = self.PatchObject(pre_upload, '_get_file_diff')
def test_good_cases(self):
@@ -110,7 +130,7 @@
self.assertTrue(failure)
self.assertEquals('Found a space in indentation (must be all tabs):',
failure.msg)
- self.assertEquals(['x.py, line %d' % line for line in xrange(1, 6)],
+ self.assertEquals(['x.ebuild, line %d' % line for line in xrange(1, 6)],
failure.items)
