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.

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 70c8a2554e..59d2ddf18e 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
-
-        if path.startswith('hacking/'):
-            return minimal
+            # changes to files which are not unit tests should trigger tests from the nearest parent directory
 
-        if path.startswith('lib/ansible/executor/powershell/'):
-            units_path = 'test/units/executor/powershell/'
+            test_path = os.path.dirname(path)
 
-            if units_path not in self.units_paths:
-                units_path = None
+            while test_path:
+                if test_path + '/' in self.units_paths:
+                    return {
+                        'units': test_path + '/',
+                    }
 
-            return {
-                'windows-integration': self.integration_all_target,
-                'units': units_path,
-            }
+                test_path = os.path.dirname(test_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 1dc8486840..6f8cbf45ca 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 8e2236295e..cfe6176f06 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 b0ab798a46..fd52173861 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 402b40f41e..49f304b14d 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 164557945d..6ef68dada2 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 44e0df63de..2cceab2640 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 080b5cf9b9..d61cf2fa05 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 3816e62c9c..c12c48649c 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.')