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

@@ -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,

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

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,

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,
                              )

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)

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)

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

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)

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):
     """