From 4afe757f93b175adf38fa005d59ebce00d577ded Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Wed, 28 Aug 2019 15:13:00 -0700 Subject: [PATCH] More ansible-test path handling updates. (#61484) * Move test results vars into layout. * Add test_path to layout. * Add sanity_path to layout. * Clean up layout. * Update change classification. * Improve classification layout. * Relocate common classification code. * Use is_subdir for relocated code. * Relocate ansible test/units/compat/ hack. * Fix variable shadowing. * Remove unused code. * Fix ordering of path tests. * Clean up ansible classification. * Fix hard-coded plugin paths in classification. * Relocate more common classification. * Fix logic. * Fix pylint issue. * Add missing licenses classification. --- .../ansible_test/_internal/classification.py | 304 ++++++++++-------- test/lib/ansible_test/_internal/data.py | 2 - test/lib/ansible_test/_internal/delegation.py | 8 +- .../ansible_test/_internal/import_analysis.py | 3 +- .../_internal/provider/layout/__init__.py | 16 +- .../_internal/provider/layout/ansible.py | 4 + .../_internal/provider/layout/collection.py | 3 + .../ansible_test/_internal/sanity/__init__.py | 2 +- .../lib/ansible_test/_internal/util_common.py | 6 +- 9 files changed, 193 insertions(+), 155 deletions(-) diff --git a/test/lib/ansible_test/_internal/classification.py b/test/lib/ansible_test/_internal/classification.py index 70c8a2554ed..59d2ddf18e7 100644 --- a/test/lib/ansible_test/_internal/classification.py +++ b/test/lib/ansible_test/_internal/classification.py @@ -7,6 +7,8 @@ import os import re import time +from . import types as t + from .target import ( walk_module_targets, walk_integration_targets, @@ -157,7 +159,7 @@ def categorize_changes(args, paths, verbose_command=None): for command in commands: commands[command].discard('none') - if any(t == 'all' for t in commands[command]): + if any(target == 'all' for target in commands[command]): commands[command] = set(['all']) commands = dict((c, sorted(commands[c])) for c in commands if commands[c]) @@ -192,23 +194,23 @@ class PathMapper: self.compile_targets = list(walk_compile_targets()) self.units_targets = list(walk_units_targets()) self.sanity_targets = list(walk_sanity_targets()) - self.powershell_targets = [t for t in self.sanity_targets if os.path.splitext(t.path)[1] in ('.ps1', '.psm1')] - self.csharp_targets = [t for t in self.sanity_targets if os.path.splitext(t.path)[1] == '.cs'] + self.powershell_targets = [target for target in self.sanity_targets if os.path.splitext(target.path)[1] in ('.ps1', '.psm1')] + self.csharp_targets = [target for target in self.sanity_targets if os.path.splitext(target.path)[1] == '.cs'] - self.units_modules = set(t.module for t in self.units_targets if t.module) - self.units_paths = set(a for t in self.units_targets for a in t.aliases) - self.sanity_paths = set(t.path for t in self.sanity_targets) + self.units_modules = set(target.module for target in self.units_targets if target.module) + self.units_paths = set(a for target in self.units_targets for a in target.aliases) + self.sanity_paths = set(target.path for target in self.sanity_targets) - self.module_names_by_path = dict((t.path, t.module) for t in self.module_targets) - self.integration_targets_by_name = dict((t.name, t) for t in self.integration_targets) - self.integration_targets_by_alias = dict((a, t) for t in self.integration_targets for a in t.aliases) + self.module_names_by_path = dict((target.path, target.module) for target in self.module_targets) + self.integration_targets_by_name = dict((target.name, target) for target in self.integration_targets) + self.integration_targets_by_alias = dict((a, target) for target in self.integration_targets for a in target.aliases) - self.posix_integration_by_module = dict((m, t.name) for t in self.integration_targets - if 'posix/' in t.aliases for m in t.modules) - self.windows_integration_by_module = dict((m, t.name) for t in self.integration_targets - if 'windows/' in t.aliases for m in t.modules) - self.network_integration_by_module = dict((m, t.name) for t in self.integration_targets - if 'network/' in t.aliases for m in t.modules) + self.posix_integration_by_module = dict((m, target.name) for target in self.integration_targets + if 'posix/' in target.aliases for m in target.modules) + self.windows_integration_by_module = dict((m, target.name) for target in self.integration_targets + if 'windows/' in target.aliases for m in target.modules) + self.network_integration_by_module = dict((m, target.name) for target in self.integration_targets + if 'network/' in target.aliases for m in target.modules) self.prefixes = load_integration_prefixes() self.integration_dependencies = analyze_integration_target_dependencies(self.integration_targets) @@ -254,7 +256,7 @@ class PathMapper: :rtype: list[str] """ paths = self.get_dependent_paths_internal(path) - paths += [t.path + '/' for t in self.paths_to_dependent_targets.get(path, set())] + paths += [target.path + '/' for target in self.paths_to_dependent_targets.get(path, set())] paths = sorted(set(paths)) return paths @@ -360,57 +362,89 @@ class PathMapper: return result - def _classify(self, path): - """ - :type path: str - :rtype: dict[str, str] | None - """ + def _classify(self, path): # type: (str) -> t.Optional[t.Dict[str, str]] + """Return the classification for the given path.""" + if data_context().content.is_ansible: + return self._classify_ansible(path) + + if data_context().content.collection: + return self._classify_collection(path) + + return None + + def _classify_common(self, path): # type: (str) -> t.Optional[t.Dict[str, str]] + """Return the classification for the given path using rules common to all layouts.""" dirname = os.path.dirname(path) filename = os.path.basename(path) name, ext = os.path.splitext(filename) minimal = {} - if path.startswith('.github/'): + if is_subdir(path, '.github'): return minimal - if path.startswith('bin/'): - return all_tests(self.args) # broad impact, run all tests + if is_subdir(path, data_context().content.integration_targets_path): + if not os.path.exists(path): + return minimal + + target = self.integration_targets_by_name.get(path.split('/')[3]) + + if not target: + display.warning('Unexpected non-target found: %s' % path) + return minimal + + if 'hidden/' in target.aliases: + return minimal # already expanded using get_dependent_paths - if path.startswith('contrib/'): return { - 'units': 'test/units/contrib/' + 'integration': target.name if 'posix/' in target.aliases else None, + 'windows-integration': target.name if 'windows/' in target.aliases else None, + 'network-integration': target.name if 'network/' in target.aliases else None, + FOCUSED_TARGET: True, } - if path.startswith('changelogs/'): - return minimal + if is_subdir(path, data_context().content.integration_path): + if dirname == data_context().content.integration_path: + for command in ( + 'integration', + 'windows-integration', + 'network-integration', + ): + if name == command and ext == '.cfg': + return { + command: self.integration_all_target, + } - if path.startswith('docs/'): - return minimal + return { + 'integration': self.integration_all_target, + 'windows-integration': self.integration_all_target, + 'network-integration': self.integration_all_target, + } - if path.startswith('examples/'): - if path == 'examples/scripts/ConfigureRemotingForAnsible.ps1': + if is_subdir(path, data_context().content.sanity_path): + return { + 'sanity': 'all', # test infrastructure, run all sanity checks + } + + if is_subdir(path, data_context().content.unit_path): + if path in self.units_paths: return { - 'windows-integration': 'connection_winrm', + 'units': path, } - return minimal + # changes to files which are not unit tests should trigger tests from the nearest parent directory - if path.startswith('hacking/'): - return minimal + test_path = os.path.dirname(path) - if path.startswith('lib/ansible/executor/powershell/'): - units_path = 'test/units/executor/powershell/' + while test_path: + if test_path + '/' in self.units_paths: + return { + 'units': test_path + '/', + } - if units_path not in self.units_paths: - units_path = None + test_path = os.path.dirname(test_path) - return { - 'windows-integration': self.integration_all_target, - 'units': units_path, - } - - if path.startswith('lib/ansible/modules/'): + if is_subdir(path, data_context().content.module_path): module_name = self.module_names_by_path.get(path) if module_name: @@ -424,7 +458,7 @@ class PathMapper: return minimal - if path.startswith('lib/ansible/module_utils/'): + if is_subdir(path, data_context().content.module_utils_path): if ext == '.cs': return minimal # already expanded using get_dependent_paths @@ -434,7 +468,7 @@ class PathMapper: if ext == '.py': return minimal # already expanded using get_dependent_paths - if path.startswith('lib/ansible/plugins/action/'): + if is_subdir(path, data_context().content.plugin_paths['action']): if ext == '.py': if name.startswith('net_'): network_target = 'network/.*_%s' % name[4:] @@ -474,7 +508,7 @@ class PathMapper: 'units': 'all', } - if path.startswith('lib/ansible/plugins/connection/'): + if is_subdir(path, data_context().content.plugin_paths['connection']): if name == '__init__': return { 'integration': self.integration_all_target, @@ -534,7 +568,12 @@ class PathMapper: 'units': units_path, } - if path.startswith('lib/ansible/plugins/inventory/'): + if is_subdir(path, data_context().content.plugin_paths['doc_fragments']): + return { + 'sanity': 'all', + } + + if is_subdir(path, data_context().content.plugin_paths['inventory']): if name == '__init__': return all_tests(self.args) # broad impact, run all tests @@ -567,9 +606,9 @@ class PathMapper: FOCUSED_TARGET: target is not None, } - if (path.startswith('lib/ansible/plugins/terminal/') or - path.startswith('lib/ansible/plugins/cliconf/') or - path.startswith('lib/ansible/plugins/netconf/')): + if (is_subdir(path, data_context().content.plugin_paths['terminal']) or + is_subdir(path, data_context().content.plugin_paths['cliconf']) or + is_subdir(path, data_context().content.plugin_paths['netconf'])): if ext == '.py': if name in self.prefixes and self.prefixes[name] == 'network': network_target = 'network/%s/' % name @@ -591,14 +630,77 @@ class PathMapper: 'units': 'all', } - if path.startswith('lib/ansible/plugins/doc_fragments/'): + return None + + def _classify_collection(self, path): # type: (str) -> t.Optional[t.Dict[str, str]] + """Return the classification for the given path using rules specific to collections.""" + result = self._classify_common(path) + + if result is not None: + return result + + return None + + def _classify_ansible(self, path): # type: (str) -> t.Optional[t.Dict[str, str]] + """Return the classification for the given path using rules specific to Ansible.""" + if path.startswith('test/units/compat/'): return { - 'sanity': 'all', + 'units': 'test/units/', + } + + result = self._classify_common(path) + + if result is not None: + return result + + dirname = os.path.dirname(path) + filename = os.path.basename(path) + name, ext = os.path.splitext(filename) + + minimal = {} + + if path.startswith('bin/'): + return all_tests(self.args) # broad impact, run all tests + + if path.startswith('changelogs/'): + return minimal + + if path.startswith('contrib/'): + return { + 'units': 'test/units/contrib/' + } + + if path.startswith('docs/'): + return minimal + + if path.startswith('examples/'): + if path == 'examples/scripts/ConfigureRemotingForAnsible.ps1': + return { + 'windows-integration': 'connection_winrm', + } + + return minimal + + if path.startswith('hacking/'): + return minimal + + if path.startswith('lib/ansible/executor/powershell/'): + units_path = 'test/units/executor/powershell/' + + if units_path not in self.units_paths: + units_path = None + + return { + 'windows-integration': self.integration_all_target, + 'units': units_path, } if path.startswith('lib/ansible/'): return all_tests(self.args) # broad impact, run all tests + if path.startswith('licenses/'): + return minimal + if path.startswith('packaging/'): if path.startswith('packaging/requirements/'): if name.startswith('requirements-') and ext == '.txt': @@ -624,72 +726,6 @@ class PathMapper: if path.startswith('test/legacy/'): return minimal - if is_subdir(path, data_context().content.integration_targets_path): - if not os.path.exists(path): - return minimal - - target = self.integration_targets_by_name.get(path.split('/')[3]) - - if not target: - display.warning('Unexpected non-target found: %s' % path) - return minimal - - if 'hidden/' in target.aliases: - return minimal # already expanded using get_dependent_paths - - return { - 'integration': target.name if 'posix/' in target.aliases else None, - 'windows-integration': target.name if 'windows/' in target.aliases else None, - 'network-integration': target.name if 'network/' in target.aliases else None, - FOCUSED_TARGET: True, - } - - if is_subdir(path, data_context().content.integration_path): - if dirname == data_context().content.integration_path: - for command in ( - 'integration', - 'windows-integration', - 'network-integration', - ): - if name == command and ext == '.cfg': - return { - command: self.integration_all_target, - } - - return { - 'integration': self.integration_all_target, - 'windows-integration': self.integration_all_target, - 'network-integration': self.integration_all_target, - } - - if path.startswith('test/sanity/'): - return { - 'sanity': 'all', # test infrastructure, run all sanity checks - } - - if path.startswith('test/units/'): - if path in self.units_paths: - return { - 'units': path, - } - - if path.startswith('test/units/compat/'): - return { - 'units': 'test/units/', - } - - # changes to files which are not unit tests should trigger tests from the nearest parent directory - - test_path = os.path.dirname(path) - - while test_path: - if test_path + '/' in self.units_paths: - return { - 'units': test_path + '/', - } - - test_path = os.path.dirname(test_path) - if path.startswith('test/lib/ansible_test/config/'): if name.startswith('cloud-config-'): cloud_target = 'cloud/%s/' % name.split('-')[2].split('.')[0] @@ -805,39 +841,29 @@ class PathMapper: if path.startswith('test/utils/'): return minimal - if path == 'test/README.md': - return minimal - - if path.startswith('ticket_stubs/'): - return minimal - if '/' not in path: if path in ( '.gitattributes', '.gitignore', - '.gitmodules', '.mailmap', - 'tox.ini', # obsolete 'COPYING', - 'VERSION', 'Makefile', ): return minimal if path in ( + 'setup.py', 'shippable.yml', ): - return all_tests(self.args) # test infrastructure, run all tests - - if path == 'setup.py': return all_tests(self.args) # broad impact, run all tests - if path == '.yamllint': - return { - 'sanity': 'all', - } - - if ext in ('.md', '.rst', '.txt', '.xml', '.in'): + if ext in ( + '.in', + '.md', + '.rst', + '.toml', + '.txt', + ): return minimal return None # unknown, will result in fall-back to run all tests diff --git a/test/lib/ansible_test/_internal/data.py b/test/lib/ansible_test/_internal/data.py index 1dc84868401..6f8cbf45cac 100644 --- a/test/lib/ansible_test/_internal/data.py +++ b/test/lib/ansible_test/_internal/data.py @@ -72,8 +72,6 @@ class DataContext: content = self.__create_content_layout(layout_providers, source_providers, current_path, True) self.content = content # type: ContentLayout - self.results_relative = os.path.join('test', 'results') - self.results = os.path.join(self.content.root, self.results_relative) def create_collection_layouts(self): # type: () -> t.List[ContentLayout] """ diff --git a/test/lib/ansible_test/_internal/delegation.py b/test/lib/ansible_test/_internal/delegation.py index 8e2236295e4..cfe6176f069 100644 --- a/test/lib/ansible_test/_internal/delegation.py +++ b/test/lib/ansible_test/_internal/delegation.py @@ -300,7 +300,7 @@ def delegate_docker(args, exclude, require, integration_targets): else: content_root = install_root - remote_results_root = os.path.join(content_root, data_context().results_relative) + remote_results_root = os.path.join(content_root, data_context().content.results_path) cmd = generate_command(args, python_interpreter, os.path.join(install_root, 'bin'), content_root, options, exclude, require) @@ -407,7 +407,7 @@ def delegate_docker(args, exclude, require, integration_targets): try: docker_exec(args, test_id, cmd, options=cmd_options) finally: - local_test_root = os.path.dirname(data_context().results) + local_test_root = os.path.dirname(os.path.join(data_context().content.root, data_context().content.results_path)) remote_test_root = os.path.dirname(remote_results_root) remote_results_name = os.path.basename(remote_results_root) @@ -530,9 +530,9 @@ def delegate_remote(args, exclude, require, integration_targets): download = False if download and content_root: - local_test_root = os.path.dirname(data_context().results) + local_test_root = os.path.dirname(os.path.join(data_context().content.root, data_context().content.results_path)) - remote_results_root = os.path.join(content_root, data_context().results_relative) + remote_results_root = os.path.join(content_root, data_context().content.results_path) remote_results_name = os.path.basename(remote_results_root) remote_temp_path = os.path.join('/tmp', remote_results_name) diff --git a/test/lib/ansible_test/_internal/import_analysis.py b/test/lib/ansible_test/_internal/import_analysis.py index b0ab798a461..fd52173861d 100644 --- a/test/lib/ansible_test/_internal/import_analysis.py +++ b/test/lib/ansible_test/_internal/import_analysis.py @@ -10,6 +10,7 @@ from . import types as t from .util import ( display, ApplicationError, + is_subdir, ) from .data import ( @@ -245,7 +246,7 @@ class ModuleUtilFinder(ast.NodeVisitor): name = '.'.join(name.split('.')[:-1]) - if self.path.startswith('test/'): + if is_subdir(self.path, data_context().content.test_path): return # invalid imports in tests are ignored # Treat this error as a warning so tests can be executed as best as possible. diff --git a/test/lib/ansible_test/_internal/provider/layout/__init__.py b/test/lib/ansible_test/_internal/provider/layout/__init__.py index 402b40f41ec..49f304b14d2 100644 --- a/test/lib/ansible_test/_internal/provider/layout/__init__.py +++ b/test/lib/ansible_test/_internal/provider/layout/__init__.py @@ -80,16 +80,22 @@ class ContentLayout(Layout): root, # type: str paths, # type: t.List[str] plugin_paths, # type: t.Dict[str, str] - collection=None, # type: t.Optional[CollectionDetail] - integration_path=None, # type: t.Optional[str] - unit_path=None, # type: t.Optional[str] - unit_module_path=None, # type: t.Optional[str] - unit_module_utils_path=None, # type: t.Optional[str] + collection, # type: t.Optional[CollectionDetail] + test_path, # type: str + results_path, # type: str + sanity_path, # type: str + integration_path, # type: str + unit_path, # type: str + unit_module_path, # type: str + unit_module_utils_path, # type: str ): # type: (...) -> None super(ContentLayout, self).__init__(root, paths) self.plugin_paths = plugin_paths self.collection = collection + self.test_path = test_path + self.results_path = results_path + self.sanity_path = sanity_path self.integration_path = integration_path self.integration_targets_path = os.path.join(integration_path, 'targets') self.integration_vars_path = os.path.join(integration_path, 'integration_config.yml') diff --git a/test/lib/ansible_test/_internal/provider/layout/ansible.py b/test/lib/ansible_test/_internal/provider/layout/ansible.py index 164557945d5..6ef68dada25 100644 --- a/test/lib/ansible_test/_internal/provider/layout/ansible.py +++ b/test/lib/ansible_test/_internal/provider/layout/ansible.py @@ -31,6 +31,10 @@ class AnsibleLayout(LayoutProvider): return ContentLayout(root, paths, plugin_paths=plugin_paths, + collection=None, + test_path='test', + results_path='test/results', + sanity_path='test/sanity', integration_path='test/integration', unit_path='test/units', unit_module_path='test/units/modules', diff --git a/test/lib/ansible_test/_internal/provider/layout/collection.py b/test/lib/ansible_test/_internal/provider/layout/collection.py index 44e0df63de3..2cceab26404 100644 --- a/test/lib/ansible_test/_internal/provider/layout/collection.py +++ b/test/lib/ansible_test/_internal/provider/layout/collection.py @@ -44,6 +44,9 @@ class CollectionLayout(LayoutProvider): namespace=collection_namespace, root=collection_root, ), + test_path='test', + results_path='test/results', + sanity_path='test/sanity', integration_path='test/integration', unit_path='test/unit', unit_module_path='test/unit/plugins/modules', diff --git a/test/lib/ansible_test/_internal/sanity/__init__.py b/test/lib/ansible_test/_internal/sanity/__init__.py index 080b5cf9b99..d61cf2fa05e 100644 --- a/test/lib/ansible_test/_internal/sanity/__init__.py +++ b/test/lib/ansible_test/_internal/sanity/__init__.py @@ -246,7 +246,7 @@ class SanityIgnoreParser: file_name = 'ignore.txt' self.args = args - self.relative_path = os.path.join('test/sanity', file_name) + self.relative_path = os.path.join(data_context().content.sanity_path, file_name) self.path = os.path.join(data_context().content.root, self.relative_path) self.ignores = collections.defaultdict(lambda: collections.defaultdict(dict)) # type: t.Dict[str, t.Dict[str, t.Dict[str, int]]] self.skips = collections.defaultdict(lambda: collections.defaultdict(int)) # type: t.Dict[str, t.Dict[str, int]] diff --git a/test/lib/ansible_test/_internal/util_common.py b/test/lib/ansible_test/_internal/util_common.py index 3816e62c9c3..c12c48649c9 100644 --- a/test/lib/ansible_test/_internal/util_common.py +++ b/test/lib/ansible_test/_internal/util_common.py @@ -58,12 +58,12 @@ class ResultType: @property def relative_path(self): # type: () -> str """The content relative path to the results.""" - return os.path.join(data_context().results_relative, self.name) + return os.path.join(data_context().content.results_path, self.name) @property def path(self): # type: () -> str """The absolute path to the results.""" - return os.path.join(data_context().results, self.name) + return os.path.join(data_context().content.root, self.relative_path) def __str__(self): # type: () -> str return self.name @@ -238,7 +238,7 @@ def get_coverage_environment(args, target_name, version, temp_path, module_cover # config is in a temporary directory # results are in the source tree coverage_config_base_path = args.coverage_config_base_path - coverage_output_base_path = data_context().results + coverage_output_base_path = os.path.join(data_context().content.root, data_context().content.results_path) else: raise Exception('No temp path and no coverage config base path. Check for missing coverage_context usage.')