From fb0d5609cbabc1dc4187b994e24aba6eaef2e198 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Fri, 30 Apr 2021 18:50:00 -0700 Subject: [PATCH] Code cleanup in ansible-test. (#74529) * Fix get_available_python_versions calls. * Make run_playbook vars optional. * Use ansible_pipelining in inventory. * Fix type hint. * Fix order of conditional evaluation. * Remove unused ibmi platform. * Add changelog fragment. --- changelogs/fragments/ansible-test-code-cleanup.yml | 2 ++ test/lib/ansible_test/_internal/ansible_util.py | 7 +++++-- test/lib/ansible_test/_internal/core_ci.py | 1 - test/lib/ansible_test/_internal/delegation.py | 2 +- test/lib/ansible_test/_internal/env.py | 6 +----- test/lib/ansible_test/_internal/manage_ci.py | 2 +- test/lib/ansible_test/_internal/sanity/__init__.py | 2 +- test/lib/ansible_test/_internal/ssh.py | 2 +- test/lib/ansible_test/_internal/units/__init__.py | 2 +- test/lib/ansible_test/_internal/util.py | 6 +++--- test/lib/ansible_test/_internal/util_common.py | 4 ++-- test/lib/ansible_test/_internal/venv.py | 3 +-- 12 files changed, 19 insertions(+), 20 deletions(-) create mode 100644 changelogs/fragments/ansible-test-code-cleanup.yml diff --git a/changelogs/fragments/ansible-test-code-cleanup.yml b/changelogs/fragments/ansible-test-code-cleanup.yml new file mode 100644 index 00000000000..69ce6c60500 --- /dev/null +++ b/changelogs/fragments/ansible-test-code-cleanup.yml @@ -0,0 +1,2 @@ +minor_changes: + - ansible-test - Minor code cleanup. diff --git a/test/lib/ansible_test/_internal/ansible_util.py b/test/lib/ansible_test/_internal/ansible_util.py index a949f903b31..2a1208f4026 100644 --- a/test/lib/ansible_test/_internal/ansible_util.py +++ b/test/lib/ansible_test/_internal/ansible_util.py @@ -298,10 +298,13 @@ def get_collection_detail(args, python): # type: (EnvironmentConfig, str) -> Co return detail -def run_playbook(args, inventory_path, playbook, run_playbook_vars): # type: (CommonConfig, str, str, t.Dict[str, t.Any]) -> None +def run_playbook(args, inventory_path, playbook, run_playbook_vars=None): # type: (CommonConfig, str, str, t.Optional[t.Dict[str, t.Any]]) -> None """Run the specified playbook using the given inventory file and playbook variables.""" playbook_path = os.path.join(ANSIBLE_TEST_DATA_ROOT, 'playbooks', playbook) - command = ['ansible-playbook', '-i', inventory_path, playbook_path, '-e', json.dumps(run_playbook_vars)] + command = ['ansible-playbook', '-i', inventory_path, playbook_path] + + if run_playbook_vars: + command.extend(['-e', json.dumps(run_playbook_vars)]) if args.verbosity: command.append('-%s' % ('v' * args.verbosity)) diff --git a/test/lib/ansible_test/_internal/core_ci.py b/test/lib/ansible_test/_internal/core_ci.py index 025969ca8c4..bd03335c424 100644 --- a/test/lib/ansible_test/_internal/core_ci.py +++ b/test/lib/ansible_test/_internal/core_ci.py @@ -77,7 +77,6 @@ class AnsibleCoreCI: ), ibmps=( 'aix', - 'ibmi', ), parallels=( 'macos', diff --git a/test/lib/ansible_test/_internal/delegation.py b/test/lib/ansible_test/_internal/delegation.py index 692635d7c96..5c05e5fc72a 100644 --- a/test/lib/ansible_test/_internal/delegation.py +++ b/test/lib/ansible_test/_internal/delegation.py @@ -477,7 +477,7 @@ def delegate_remote(args, exclude, require): # AIX cp and GNU cp provide different options, no way could be found to have a common # pattern and achieve the same goal - cp_opts = '-hr' if remote.platform in ['aix', 'ibmi'] else '-a' + cp_opts = '-hr' if remote.platform == 'aix' else '-a' try: command = 'rm -rf {0} && mkdir {0} && cp {1} {2}/* {0}/ && chmod -R a+r {0}'.format(remote_temp_path, cp_opts, remote_results_root) diff --git a/test/lib/ansible_test/_internal/env.py b/test/lib/ansible_test/_internal/env.py index ef04c692fcd..8b1b002ad84 100644 --- a/test/lib/ansible_test/_internal/env.py +++ b/test/lib/ansible_test/_internal/env.py @@ -52,10 +52,6 @@ from .test import ( TestTimeout, ) -from .executor import ( - SUPPORTED_PYTHON_VERSIONS, -) - from .ci import ( get_ci_provider, ) @@ -115,7 +111,7 @@ def show_dump_env(args): executable=sys.executable, version=platform.python_version(), ), - interpreters=get_available_python_versions(SUPPORTED_PYTHON_VERSIONS), + interpreters=get_available_python_versions(), ) if args.show: diff --git a/test/lib/ansible_test/_internal/manage_ci.py b/test/lib/ansible_test/_internal/manage_ci.py index 8cb09ba275a..8897150d17f 100644 --- a/test/lib/ansible_test/_internal/manage_ci.py +++ b/test/lib/ansible_test/_internal/manage_ci.py @@ -225,7 +225,7 @@ class ManagePosixCI: self.become = ['sudo', '-in', 'PATH=/usr/local/bin:$PATH'] elif self.core_ci.platform == 'rhel': self.become = ['sudo', '-in', 'bash', '-c'] - elif self.core_ci.platform in ['aix', 'ibmi']: + elif self.core_ci.platform == 'aix': self.become = [] if self.become is None: diff --git a/test/lib/ansible_test/_internal/sanity/__init__.py b/test/lib/ansible_test/_internal/sanity/__init__.py index 30cb6dd47a0..0804b4198dc 100644 --- a/test/lib/ansible_test/_internal/sanity/__init__.py +++ b/test/lib/ansible_test/_internal/sanity/__init__.py @@ -121,7 +121,7 @@ def command_sanity(args): display.info(test.name) continue - available_versions = sorted(get_available_python_versions(SUPPORTED_PYTHON_VERSIONS).keys()) + available_versions = sorted(get_available_python_versions().keys()) if args.python: # specific version selected diff --git a/test/lib/ansible_test/_internal/ssh.py b/test/lib/ansible_test/_internal/ssh.py index 6d16e78f3ef..acc6f5d24d9 100644 --- a/test/lib/ansible_test/_internal/ssh.py +++ b/test/lib/ansible_test/_internal/ssh.py @@ -248,7 +248,7 @@ def generate_ssh_inventory(ssh_connections): # type: (t.List[SshConnectionDetai ansible_user=ssh.user, ansible_ssh_private_key_file=os.path.abspath(ssh.identity_file), ansible_connection='ssh', - ansible_ssh_pipelining='yes', + ansible_pipelining='yes', ansible_python_interpreter=ssh.python_interpreter, ansible_shell_type=ssh.shell_type, ansible_ssh_extra_args='-o UserKnownHostsFile=/dev/null', # avoid changing the test environment diff --git a/test/lib/ansible_test/_internal/units/__init__.py b/test/lib/ansible_test/_internal/units/__init__.py index a2f49e29bd3..870a6f97a60 100644 --- a/test/lib/ansible_test/_internal/units/__init__.py +++ b/test/lib/ansible_test/_internal/units/__init__.py @@ -129,7 +129,7 @@ def command_units(args): test_sets = [] - available_versions = sorted(get_available_python_versions(list(SUPPORTED_PYTHON_VERSIONS)).keys()) + available_versions = sorted(get_available_python_versions().keys()) for version in SUPPORTED_PYTHON_VERSIONS: # run all versions unless version given, in which case run only that version diff --git a/test/lib/ansible_test/_internal/util.py b/test/lib/ansible_test/_internal/util.py index 3c4549a5224..16ba2c2f97a 100644 --- a/test/lib/ansible_test/_internal/util.py +++ b/test/lib/ansible_test/_internal/util.py @@ -246,15 +246,15 @@ def get_ansible_version(): # type: () -> str return ansible_version -def get_available_python_versions(versions): # type: (t.List[str]) -> t.Dict[str, str] - """Return a dictionary indicating which of the requested Python versions are available.""" +def get_available_python_versions(): # type: () -> t.Dict[str, str] + """Return a dictionary indicating which supported Python versions are available.""" try: return get_available_python_versions.result except AttributeError: pass get_available_python_versions.result = dict((version, path) for version, path in - ((version, find_python(version, required=False)) for version in versions) if path) + ((version, find_python(version, required=False)) for version in SUPPORTED_PYTHON_VERSIONS) if path) return get_available_python_versions.result diff --git a/test/lib/ansible_test/_internal/util_common.py b/test/lib/ansible_test/_internal/util_common.py index 370a8ca03c9..6e7339df2b4 100644 --- a/test/lib/ansible_test/_internal/util_common.py +++ b/test/lib/ansible_test/_internal/util_common.py @@ -53,7 +53,7 @@ NETWORK_COMPLETION = {} # type: t.Dict[str, t.Dict[str, str]] class ShellScriptTemplate: """A simple substition template for shell scripts.""" - def __init__(self, template): # type: (str) -> None + def __init__(self, template): # type: (t.Text) -> None self.template = template def substitute(self, **kwargs): @@ -427,7 +427,7 @@ def intercept_command(args, cmd, target_name, env, capture=False, data=None, cwd env['ANSIBLE_TEST_PYTHON_VERSION'] = version env['ANSIBLE_TEST_PYTHON_INTERPRETER'] = interpreter - if args.coverage and not disable_coverage: + if not disable_coverage and args.coverage: # add the necessary environment variables to enable code coverage collection env.update(get_coverage_environment(args, target_name, version, temp_path, module_coverage, remote_temp_path=remote_temp_path)) diff --git a/test/lib/ansible_test/_internal/venv.py b/test/lib/ansible_test/_internal/venv.py index 37eef367b76..49235f8662a 100644 --- a/test/lib/ansible_test/_internal/venv.py +++ b/test/lib/ansible_test/_internal/venv.py @@ -16,7 +16,6 @@ from .util import ( find_python, SubprocessError, get_available_python_versions, - SUPPORTED_PYTHON_VERSIONS, ANSIBLE_TEST_DATA_ROOT, display, remove_tree, @@ -63,7 +62,7 @@ def create_virtual_environment(args, # type: EnvironmentConfig display.info('Created Python %s virtual environment using "virtualenv": %s' % (version, path), verbosity=1) return True - available_pythons = get_available_python_versions(SUPPORTED_PYTHON_VERSIONS) + available_pythons = get_available_python_versions() for available_python_version, available_python_interpreter in sorted(available_pythons.items()): virtualenv_version = get_virtualenv_version(args, available_python_interpreter)