diff options
Diffstat (limited to 'test')
11 files changed, 193 insertions, 176 deletions
diff --git a/test/lib/ansible_test/_internal/ci/__init__.py b/test/lib/ansible_test/_internal/ci/__init__.py index cc75a5b63e..5e53b15075 100644 --- a/test/lib/ansible_test/_internal/ci/__init__.py +++ b/test/lib/ansible_test/_internal/ci/__init__.py @@ -62,8 +62,8 @@ class CIProvider(metaclass=abc.ABCMeta): """Return a resource prefix specific to this CI provider.""" @abc.abstractmethod - def get_base_branch(self) -> str: - """Return the base branch or an empty string.""" + def get_base_commit(self, args: CommonConfig) -> str: + """Return the base commit or an empty string.""" @abc.abstractmethod def detect_changes(self, args: TestConfig) -> t.Optional[list[str]]: diff --git a/test/lib/ansible_test/_internal/ci/azp.py b/test/lib/ansible_test/_internal/ci/azp.py index 12e041fa84..404f8056af 100644 --- a/test/lib/ansible_test/_internal/ci/azp.py +++ b/test/lib/ansible_test/_internal/ci/azp.py @@ -44,6 +44,8 @@ class AzurePipelines(CIProvider): def __init__(self) -> None: self.auth = AzurePipelinesAuthHelper() + self._changes: AzurePipelinesChanges | None = None + @staticmethod def is_supported() -> bool: """Return True if this provider is supported in the current running environment.""" @@ -72,18 +74,20 @@ class AzurePipelines(CIProvider): return prefix - def get_base_branch(self) -> str: - """Return the base branch or an empty string.""" - base_branch = os.environ.get('SYSTEM_PULLREQUEST_TARGETBRANCH') or os.environ.get('BUILD_SOURCEBRANCHNAME') + def get_base_commit(self, args: CommonConfig) -> str: + """Return the base commit or an empty string.""" + return self._get_changes(args).base_commit or '' - if base_branch: - base_branch = 'origin/%s' % base_branch + def _get_changes(self, args: CommonConfig) -> AzurePipelinesChanges: + """Return an AzurePipelinesChanges instance, which will be created on first use.""" + if not self._changes: + self._changes = AzurePipelinesChanges(args) - return base_branch or '' + return self._changes def detect_changes(self, args: TestConfig) -> t.Optional[list[str]]: """Initialize change detection.""" - result = AzurePipelinesChanges(args) + result = self._get_changes(args) if result.is_pr: job_type = 'pull request' @@ -129,7 +133,7 @@ class AzurePipelines(CIProvider): def get_git_details(self, args: CommonConfig) -> t.Optional[dict[str, t.Any]]: """Return details about git in the current environment.""" - changes = AzurePipelinesChanges(args) + changes = self._get_changes(args) details = dict( base_commit=changes.base_commit, diff --git a/test/lib/ansible_test/_internal/ci/local.py b/test/lib/ansible_test/_internal/ci/local.py index fef5435bb2..4b9ab13ef7 100644 --- a/test/lib/ansible_test/_internal/ci/local.py +++ b/test/lib/ansible_test/_internal/ci/local.py @@ -63,8 +63,8 @@ class Local(CIProvider): return prefix - def get_base_branch(self) -> str: - """Return the base branch or an empty string.""" + def get_base_commit(self, args: CommonConfig) -> str: + """Return the base commit or an empty string.""" return '' def detect_changes(self, args: TestConfig) -> t.Optional[list[str]]: diff --git a/test/lib/ansible_test/_internal/cli/commands/sanity.py b/test/lib/ansible_test/_internal/cli/commands/sanity.py index 1e143cbfde..c4f0c0a0ed 100644 --- a/test/lib/ansible_test/_internal/cli/commands/sanity.py +++ b/test/lib/ansible_test/_internal/cli/commands/sanity.py @@ -16,10 +16,6 @@ from ...target import ( walk_sanity_targets, ) -from ...data import ( - data_context, -) - from ..environments import ( CompositeActionCompletionFinder, ControllerMode, @@ -82,17 +78,6 @@ def do_sanity( help='enable optional errors', ) - if data_context().content.is_ansible: - sanity.add_argument( - '--keep-git', - action='store_true', - help='transfer git related files to the remote host/container', - ) - else: - sanity.set_defaults( - keep_git=False, - ) - sanity.add_argument( '--lint', action='store_true', diff --git a/test/lib/ansible_test/_internal/commands/sanity/__init__.py b/test/lib/ansible_test/_internal/commands/sanity/__init__.py index 4089dce51d..f399f2ad95 100644 --- a/test/lib/ansible_test/_internal/commands/sanity/__init__.py +++ b/test/lib/ansible_test/_internal/commands/sanity/__init__.py @@ -160,6 +160,10 @@ def command_sanity(args: SanityConfig) -> None: if args.skip_test: tests = [target for target in tests if target.name not in args.skip_test] + if not args.host_path: + for test in tests: + test.origin_hook(args) + targets_use_pypi = any(isinstance(test, SanityMultipleVersion) and test.needs_pypi for test in tests) and not args.list_tests host_state = prepare_profiles(args, targets_use_pypi=targets_use_pypi) # sanity @@ -765,6 +769,9 @@ class SanityTest(metaclass=abc.ABCMeta): """A tuple of supported Python versions or None if the test does not depend on specific Python versions.""" return CONTROLLER_PYTHON_VERSIONS + def origin_hook(self, args: SanityConfig) -> None: + """This method is called on the origin, before the test runs or delegation occurs.""" + def filter_targets(self, targets: list[TestTarget]) -> list[TestTarget]: # pylint: disable=unused-argument """Return the given list of test targets, filtered to include only those relevant for the test.""" if self.no_targets: diff --git a/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py b/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py index 182c30574b..ab7dd93c0a 100644 --- a/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py +++ b/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py @@ -1,9 +1,12 @@ """Sanity test using validate-modules.""" from __future__ import annotations +import atexit import collections +import contextlib import json import os +import tarfile import typing as t from . import ( @@ -16,6 +19,10 @@ from . import ( SANITY_ROOT, ) +from ...io import ( + make_dirs, +) + from ...test import ( TestResult, ) @@ -30,7 +37,9 @@ from ...util import ( ) from ...util_common import ( + process_scoped_temporary_directory, run_command, + ResultType, ) from ...ansible_util import ( @@ -49,12 +58,21 @@ from ...ci import ( from ...data import ( data_context, + PayloadConfig, ) from ...host_configs import ( PythonConfig, ) +from ...git import ( + Git, +) + +from ...provider.source import ( + SourceProvider as GitSourceProvider, +) + class ValidateModulesTest(SanitySingleVersion): """Sanity test using validate-modules.""" @@ -130,14 +148,17 @@ class ValidateModulesTest(SanitySingleVersion): except CollectionDetailError as ex: display.warning('Skipping validate-modules collection version checks since collection detail loading failed: %s' % ex.reason) else: - base_branch = args.base_branch or get_ci_provider().get_base_branch() + path = self.get_archive_path(args) + + if os.path.exists(path): + temp_dir = process_scoped_temporary_directory(args) + + with tarfile.open(path) as file: + file.extractall(temp_dir) - if base_branch: cmd.extend([ - '--base-branch', base_branch, + '--original-plugins', temp_dir, ]) - else: - display.warning('Cannot perform module comparison against the base branch because the base branch was not detected.') errors = [] @@ -188,3 +209,43 @@ class ValidateModulesTest(SanitySingleVersion): return SanityFailure(self.name, messages=all_errors) return SanitySuccess(self.name) + + def origin_hook(self, args: SanityConfig) -> None: + """This method is called on the origin, before the test runs or delegation occurs.""" + if not data_context().content.is_ansible: + return + + if not isinstance(data_context().source_provider, GitSourceProvider): + display.warning('The validate-modules sanity test cannot compare against the base commit because git is not being used.') + return + + base_commit = args.base_branch or get_ci_provider().get_base_commit(args) + + if not base_commit: + display.warning('The validate-modules sanity test cannot compare against the base commit because it was not detected.') + return + + path = self.get_archive_path(args) + + def cleanup() -> None: + """Cleanup callback called when the process exits.""" + with contextlib.suppress(FileNotFoundError): + os.unlink(path) + + def git_callback(payload_config: PayloadConfig) -> None: + """Include the previous plugin content archive in the payload.""" + files = payload_config.files + files.append((path, os.path.relpath(path, data_context().content.root))) + + atexit.register(cleanup) + data_context().register_payload_callback(git_callback) + + make_dirs(os.path.dirname(path)) + + git = Git() + git.run_git(['archive', '--output', path, base_commit, 'lib/ansible/modules/', 'lib/ansible/plugins/']) + + @staticmethod + def get_archive_path(args: SanityConfig) -> str: + """Return the path to the original plugin content archive.""" + return os.path.join(ResultType.TMP.path, f'validate-modules-{args.metadata.session_id}.tar') diff --git a/test/lib/ansible_test/_internal/config.py b/test/lib/ansible_test/_internal/config.py index a0e0eb64fd..4e69793300 100644 --- a/test/lib/ansible_test/_internal/config.py +++ b/test/lib/ansible_test/_internal/config.py @@ -269,23 +269,10 @@ class SanityConfig(TestConfig): self.list_tests: bool = args.list_tests self.allow_disabled: bool = args.allow_disabled self.enable_optional_errors: bool = args.enable_optional_errors - self.keep_git: bool = args.keep_git self.prime_venvs: bool = args.prime_venvs self.display_stderr = self.lint or self.list_tests - if self.keep_git: - - def git_callback(payload_config: PayloadConfig) -> None: - """Add files from the content root .git directory to the payload file list.""" - files = payload_config.files - - for dirpath, _dirnames, filenames in os.walk(os.path.join(data_context().content.root, '.git')): - paths = [os.path.join(dirpath, filename) for filename in filenames] - files.extend((path, os.path.relpath(path, data_context().content.root)) for path in paths) - - data_context().register_payload_callback(git_callback) - class IntegrationConfig(TestConfig): """Configuration for the integration command.""" diff --git a/test/lib/ansible_test/_internal/data.py b/test/lib/ansible_test/_internal/data.py index 68392a3cdd..379ee7b011 100644 --- a/test/lib/ansible_test/_internal/data.py +++ b/test/lib/ansible_test/_internal/data.py @@ -75,13 +75,14 @@ class DataContext: self.payload_callbacks: list[c.Callable[[PayloadConfig], None]] = [] if content_path: - content = self.__create_content_layout(layout_providers, source_providers, content_path, False) + content, source_provider = self.__create_content_layout(layout_providers, source_providers, content_path, False) elif ANSIBLE_SOURCE_ROOT and is_subdir(current_path, ANSIBLE_SOURCE_ROOT): - content = self.__create_content_layout(layout_providers, source_providers, ANSIBLE_SOURCE_ROOT, False) + content, source_provider = self.__create_content_layout(layout_providers, source_providers, ANSIBLE_SOURCE_ROOT, False) else: - content = self.__create_content_layout(layout_providers, source_providers, current_path, True) + content, source_provider = self.__create_content_layout(layout_providers, source_providers, current_path, True) self.content: ContentLayout = content + self.source_provider = source_provider def create_collection_layouts(self) -> list[ContentLayout]: """ @@ -109,7 +110,7 @@ class DataContext: if collection_path == os.path.join(collection.root, collection.directory): collection_layout = layout else: - collection_layout = self.__create_content_layout(self.__layout_providers, self.__source_providers, collection_path, False) + collection_layout = self.__create_content_layout(self.__layout_providers, self.__source_providers, collection_path, False)[0] file_count = len(collection_layout.all_files()) @@ -127,7 +128,7 @@ class DataContext: source_providers: list[t.Type[SourceProvider]], root: str, walk: bool, - ) -> ContentLayout: + ) -> t.Tuple[ContentLayout, SourceProvider]: """Create a content layout using the given providers and root path.""" try: layout_provider = find_path_provider(LayoutProvider, layout_providers, root, walk) @@ -148,7 +149,7 @@ class DataContext: layout = layout_provider.create(layout_provider.root, source_provider.get_paths(layout_provider.root)) - return layout + return layout, source_provider def __create_ansible_source(self): """Return a tuple of Ansible source files with both absolute and relative paths.""" diff --git a/test/lib/ansible_test/_internal/delegation.py b/test/lib/ansible_test/_internal/delegation.py index 10f0398258..7114f2abd4 100644 --- a/test/lib/ansible_test/_internal/delegation.py +++ b/test/lib/ansible_test/_internal/delegation.py @@ -346,7 +346,7 @@ def filter_options( ('--metadata', 1, args.metadata_path), ('--exclude', 1, exclude), ('--require', 1, require), - ('--base-branch', 1, args.base_branch or get_ci_provider().get_base_branch()), + ('--base-branch', 1, False), ]) pass_through_args: list[str] = [] diff --git a/test/lib/ansible_test/_internal/metadata.py b/test/lib/ansible_test/_internal/metadata.py index 2d99df4ccc..b8b598e88a 100644 --- a/test/lib/ansible_test/_internal/metadata.py +++ b/test/lib/ansible_test/_internal/metadata.py @@ -4,6 +4,7 @@ import typing as t from .util import ( display, + generate_name, ) from .io import ( @@ -26,6 +27,7 @@ class Metadata: self.cloud_config: t.Optional[dict[str, dict[str, t.Union[int, str, bool]]]] = None self.change_description: t.Optional[ChangeDescription] = None self.ci_provider: t.Optional[str] = None + self.session_id = generate_name() def populate_changes(self, diff: t.Optional[list[str]]) -> None: """Populate the changeset using the given diff.""" @@ -53,6 +55,7 @@ class Metadata: cloud_config=self.cloud_config, ci_provider=self.ci_provider, change_description=self.change_description.to_dict(), + session_id=self.session_id, ) def to_file(self, path: str) -> None: @@ -77,6 +80,7 @@ class Metadata: metadata.cloud_config = data['cloud_config'] metadata.ci_provider = data['ci_provider'] metadata.change_description = ChangeDescription.from_dict(data['change_description']) + metadata.session_id = data['session_id'] return metadata diff --git a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py index 270c9f4444..25c61798e9 100644 --- a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py +++ b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py @@ -24,9 +24,7 @@ import datetime import json import os import re -import subprocess import sys -import tempfile import traceback import warnings @@ -301,8 +299,8 @@ class ModuleValidator(Validator): ACCEPTLIST_FUTURE_IMPORTS = frozenset(('absolute_import', 'division', 'print_function')) - def __init__(self, path, analyze_arg_spec=False, collection=None, collection_version=None, - base_branch=None, git_cache=None, reporter=None, routing=None, plugin_type='module'): + def __init__(self, path, git_cache: GitCache, analyze_arg_spec=False, collection=None, collection_version=None, + reporter=None, routing=None, plugin_type='module'): super(ModuleValidator, self).__init__(reporter=reporter or Reporter()) self.path = path @@ -328,8 +326,8 @@ class ModuleValidator(Validator): self.collection_version_str = collection_version self.collection_version = SemanticVersion(collection_version) - self.base_branch = base_branch - self.git_cache = git_cache or GitCache() + self.git_cache = git_cache + self.base_module = self.git_cache.get_original_path(self.path) self._python_module_override = False @@ -341,11 +339,6 @@ class ModuleValidator(Validator): except Exception: self.ast = None - if base_branch: - self.base_module = self._get_base_file() - else: - self.base_module = None - def _create_version(self, v, collection_name=None): if not v: raise ValueError('Empty string is not a valid version') @@ -368,13 +361,7 @@ class ModuleValidator(Validator): return self def __exit__(self, exc_type, exc_value, traceback): - if not self.base_module: - return - - try: - os.remove(self.base_module) - except Exception: - pass + pass @property def object_name(self): @@ -426,36 +413,9 @@ class ModuleValidator(Validator): except AttributeError: return False - def _get_base_branch_module_path(self): - """List all paths within lib/ansible/modules to try and match a moved module""" - return self.git_cache.base_module_paths.get(self.object_name) - - def _has_alias(self): - """Return true if the module has any aliases.""" - return self.object_name in self.git_cache.head_aliased_modules - - def _get_base_file(self): - # In case of module moves, look for the original location - base_path = self._get_base_branch_module_path() - ext = os.path.splitext(base_path or self.path)[1] - - command = ['git', 'show', '%s:%s' % (self.base_branch, base_path or self.path)] - p = subprocess.run(command, stdin=subprocess.DEVNULL, capture_output=True, check=False) - - if int(p.returncode) != 0: - return None - - t = tempfile.NamedTemporaryFile(delete=False, suffix=ext) - t.write(p.stdout) - t.close() - - return t.name - - def _is_new_module(self): - if self._has_alias(): - return False - - return not self.object_name.startswith('_') and bool(self.base_branch) and not bool(self.base_module) + def _is_new_module(self) -> bool | None: + """Return True if the content is new, False if it is not and None if the information is not available.""" + return self.git_cache.is_new(self.path) def _check_interpreter(self, powershell=False): if powershell: @@ -2050,7 +2010,7 @@ class ModuleValidator(Validator): ) def _check_for_new_args(self, doc): - if not self.base_branch or self._is_new_module(): + if not self.base_module: return with CaptureStd(): @@ -2284,7 +2244,7 @@ class ModuleValidator(Validator): # We can only validate PowerShell arg spec if it is using the new Ansible.Basic.AnsibleModule util pattern = r'(?im)^#\s*ansiblerequires\s+\-csharputil\s*Ansible\.Basic' if re.search(pattern, self.text) and self.object_name not in self.PS_ARG_VALIDATE_REJECTLIST: - with ModuleValidator(docs_path, base_branch=self.base_branch, git_cache=self.git_cache) as docs_mv: + with ModuleValidator(docs_path, git_cache=self.git_cache) as docs_mv: docs = docs_mv._validate_docs()[1] self._validate_ansible_module_call(docs) @@ -2329,6 +2289,84 @@ class PythonPackageValidator(Validator): ) +class GitCache(metaclass=abc.ABCMeta): + """Base class for access to original files.""" + @abc.abstractmethod + def get_original_path(self, path: str) -> str | None: + """Return the path to the original version of the specified file, or None if there isn't one.""" + + @abc.abstractmethod + def is_new(self, path: str) -> bool | None: + """Return True if the content is new, False if it is not and None if the information is not available.""" + + @staticmethod + def create(original_plugins: str | None, plugin_type: str) -> GitCache: + return CoreGitCache(original_plugins, plugin_type) if original_plugins else NoOpGitCache() + + +class CoreGitCache(GitCache): + """Provides access to original files when testing core.""" + def __init__(self, original_plugins: str | None, plugin_type: str) -> None: + super().__init__() + + self.original_plugins = original_plugins + + rel_path = 'lib/ansible/modules/' if plugin_type == 'module' else f'lib/ansible/plugins/{plugin_type}/' + head_tree = self._find_files(rel_path) + + head_aliased_modules = set() + + for path in head_tree: + filename = os.path.basename(path) + + if filename.startswith('_') and filename != '__init__.py': + if os.path.islink(path): + head_aliased_modules.add(os.path.basename(os.path.realpath(path))) + + self._head_aliased_modules = head_aliased_modules + + def get_original_path(self, path: str) -> str | None: + """Return the path to the original version of the specified file, or None if there isn't one.""" + path = os.path.join(self.original_plugins, path) + + if not os.path.exists(path): + path = None + + return path + + def is_new(self, path: str) -> bool | None: + """Return True if the content is new, False if it is not and None if the information is not available.""" + if os.path.basename(path).startswith('_'): + return False + + if os.path.basename(path) in self._head_aliased_modules: + return False + + return not self.get_original_path(path) + + @staticmethod + def _find_files(path: str) -> list[str]: + """Return a list of files found in the specified directory.""" + paths = [] + + for (dir_path, dir_names, file_names) in os.walk(path): + for file_name in file_names: + paths.append(os.path.join(dir_path, file_name)) + + return sorted(paths) + + +class NoOpGitCache(GitCache): + """Provides a no-op interface for access to original files.""" + def get_original_path(self, path: str) -> str | None: + """Return the path to the original version of the specified file, or None if there isn't one.""" + return None + + def is_new(self, path: str) -> bool | None: + """Return True if the content is new, False if it is not and None if the information is not available.""" + return None + + def re_compile(value): """ Argparse expects things to raise TypeError, re.compile raises an re.error @@ -2354,8 +2392,6 @@ def run(): type=re_compile) parser.add_argument('--arg-spec', help='Analyze module argument spec', action='store_true', default=False) - parser.add_argument('--base-branch', default=None, - help='Used in determining if new options were added') parser.add_argument('--format', choices=['json', 'plain'], default='plain', help='Output format. Default: "%(default)s"') parser.add_argument('--output', default='-', @@ -2372,13 +2408,14 @@ def run(): parser.add_argument('--plugin-type', default='module', help='The plugin type to validate. Defaults to %(default)s') + parser.add_argument('--original-plugins') args = parser.parse_args() args.plugins = [m.rstrip('/') for m in args.plugins] reporter = Reporter() - git_cache = GitCache(args.base_branch, args.plugin_type) + git_cache = GitCache.create(args.original_plugins, args.plugin_type) check_dirs = set() @@ -2403,7 +2440,7 @@ def run(): if ModuleValidator.is_on_rejectlist(path): continue with ModuleValidator(path, collection=args.collection, collection_version=args.collection_version, - analyze_arg_spec=args.arg_spec, base_branch=args.base_branch, + analyze_arg_spec=args.arg_spec, git_cache=git_cache, reporter=reporter, routing=routing, plugin_type=args.plugin_type) as mv1: mv1.validate() @@ -2428,7 +2465,7 @@ def run(): if ModuleValidator.is_on_rejectlist(path): continue with ModuleValidator(path, collection=args.collection, collection_version=args.collection_version, - analyze_arg_spec=args.arg_spec, base_branch=args.base_branch, + analyze_arg_spec=args.arg_spec, git_cache=git_cache, reporter=reporter, routing=routing, plugin_type=args.plugin_type) as mv2: mv2.validate() @@ -2444,75 +2481,6 @@ def run(): sys.exit(reporter.json(warnings=args.warnings, output=args.output)) -class GitCache: - def __init__(self, base_branch, plugin_type): - self.base_branch = base_branch - self.plugin_type = plugin_type - - self.rel_path = 'lib/ansible/modules/' - if plugin_type != 'module': - self.rel_path = 'lib/ansible/plugins/%s/' % plugin_type - - if self.base_branch: - self.base_tree = self._git(['ls-tree', '-r', '--name-only', self.base_branch, self.rel_path]) - else: - self.base_tree = [] - - try: - self.head_tree = self._git(['ls-tree', '-r', '--name-only', 'HEAD', self.rel_path]) - except GitError as ex: - if ex.status == 128: - # fallback when there is no .git directory - self.head_tree = self._get_module_files() - else: - raise - except FileNotFoundError: - # fallback when git is not installed - self.head_tree = self._get_module_files() - - allowed_exts = ('.py', '.ps1') - if plugin_type != 'module': - allowed_exts = ('.py', ) - self.base_module_paths = dict((os.path.basename(p), p) for p in self.base_tree if os.path.splitext(p)[1] in allowed_exts) - - self.base_module_paths.pop('__init__.py', None) - - self.head_aliased_modules = set() - - for path in self.head_tree: - filename = os.path.basename(path) - - if filename.startswith('_') and filename != '__init__.py': - if os.path.islink(path): - self.head_aliased_modules.add(os.path.basename(os.path.realpath(path))) - - def _get_module_files(self): - module_files = [] - - for (dir_path, dir_names, file_names) in os.walk(self.rel_path): - for file_name in file_names: - module_files.append(os.path.join(dir_path, file_name)) - - return module_files - - @staticmethod - def _git(args): - cmd = ['git'] + args - p = subprocess.run(cmd, stdin=subprocess.DEVNULL, capture_output=True, text=True, check=False) - - if p.returncode != 0: - raise GitError(p.stderr, p.returncode) - - return p.stdout.splitlines() - - -class GitError(Exception): - def __init__(self, message, status): - super(GitError, self).__init__(message) - - self.status = status - - def main(): try: run() |