From f61b044bf0aa730556252bca90f99b99f0a7260e Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Tue, 30 Jul 2019 16:03:27 -0700 Subject: [PATCH] Support directories in sanity tests. --- test/runner/lib/sanity/__init__.py | 35 ++++++++++++++++--- test/runner/lib/sanity/ignores.py | 4 ++- .../code-smell/no-illegal-filenames.json | 2 +- .../sanity/code-smell/no-illegal-filenames.py | 23 +++--------- 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/test/runner/lib/sanity/__init__.py b/test/runner/lib/sanity/__init__.py index c0983352630..f747e07bc3d 100644 --- a/test/runner/lib/sanity/__init__.py +++ b/test/runner/lib/sanity/__init__.py @@ -25,6 +25,7 @@ from lib.util import ( get_available_python_versions, find_python, is_subdir, + paths_to_dirs, ) from lib.util_common import ( @@ -164,10 +165,13 @@ def command_sanity(args): if test.all_targets: usable_targets = targets.targets elif test.no_targets: - usable_targets = [] + usable_targets = tuple() else: usable_targets = targets.include + if test.include_directories: + usable_targets += tuple(TestTarget(path, None, None, '') for path in paths_to_dirs([target.path for target in usable_targets])) + usable_targets = sorted(test.filter_targets(list(usable_targets))) usable_targets = settings.filter_skipped_targets(usable_targets) sanity_targets = SanityTargets(targets.targets, tuple(usable_targets)) @@ -258,6 +262,7 @@ class SanityIgnoreParser: tests_by_name = {} # type: t.Dict[str, SanityTest] versioned_test_names = set() # type: t.Set[str] unversioned_test_names = {} # type: t.Dict[str, str] + directories = paths_to_dirs(list(paths)) display.info('Read %d sanity test ignore line(s) for %s from: %s' % (len(lines), ansible_label, self.relative_path), verbosity=1) @@ -282,9 +287,14 @@ class SanityIgnoreParser: self.parse_errors.append((line_no, 1, "Line cannot start with a space")) continue - if path not in paths: - self.file_not_found_errors.append((line_no, path)) - continue + if path.endswith(os.path.sep): + if path not in directories: + self.file_not_found_errors.append((line_no, path)) + continue + else: + if path not in paths: + self.file_not_found_errors.append((line_no, path)) + continue if not codes: self.parse_errors.append((line_no, len(path), "Error code required after path")) @@ -324,6 +334,10 @@ class SanityIgnoreParser: continue + if path.endswith(os.path.sep) and not test.include_directories: + self.parse_errors.append((line_no, 1, "Sanity test '%s' does not support directory paths" % test_name)) + continue + if commands and error_codes: self.parse_errors.append((line_no, len(path) + len(test_name) + 2, "Error code cannot contain both '!' and ':' characters")) continue @@ -564,6 +578,11 @@ class SanityTest(ABC): """True if the test does not use test targets. Mutually exclusive with all_targets.""" return False + @property + def include_directories(self): # type: () -> bool + """True if the test targets should include directories.""" + return False + @property def supported_python_versions(self): # type: () -> t.Optional[t.Tuple[str, ...]] """A tuple of supported Python versions or None if the test does not depend on specific Python versions.""" @@ -605,6 +624,7 @@ class SanityCodeSmellTest(SanityTest): self.__all_targets = self.config.get('all_targets') # type: bool self.__no_targets = self.config.get('no_targets') # type: bool + self.__include_directories = self.config.get('include_directories') # type: bool else: self.output = None self.extensions = [] @@ -615,6 +635,7 @@ class SanityCodeSmellTest(SanityTest): self.__all_targets = False self.__no_targets = True + self.__include_directories = False if self.no_targets: mutually_exclusive = ( @@ -624,6 +645,7 @@ class SanityCodeSmellTest(SanityTest): 'text', 'ignore_self', 'all_targets', + 'include_directories', ) problems = sorted(name for name in mutually_exclusive if getattr(self, name)) @@ -641,6 +663,11 @@ class SanityCodeSmellTest(SanityTest): """True if the test does not use test targets. Mutually exclusive with all_targets.""" return self.__no_targets + @property + def include_directories(self): # type: () -> bool + """True if the test targets should include directories.""" + return self.__include_directories + def filter_targets(self, targets): # type: (t.List[TestTarget]) -> t.List[TestTarget] """Return the given list of test targets, filtered to include only those relevant for the test.""" if self.no_targets: diff --git a/test/runner/lib/sanity/ignores.py b/test/runner/lib/sanity/ignores.py index 4a4740a88f7..3cee330a688 100644 --- a/test/runner/lib/sanity/ignores.py +++ b/test/runner/lib/sanity/ignores.py @@ -2,6 +2,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import os + from lib.sanity import ( SanityFailure, SanityIgnoreParser, @@ -56,7 +58,7 @@ class IgnoresTest(SanityVersionNeutral): # file not found errors messages.extend(SanityMessage( - message="File '%s' does not exist" % path, + message="%s '%s' does not exist" % ("Directory" if path.endswith(os.path.sep) else "File", path), path=sanity_ignore.relative_path, line=line, column=1, diff --git a/test/sanity/code-smell/no-illegal-filenames.json b/test/sanity/code-smell/no-illegal-filenames.json index 593b765d14a..cc4333bff5d 100644 --- a/test/sanity/code-smell/no-illegal-filenames.json +++ b/test/sanity/code-smell/no-illegal-filenames.json @@ -1,4 +1,4 @@ { - "no_targets": true, + "include_directories": true, "output": "path-message" } diff --git a/test/sanity/code-smell/no-illegal-filenames.py b/test/sanity/code-smell/no-illegal-filenames.py index b233a939100..99432ea133c 100755 --- a/test/sanity/code-smell/no-illegal-filenames.py +++ b/test/sanity/code-smell/no-illegal-filenames.py @@ -7,8 +7,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import os -import re import struct +import sys from ansible.module_utils.basic import to_bytes @@ -57,7 +57,7 @@ ILLEGAL_END_CHARS = [ def check_path(path, is_dir=False): type_name = 'directory' if is_dir else 'file' - file_name = os.path.basename(path) + file_name = os.path.basename(path.rstrip(os.path.sep)) name = os.path.splitext(file_name)[0] if name.upper() in ILLEGAL_NAMES: @@ -74,23 +74,8 @@ def check_path(path, is_dir=False): def main(): - pattern = re.compile("^test/integration/targets/.*/backup") - - for root, dirs, files in os.walk('.'): - if root == '.': - root = '' - elif root.startswith('./'): - root = root[2:] - - # ignore test/integration/targets/*/backup - if pattern.match(root): - continue - - for dir_name in dirs: - check_path(os.path.join(root, dir_name), is_dir=True) - - for file_name in files: - check_path(os.path.join(root, file_name), is_dir=False) + for path in sys.argv[1:] or sys.stdin.read().splitlines(): + check_path(path, is_dir=path.endswith(os.path.sep)) if __name__ == '__main__':