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.
This commit is contained in:
Matt Clay 2019-08-28 15:13:00 -07:00 committed by GitHub
parent 7ff7ce0123
commit 4afe757f93
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 193 additions and 155 deletions

View file

@ -7,6 +7,8 @@ import os
import re import re
import time import time
from . import types as t
from .target import ( from .target import (
walk_module_targets, walk_module_targets,
walk_integration_targets, walk_integration_targets,
@ -157,7 +159,7 @@ def categorize_changes(args, paths, verbose_command=None):
for command in commands: for command in commands:
commands[command].discard('none') 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[command] = set(['all'])
commands = dict((c, sorted(commands[c])) for c in commands if commands[c]) 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.compile_targets = list(walk_compile_targets())
self.units_targets = list(walk_units_targets()) self.units_targets = list(walk_units_targets())
self.sanity_targets = list(walk_sanity_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.powershell_targets = [target for target in self.sanity_targets if os.path.splitext(target.path)[1] in ('.ps1', '.psm1')]
self.csharp_targets = [t for t in self.sanity_targets if os.path.splitext(t.path)[1] == '.cs'] 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_modules = set(target.module for target in self.units_targets if target.module)
self.units_paths = set(a for t in self.units_targets for a in t.aliases) self.units_paths = set(a for target in self.units_targets for a in target.aliases)
self.sanity_paths = set(t.path for t in self.sanity_targets) 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.module_names_by_path = dict((target.path, target.module) for target in self.module_targets)
self.integration_targets_by_name = dict((t.name, t) for t in self.integration_targets) self.integration_targets_by_name = dict((target.name, target) for target in self.integration_targets)
self.integration_targets_by_alias = dict((a, t) for t in self.integration_targets for a in t.aliases) 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 self.posix_integration_by_module = dict((m, target.name) for target in self.integration_targets
if 'posix/' in t.aliases for m in t.modules) if 'posix/' in target.aliases for m in target.modules)
self.windows_integration_by_module = dict((m, t.name) for t in self.integration_targets self.windows_integration_by_module = dict((m, target.name) for target in self.integration_targets
if 'windows/' in t.aliases for m in t.modules) if 'windows/' in target.aliases for m in target.modules)
self.network_integration_by_module = dict((m, t.name) for t in self.integration_targets self.network_integration_by_module = dict((m, target.name) for target in self.integration_targets
if 'network/' in t.aliases for m in t.modules) if 'network/' in target.aliases for m in target.modules)
self.prefixes = load_integration_prefixes() self.prefixes = load_integration_prefixes()
self.integration_dependencies = analyze_integration_target_dependencies(self.integration_targets) self.integration_dependencies = analyze_integration_target_dependencies(self.integration_targets)
@ -254,7 +256,7 @@ class PathMapper:
:rtype: list[str] :rtype: list[str]
""" """
paths = self.get_dependent_paths_internal(path) 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)) paths = sorted(set(paths))
return paths return paths
@ -360,57 +362,89 @@ class PathMapper:
return result return result
def _classify(self, path): def _classify(self, path): # type: (str) -> t.Optional[t.Dict[str, str]]
""" """Return the classification for the given path."""
:type path: str if data_context().content.is_ansible:
:rtype: dict[str, str] | None 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) dirname = os.path.dirname(path)
filename = os.path.basename(path) filename = os.path.basename(path)
name, ext = os.path.splitext(filename) name, ext = os.path.splitext(filename)
minimal = {} minimal = {}
if path.startswith('.github/'): if is_subdir(path, '.github'):
return minimal return minimal
if path.startswith('bin/'): if is_subdir(path, data_context().content.integration_targets_path):
return all_tests(self.args) # broad impact, run all tests 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 { 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/'): if is_subdir(path, data_context().content.integration_path):
return minimal 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 {
return minimal 'integration': self.integration_all_target,
'windows-integration': self.integration_all_target,
'network-integration': self.integration_all_target,
}
if path.startswith('examples/'): if is_subdir(path, data_context().content.sanity_path):
if path == 'examples/scripts/ConfigureRemotingForAnsible.ps1': 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 { 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/'): test_path = os.path.dirname(path)
return minimal
if path.startswith('lib/ansible/executor/powershell/'): while test_path:
units_path = 'test/units/executor/powershell/' if test_path + '/' in self.units_paths:
return {
'units': test_path + '/',
}
if units_path not in self.units_paths: test_path = os.path.dirname(test_path)
units_path = None
return { if is_subdir(path, data_context().content.module_path):
'windows-integration': self.integration_all_target,
'units': units_path,
}
if path.startswith('lib/ansible/modules/'):
module_name = self.module_names_by_path.get(path) module_name = self.module_names_by_path.get(path)
if module_name: if module_name:
@ -424,7 +458,7 @@ class PathMapper:
return minimal return minimal
if path.startswith('lib/ansible/module_utils/'): if is_subdir(path, data_context().content.module_utils_path):
if ext == '.cs': if ext == '.cs':
return minimal # already expanded using get_dependent_paths return minimal # already expanded using get_dependent_paths
@ -434,7 +468,7 @@ class PathMapper:
if ext == '.py': if ext == '.py':
return minimal # already expanded using get_dependent_paths 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 ext == '.py':
if name.startswith('net_'): if name.startswith('net_'):
network_target = 'network/.*_%s' % name[4:] network_target = 'network/.*_%s' % name[4:]
@ -474,7 +508,7 @@ class PathMapper:
'units': 'all', 'units': 'all',
} }
if path.startswith('lib/ansible/plugins/connection/'): if is_subdir(path, data_context().content.plugin_paths['connection']):
if name == '__init__': if name == '__init__':
return { return {
'integration': self.integration_all_target, 'integration': self.integration_all_target,
@ -534,7 +568,12 @@ class PathMapper:
'units': units_path, '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__': if name == '__init__':
return all_tests(self.args) # broad impact, run all tests return all_tests(self.args) # broad impact, run all tests
@ -567,9 +606,9 @@ class PathMapper:
FOCUSED_TARGET: target is not None, FOCUSED_TARGET: target is not None,
} }
if (path.startswith('lib/ansible/plugins/terminal/') or if (is_subdir(path, data_context().content.plugin_paths['terminal']) or
path.startswith('lib/ansible/plugins/cliconf/') or is_subdir(path, data_context().content.plugin_paths['cliconf']) or
path.startswith('lib/ansible/plugins/netconf/')): is_subdir(path, data_context().content.plugin_paths['netconf'])):
if ext == '.py': if ext == '.py':
if name in self.prefixes and self.prefixes[name] == 'network': if name in self.prefixes and self.prefixes[name] == 'network':
network_target = 'network/%s/' % name network_target = 'network/%s/' % name
@ -591,14 +630,77 @@ class PathMapper:
'units': 'all', '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 { 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/'): if path.startswith('lib/ansible/'):
return all_tests(self.args) # broad impact, run all tests return all_tests(self.args) # broad impact, run all tests
if path.startswith('licenses/'):
return minimal
if path.startswith('packaging/'): if path.startswith('packaging/'):
if path.startswith('packaging/requirements/'): if path.startswith('packaging/requirements/'):
if name.startswith('requirements-') and ext == '.txt': if name.startswith('requirements-') and ext == '.txt':
@ -624,72 +726,6 @@ class PathMapper:
if path.startswith('test/legacy/'): if path.startswith('test/legacy/'):
return minimal 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 path.startswith('test/lib/ansible_test/config/'):
if name.startswith('cloud-config-'): if name.startswith('cloud-config-'):
cloud_target = 'cloud/%s/' % name.split('-')[2].split('.')[0] cloud_target = 'cloud/%s/' % name.split('-')[2].split('.')[0]
@ -805,39 +841,29 @@ class PathMapper:
if path.startswith('test/utils/'): if path.startswith('test/utils/'):
return minimal return minimal
if path == 'test/README.md':
return minimal
if path.startswith('ticket_stubs/'):
return minimal
if '/' not in path: if '/' not in path:
if path in ( if path in (
'.gitattributes', '.gitattributes',
'.gitignore', '.gitignore',
'.gitmodules',
'.mailmap', '.mailmap',
'tox.ini', # obsolete
'COPYING', 'COPYING',
'VERSION',
'Makefile', 'Makefile',
): ):
return minimal return minimal
if path in ( if path in (
'setup.py',
'shippable.yml', '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 return all_tests(self.args) # broad impact, run all tests
if path == '.yamllint': if ext in (
return { '.in',
'sanity': 'all', '.md',
} '.rst',
'.toml',
if ext in ('.md', '.rst', '.txt', '.xml', '.in'): '.txt',
):
return minimal return minimal
return None # unknown, will result in fall-back to run all tests return None # unknown, will result in fall-back to run all tests

View file

@ -72,8 +72,6 @@ class DataContext:
content = self.__create_content_layout(layout_providers, source_providers, current_path, True) content = self.__create_content_layout(layout_providers, source_providers, current_path, True)
self.content = content # type: ContentLayout 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] def create_collection_layouts(self): # type: () -> t.List[ContentLayout]
""" """

View file

@ -300,7 +300,7 @@ def delegate_docker(args, exclude, require, integration_targets):
else: else:
content_root = install_root 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) 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: try:
docker_exec(args, test_id, cmd, options=cmd_options) docker_exec(args, test_id, cmd, options=cmd_options)
finally: 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_test_root = os.path.dirname(remote_results_root)
remote_results_name = os.path.basename(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 download = False
if download and content_root: 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_results_name = os.path.basename(remote_results_root)
remote_temp_path = os.path.join('/tmp', remote_results_name) remote_temp_path = os.path.join('/tmp', remote_results_name)

View file

@ -10,6 +10,7 @@ from . import types as t
from .util import ( from .util import (
display, display,
ApplicationError, ApplicationError,
is_subdir,
) )
from .data import ( from .data import (
@ -245,7 +246,7 @@ class ModuleUtilFinder(ast.NodeVisitor):
name = '.'.join(name.split('.')[:-1]) 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 return # invalid imports in tests are ignored
# Treat this error as a warning so tests can be executed as best as possible. # Treat this error as a warning so tests can be executed as best as possible.

View file

@ -80,16 +80,22 @@ class ContentLayout(Layout):
root, # type: str root, # type: str
paths, # type: t.List[str] paths, # type: t.List[str]
plugin_paths, # type: t.Dict[str, str] plugin_paths, # type: t.Dict[str, str]
collection=None, # type: t.Optional[CollectionDetail] collection, # type: t.Optional[CollectionDetail]
integration_path=None, # type: t.Optional[str] test_path, # type: str
unit_path=None, # type: t.Optional[str] results_path, # type: str
unit_module_path=None, # type: t.Optional[str] sanity_path, # type: str
unit_module_utils_path=None, # type: t.Optional[str] integration_path, # type: str
unit_path, # type: str
unit_module_path, # type: str
unit_module_utils_path, # type: str
): # type: (...) -> None ): # type: (...) -> None
super(ContentLayout, self).__init__(root, paths) super(ContentLayout, self).__init__(root, paths)
self.plugin_paths = plugin_paths self.plugin_paths = plugin_paths
self.collection = collection 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_path = integration_path
self.integration_targets_path = os.path.join(integration_path, 'targets') self.integration_targets_path = os.path.join(integration_path, 'targets')
self.integration_vars_path = os.path.join(integration_path, 'integration_config.yml') self.integration_vars_path = os.path.join(integration_path, 'integration_config.yml')

View file

@ -31,6 +31,10 @@ class AnsibleLayout(LayoutProvider):
return ContentLayout(root, return ContentLayout(root,
paths, paths,
plugin_paths=plugin_paths, plugin_paths=plugin_paths,
collection=None,
test_path='test',
results_path='test/results',
sanity_path='test/sanity',
integration_path='test/integration', integration_path='test/integration',
unit_path='test/units', unit_path='test/units',
unit_module_path='test/units/modules', unit_module_path='test/units/modules',

View file

@ -44,6 +44,9 @@ class CollectionLayout(LayoutProvider):
namespace=collection_namespace, namespace=collection_namespace,
root=collection_root, root=collection_root,
), ),
test_path='test',
results_path='test/results',
sanity_path='test/sanity',
integration_path='test/integration', integration_path='test/integration',
unit_path='test/unit', unit_path='test/unit',
unit_module_path='test/unit/plugins/modules', unit_module_path='test/unit/plugins/modules',

View file

@ -246,7 +246,7 @@ class SanityIgnoreParser:
file_name = 'ignore.txt' file_name = 'ignore.txt'
self.args = args 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.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.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]] self.skips = collections.defaultdict(lambda: collections.defaultdict(int)) # type: t.Dict[str, t.Dict[str, int]]

View file

@ -58,12 +58,12 @@ class ResultType:
@property @property
def relative_path(self): # type: () -> str def relative_path(self): # type: () -> str
"""The content relative path to the results.""" """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 @property
def path(self): # type: () -> str def path(self): # type: () -> str
"""The absolute path to the results.""" """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 def __str__(self): # type: () -> str
return self.name 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 # config is in a temporary directory
# results are in the source tree # results are in the source tree
coverage_config_base_path = args.coverage_config_base_path 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: else:
raise Exception('No temp path and no coverage config base path. Check for missing coverage_context usage.') raise Exception('No temp path and no coverage config base path. Check for missing coverage_context usage.')