From 0012263c7acd971335d9f1c8a0ccc69b7e3325ee Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Thu, 21 Jul 2022 13:00:36 -0700 Subject: [PATCH] ansible-test - Fix handling of args after `--` (#78328) --- .../fragments/ansible-test-filter-options.yml | 2 + test/lib/ansible_test/_internal/config.py | 4 +- test/lib/ansible_test/_internal/delegation.py | 110 +++++++----------- 3 files changed, 46 insertions(+), 70 deletions(-) create mode 100644 changelogs/fragments/ansible-test-filter-options.yml diff --git a/changelogs/fragments/ansible-test-filter-options.yml b/changelogs/fragments/ansible-test-filter-options.yml new file mode 100644 index 00000000000..4589b919229 --- /dev/null +++ b/changelogs/fragments/ansible-test-filter-options.yml @@ -0,0 +1,2 @@ +bugfixes: + - ansible-test - Delegation now properly handles arguments given after ``--`` on the command line. diff --git a/test/lib/ansible_test/_internal/config.py b/test/lib/ansible_test/_internal/config.py index e5320f48f98..1e08cfb4782 100644 --- a/test/lib/ansible_test/_internal/config.py +++ b/test/lib/ansible_test/_internal/config.py @@ -269,8 +269,8 @@ class IntegrationConfig(TestConfig): self.tags = args.tags self.skip_tags = args.skip_tags self.diff = args.diff - self.no_temp_workdir = args.no_temp_workdir - self.no_temp_unicode = args.no_temp_unicode + self.no_temp_workdir = args.no_temp_workdir # type: bool + self.no_temp_unicode = args.no_temp_unicode # type: bool if self.list_targets: self.explain = True diff --git a/test/lib/ansible_test/_internal/delegation.py b/test/lib/ansible_test/_internal/delegation.py index 33e3f621d95..79d2dc7cbc8 100644 --- a/test/lib/ansible_test/_internal/delegation.py +++ b/test/lib/ansible_test/_internal/delegation.py @@ -15,7 +15,6 @@ from .config import ( CommonConfig, EnvironmentConfig, IntegrationConfig, - SanityConfig, ShellConfig, TestConfig, UnitsConfig, @@ -248,11 +247,6 @@ def generate_command( require, # type: t.List[str] ): # type: (...) -> t.List[str] """Generate the command necessary to delegate ansible-test.""" - options = { - '--color': 1, - '--docker-no-pull': 0, - } - cmd = [os.path.join(ansible_bin_path, 'ansible-test')] cmd = [python.path] + cmd @@ -288,16 +282,7 @@ def generate_command( cmd = ['/usr/bin/env'] + env_args + cmd - cmd += list(filter_options(args, args.host_settings.filtered_args, options, exclude, require)) - cmd += ['--color', 'yes' if args.color else 'no'] - - if isinstance(args, SanityConfig): - base_branch = args.base_branch or get_ci_provider().get_base_branch() - - if base_branch: - cmd += ['--base-branch', base_branch] - - cmd.extend(['--host-path', args.host_path]) + cmd += list(filter_options(args, args.host_settings.filtered_args, exclude, require)) return cmd @@ -305,66 +290,55 @@ def generate_command( def filter_options( args, # type: EnvironmentConfig argv, # type: t.List[str] - options, # type: t.Dict[str, int] exclude, # type: t.List[str] require, # type: t.List[str] ): # type: (...) -> t.Iterable[str] """Return an iterable that filters out unwanted CLI options and injects new ones as requested.""" - options = options.copy() - - options['--truncate'] = 1 - options['--redact'] = 0 - options['--no-redact'] = 0 + replace: list[tuple[str, int, t.Optional[t.Union[bool, str, list[str]]]]] = [ + ('--docker-no-pull', 0, False), + ('--truncate', 1, str(args.truncate)), + ('--color', 1, 'yes' if args.color else 'no'), + ('--redact', 0, False), + ('--no-redact', 0, not args.redact), + ('--host-path', 1, args.host_path), + ] if isinstance(args, TestConfig): - options.update({ - '--changed': 0, - '--tracked': 0, - '--untracked': 0, - '--ignore-committed': 0, - '--ignore-staged': 0, - '--ignore-unstaged': 0, - '--changed-from': 1, - '--changed-path': 1, - '--metadata': 1, - '--exclude': 1, - '--require': 1, - }) - elif isinstance(args, SanityConfig): - options.update({ - '--base-branch': 1, - }) - - if isinstance(args, IntegrationConfig): - options.update({ - '--no-temp-unicode': 0, - }) - - for arg in filter_args(argv, options): - yield arg + replace.extend([ + ('--changed', 0, False), + ('--tracked', 0, False), + ('--untracked', 0, False), + ('--ignore-committed', 0, False), + ('--ignore-staged', 0, False), + ('--ignore-unstaged', 0, False), + ('--changed-from', 1, False), + ('--changed-path', 1, False), + ('--metadata', 1, args.metadata_path), + ('--exclude', 1, exclude), + ('--require', 1, require), + ('--base-branch', 1, args.base_branch or get_ci_provider().get_base_branch()), + ]) + + pass_through_args: list[str] = [] + + for arg in filter_args(argv, {option: count for option, count, replacement in replace}): + if arg == '--' or pass_through_args: + pass_through_args.append(arg) + continue - for arg in args.delegate_args: yield arg - for target in exclude: - yield '--exclude' - yield target - - for target in require: - yield '--require' - yield target - - if isinstance(args, TestConfig): - if args.metadata_path: - yield '--metadata' - yield args.metadata_path - - yield '--truncate' - yield '%d' % args.truncate + for option, _count, replacement in replace: + if not replacement: + continue - if not args.redact: - yield '--no-redact' + if isinstance(replacement, bool): + yield option + elif isinstance(replacement, str): + yield from [option, replacement] + elif isinstance(replacement, list): + for item in replacement: + yield from [option, item] - if isinstance(args, IntegrationConfig): - if args.no_temp_unicode: - yield '--no-temp-unicode' + yield from args.delegate_args + yield from pass_through_args