From f90587069cef9d96fe24265866c59c0e841008d5 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Thu, 29 Aug 2019 03:39:03 -0700 Subject: [PATCH] Update collections integration targets path. (#61523) * Update collections integration targets path. * Fix integration path handling. * Add collections test target fallback. Also add warnings and errors for common path mistakes. * Improve role target detection. --- test/lib/ansible_test/_internal/executor.py | 11 ++- .../_internal/integration/__init__.py | 16 +++-- .../_internal/provider/layout/__init__.py | 14 ++++ .../_internal/provider/layout/ansible.py | 3 + .../_internal/provider/layout/collection.py | 69 ++++++++++++++++++- .../ansible_test/_internal/sanity/__init__.py | 3 + test/lib/ansible_test/_internal/target.py | 51 +++++++++++++- .../ansible_test/_internal/units/__init__.py | 3 + .../lib/ansible_test/_internal/util_common.py | 20 ++++++ 9 files changed, 179 insertions(+), 11 deletions(-) diff --git a/test/lib/ansible_test/_internal/executor.py b/test/lib/ansible_test/_internal/executor.py index 3de843d6123..a3f008d49b8 100644 --- a/test/lib/ansible_test/_internal/executor.py +++ b/test/lib/ansible_test/_internal/executor.py @@ -74,6 +74,7 @@ from .util_common import ( write_text_file, write_json_test_results, ResultType, + handle_layout_messages, ) from .docker_util import ( @@ -363,6 +364,8 @@ def command_posix_integration(args): """ :type args: PosixIntegrationConfig """ + handle_layout_messages(data_context().content.integration_messages) + inventory_relative_path = get_inventory_relative_path(args) inventory_path = os.path.join(ANSIBLE_TEST_DATA_ROOT, os.path.basename(inventory_relative_path)) @@ -375,6 +378,8 @@ def command_network_integration(args): """ :type args: NetworkIntegrationConfig """ + handle_layout_messages(data_context().content.integration_messages) + inventory_relative_path = get_inventory_relative_path(args) template_path = os.path.join(ANSIBLE_TEST_CONFIG_ROOT, os.path.basename(inventory_relative_path)) + '.template' @@ -556,6 +561,8 @@ def command_windows_integration(args): """ :type args: WindowsIntegrationConfig """ + handle_layout_messages(data_context().content.integration_messages) + inventory_relative_path = get_inventory_relative_path(args) template_path = os.path.join(ANSIBLE_TEST_CONFIG_ROOT, os.path.basename(inventory_relative_path)) + '.template' @@ -1316,7 +1323,7 @@ def command_integration_script(args, target, test_dir, inventory_path, temp_path cmd.append('-' + ('v' * args.verbosity)) env = integration_environment(args, target, test_dir, test_env.inventory_path, test_env.ansible_config, env_config) - cwd = os.path.join(test_env.integration_dir, 'targets', target.name) + cwd = os.path.join(test_env.targets_dir, target.relative_path) if env_config and env_config.env_vars: env.update(env_config.env_vars) @@ -1421,7 +1428,7 @@ def command_integration_role(args, target, start_at_task, test_dir, inventory_pa env = integration_environment(args, target, test_dir, test_env.inventory_path, test_env.ansible_config, env_config) cwd = test_env.integration_dir - env['ANSIBLE_ROLES_PATH'] = os.path.abspath(os.path.join(test_env.integration_dir, 'targets')) + env['ANSIBLE_ROLES_PATH'] = test_env.targets_dir module_coverage = 'non_local/' not in target.aliases intercept_command(args, cmd, target_name=target.name, env=env, cwd=cwd, temp_path=temp_path, diff --git a/test/lib/ansible_test/_internal/integration/__init__.py b/test/lib/ansible_test/_internal/integration/__init__.py index 209eaea98b1..85fc5978d58 100644 --- a/test/lib/ansible_test/_internal/integration/__init__.py +++ b/test/lib/ansible_test/_internal/integration/__init__.py @@ -195,17 +195,18 @@ def integration_test_environment(args, target, inventory_path_src): :type inventory_path_src: str """ ansible_config_src = args.get_ansible_config() - ansible_config_relative = os.path.relpath(ansible_config_src, data_context().content.root) + ansible_config_relative = os.path.join(data_context().content.integration_path, '%s.cfg' % args.command) if args.no_temp_workdir or 'no/temp_workdir/' in target.aliases: display.warning('Disabling the temp work dir is a temporary debugging feature that may be removed in the future without notice.') integration_dir = os.path.join(data_context().content.root, data_context().content.integration_path) + targets_dir = os.path.join(data_context().content.root, data_context().content.integration_targets_path) inventory_path = inventory_path_src ansible_config = ansible_config_src vars_file = os.path.join(data_context().content.root, data_context().content.integration_vars_path) - yield IntegrationEnvironment(integration_dir, inventory_path, ansible_config, vars_file) + yield IntegrationEnvironment(integration_dir, targets_dir, inventory_path, ansible_config, vars_file) return root_temp_dir = os.path.expanduser('~/.ansible/test/tmp') @@ -236,6 +237,7 @@ def integration_test_environment(args, target, inventory_path_src): files_needed = get_files_needed(target_dependencies) integration_dir = os.path.join(temp_dir, data_context().content.integration_path) + targets_dir = os.path.join(temp_dir, data_context().content.integration_targets_path) ansible_config = os.path.join(temp_dir, ansible_config_relative) vars_file_src = os.path.join(data_context().content.root, data_context().content.integration_vars_path) @@ -254,7 +256,10 @@ def integration_test_environment(args, target, inventory_path_src): integration_targets_relative_path = data_context().content.integration_targets_path directory_copies = [ - (os.path.join(integration_targets_relative_path, target.name), os.path.join(temp_dir, integration_targets_relative_path, target.name)) + ( + os.path.join(integration_targets_relative_path, target.relative_path), + os.path.join(temp_dir, integration_targets_relative_path, target.relative_path) + ) for target in target_dependencies ] @@ -277,7 +282,7 @@ def integration_test_environment(args, target, inventory_path_src): make_dirs(os.path.dirname(file_dst)) shutil.copy2(file_src, file_dst) - yield IntegrationEnvironment(integration_dir, inventory_path, ansible_config, vars_file) + yield IntegrationEnvironment(integration_dir, targets_dir, inventory_path, ansible_config, vars_file) finally: if not args.explain: shutil.rmtree(temp_dir) @@ -315,8 +320,9 @@ def integration_test_config_file(args, env_config, integration_dir): class IntegrationEnvironment: """Details about the integration environment.""" - def __init__(self, integration_dir, inventory_path, ansible_config, vars_file): + def __init__(self, integration_dir, targets_dir, inventory_path, ansible_config, vars_file): self.integration_dir = integration_dir + self.targets_dir = targets_dir self.inventory_path = inventory_path self.ansible_config = ansible_config self.vars_file = vars_file diff --git a/test/lib/ansible_test/_internal/provider/layout/__init__.py b/test/lib/ansible_test/_internal/provider/layout/__init__.py index 54fa0e88c12..502b385982f 100644 --- a/test/lib/ansible_test/_internal/provider/layout/__init__.py +++ b/test/lib/ansible_test/_internal/provider/layout/__init__.py @@ -84,12 +84,15 @@ class ContentLayout(Layout): test_path, # type: str results_path, # type: str sanity_path, # type: str + sanity_messages, # type: t.Optional[LayoutMessages] integration_path, # type: str integration_targets_path, # type: str integration_vars_path, # type: str + integration_messages, # type: t.Optional[LayoutMessages] unit_path, # type: str unit_module_path, # type: str unit_module_utils_path, # type: str + unit_messages, # type: t.Optional[LayoutMessages] ): # type: (...) -> None super(ContentLayout, self).__init__(root, paths) @@ -98,12 +101,15 @@ class ContentLayout(Layout): self.test_path = test_path self.results_path = results_path self.sanity_path = sanity_path + self.sanity_messages = sanity_messages self.integration_path = integration_path self.integration_targets_path = integration_targets_path self.integration_vars_path = integration_vars_path + self.integration_messages = integration_messages self.unit_path = unit_path self.unit_module_path = unit_module_path self.unit_module_utils_path = unit_module_utils_path + self.unit_messages = unit_messages self.is_ansible = root == ANSIBLE_SOURCE_ROOT @@ -142,6 +148,14 @@ class ContentLayout(Layout): return self.plugin_paths.get('module_utils') +class LayoutMessages: + """Messages generated during layout creation that should be deferred for later display.""" + def __init__(self): + self.info = [] # type: t.List[str] + self.warning = [] # type: t.List[str] + self.error = [] # type: t.List[str] + + class CollectionDetail: """Details about the layout of the current collection.""" def __init__(self, diff --git a/test/lib/ansible_test/_internal/provider/layout/ansible.py b/test/lib/ansible_test/_internal/provider/layout/ansible.py index b393d7907a0..49ca482b7b4 100644 --- a/test/lib/ansible_test/_internal/provider/layout/ansible.py +++ b/test/lib/ansible_test/_internal/provider/layout/ansible.py @@ -35,10 +35,13 @@ class AnsibleLayout(LayoutProvider): test_path='test', results_path='test/results', sanity_path='test/sanity', + sanity_messages=None, integration_path='test/integration', integration_targets_path='test/integration/targets', integration_vars_path='test/integration/integration_config.yml', + integration_messages=None, unit_path='test/units', unit_module_path='test/units/modules', unit_module_utils_path='test/units/module_utils', + unit_messages=None, ) diff --git a/test/lib/ansible_test/_internal/provider/layout/collection.py b/test/lib/ansible_test/_internal/provider/layout/collection.py index 3f4ddbd858f..77fc43e0460 100644 --- a/test/lib/ansible_test/_internal/provider/layout/collection.py +++ b/test/lib/ansible_test/_internal/provider/layout/collection.py @@ -10,6 +10,7 @@ from . import ( ContentLayout, LayoutProvider, CollectionDetail, + LayoutMessages, ) @@ -36,6 +37,19 @@ class CollectionLayout(LayoutProvider): collection_root = os.path.dirname(collection_root) + sanity_messages = LayoutMessages() + integration_messages = LayoutMessages() + unit_messages = LayoutMessages() + + # these apply to all test commands + self.__check_test_path(paths, sanity_messages) + self.__check_test_path(paths, integration_messages) + self.__check_test_path(paths, unit_messages) + + # these apply to specific test commands + integration_targets_path = self.__check_integration_path(paths, integration_messages) + self.__check_unit_path(paths, unit_messages) + return ContentLayout(root, paths, plugin_paths=plugin_paths, @@ -47,10 +61,63 @@ class CollectionLayout(LayoutProvider): test_path='tests', results_path='tests/output', sanity_path='tests/sanity', + sanity_messages=sanity_messages, integration_path='tests/integration', - integration_targets_path='tests/integration/targets', + integration_targets_path=integration_targets_path.rstrip(os.path.sep), integration_vars_path='tests/integration/integration_config.yml', + integration_messages=integration_messages, unit_path='tests/unit', unit_module_path='tests/unit/plugins/modules', unit_module_utils_path='tests/unit/plugins/module_utils', + unit_messages=unit_messages, ) + + @staticmethod + def __check_test_path(paths, messages): # type: (t.List[str], LayoutMessages) -> None + modern_test_path = 'tests/' + modern_test_path_found = any(path.startswith(modern_test_path) for path in paths) + legacy_test_path = 'test/' + legacy_test_path_found = any(path.startswith(legacy_test_path) for path in paths) + + if modern_test_path_found and legacy_test_path_found: + messages.warning.append('Ignoring tests in "%s" in favor of "%s".' % (legacy_test_path, modern_test_path)) + elif legacy_test_path_found: + messages.warning.append('Ignoring tests in "%s" that should be in "%s".' % (legacy_test_path, modern_test_path)) + + @staticmethod + def __check_integration_path(paths, messages): # type: (t.List[str], LayoutMessages) -> str + modern_integration_path = 'roles/test/' + modern_integration_path_found = any(path.startswith(modern_integration_path) for path in paths) + legacy_integration_path = 'tests/integration/targets/' + legacy_integration_path_found = any(path.startswith(legacy_integration_path) for path in paths) + + if modern_integration_path_found and legacy_integration_path_found: + messages.warning.append('Ignoring tests in "%s" in favor of "%s".' % (legacy_integration_path, modern_integration_path)) + integration_targets_path = modern_integration_path + elif legacy_integration_path_found: + messages.info.append('Falling back to tests in "%s" because "%s" was not found.' % (legacy_integration_path, modern_integration_path)) + integration_targets_path = legacy_integration_path + elif modern_integration_path_found: + messages.info.append('Loading tests from "%s".') + integration_targets_path = modern_integration_path + else: + messages.error.append('Cannot run integration tests without "%s" or "%s".' % (modern_integration_path, legacy_integration_path)) + integration_targets_path = modern_integration_path + + return integration_targets_path + + @staticmethod + def __check_unit_path(paths, messages): # type: (t.List[str], LayoutMessages) -> None + modern_unit_path = 'tests/unit/' + modern_unit_path_found = any(path.startswith(modern_unit_path) for path in paths) + legacy_unit_path = 'tests/units/' # test/units/ will be covered by the warnings for test/ vs tests/ + legacy_unit_path_found = any(path.startswith(legacy_unit_path) for path in paths) + + if modern_unit_path_found and legacy_unit_path_found: + messages.warning.append('Ignoring tests in "%s" in favor of "%s".' % (legacy_unit_path, modern_unit_path)) + elif legacy_unit_path_found: + messages.warning.append('Rename "%s" to "%s" to run unit tests.' % (legacy_unit_path, modern_unit_path)) + elif modern_unit_path_found: + pass # unit tests only run from one directory so no message is needed + else: + messages.error.append('Cannot run unit tests without "%s".' % modern_unit_path) diff --git a/test/lib/ansible_test/_internal/sanity/__init__.py b/test/lib/ansible_test/_internal/sanity/__init__.py index d61cf2fa05e..119c8305b6a 100644 --- a/test/lib/ansible_test/_internal/sanity/__init__.py +++ b/test/lib/ansible_test/_internal/sanity/__init__.py @@ -31,6 +31,7 @@ from ..util import ( from ..util_common import ( run_command, + handle_layout_messages, ) from ..ansible_util import ( @@ -76,6 +77,8 @@ def command_sanity(args): """ :type args: SanityConfig """ + handle_layout_messages(data_context().content.sanity_messages) + changes = get_changes_filter(args) require = args.require + changes targets = SanityTargets.create(args.include, args.exclude, require) diff --git a/test/lib/ansible_test/_internal/target.py b/test/lib/ansible_test/_internal/target.py index c0cf8c5b10d..cdbf86406b6 100644 --- a/test/lib/ansible_test/_internal/target.py +++ b/test/lib/ansible_test/_internal/target.py @@ -5,7 +5,6 @@ __metaclass__ = type import collections import os import re -import errno import itertools import abc @@ -230,8 +229,53 @@ def walk_integration_targets(): """ path = data_context().content.integration_targets_path modules = frozenset(target.module for target in walk_module_targets()) - paths = data_context().content.get_dirs(path) + paths = data_context().content.walk_files(path) prefixes = load_integration_prefixes() + targets_path_tuple = tuple(path.split(os.path.sep)) + + entry_dirs = ( + 'defaults', + 'files', + 'handlers', + 'meta', + 'tasks', + 'templates', + 'vars', + ) + + entry_files = ( + 'main.yml', + 'main.yaml', + ) + + entry_points = [] + + for entry_dir in entry_dirs: + for entry_file in entry_files: + entry_points.append(os.path.join(os.path.sep, entry_dir, entry_file)) + + # any directory with at least one file is a target + path_tuples = set(tuple(os.path.dirname(p).split(os.path.sep)) + for p in paths) + + # also detect targets which are ansible roles, looking for standard entry points + path_tuples.update(tuple(os.path.dirname(os.path.dirname(p)).split(os.path.sep)) + for p in paths if any(p.endswith(entry_point) for entry_point in entry_points)) + + # remove the top-level directory if it was included + if targets_path_tuple in path_tuples: + path_tuples.remove(targets_path_tuple) + + previous_path_tuple = None + paths = [] + + for path_tuple in sorted(path_tuples): + if previous_path_tuple and previous_path_tuple == path_tuple[:len(previous_path_tuple)]: + # ignore nested directories + continue + + previous_path_tuple = path_tuple + paths.append(os.path.sep.join(path_tuple)) for path in paths: yield IntegrationTarget(path, modules, prefixes) @@ -515,7 +559,8 @@ class IntegrationTarget(CompletionTarget): """ super(IntegrationTarget, self).__init__() - self.name = os.path.basename(path) + self.relative_path = os.path.relpath(path, data_context().content.integration_targets_path) + self.name = self.relative_path.replace(os.path.sep, '.') self.path = path # script_path and type diff --git a/test/lib/ansible_test/_internal/units/__init__.py b/test/lib/ansible_test/_internal/units/__init__.py index 2ca68f3b322..15655c4c1d3 100644 --- a/test/lib/ansible_test/_internal/units/__init__.py +++ b/test/lib/ansible_test/_internal/units/__init__.py @@ -17,6 +17,7 @@ from ..util import ( from ..util_common import ( intercept_command, ResultType, + handle_layout_messages, ) from ..ansible_util import ( @@ -54,6 +55,8 @@ def command_units(args): """ :type args: UnitsConfig """ + handle_layout_messages(data_context().content.unit_messages) + changes = get_changes_filter(args) require = args.require + changes include = walk_internal_targets(walk_units_targets(), args.include, args.exclude, require) diff --git a/test/lib/ansible_test/_internal/util_common.py b/test/lib/ansible_test/_internal/util_common.py index c12c48649c9..3cfcfca0fe3 100644 --- a/test/lib/ansible_test/_internal/util_common.py +++ b/test/lib/ansible_test/_internal/util_common.py @@ -25,12 +25,17 @@ from .util import ( to_bytes, ANSIBLE_TEST_DATA_ROOT, make_dirs, + ApplicationError, ) from .data import ( data_context, ) +from .provider.layout import ( + LayoutMessages, +) + class ResultType: """Test result type.""" @@ -99,6 +104,21 @@ class CommonConfig: return os.path.join(ANSIBLE_TEST_DATA_ROOT, 'ansible.cfg') +def handle_layout_messages(messages): # type: (t.Optional[LayoutMessages]) -> None + """Display the given layout messages.""" + if not messages: + return + + for message in messages.info: + display.info(message, verbosity=1) + + for message in messages.warning: + display.warning(message) + + if messages.error: + raise ApplicationError('\n'.join(messages.error)) + + @contextlib.contextmanager def named_temporary_file(args, prefix, suffix, directory, content): """