From 26fd5a8c3a92b1cc470efc81ef95e9aab7c5de03 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Mon, 25 Apr 2022 15:38:35 -0700 Subject: [PATCH] Revert "ansible-test - Fix subprocess management. (#77638)" This reverts commit 62d03c8e752ee35057031a91d7028e0a2e5d43e4. --- .../ansible-test-subprocess-isolation.yml | 10 --------- .../targets/ansible-test-no-tty/aliases | 2 -- .../targets/ansible-test-no-tty/runme.py | 7 ------ .../targets/ansible-test-no-tty/runme.sh | 5 ----- .../ansible_test/_internal/ansible_util.py | 6 ++--- .../_internal/commands/coverage/__init__.py | 11 +--------- .../_internal/commands/coverage/combine.py | 4 ++-- .../commands/integration/cloud/cs.py | 2 +- .../_internal/commands/sanity/__init__.py | 1 - .../_internal/commands/sanity/pylint.py | 2 +- .../commands/sanity/validate_modules.py | 2 +- .../_internal/commands/shell/__init__.py | 6 +---- test/lib/ansible_test/_internal/config.py | 1 - .../lib/ansible_test/_internal/connections.py | 16 +++++--------- test/lib/ansible_test/_internal/core_ci.py | 2 +- test/lib/ansible_test/_internal/delegation.py | 13 +++++------ .../lib/ansible_test/_internal/docker_util.py | 8 +++---- .../ansible_test/_internal/host_profiles.py | 10 ++++----- test/lib/ansible_test/_internal/util.py | 20 ++--------------- .../lib/ansible_test/_internal/util_common.py | 4 +--- test/lib/ansible_test/_internal/venv.py | 13 +++++------ .../controller/sanity/code-smell/changelog.py | 6 +---- .../validate-modules/validate_modules/main.py | 16 +++++++------- .../validate_modules/module_args.py | 8 ++++--- .../_util/target/cli/ansible_test_cli_stub.py | 3 --- test/sanity/code-smell/docs-build.py | 7 +++--- test/sanity/code-smell/package-data.py | 22 +++++++++---------- test/units/test_no_tty.py | 7 ------ 28 files changed, 66 insertions(+), 148 deletions(-) delete mode 100644 changelogs/fragments/ansible-test-subprocess-isolation.yml delete mode 100644 test/integration/targets/ansible-test-no-tty/aliases delete mode 100755 test/integration/targets/ansible-test-no-tty/runme.py delete mode 100755 test/integration/targets/ansible-test-no-tty/runme.sh delete mode 100644 test/units/test_no_tty.py diff --git a/changelogs/fragments/ansible-test-subprocess-isolation.yml b/changelogs/fragments/ansible-test-subprocess-isolation.yml deleted file mode 100644 index 3be259d6089..00000000000 --- a/changelogs/fragments/ansible-test-subprocess-isolation.yml +++ /dev/null @@ -1,10 +0,0 @@ -bugfixes: - - ansible-test - Subprocesses are now isolated from the stdin, stdout and stderr of ansible-test. - This avoids issues with subprocesses tampering with the file descriptors, such as SSH making them non-blocking. - As a result of this change, subprocess output from unit and integration tests on stderr now go to stdout. - - ansible-test - Subprocesses no longer have access to the TTY ansible-test is connected to, if any. - This maintains consistent behavior between local testing and CI systems, which typically do not provide a TTY. - Tests which require a TTY should use pexpect or another mechanism to create a PTY. -minor_changes: - - ansible-test - Blocking mode is now enforced for stdin, stdout and stderr. - If any of these are non-blocking then ansible-test will exit during startup with an error. diff --git a/test/integration/targets/ansible-test-no-tty/aliases b/test/integration/targets/ansible-test-no-tty/aliases deleted file mode 100644 index 0ac86c9200c..00000000000 --- a/test/integration/targets/ansible-test-no-tty/aliases +++ /dev/null @@ -1,2 +0,0 @@ -context/controller -shippable/posix/group1 diff --git a/test/integration/targets/ansible-test-no-tty/runme.py b/test/integration/targets/ansible-test-no-tty/runme.py deleted file mode 100755 index c8c5cfccce6..00000000000 --- a/test/integration/targets/ansible-test-no-tty/runme.py +++ /dev/null @@ -1,7 +0,0 @@ -#!/usr/bin/env python - -import sys - -assert not sys.stdin.isatty() -assert not sys.stdout.isatty() -assert not sys.stderr.isatty() diff --git a/test/integration/targets/ansible-test-no-tty/runme.sh b/test/integration/targets/ansible-test-no-tty/runme.sh deleted file mode 100755 index bae5220e2c2..00000000000 --- a/test/integration/targets/ansible-test-no-tty/runme.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env bash - -set -eux - -./runme.py diff --git a/test/lib/ansible_test/_internal/ansible_util.py b/test/lib/ansible_test/_internal/ansible_util.py index c6f231bc7dc..a3582dc89f4 100644 --- a/test/lib/ansible_test/_internal/ansible_util.py +++ b/test/lib/ansible_test/_internal/ansible_util.py @@ -22,11 +22,11 @@ from .util import ( ANSIBLE_SOURCE_ROOT, ANSIBLE_TEST_TOOLS_ROOT, get_ansible_version, - raw_command, ) from .util_common import ( create_temp_dir, + run_command, ResultType, intercept_python, get_injector_path, @@ -258,12 +258,12 @@ class CollectionDetailError(ApplicationError): self.reason = reason -def get_collection_detail(python): # type: (PythonConfig) -> CollectionDetail +def get_collection_detail(args, python): # type: (EnvironmentConfig, PythonConfig) -> CollectionDetail """Return collection detail.""" collection = data_context().content.collection directory = os.path.join(collection.root, collection.directory) - stdout = raw_command([python.path, os.path.join(ANSIBLE_TEST_TOOLS_ROOT, 'collection_detail.py'), directory], capture=True)[0] + stdout = run_command(args, [python.path, os.path.join(ANSIBLE_TEST_TOOLS_ROOT, 'collection_detail.py'), directory], capture=True, always=True)[0] result = json.loads(stdout) error = result.get('error') diff --git a/test/lib/ansible_test/_internal/commands/coverage/__init__.py b/test/lib/ansible_test/_internal/commands/coverage/__init__.py index 3d27b64bb6e..8cae31298c1 100644 --- a/test/lib/ansible_test/_internal/commands/coverage/__init__.py +++ b/test/lib/ansible_test/_internal/commands/coverage/__init__.py @@ -100,16 +100,7 @@ def run_coverage(args, host_state, output_file, command, cmd): # type: (Coverag cmd = ['python', '-m', 'coverage.__main__', command, '--rcfile', COVERAGE_CONFIG_PATH] + cmd - stdout, stderr = intercept_python(args, host_state.controller_profile.python, cmd, env, capture=True) - - stdout = (stdout or '').strip() - stderr = (stderr or '').strip() - - if stdout: - display.info(stdout) - - if stderr: - display.warning(stderr) + intercept_python(args, host_state.controller_profile.python, cmd, env) def get_all_coverage_files(): # type: () -> t.List[str] diff --git a/test/lib/ansible_test/_internal/commands/coverage/combine.py b/test/lib/ansible_test/_internal/commands/coverage/combine.py index ea1851a1c9d..3356cc857c9 100644 --- a/test/lib/ansible_test/_internal/commands/coverage/combine.py +++ b/test/lib/ansible_test/_internal/commands/coverage/combine.py @@ -18,11 +18,11 @@ from ...util import ( ANSIBLE_TEST_TOOLS_ROOT, display, ApplicationError, - raw_command, ) from ...util_common import ( ResultType, + run_command, write_json_file, write_json_test_results, ) @@ -196,7 +196,7 @@ def _command_coverage_combine_powershell(args): # type: (CoverageCombineConfig) cmd = ['pwsh', os.path.join(ANSIBLE_TEST_TOOLS_ROOT, 'coverage_stub.ps1')] cmd.extend(source_paths) - stubs = json.loads(raw_command(cmd, capture=True)[0]) + stubs = json.loads(run_command(args, cmd, capture=True, always=True)[0]) return dict((d['Path'], dict((line, 0) for line in d['Lines'])) for d in stubs) diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py b/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py index 8ffcabfb32e..f20a7d887ef 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py @@ -106,7 +106,7 @@ class CsCloudProvider(CloudProvider): # apply work-around for OverlayFS issue # https://github.com/docker/for-linux/issues/72#issuecomment-319904698 - docker_exec(self.args, self.DOCKER_SIMULATOR_NAME, ['find', '/var/lib/mysql', '-type', 'f', '-exec', 'touch', '{}', ';'], capture=True) + docker_exec(self.args, self.DOCKER_SIMULATOR_NAME, ['find', '/var/lib/mysql', '-type', 'f', '-exec', 'touch', '{}', ';']) if self.args.explain: values = dict( diff --git a/test/lib/ansible_test/_internal/commands/sanity/__init__.py b/test/lib/ansible_test/_internal/commands/sanity/__init__.py index 7b7e6196fee..d819c37e33f 100644 --- a/test/lib/ansible_test/_internal/commands/sanity/__init__.py +++ b/test/lib/ansible_test/_internal/commands/sanity/__init__.py @@ -952,7 +952,6 @@ class SanityCodeSmellTest(SanitySingleVersion): cmd = [python.path, self.path] env = ansible_environment(args, color=False) - env.update(PYTHONUTF8='1') # force all code-smell sanity tests to run with Python UTF-8 Mode enabled pattern = None data = None diff --git a/test/lib/ansible_test/_internal/commands/sanity/pylint.py b/test/lib/ansible_test/_internal/commands/sanity/pylint.py index 370e8998438..0e6ace8ea99 100644 --- a/test/lib/ansible_test/_internal/commands/sanity/pylint.py +++ b/test/lib/ansible_test/_internal/commands/sanity/pylint.py @@ -141,7 +141,7 @@ class PylintTest(SanitySingleVersion): if data_context().content.collection: try: - collection_detail = get_collection_detail(python) + collection_detail = get_collection_detail(args, python) if not collection_detail.version: display.warning('Skipping pylint collection version checks since no collection version was found.') 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 f1d448804c1..e0fbac6439e 100644 --- a/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py +++ b/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py @@ -121,7 +121,7 @@ class ValidateModulesTest(SanitySingleVersion): cmd.extend(['--collection', data_context().content.collection.directory]) try: - collection_detail = get_collection_detail(python) + collection_detail = get_collection_detail(args, python) if collection_detail.version: cmd.extend(['--collection-version', collection_detail.version]) diff --git a/test/lib/ansible_test/_internal/commands/shell/__init__.py b/test/lib/ansible_test/_internal/commands/shell/__init__.py index c518af94c9d..4b205171a38 100644 --- a/test/lib/ansible_test/_internal/commands/shell/__init__.py +++ b/test/lib/ansible_test/_internal/commands/shell/__init__.py @@ -2,7 +2,6 @@ from __future__ import annotations import os -import sys import typing as t from ...util import ( @@ -45,9 +44,6 @@ def command_shell(args): # type: (ShellConfig) -> None if args.raw and isinstance(args.targets[0], ControllerConfig): raise ApplicationError('The --raw option has no effect on the controller.') - if not sys.stdin.isatty(): - raise ApplicationError('Standard input must be a TTY to launch a shell.') - host_state = prepare_profiles(args, skip_setup=args.raw) # shell if args.delegate: @@ -91,4 +87,4 @@ def command_shell(args): # type: (ShellConfig) -> None else: cmd = [] - con.run(cmd, interactive=True) + con.run(cmd) diff --git a/test/lib/ansible_test/_internal/config.py b/test/lib/ansible_test/_internal/config.py index 5da5fc00e4d..0a14a806ca8 100644 --- a/test/lib/ansible_test/_internal/config.py +++ b/test/lib/ansible_test/_internal/config.py @@ -238,7 +238,6 @@ class ShellConfig(EnvironmentConfig): super().__init__(args, 'shell') self.raw = args.raw # type: bool - self.interactive = True class SanityConfig(TestConfig): diff --git a/test/lib/ansible_test/_internal/connections.py b/test/lib/ansible_test/_internal/connections.py index b0c7d948ad1..14234b2d936 100644 --- a/test/lib/ansible_test/_internal/connections.py +++ b/test/lib/ansible_test/_internal/connections.py @@ -3,6 +3,7 @@ from __future__ import annotations import abc import shlex +import sys import tempfile import typing as t @@ -46,7 +47,6 @@ class Connection(metaclass=abc.ABCMeta): def run(self, command, # type: t.List[str] capture=False, # type: bool - interactive=False, # type: bool data=None, # type: t.Optional[str] stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] @@ -60,7 +60,7 @@ class Connection(metaclass=abc.ABCMeta): """Extract the given archive file stream in the specified directory.""" tar_cmd = ['tar', 'oxzf', '-', '-C', chdir] - retry(lambda: self.run(tar_cmd, stdin=src, capture=True)) + retry(lambda: self.run(tar_cmd, stdin=src)) def create_archive(self, chdir, # type: str @@ -82,7 +82,7 @@ class Connection(metaclass=abc.ABCMeta): sh_cmd = ['sh', '-c', ' | '.join(' '.join(shlex.quote(cmd) for cmd in command) for command in commands)] - retry(lambda: self.run(sh_cmd, stdout=dst, capture=True)) + retry(lambda: self.run(sh_cmd, stdout=dst)) class LocalConnection(Connection): @@ -93,7 +93,6 @@ class LocalConnection(Connection): def run(self, command, # type: t.List[str] capture=False, # type: bool - interactive=False, # type: bool data=None, # type: t.Optional[str] stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] @@ -106,7 +105,6 @@ class LocalConnection(Connection): data=data, stdin=stdin, stdout=stdout, - interactive=interactive, ) @@ -133,7 +131,6 @@ class SshConnection(Connection): def run(self, command, # type: t.List[str] capture=False, # type: bool - interactive=False, # type: bool data=None, # type: t.Optional[str] stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] @@ -146,7 +143,7 @@ class SshConnection(Connection): options.append('-q') - if interactive: + if not data and not stdin and not stdout and sys.stdin.isatty(): options.append('-tt') with tempfile.NamedTemporaryFile(prefix='ansible-test-ssh-debug-', suffix='.log') as ssh_logfile: @@ -169,7 +166,6 @@ class SshConnection(Connection): data=data, stdin=stdin, stdout=stdout, - interactive=interactive, error_callback=error_callback, ) @@ -213,7 +209,6 @@ class DockerConnection(Connection): def run(self, command, # type: t.List[str] capture=False, # type: bool - interactive=False, # type: bool data=None, # type: t.Optional[str] stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] @@ -224,7 +219,7 @@ class DockerConnection(Connection): if self.user: options.extend(['--user', self.user]) - if interactive: + if not data and not stdin and not stdout and sys.stdin.isatty(): options.append('-it') return docker_exec( @@ -236,7 +231,6 @@ class DockerConnection(Connection): data=data, stdin=stdin, stdout=stdout, - interactive=interactive, ) def inspect(self): # type: () -> DockerInspect diff --git a/test/lib/ansible_test/_internal/core_ci.py b/test/lib/ansible_test/_internal/core_ci.py index 37e4ac061f0..dbb428aeebf 100644 --- a/test/lib/ansible_test/_internal/core_ci.py +++ b/test/lib/ansible_test/_internal/core_ci.py @@ -469,7 +469,7 @@ class SshKey: make_dirs(os.path.dirname(key)) if not os.path.isfile(key) or not os.path.isfile(pub): - run_command(args, ['ssh-keygen', '-m', 'PEM', '-q', '-t', self.KEY_TYPE, '-N', '', '-f', key], capture=True) + run_command(args, ['ssh-keygen', '-m', 'PEM', '-q', '-t', self.KEY_TYPE, '-N', '', '-f', key]) if args.explain: return key, pub diff --git a/test/lib/ansible_test/_internal/delegation.py b/test/lib/ansible_test/_internal/delegation.py index 7ee5bd971db..6298bf24058 100644 --- a/test/lib/ansible_test/_internal/delegation.py +++ b/test/lib/ansible_test/_internal/delegation.py @@ -160,12 +160,11 @@ def delegate_command(args, host_state, exclude, require): # type: (EnvironmentC os.path.join(content_root, ResultType.COVERAGE.relative_path), ] - con.run(['mkdir', '-p'] + writable_dirs, capture=True) - con.run(['chmod', '777'] + writable_dirs, capture=True) - con.run(['chmod', '755', working_directory], capture=True) - con.run(['chmod', '644', os.path.join(content_root, args.metadata_path)], capture=True) - con.run(['useradd', pytest_user, '--create-home'], capture=True) - + con.run(['mkdir', '-p'] + writable_dirs) + con.run(['chmod', '777'] + writable_dirs) + con.run(['chmod', '755', working_directory]) + con.run(['chmod', '644', os.path.join(content_root, args.metadata_path)]) + con.run(['useradd', pytest_user, '--create-home']) con.run(insert_options(command, options + ['--requirements-mode', 'only'])) container = con.inspect() @@ -192,7 +191,7 @@ def delegate_command(args, host_state, exclude, require): # type: (EnvironmentC success = False try: - con.run(insert_options(command, options), interactive=args.interactive) + con.run(insert_options(command, options)) success = True finally: if host_delegation: diff --git a/test/lib/ansible_test/_internal/docker_util.py b/test/lib/ansible_test/_internal/docker_util.py index b8bc838c4be..12509076776 100644 --- a/test/lib/ansible_test/_internal/docker_util.py +++ b/test/lib/ansible_test/_internal/docker_util.py @@ -279,7 +279,7 @@ def docker_pull(args, image): # type: (EnvironmentConfig, str) -> None def docker_cp_to(args, container_id, src, dst): # type: (EnvironmentConfig, str, str, str) -> None """Copy a file to the specified container.""" - docker_command(args, ['cp', src, '%s:%s' % (container_id, dst)], capture=True) + docker_command(args, ['cp', src, '%s:%s' % (container_id, dst)]) def docker_run( @@ -514,7 +514,6 @@ def docker_exec( capture=False, # type: bool stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] - interactive=False, # type: bool data=None, # type: t.Optional[str] ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Execute the given command in the specified container.""" @@ -524,7 +523,7 @@ def docker_exec( if data or stdin or stdout: options.append('-i') - return docker_command(args, ['exec'] + options + [container_id] + cmd, capture=capture, stdin=stdin, stdout=stdout, interactive=interactive, data=data) + return docker_command(args, ['exec'] + options + [container_id] + cmd, capture=capture, stdin=stdin, stdout=stdout, data=data) def docker_info(args): # type: (CommonConfig) -> t.Dict[str, t.Any] @@ -545,7 +544,6 @@ def docker_command( capture=False, # type: bool stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] - interactive=False, # type: bool always=False, # type: bool data=None, # type: t.Optional[str] ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] @@ -554,7 +552,7 @@ def docker_command( command = [require_docker().command] if command[0] == 'podman' and _get_podman_remote(): command.append('--remote') - return run_command(args, command + cmd, env=env, capture=capture, stdin=stdin, stdout=stdout, interactive=interactive, always=always, data=data) + return run_command(args, command + cmd, env=env, capture=capture, stdin=stdin, stdout=stdout, always=always, data=data) def docker_environment(): # type: () -> t.Dict[str, str] diff --git a/test/lib/ansible_test/_internal/host_profiles.py b/test/lib/ansible_test/_internal/host_profiles.py index e5a3dbd8256..9079c7e9244 100644 --- a/test/lib/ansible_test/_internal/host_profiles.py +++ b/test/lib/ansible_test/_internal/host_profiles.py @@ -484,9 +484,8 @@ class NetworkRemoteProfile(RemoteProfile[NetworkRemoteConfig]): for dummy in range(1, 90): try: - intercept_python(self.args, self.args.controller_python, cmd, env, capture=True) - except SubprocessError as ex: - display.warning(str(ex)) + intercept_python(self.args, self.args.controller_python, cmd, env) + except SubprocessError: time.sleep(10) else: return @@ -718,9 +717,8 @@ class WindowsRemoteProfile(RemoteProfile[WindowsRemoteConfig]): for dummy in range(1, 120): try: - intercept_python(self.args, self.args.controller_python, cmd, env, capture=True) - except SubprocessError as ex: - display.warning(str(ex)) + intercept_python(self.args, self.args.controller_python, cmd, env) + except SubprocessError: time.sleep(10) else: return diff --git a/test/lib/ansible_test/_internal/util.py b/test/lib/ansible_test/_internal/util.py index a0fd6fa70d9..9f1b7a4fa77 100644 --- a/test/lib/ansible_test/_internal/util.py +++ b/test/lib/ansible_test/_internal/util.py @@ -261,7 +261,6 @@ def raw_command( explain=False, # type: bool stdin=None, # type: t.Optional[t.Union[t.IO[bytes], int]] stdout=None, # type: t.Optional[t.Union[t.IO[bytes], int]] - interactive=False, # type: bool cmd_verbosity=1, # type: int str_errors='strict', # type: str error_callback=None, # type: t.Optional[t.Callable[[SubprocessError], None]] @@ -275,14 +274,6 @@ def raw_command( cmd = list(cmd) - if not capture and not interactive: - # When not capturing stdout/stderr and not running interactively, send subprocess stdout/stderr through a shell to the current stdout. - # Additionally send output through a pipe to `tee` to block access to the current TTY, if any. - # This prevents subprocesses from sharing stdin/stdout/stderr with the current process or each other. - # Doing so allows subprocesses to safely make changes to their file handles, such as making them non-blocking (ssh does this). - # This also maintains consistency between local testing and CI systems, which typically do not provide a TTY. - cmd = ['/bin/sh', '-c', f'{shlex.join(cmd)} 2>&1 | tee'] - escaped_cmd = ' '.join(shlex.quote(c) for c in cmd) display.info('Run command: %s' % escaped_cmd, verbosity=cmd_verbosity, truncate=True) @@ -307,10 +298,6 @@ def raw_command( elif data is not None: stdin = subprocess.PIPE communicate = True - elif interactive: - pass # allow the subprocess access to our stdin - else: - stdin = subprocess.DEVNULL if stdout: communicate = True @@ -667,15 +654,12 @@ class MissingEnvironmentVariable(ApplicationError): self.name = name -def retry(func, ex_type=SubprocessError, sleep=10, attempts=10, warn=True): +def retry(func, ex_type=SubprocessError, sleep=10, attempts=10): """Retry the specified function on failure.""" for dummy in range(1, attempts): try: return func() - except ex_type as ex: - if warn: - display.warning(str(ex)) - + except ex_type: time.sleep(sleep) return func() diff --git a/test/lib/ansible_test/_internal/util_common.py b/test/lib/ansible_test/_internal/util_common.py index 2f68a6dfe5e..99d22c2bcae 100644 --- a/test/lib/ansible_test/_internal/util_common.py +++ b/test/lib/ansible_test/_internal/util_common.py @@ -126,7 +126,6 @@ class CommonConfig: """Configuration common to all commands.""" def __init__(self, args, command): # type: (t.Any, str) -> None self.command = command - self.interactive = False self.success = None # type: t.Optional[bool] self.color = args.color # type: bool @@ -407,14 +406,13 @@ def run_command( always=False, # type: bool stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] - interactive=False, # type: bool cmd_verbosity=1, # type: int str_errors='strict', # type: str error_callback=None, # type: t.Optional[t.Callable[[SubprocessError], None]] ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Run the specified command and return stdout and stderr as a tuple.""" explain = args.explain and not always - return raw_command(cmd, capture=capture, env=env, data=data, cwd=cwd, explain=explain, stdin=stdin, stdout=stdout, interactive=interactive, + return raw_command(cmd, capture=capture, env=env, data=data, cwd=cwd, explain=explain, stdin=stdin, stdout=stdout, cmd_verbosity=cmd_verbosity, str_errors=str_errors, error_callback=error_callback) diff --git a/test/lib/ansible_test/_internal/venv.py b/test/lib/ansible_test/_internal/venv.py index 21dded627cb..64d8d04ce11 100644 --- a/test/lib/ansible_test/_internal/venv.py +++ b/test/lib/ansible_test/_internal/venv.py @@ -20,7 +20,6 @@ from .util import ( remove_tree, ApplicationError, str_to_version, - raw_command, ) from .util_common import ( @@ -93,7 +92,7 @@ def create_virtual_environment(args, # type: EnvironmentConfig # creating a virtual environment using 'venv' when running in a virtual environment created by 'virtualenv' results # in a copy of the original virtual environment instead of creation of a new one # avoid this issue by only using "real" python interpreters to invoke 'venv' - for real_python in iterate_real_pythons(python.version): + for real_python in iterate_real_pythons(args, python.version): if run_venv(args, real_python, system_site_packages, pip, path): display.info('Created Python %s virtual environment using "venv": %s' % (python.version, path), verbosity=1) return True @@ -129,7 +128,7 @@ def create_virtual_environment(args, # type: EnvironmentConfig return False -def iterate_real_pythons(version): # type: (str) -> t.Iterable[str] +def iterate_real_pythons(args, version): # type: (EnvironmentConfig, str) -> t.Iterable[str] """ Iterate through available real python interpreters of the requested version. The current interpreter will be checked and then the path will be searched. @@ -139,7 +138,7 @@ def iterate_real_pythons(version): # type: (str) -> t.Iterable[str] if version_info == sys.version_info[:len(version_info)]: current_python = sys.executable - real_prefix = get_python_real_prefix(current_python) + real_prefix = get_python_real_prefix(args, current_python) if real_prefix: current_python = find_python(version, os.path.join(real_prefix, 'bin')) @@ -160,7 +159,7 @@ def iterate_real_pythons(version): # type: (str) -> t.Iterable[str] if found_python == current_python: return - real_prefix = get_python_real_prefix(found_python) + real_prefix = get_python_real_prefix(args, found_python) if real_prefix: found_python = find_python(version, os.path.join(real_prefix, 'bin')) @@ -169,12 +168,12 @@ def iterate_real_pythons(version): # type: (str) -> t.Iterable[str] yield found_python -def get_python_real_prefix(python_path): # type: (str) -> t.Optional[str] +def get_python_real_prefix(args, python_path): # type: (EnvironmentConfig, str) -> t.Optional[str] """ Return the real prefix of the specified interpreter or None if the interpreter is not a virtual environment created by 'virtualenv'. """ cmd = [python_path, os.path.join(os.path.join(ANSIBLE_TEST_TARGET_TOOLS_ROOT, 'virtualenvcheck.py'))] - check_result = json.loads(raw_command(cmd, capture=True)[0]) + check_result = json.loads(run_command(args, cmd, capture=True, always=True)[0]) real_prefix = check_result['real_prefix'] return real_prefix diff --git a/test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py b/test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py index 983eaeb4266..fe2ba5e3727 100644 --- a/test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py +++ b/test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py @@ -47,11 +47,7 @@ def main(): env = os.environ.copy() env.update(PYTHONPATH='%s:%s' % (os.path.join(os.path.dirname(__file__), 'changelog'), env['PYTHONPATH'])) - # ignore the return code, rely on the output instead - process = subprocess.run(cmd, stdin=subprocess.DEVNULL, capture_output=True, text=True, env=env, check=False) - - sys.stdout.write(process.stdout) - sys.stderr.write(process.stderr) + subprocess.call(cmd, env=env) # ignore the return code, rely on the output instead if __name__ == '__main__': 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 f18477d529b..0bdd9dee21c 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 @@ -436,13 +436,14 @@ class ModuleValidator(Validator): base_path = self._get_base_branch_module_path() 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) - + p = subprocess.Popen(command, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + stdout, stderr = p.communicate() if int(p.returncode) != 0: return None t = tempfile.NamedTemporaryFile(delete=False) - t.write(p.stdout) + t.write(stdout) t.close() return t.name @@ -2455,12 +2456,11 @@ class GitCache: @staticmethod def _git(args): cmd = ['git'] + args - p = subprocess.run(cmd, stdin=subprocess.DEVNULL, capture_output=True, text=True, check=False) - + p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + stdout, stderr = p.communicate() if p.returncode != 0: - raise GitError(p.stderr, p.returncode) - - return p.stdout.splitlines() + raise GitError(stderr, p.returncode) + return stdout.decode('utf-8').splitlines() class GitError(Exception): diff --git a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py index 03a14019274..ee938142ff8 100644 --- a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py +++ b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py @@ -122,12 +122,14 @@ def get_ps_argument_spec(filename, collection): }) script_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'ps_argspec.ps1') - proc = subprocess.run(['pwsh', script_path, util_manifest], stdin=subprocess.DEVNULL, capture_output=True, text=True, check=False) + proc = subprocess.Popen(['pwsh', script_path, util_manifest], stdout=subprocess.PIPE, stderr=subprocess.PIPE, + shell=False) + stdout, stderr = proc.communicate() if proc.returncode != 0: - raise AnsibleModuleImportError("STDOUT:\n%s\nSTDERR:\n%s" % (proc.stdout, proc.stderr)) + raise AnsibleModuleImportError("STDOUT:\n%s\nSTDERR:\n%s" % (stdout.decode('utf-8'), stderr.decode('utf-8'))) - kwargs = json.loads(proc.stdout) + kwargs = json.loads(stdout) # the validate-modules code expects the options spec to be under the argument_spec key not options as set in PS kwargs['argument_spec'] = kwargs.pop('options', {}) diff --git a/test/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py b/test/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py index 930654fc1e7..95209493089 100755 --- a/test/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py +++ b/test/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py @@ -27,9 +27,6 @@ def main(args=None): raise SystemExit('This version of ansible-test cannot be executed with Python version %s. Supported Python versions are: %s' % ( version_to_str(sys.version_info[:3]), ', '.join(CONTROLLER_PYTHON_VERSIONS))) - if any(not os.get_blocking(handle.fileno()) for handle in (sys.stdin, sys.stdout, sys.stderr)): - raise SystemExit('Standard input, output and error file handles must be blocking to run ansible-test.') - # noinspection PyProtectedMember from ansible_test._internal import main as cli_main diff --git a/test/sanity/code-smell/docs-build.py b/test/sanity/code-smell/docs-build.py index aaa69378c7f..9461620a9a3 100644 --- a/test/sanity/code-smell/docs-build.py +++ b/test/sanity/code-smell/docs-build.py @@ -29,12 +29,13 @@ def main(): try: cmd = ['make', 'core_singlehtmldocs'] - sphinx = subprocess.run(cmd, stdin=subprocess.DEVNULL, capture_output=True, cwd=docs_dir, check=False, text=True) + sphinx = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=docs_dir) + stdout, stderr = sphinx.communicate() finally: shutil.move(tmp, requirements_txt) - stdout = sphinx.stdout - stderr = sphinx.stderr + stdout = stdout.decode('utf-8') + stderr = stderr.decode('utf-8') if sphinx.returncode != 0: sys.stderr.write("Command '%s' failed with status code: %d\n" % (' '.join(cmd), sphinx.returncode)) diff --git a/test/sanity/code-smell/package-data.py b/test/sanity/code-smell/package-data.py index e707b0eb4f5..110b1635628 100644 --- a/test/sanity/code-smell/package-data.py +++ b/test/sanity/code-smell/package-data.py @@ -172,15 +172,14 @@ def clean_repository(file_list): def create_sdist(tmp_dir): """Create an sdist in the repository""" - create = subprocess.run( + create = subprocess.Popen( ['make', 'snapshot', 'SDIST_DIR=%s' % tmp_dir], - stdin=subprocess.DEVNULL, - capture_output=True, - text=True, - check=False, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, ) - stderr = create.stderr + stderr = create.communicate()[1] if create.returncode != 0: raise Exception('make snapshot failed:\n%s' % stderr) @@ -221,16 +220,15 @@ def extract_sdist(sdist_path, tmp_dir): def install_sdist(tmp_dir, sdist_dir): """Install the extracted sdist into the temporary directory""" - install = subprocess.run( + install = subprocess.Popen( ['python', 'setup.py', 'install', '--root=%s' % tmp_dir], - stdin=subprocess.DEVNULL, - capture_output=True, - text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, cwd=os.path.join(tmp_dir, sdist_dir), - check=False, ) - stdout, stderr = install.stdout, install.stderr + stdout, stderr = install.communicate() if install.returncode != 0: raise Exception('sdist install failed:\n%s' % stderr) diff --git a/test/units/test_no_tty.py b/test/units/test_no_tty.py deleted file mode 100644 index 290c0b922ab..00000000000 --- a/test/units/test_no_tty.py +++ /dev/null @@ -1,7 +0,0 @@ -import sys - - -def test_no_tty(): - assert not sys.stdin.isatty() - assert not sys.stdout.isatty() - assert not sys.stderr.isatty()