From e07c4f41d71d0c78c6622d115059a7264e0de847 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Tue, 30 Jul 2019 11:10:55 -0700 Subject: [PATCH] Standardize ansible-test sanity test filters. --- test/runner/lib/sanity/__init__.py | 190 ++++++++++++------ test/runner/lib/sanity/ansible_doc.py | 74 ++++--- test/runner/lib/sanity/compile.py | 19 +- test/runner/lib/sanity/ignores.py | 6 +- test/runner/lib/sanity/import.py | 24 +-- test/runner/lib/sanity/integration_aliases.py | 6 +- test/runner/lib/sanity/pep8.py | 12 +- test/runner/lib/sanity/pslint.py | 14 +- test/runner/lib/sanity/pylint.py | 11 +- test/runner/lib/sanity/rstcheck.py | 17 +- test/runner/lib/sanity/sanity_docs.py | 6 +- test/runner/lib/sanity/shellcheck.py | 14 +- test/runner/lib/sanity/validate_modules.py | 15 +- test/runner/lib/sanity/yamllint.py | 36 ++-- .../sanity/code-smell/action-plugin-docs.json | 2 +- .../sanity/code-smell/azure-requirements.json | 2 +- test/sanity/code-smell/botmeta.json | 2 +- .../code-smell/configure-remoting-ps1.json | 2 +- test/sanity/code-smell/deprecated-config.json | 2 +- test/sanity/code-smell/docs-build.json | 2 +- .../code-smell/no-illegal-filenames.json | 2 +- test/sanity/code-smell/package-data.json | 2 +- test/sanity/code-smell/symlinks.json | 2 +- test/sanity/code-smell/update-bundled.json | 2 +- test/sanity/ignore.txt | 1 + 25 files changed, 289 insertions(+), 176 deletions(-) diff --git a/test/runner/lib/sanity/__init__.py b/test/runner/lib/sanity/__init__.py index 3fe9ed56b27..c0983352630 100644 --- a/test/runner/lib/sanity/__init__.py +++ b/test/runner/lib/sanity/__init__.py @@ -24,6 +24,7 @@ from lib.util import ( read_lines_without_comments, get_available_python_versions, find_python, + is_subdir, ) from lib.util_common import ( @@ -78,7 +79,7 @@ def command_sanity(args): """ changes = get_changes_filter(args) require = args.require + changes - targets = SanityTargets(args.include, args.exclude, require) + targets = SanityTargets.create(args.include, args.exclude, require) if not targets.include: raise AllTargetsSkipped() @@ -128,14 +129,19 @@ def command_sanity(args): versions = versions[:1] or (args.python_version,) for version in versions: + if isinstance(test, SanityMultipleVersion): + skip_version = version + else: + skip_version = None + options = '' if test.supported_python_versions and version not in test.supported_python_versions: display.warning("Skipping sanity test '%s' on unsupported Python %s." % (test.name, version)) - result = SanitySkipped(test.name) + result = SanitySkipped(test.name, skip_version) elif not args.python and version not in available_versions: display.warning("Skipping sanity test '%s' on Python %s due to missing interpreter." % (test.name, version)) - result = SanitySkipped(test.name) + result = SanitySkipped(test.name, skip_version) else: check_pyyaml(args, version) @@ -145,17 +151,42 @@ def command_sanity(args): display.info("Running sanity test '%s'" % test.name) if isinstance(test, SanityCodeSmellTest): - result = test.test(args, targets, version) + settings = test.load_processor(args) elif isinstance(test, SanityMultipleVersion): - result = test.test(args, targets, version) - options = ' --python %s' % version + settings = test.load_processor(args, version) elif isinstance(test, SanitySingleVersion): - result = test.test(args, targets, version) + settings = test.load_processor(args) elif isinstance(test, SanityVersionNeutral): - result = test.test(args, targets) + settings = test.load_processor(args) else: raise Exception('Unsupported test type: %s' % type(test)) + if test.all_targets: + usable_targets = targets.targets + elif test.no_targets: + usable_targets = [] + else: + usable_targets = targets.include + + 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)) + + if usable_targets or test.no_targets: + if isinstance(test, SanityCodeSmellTest): + result = test.test(args, sanity_targets, version) + elif isinstance(test, SanityMultipleVersion): + result = test.test(args, sanity_targets, version) + options = ' --python %s' % version + elif isinstance(test, SanitySingleVersion): + result = test.test(args, sanity_targets, version) + elif isinstance(test, SanityVersionNeutral): + result = test.test(args, sanity_targets) + else: + raise Exception('Unsupported test type: %s' % type(test)) + else: + result = SanitySkipped(test.name, skip_version) + result.write(args) total += 1 @@ -388,10 +419,6 @@ class SanityIgnoreProcessor: self.skip_entries = self.parser.skips.get(full_name, {}) self.used_line_numbers = set() # type: t.Set[int] - def filter_skipped_paths(self, paths): # type: (t.List[str]) -> t.List[str] - """Return the given paths, with any skipped paths filtered out.""" - return sorted(set(paths) - set(self.skip_entries.keys())) - def filter_skipped_targets(self, targets): # type: (t.List[TestTarget]) -> t.List[TestTarget] """Return the given targets, with any skipped paths filtered out.""" return sorted(target for target in targets if target.path not in self.skip_entries) @@ -490,15 +517,16 @@ class SanityMessage(TestMessage): class SanityTargets: """Sanity test target information.""" - def __init__(self, include, exclude, require): - """ - :type include: list[str] - :type exclude: list[str] - :type require: list[str] - """ - self.all = not include - self.targets = tuple(sorted(walk_sanity_targets())) - self.include = walk_internal_targets(self.targets, include, exclude, require) + def __init__(self, targets, include): # type: (t.Tuple[TestTarget], t.Tuple[TestTarget]) -> None + self.targets = targets + self.include = include + + @staticmethod + def create(include, exclude, require): # type: (t.List[str], t.List[str], t.List[str]) -> SanityTargets + """Create a SanityTargets instance from the given include, exclude and require lists.""" + _targets = tuple(sorted(walk_sanity_targets())) + _include = walk_internal_targets(_targets, include, exclude, require) + return SanityTargets(_targets, _include) class SanityTest(ABC): @@ -524,13 +552,30 @@ class SanityTest(ABC): @property def can_skip(self): # type: () -> bool """True if the test supports skip entries.""" - return True + return not self.all_targets and not self.no_targets + + @property + def all_targets(self): # type: () -> bool + """True if test targets will not be filtered using includes, excludes, requires or changes. Mutually exclusive with no_targets.""" + return False + + @property + def no_targets(self): # type: () -> bool + """True if the test does not use test targets. Mutually exclusive with all_targets.""" + 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.""" return tuple(python_version for python_version in SUPPORTED_PYTHON_VERSIONS if python_version.startswith('3.')) + 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: + return [] + + raise NotImplementedError('Sanity test "%s" must implement "filter_targets" or set "no_targets" to True.' % self.name) + class SanityCodeSmellTest(SanityTest): """Sanity test script.""" @@ -555,27 +600,73 @@ class SanityCodeSmellTest(SanityTest): self.extensions = self.config.get('extensions') # type: t.List[str] self.prefixes = self.config.get('prefixes') # type: t.List[str] self.files = self.config.get('files') # type: t.List[str] - self.always = self.config.get('always') # type: bool self.text = self.config.get('text') # type: t.Optional[bool] - self.ignore_changes = self.config.get('ignore_changes') # type: bool self.ignore_self = self.config.get('ignore_self') # type: bool - if self.ignore_changes: - self.always = False + self.__all_targets = self.config.get('all_targets') # type: bool + self.__no_targets = self.config.get('no_targets') # type: bool else: self.output = None self.extensions = [] self.prefixes = [] self.files = [] - self.always = False self.text = None # type: t.Optional[bool] - self.ignore_changes = False self.ignore_self = False + self.__all_targets = False + self.__no_targets = True + + if self.no_targets: + mutually_exclusive = ( + 'extensions', + 'prefixes', + 'files', + 'text', + 'ignore_self', + 'all_targets', + ) + + problems = sorted(name for name in mutually_exclusive if getattr(self, name)) + + if problems: + raise ApplicationError('Sanity test "%s" option "no_targets" is mutually exclusive with options: %s' % (self.name, ', '.join(problems))) + @property - def can_skip(self): # type: () -> bool - """True if the test supports skip entries.""" - return not self.always + def all_targets(self): # type: () -> bool + """True if test targets will not be filtered using includes, excludes, requires or changes. Mutually exclusive with no_targets.""" + return self.__all_targets + + @property + def no_targets(self): # type: () -> bool + """True if the test does not use test targets. Mutually exclusive with all_targets.""" + return self.__no_targets + + 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: + return [] + + if self.text is not None: + if self.text: + targets = [target for target in targets if not is_binary_file(target.path)] + else: + targets = [target for target in targets if is_binary_file(target.path)] + + if self.extensions: + targets = [target for target in targets if os.path.splitext(target.path)[1] in self.extensions + or (is_subdir(target.path, 'bin') and '.py' in self.extensions)] + + if self.prefixes: + targets = [target for target in targets if any(target.path.startswith(pre) for pre in self.prefixes)] + + if self.files: + targets = [target for target in targets if os.path.basename(target.path) in self.files] + + if self.ignore_self and data_context().content.is_ansible: + relative_self_path = os.path.relpath(self.path, data_context().content.root) + targets = [target for target in targets if target.path != relative_self_path] + + return targets def test(self, args, targets, python_version): """ @@ -593,7 +684,7 @@ class SanityCodeSmellTest(SanityTest): settings = self.load_processor(args) - paths = [] + paths = [target.path for target in targets.include] if self.config: if self.output == 'path-line-column-message': @@ -603,40 +694,7 @@ class SanityCodeSmellTest(SanityTest): else: pattern = ApplicationError('Unsupported output type: %s' % self.output) - if self.ignore_changes: - paths = sorted(i.path for i in targets.targets) - else: - paths = sorted(i.path for i in targets.include) - - if self.always: - paths = [] - - if self.text is not None: - if self.text: - paths = [p for p in paths if not is_binary_file(p)] - else: - paths = [p for p in paths if is_binary_file(p)] - - if self.extensions: - paths = [p for p in paths if os.path.splitext(p)[1] in self.extensions or (p.startswith('bin/') and '.py' in self.extensions)] - - if self.prefixes: - paths = [p for p in paths if any(p.startswith(pre) for pre in self.prefixes)] - - if self.files: - paths = [p for p in paths if os.path.basename(p) in self.files] - - if self.ignore_self and data_context().content.is_ansible: - relative_self_path = os.path.relpath(self.path, data_context().content.root) - - if relative_self_path in paths: - paths.remove(relative_self_path) - - paths = settings.filter_skipped_paths(paths) - - if not paths and not self.always: - return SanitySkipped(self.name) - + if not self.no_targets: data = '\n'.join(paths) if data: diff --git a/test/runner/lib/sanity/ansible_doc.py b/test/runner/lib/sanity/ansible_doc.py index 0f047467f95..805215136c0 100644 --- a/test/runner/lib/sanity/ansible_doc.py +++ b/test/runner/lib/sanity/ansible_doc.py @@ -6,17 +6,23 @@ import collections import os import re +import lib.types as t + from lib.sanity import ( SanitySingleVersion, SanityFailure, SanitySuccess, - SanitySkipped, SanityMessage, ) +from lib.target import ( + TestTarget, +) + from lib.util import ( SubprocessError, display, + is_subdir, ) from lib.util_common import ( @@ -42,6 +48,28 @@ from lib.coverage_util import ( class AnsibleDocTest(SanitySingleVersion): """Sanity test for ansible-doc.""" + 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.""" + # This should use documentable plugins from constants instead + plugin_type_blacklist = set([ + # not supported by ansible-doc + 'action', + 'doc_fragments', + 'filter', + 'module_utils', + 'netconf', + 'terminal', + 'test', + ]) + + plugin_paths = [plugin_path for plugin_type, plugin_path in data_context().content.plugin_paths.items() if plugin_type not in plugin_type_blacklist] + + return [target for target in targets + if os.path.splitext(target.path)[1] == '.py' + and os.path.basename(target.path) != '__init__.py' + and any(is_subdir(target.path, path) for path in plugin_paths) + ] + def test(self, args, targets, python_version): """ :type args: SanityConfig @@ -51,46 +79,26 @@ class AnsibleDocTest(SanitySingleVersion): """ settings = self.load_processor(args) - targets_include = [target for target in targets.include if os.path.splitext(target.path)[1] == '.py'] - targets_include = settings.filter_skipped_targets(targets_include) - - paths = [target.path for target in targets_include] - - # This should use documentable plugins from constants instead - plugin_type_blacklist = set([ - # not supported by ansible-doc - 'action', - 'doc_fragments', - 'filter', - 'netconf', - 'terminal', - 'test', - ]) - - modules = sorted(set(m for i in targets_include for m in i.modules)) - - plugins = [os.path.splitext(i.path)[0].split('/')[-2:] + [i.path] for i in targets_include if - os.path.basename(i.path) != '__init__.py' and - re.search(r'^lib/ansible/plugins/[^/]+/', i.path) - and i.path != 'lib/ansible/plugins/cache/base.py'] + paths = [target.path for target in targets.include] doc_targets = collections.defaultdict(list) target_paths = collections.defaultdict(dict) - for module in modules: - doc_targets['module'].append(data_context().content.prefix + module) + remap_types = dict( + modules='module', + ) - for plugin_type, plugin_name, plugin_path in plugins: - if plugin_type in plugin_type_blacklist: - continue + for plugin_type, plugin_path in data_context().content.plugin_paths.items(): + plugin_type = remap_types.get(plugin_type, plugin_type) - doc_targets[plugin_type].append(data_context().content.prefix + plugin_name) - target_paths[plugin_type][data_context().content.prefix + plugin_name] = plugin_path + for plugin_file_path in [target.name for target in targets.include if is_subdir(target.path, plugin_path)]: + plugin_name = os.path.splitext(os.path.basename(plugin_file_path))[0] - if not doc_targets: - return SanitySkipped(self.name) + if plugin_name.startswith('_'): + plugin_name = plugin_name[1:] - target_paths['module'] = dict((data_context().content.prefix + t.module, t.path) for t in targets.targets if t.module) + doc_targets[plugin_type].append(data_context().content.prefix + plugin_name) + target_paths[plugin_type][data_context().content.prefix + plugin_name] = plugin_file_path env = ansible_environment(args, color=False) error_messages = [] diff --git a/test/runner/lib/sanity/compile.py b/test/runner/lib/sanity/compile.py index 38298af08de..43661985ee1 100644 --- a/test/runner/lib/sanity/compile.py +++ b/test/runner/lib/sanity/compile.py @@ -4,12 +4,18 @@ __metaclass__ = type import os +import lib.types as t + from lib.sanity import ( SanityMultipleVersion, SanityMessage, SanityFailure, SanitySuccess, - SanitySkipped, + SanityTargets, +) + +from lib.target import ( + TestTarget, ) from lib.util import ( @@ -18,6 +24,7 @@ from lib.util import ( find_python, parse_to_list_of_dict, ANSIBLE_ROOT, + is_subdir, ) from lib.util_common import ( @@ -31,6 +38,10 @@ from lib.config import ( class CompileTest(SanityMultipleVersion): """Sanity test for proper python syntax.""" + 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.""" + return [target for target in targets if os.path.splitext(target.path)[1] == '.py' or is_subdir(target.path, 'bin')] + def test(self, args, targets, python_version): """ :type args: SanityConfig @@ -40,11 +51,7 @@ class CompileTest(SanityMultipleVersion): """ settings = self.load_processor(args, python_version) - paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.py' or i.path.startswith('bin/')) - paths = settings.filter_skipped_paths(paths) - - if not paths: - return SanitySkipped(self.name, python_version=python_version) + paths = [target.path for target in targets.include] cmd = [find_python(python_version), os.path.join(ANSIBLE_ROOT, 'test/sanity/compile/compile.py')] diff --git a/test/runner/lib/sanity/ignores.py b/test/runner/lib/sanity/ignores.py index 701d38ae2d2..4a4740a88f7 100644 --- a/test/runner/lib/sanity/ignores.py +++ b/test/runner/lib/sanity/ignores.py @@ -28,9 +28,9 @@ class IgnoresTest(SanityVersionNeutral): return False @property - def can_skip(self): # type: () -> bool - """True if the test supports skip entries.""" - return False + def no_targets(self): # type: () -> bool + """True if the test does not use test targets. Mutually exclusive with all_targets.""" + return True # noinspection PyUnusedLocal def test(self, args, targets): # pylint: disable=locally-disabled, unused-argument diff --git a/test/runner/lib/sanity/import.py b/test/runner/lib/sanity/import.py index f3d18003465..9e585a5abfc 100644 --- a/test/runner/lib/sanity/import.py +++ b/test/runner/lib/sanity/import.py @@ -4,12 +4,17 @@ __metaclass__ = type import os +import lib.types as t + from lib.sanity import ( SanityMultipleVersion, SanityMessage, SanityFailure, SanitySuccess, - SanitySkipped, +) + +from lib.target import ( + TestTarget, ) from lib.util import ( @@ -51,6 +56,11 @@ from lib.data import ( class ImportTest(SanityMultipleVersion): """Sanity test for proper import exception handling.""" + 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.""" + return [target for target in targets if os.path.splitext(target.path)[1] == '.py' and + (is_subdir(target.path, data_context().content.module_path) or is_subdir(target.path, data_context().content.module_utils_path))] + def test(self, args, targets, python_version): """ :type args: SanityConfig @@ -60,17 +70,7 @@ class ImportTest(SanityMultipleVersion): """ settings = self.load_processor(args, python_version) - paths = sorted( - i.path - for i in targets.include - if os.path.splitext(i.path)[1] == '.py' and - (is_subdir(i.path, data_context().content.module_path) or is_subdir(i.path, data_context().content.module_utils_path)) - ) - - paths = settings.filter_skipped_paths(paths) - - if not paths: - return SanitySkipped(self.name, python_version=python_version) + paths = [target.path for target in targets.include] env = ansible_environment(args, color=False) diff --git a/test/runner/lib/sanity/integration_aliases.py b/test/runner/lib/sanity/integration_aliases.py index d568a6b392f..81324318dad 100644 --- a/test/runner/lib/sanity/integration_aliases.py +++ b/test/runner/lib/sanity/integration_aliases.py @@ -94,9 +94,9 @@ class IntegrationAliasesTest(SanityVersionNeutral): return False @property - def can_skip(self): # type: () -> bool - """True if the test supports skip entries.""" - return False + def no_targets(self): # type: () -> bool + """True if the test does not use test targets. Mutually exclusive with all_targets.""" + return True @property def shippable_yml_lines(self): diff --git a/test/runner/lib/sanity/pep8.py b/test/runner/lib/sanity/pep8.py index c8f29a547d0..24e33200474 100644 --- a/test/runner/lib/sanity/pep8.py +++ b/test/runner/lib/sanity/pep8.py @@ -13,12 +13,17 @@ from lib.sanity import ( SanitySuccess, ) +from lib.target import ( + TestTarget, +) + from lib.util import ( SubprocessError, read_lines_without_comments, parse_to_list_of_dict, ANSIBLE_ROOT, find_python, + is_subdir, ) from lib.util_common import ( @@ -37,6 +42,10 @@ class Pep8Test(SanitySingleVersion): """Error code for ansible-test matching the format used by the underlying test program, or None if the program does not use error codes.""" return 'A100' + 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.""" + return [target for target in targets if os.path.splitext(target.path)[1] == '.py' or is_subdir(target.path, 'bin')] + def test(self, args, targets, python_version): """ :type args: SanityConfig @@ -49,8 +58,7 @@ class Pep8Test(SanitySingleVersion): settings = self.load_processor(args) - paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.py' or i.path.startswith('bin/')) - paths = settings.filter_skipped_paths(paths) + paths = [target.path for target in targets.include] cmd = [ find_python(python_version), diff --git a/test/runner/lib/sanity/pslint.py b/test/runner/lib/sanity/pslint.py index 31058708659..e48bfb5def4 100644 --- a/test/runner/lib/sanity/pslint.py +++ b/test/runner/lib/sanity/pslint.py @@ -16,6 +16,10 @@ from lib.sanity import ( SanitySkipped, ) +from lib.target import ( + TestTarget, +) + from lib.util import ( SubprocessError, find_executable, @@ -42,6 +46,10 @@ class PslintTest(SanityVersionNeutral): """Error code for ansible-test matching the format used by the underlying test program, or None if the program does not use error codes.""" return 'AnsibleTest' + 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.""" + return [target for target in targets if os.path.splitext(target.path)[1] in ('.ps1', '.psm1', '.psd1')] + def test(self, args, targets): """ :type args: SanityConfig @@ -50,11 +58,7 @@ class PslintTest(SanityVersionNeutral): """ settings = self.load_processor(args) - paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.ps1', '.psm1', '.psd1')) - paths = settings.filter_skipped_paths(paths) - - if not paths: - return SanitySkipped(self.name) + paths = [target.path for target in targets.include] if not find_executable('pwsh', required='warning'): return SanitySkipped(self.name) diff --git a/test/runner/lib/sanity/pylint.py b/test/runner/lib/sanity/pylint.py index 51490853f9a..212bfb8945a 100644 --- a/test/runner/lib/sanity/pylint.py +++ b/test/runner/lib/sanity/pylint.py @@ -16,6 +16,10 @@ from lib.sanity import ( SanitySuccess, ) +from lib.target import ( + TestTarget, +) + from lib.util import ( SubprocessError, display, @@ -49,6 +53,10 @@ class PylintTest(SanitySingleVersion): """Error code for ansible-test matching the format used by the underlying test program, or None if the program does not use error codes.""" return 'ansible-test' + 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.""" + return [target for target in targets if os.path.splitext(target.path)[1] == '.py' or is_subdir(target.path, 'bin')] + def test(self, args, targets, python_version): """ :type args: SanityConfig @@ -62,8 +70,7 @@ class PylintTest(SanitySingleVersion): settings = self.load_processor(args) - paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.py' or is_subdir(i.path, 'bin/')) - paths = settings.filter_skipped_paths(paths) + paths = [target.path for target in targets.include] module_paths = [os.path.relpath(p, data_context().content.module_path).split(os.path.sep) for p in paths if is_subdir(p, data_context().content.module_path)] diff --git a/test/runner/lib/sanity/rstcheck.py b/test/runner/lib/sanity/rstcheck.py index bcc0d8ad5f2..759ca5e09f9 100644 --- a/test/runner/lib/sanity/rstcheck.py +++ b/test/runner/lib/sanity/rstcheck.py @@ -4,12 +4,17 @@ __metaclass__ = type import os +import lib.types as t + from lib.sanity import ( SanitySingleVersion, SanityMessage, SanityFailure, SanitySuccess, - SanitySkipped, +) + +from lib.target import ( + TestTarget, ) from lib.util import ( @@ -31,6 +36,10 @@ from lib.config import ( class RstcheckTest(SanitySingleVersion): """Sanity test using rstcheck.""" + 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.""" + return [target for target in targets if os.path.splitext(target.path)[1] in ('.rst',)] + def test(self, args, targets, python_version): """ :type args: SanityConfig @@ -43,11 +52,7 @@ class RstcheckTest(SanitySingleVersion): settings = self.load_processor(args) - paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.rst',)) - paths = settings.filter_skipped_paths(paths) - - if not paths: - return SanitySkipped(self.name) + paths = [target.path for target in targets.include] cmd = [ find_python(python_version), diff --git a/test/runner/lib/sanity/sanity_docs.py b/test/runner/lib/sanity/sanity_docs.py index e3752181f2b..21e38582c7f 100644 --- a/test/runner/lib/sanity/sanity_docs.py +++ b/test/runner/lib/sanity/sanity_docs.py @@ -31,9 +31,9 @@ class SanityDocsTest(SanityVersionNeutral): return False @property - def can_skip(self): # type: () -> bool - """True if the test supports skip entries.""" - return False + def no_targets(self): # type: () -> bool + """True if the test does not use test targets. Mutually exclusive with all_targets.""" + return True # noinspection PyUnusedLocal def test(self, args, targets): # pylint: disable=locally-disabled, unused-argument diff --git a/test/runner/lib/sanity/shellcheck.py b/test/runner/lib/sanity/shellcheck.py index 1935b1745f6..f3839810fe7 100644 --- a/test/runner/lib/sanity/shellcheck.py +++ b/test/runner/lib/sanity/shellcheck.py @@ -19,10 +19,15 @@ from lib.sanity import ( SanitySkipped, ) +from lib.target import ( + TestTarget, +) + from lib.util import ( SubprocessError, read_lines_without_comments, ANSIBLE_ROOT, + find_executable, ) from lib.util_common import ( @@ -41,6 +46,10 @@ class ShellcheckTest(SanityVersionNeutral): """Error code for ansible-test matching the format used by the underlying test program, or None if the program does not use error codes.""" return 'AT1000' + 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.""" + return [target for target in targets if os.path.splitext(target.path)[1] == '.sh'] + def test(self, args, targets): """ :type args: SanityConfig @@ -52,10 +61,9 @@ class ShellcheckTest(SanityVersionNeutral): settings = self.load_processor(args) - paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.sh') - paths = settings.filter_skipped_paths(paths) + paths = [target.path for target in targets.include] - if not paths: + if not find_executable('shellcheck', required='warning'): return SanitySkipped(self.name) cmd = [ diff --git a/test/runner/lib/sanity/validate_modules.py b/test/runner/lib/sanity/validate_modules.py index 0445af01a3f..d6dff148cdb 100644 --- a/test/runner/lib/sanity/validate_modules.py +++ b/test/runner/lib/sanity/validate_modules.py @@ -12,7 +12,10 @@ from lib.sanity import ( SanityMessage, SanityFailure, SanitySuccess, - SanitySkipped, +) + +from lib.target import ( + TestTarget, ) from lib.util import ( @@ -46,6 +49,10 @@ class ValidateModulesTest(SanitySingleVersion): """Error code for ansible-test matching the format used by the underlying test program, or None if the program does not use error codes.""" return 'A100' + 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.""" + return [target for target in targets if target.module] + def test(self, args, targets, python_version): """ :type args: SanityConfig @@ -64,11 +71,7 @@ class ValidateModulesTest(SanitySingleVersion): settings = self.load_processor(args) - paths = sorted(i.path for i in targets.include if i.module) - paths = settings.filter_skipped_paths(paths) - - if not paths: - return SanitySkipped(self.name) + paths = [target.path for target in targets.include] cmd = [ find_python(python_version), diff --git a/test/runner/lib/sanity/yamllint.py b/test/runner/lib/sanity/yamllint.py index 691d53515bb..24b3aa2e90f 100644 --- a/test/runner/lib/sanity/yamllint.py +++ b/test/runner/lib/sanity/yamllint.py @@ -12,7 +12,10 @@ from lib.sanity import ( SanityMessage, SanityFailure, SanitySuccess, - SanitySkipped, +) + +from lib.target import ( + TestTarget, ) from lib.util import ( @@ -43,6 +46,21 @@ class YamllintTest(SanitySingleVersion): """Error code for ansible-test matching the format used by the underlying test program, or None if the program does not use error codes.""" return 'ansible-test' + 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.""" + yaml_targets = [target for target in targets if os.path.splitext(target.path)[1] in ('.yml', '.yaml')] + + for plugin_type, plugin_path in sorted(data_context().content.plugin_paths.items()): + if plugin_type == 'module_utils': + continue + + yaml_targets.extend([target for target in targets if + os.path.splitext(target.path)[1] == '.py' and + os.path.basename(target.path) != '__init__.py' and + is_subdir(target.path, plugin_path)]) + + return yaml_targets + def test(self, args, targets, python_version): """ :type args: SanityConfig @@ -52,21 +70,7 @@ class YamllintTest(SanitySingleVersion): """ settings = self.load_processor(args) - paths = [i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.yml', '.yaml')] - - for plugin_type, plugin_path in sorted(data_context().content.plugin_paths.items()): - if plugin_type == 'module_utils': - continue - - paths.extend([target.path for target in targets.include if - os.path.splitext(target.path)[1] == '.py' and - os.path.basename(target.path) != '__init__.py' and - is_subdir(target.path, plugin_path)]) - - paths = settings.filter_skipped_paths(paths) - - if not paths: - return SanitySkipped(self.name) + paths = [target.path for target in targets.include] python = find_python(python_version) diff --git a/test/sanity/code-smell/action-plugin-docs.json b/test/sanity/code-smell/action-plugin-docs.json index d029aaf7d67..6ff1c681f92 100644 --- a/test/sanity/code-smell/action-plugin-docs.json +++ b/test/sanity/code-smell/action-plugin-docs.json @@ -1,5 +1,5 @@ { - "ignore_changes": true, + "all_targets": true, "prefixes": [ "lib/ansible/modules/", "lib/ansible/plugins/action/" diff --git a/test/sanity/code-smell/azure-requirements.json b/test/sanity/code-smell/azure-requirements.json index 39ac4bd57f0..593b765d14a 100644 --- a/test/sanity/code-smell/azure-requirements.json +++ b/test/sanity/code-smell/azure-requirements.json @@ -1,4 +1,4 @@ { - "always": true, + "no_targets": true, "output": "path-message" } diff --git a/test/sanity/code-smell/botmeta.json b/test/sanity/code-smell/botmeta.json index 376970e6824..56455d209bb 100644 --- a/test/sanity/code-smell/botmeta.json +++ b/test/sanity/code-smell/botmeta.json @@ -1,4 +1,4 @@ { - "always": true, + "no_targets": true, "output": "path-line-column-message" } diff --git a/test/sanity/code-smell/configure-remoting-ps1.json b/test/sanity/code-smell/configure-remoting-ps1.json index 39ac4bd57f0..593b765d14a 100644 --- a/test/sanity/code-smell/configure-remoting-ps1.json +++ b/test/sanity/code-smell/configure-remoting-ps1.json @@ -1,4 +1,4 @@ { - "always": true, + "no_targets": true, "output": "path-message" } diff --git a/test/sanity/code-smell/deprecated-config.json b/test/sanity/code-smell/deprecated-config.json index 35ad9c94d03..4a884860666 100644 --- a/test/sanity/code-smell/deprecated-config.json +++ b/test/sanity/code-smell/deprecated-config.json @@ -1,5 +1,5 @@ { - "always": true, + "all_targets": true, "output": "path-message", "extensions": [ ".py" diff --git a/test/sanity/code-smell/docs-build.json b/test/sanity/code-smell/docs-build.json index 9b608c91f97..a43fa923b2b 100644 --- a/test/sanity/code-smell/docs-build.json +++ b/test/sanity/code-smell/docs-build.json @@ -1,5 +1,5 @@ { "disabled": true, - "always": true, + "no_targets": true, "output": "path-line-column-message" } diff --git a/test/sanity/code-smell/no-illegal-filenames.json b/test/sanity/code-smell/no-illegal-filenames.json index 39ac4bd57f0..593b765d14a 100644 --- a/test/sanity/code-smell/no-illegal-filenames.json +++ b/test/sanity/code-smell/no-illegal-filenames.json @@ -1,4 +1,4 @@ { - "always": true, + "no_targets": true, "output": "path-message" } diff --git a/test/sanity/code-smell/package-data.json b/test/sanity/code-smell/package-data.json index db6ca35c2a6..7cc6d71398f 100644 --- a/test/sanity/code-smell/package-data.json +++ b/test/sanity/code-smell/package-data.json @@ -1,5 +1,5 @@ { "disabled": true, - "always": true, + "no_targets": true, "output": "path-message" } diff --git a/test/sanity/code-smell/symlinks.json b/test/sanity/code-smell/symlinks.json index 39ac4bd57f0..593b765d14a 100644 --- a/test/sanity/code-smell/symlinks.json +++ b/test/sanity/code-smell/symlinks.json @@ -1,4 +1,4 @@ { - "always": true, + "no_targets": true, "output": "path-message" } diff --git a/test/sanity/code-smell/update-bundled.json b/test/sanity/code-smell/update-bundled.json index 0f30a449538..77c7254b55c 100644 --- a/test/sanity/code-smell/update-bundled.json +++ b/test/sanity/code-smell/update-bundled.json @@ -1,5 +1,5 @@ { - "ignore_changes": true, + "all_targets": true, "extensions": [ ".py" ], diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 1db3dbdbd90..413c9d069a5 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -6889,6 +6889,7 @@ lib/ansible/parsing/vault/__init__.py pylint:blacklisted-name lib/ansible/playbook/base.py pylint:blacklisted-name lib/ansible/playbook/helpers.py pylint:blacklisted-name lib/ansible/playbook/role/__init__.py pylint:blacklisted-name +lib/ansible/plugins/cache/base.py ansible-doc # not a plugin, but a stub for backwards compatibility lib/ansible/plugins/callback/hipchat.py pylint:blacklisted-name lib/ansible/plugins/connection/lxc.py pylint:blacklisted-name lib/ansible/plugins/doc_fragments/a10.py future-import-boilerplate