ansible-test - Fix subprocess management. (#77638)

* Run code-smell sanity tests in UTF-8 Mode.
* Update subprocess use in sanity test programs.
* Use raw_command instead of run_command with always=True set.
* Add more capture=True usage.
* Don't expose stdin to subprocesses.
* Capture more output. Warn on retry.
* Add more captures.
* Capture coverage cli output.
* Capture windows and network host checks.
* Be explicit about interactive usage.
* Use a shell for non-captured, non-interactive subprocesses.
* Add integration test to assert no TTY.
* Add unit test to assert no TTY.
* Require blocking stdin/stdout/stderr.
* Use subprocess.run in ansible-core sanity tests.
* Remove unused arg.
* Be explicit with subprocess.run check=False.
* Add changelog.

changelogs/fragments/ansible-test-subprocess-isolation.yml

@@ -0,0 +1,10 @@
+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.

test/integration/targets/ansible-test-no-tty/aliases

@@ -0,0 +1,2 @@
+context/controller
+shippable/posix/group1

test/integration/targets/ansible-test-no-tty/runme.py

@@ -0,0 +1,7 @@
+#!/usr/bin/env python
+
+import sys
+
+assert not sys.stdin.isatty()
+assert not sys.stdout.isatty()
+assert not sys.stderr.isatty()

test/integration/targets/ansible-test-no-tty/runme.sh

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+
+set -eux
+
+./runme.py

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(args, python):  # type: (EnvironmentConfig, PythonConfig) -> CollectionDetail
+def get_collection_detail(python):  # type: (PythonConfig) -> CollectionDetail
     """Return collection detail."""
     collection = data_context().content.collection
     directory = os.path.join(collection.root, collection.directory)
 
-    stdout = run_command(args, [python.path, os.path.join(ANSIBLE_TEST_TOOLS_ROOT, 'collection_detail.py'), directory], capture=True, always=True)[0]
+    stdout = raw_command([python.path, os.path.join(ANSIBLE_TEST_TOOLS_ROOT, 'collection_detail.py'), directory], capture=True)[0]
     result = json.loads(stdout)
     error = result.get('error')
 

test/lib/ansible_test/_internal/commands/coverage/__init__.py

@@ -100,7 +100,16 @@ def run_coverage(args, host_state, output_file, command, cmd):  # type: (Coverag
 
     cmd = ['python', '-m', 'coverage.__main__', command, '--rcfile', COVERAGE_CONFIG_PATH] + cmd
 
-    intercept_python(args, host_state.controller_profile.python, cmd, env)
+    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)
 
 
 def get_all_coverage_files():  # type: () -> t.List[str]

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(run_command(args, cmd, capture=True, always=True)[0])
+        stubs = json.loads(raw_command(cmd, capture=True)[0])
 
         return dict((d['Path'], dict((line, 0) for line in d['Lines'])) for d in stubs)
 

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', '{}', ';'])
+        docker_exec(self.args, self.DOCKER_SIMULATOR_NAME, ['find', '/var/lib/mysql', '-type', 'f', '-exec', 'touch', '{}', ';'], capture=True)
 
         if self.args.explain:
             values = dict(

test/lib/ansible_test/_internal/commands/sanity/__init__.py

@@ -952,6 +952,7 @@ 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

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(args, python)
+                collection_detail = get_collection_detail(python)
 
                 if not collection_detail.version:
                     display.warning('Skipping pylint collection version checks since no collection version was found.')

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(args, python)
+                collection_detail = get_collection_detail(python)
 
                 if collection_detail.version:
                     cmd.extend(['--collection-version', collection_detail.version])

test/lib/ansible_test/_internal/commands/shell/__init__.py

@@ -2,6 +2,7 @@
 from __future__ import annotations
 
 import os
+import sys
 import typing as t
 
 from ...util import (
@@ -44,6 +45,9 @@ 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:
@@ -87,4 +91,4 @@ def command_shell(args):  # type: (ShellConfig) -> None
     else:
         cmd = []
 
-    con.run(cmd)
+    con.run(cmd, interactive=True)

test/lib/ansible_test/_internal/config.py

@@ -238,6 +238,7 @@ class ShellConfig(EnvironmentConfig):
         super().__init__(args, 'shell')
 
         self.raw = args.raw  # type: bool
+        self.interactive = True
 
 
 class SanityConfig(TestConfig):

test/lib/ansible_test/_internal/connections.py

@@ -3,7 +3,6 @@ from __future__ import annotations
 
 import abc
 import shlex
-import sys
 import tempfile
 import typing as t
 
@@ -47,6 +46,7 @@ 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))
+        retry(lambda: self.run(tar_cmd, stdin=src, capture=True))
 
     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))
+        retry(lambda: self.run(sh_cmd, stdout=dst, capture=True))
 
 
 class LocalConnection(Connection):
@@ -93,6 +93,7 @@ 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]]
@@ -105,6 +106,7 @@ class LocalConnection(Connection):
             data=data,
             stdin=stdin,
             stdout=stdout,
+            interactive=interactive,
         )
 
 
@@ -131,6 +133,7 @@ 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]]
@@ -143,7 +146,7 @@ class SshConnection(Connection):
 
         options.append('-q')
 
-        if not data and not stdin and not stdout and sys.stdin.isatty():
+        if interactive:
             options.append('-tt')
 
         with tempfile.NamedTemporaryFile(prefix='ansible-test-ssh-debug-', suffix='.log') as ssh_logfile:
@@ -166,6 +169,7 @@ class SshConnection(Connection):
                 data=data,
                 stdin=stdin,
                 stdout=stdout,
+                interactive=interactive,
                 error_callback=error_callback,
             )
 
@@ -209,6 +213,7 @@ 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]]
@@ -219,7 +224,7 @@ class DockerConnection(Connection):
         if self.user:
             options.extend(['--user', self.user])
 
-        if not data and not stdin and not stdout and sys.stdin.isatty():
+        if interactive:
             options.append('-it')
 
         return docker_exec(
@@ -231,6 +236,7 @@ class DockerConnection(Connection):
             data=data,
             stdin=stdin,
             stdout=stdout,
+            interactive=interactive,
         )
 
     def inspect(self):  # type: () -> DockerInspect

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])
+            run_command(args, ['ssh-keygen', '-m', 'PEM', '-q', '-t', self.KEY_TYPE, '-N', '', '-f', key], capture=True)
 
             if args.explain:
                 return key, pub

test/lib/ansible_test/_internal/delegation.py

@@ -160,11 +160,12 @@ 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)
-            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(['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(insert_options(command, options + ['--requirements-mode', 'only']))
 
             container = con.inspect()
@@ -191,7 +192,7 @@ def delegate_command(args, host_state, exclude, require):  # type: (EnvironmentC
         success = False
 
         try:
-            con.run(insert_options(command, options))
+            con.run(insert_options(command, options), interactive=args.interactive)
             success = True
         finally:
             if host_delegation:

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)])
+    docker_command(args, ['cp', src, '%s:%s' % (container_id, dst)], capture=True)
 
 
 def docker_run(
@@ -514,6 +514,7 @@ 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."""
@@ -523,7 +524,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, data=data)
+    return docker_command(args, ['exec'] + options + [container_id] + cmd, capture=capture, stdin=stdin, stdout=stdout, interactive=interactive, data=data)
 
 
 def docker_info(args):  # type: (CommonConfig) -> t.Dict[str, t.Any]
@@ -544,6 +545,7 @@ 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]]
@@ -552,7 +554,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, always=always, data=data)
+    return run_command(args, command + cmd, env=env, capture=capture, stdin=stdin, stdout=stdout, interactive=interactive, always=always, data=data)
 
 
 def docker_environment():  # type: () -> t.Dict[str, str]

test/lib/ansible_test/_internal/host_profiles.py

@@ -484,8 +484,9 @@ class NetworkRemoteProfile(RemoteProfile[NetworkRemoteConfig]):
 
             for dummy in range(1, 90):
                 try:
-                    intercept_python(self.args, self.args.controller_python, cmd, env)
-                except SubprocessError:
+                    intercept_python(self.args, self.args.controller_python, cmd, env, capture=True)
+                except SubprocessError as ex:
+                    display.warning(str(ex))
                     time.sleep(10)
                 else:
                     return
@@ -717,8 +718,9 @@ class WindowsRemoteProfile(RemoteProfile[WindowsRemoteConfig]):
 
             for dummy in range(1, 120):
                 try:
-                    intercept_python(self.args, self.args.controller_python, cmd, env)
-                except SubprocessError:
+                    intercept_python(self.args, self.args.controller_python, cmd, env, capture=True)
+                except SubprocessError as ex:
+                    display.warning(str(ex))
                     time.sleep(10)
                 else:
                     return

test/lib/ansible_test/_internal/util.py

@@ -261,6 +261,7 @@ 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]]
@@ -274,6 +275,14 @@ 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)
@@ -298,6 +307,10 @@ 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
@@ -654,12 +667,15 @@ class MissingEnvironmentVariable(ApplicationError):
         self.name = name
 
 
-def retry(func, ex_type=SubprocessError, sleep=10, attempts=10):
+def retry(func, ex_type=SubprocessError, sleep=10, attempts=10, warn=True):
     """Retry the specified function on failure."""
     for dummy in range(1, attempts):
         try:
             return func()
-        except ex_type:
+        except ex_type as ex:
+            if warn:
+                display.warning(str(ex))
+
             time.sleep(sleep)
 
     return func()

test/lib/ansible_test/_internal/util_common.py

@@ -126,6 +126,7 @@ 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
@@ -406,13 +407,14 @@ 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,
+    return raw_command(cmd, capture=capture, env=env, data=data, cwd=cwd, explain=explain, stdin=stdin, stdout=stdout, interactive=interactive,
                        cmd_verbosity=cmd_verbosity, str_errors=str_errors, error_callback=error_callback)
 
 

test/lib/ansible_test/_internal/venv.py

@@ -20,6 +20,7 @@ from .util import (
     remove_tree,
     ApplicationError,
     str_to_version,
+    raw_command,
 )
 
 from .util_common import (
@@ -92,7 +93,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(args, python.version):
+        for real_python in iterate_real_pythons(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
@@ -128,7 +129,7 @@ def create_virtual_environment(args,  # type: EnvironmentConfig
     return False
 
 
-def iterate_real_pythons(args, version):  # type: (EnvironmentConfig, str) -> t.Iterable[str]
+def iterate_real_pythons(version):  # type: (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.
@@ -138,7 +139,7 @@ def iterate_real_pythons(args, version):  # type: (EnvironmentConfig, str) -> t.
 
     if version_info == sys.version_info[:len(version_info)]:
         current_python = sys.executable
-        real_prefix = get_python_real_prefix(args, current_python)
+        real_prefix = get_python_real_prefix(current_python)
 
         if real_prefix:
             current_python = find_python(version, os.path.join(real_prefix, 'bin'))
@@ -159,7 +160,7 @@ def iterate_real_pythons(args, version):  # type: (EnvironmentConfig, str) -> t.
     if found_python == current_python:
         return
 
-    real_prefix = get_python_real_prefix(args, found_python)
+    real_prefix = get_python_real_prefix(found_python)
 
     if real_prefix:
         found_python = find_python(version, os.path.join(real_prefix, 'bin'))
@@ -168,12 +169,12 @@ def iterate_real_pythons(args, version):  # type: (EnvironmentConfig, str) -> t.
         yield found_python
 
 
-def get_python_real_prefix(args, python_path):  # type: (EnvironmentConfig, str) -> t.Optional[str]
+def get_python_real_prefix(python_path):  # type: (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(run_command(args, cmd, capture=True, always=True)[0])
+    check_result = json.loads(raw_command(cmd, capture=True)[0])
     real_prefix = check_result['real_prefix']
     return real_prefix
 

test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py

@@ -47,7 +47,11 @@ def main():
     env = os.environ.copy()
     env.update(PYTHONPATH='%s:%s' % (os.path.join(os.path.dirname(__file__), 'changelog'), env['PYTHONPATH']))
 
-    subprocess.call(cmd, env=env)  # ignore the return code, rely on the output instead
+    # 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)
 
 
 if __name__ == '__main__':

test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py

@@ -436,14 +436,13 @@ 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.Popen(command, stdout=subprocess.PIPE,
-                             stderr=subprocess.PIPE)
-        stdout, stderr = p.communicate()
+        p = subprocess.run(command, stdin=subprocess.DEVNULL, capture_output=True, check=False)
+
         if int(p.returncode) != 0:
             return None
 
         t = tempfile.NamedTemporaryFile(delete=False)
-        t.write(stdout)
+        t.write(p.stdout)
         t.close()
 
         return t.name
@@ -2456,11 +2455,12 @@ class GitCache:
     @staticmethod
     def _git(args):
         cmd = ['git'] + args
-        p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        stdout, stderr = p.communicate()
+        p = subprocess.run(cmd, stdin=subprocess.DEVNULL, capture_output=True, text=True, check=False)
+
         if p.returncode != 0:
-            raise GitError(stderr, p.returncode)
-        return stdout.decode('utf-8').splitlines()
+            raise GitError(p.stderr, p.returncode)
+
+        return p.stdout.splitlines()
 
 
 class GitError(Exception):

test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py

@@ -122,14 +122,12 @@ def get_ps_argument_spec(filename, collection):
     })
 
     script_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'ps_argspec.ps1')
-    proc = subprocess.Popen(['pwsh', script_path, util_manifest], stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                            shell=False)
-    stdout, stderr = proc.communicate()
+    proc = subprocess.run(['pwsh', script_path, util_manifest], stdin=subprocess.DEVNULL, capture_output=True, text=True, check=False)
 
     if proc.returncode != 0:
-        raise AnsibleModuleImportError("STDOUT:\n%s\nSTDERR:\n%s" % (stdout.decode('utf-8'), stderr.decode('utf-8')))
+        raise AnsibleModuleImportError("STDOUT:\n%s\nSTDERR:\n%s" % (proc.stdout, proc.stderr))
 
-    kwargs = json.loads(stdout)
+    kwargs = json.loads(proc.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', {})

test/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py

@@ -27,6 +27,9 @@ 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
 

test/sanity/code-smell/docs-build.py

@@ -29,13 +29,12 @@ def main():
 
     try:
         cmd = ['make', 'core_singlehtmldocs']
-        sphinx = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=docs_dir)
-        stdout, stderr = sphinx.communicate()
+        sphinx = subprocess.run(cmd, stdin=subprocess.DEVNULL, capture_output=True, cwd=docs_dir, check=False, text=True)
     finally:
         shutil.move(tmp, requirements_txt)
 
-    stdout = stdout.decode('utf-8')
-    stderr = stderr.decode('utf-8')
+    stdout = sphinx.stdout
+    stderr = sphinx.stderr
 
     if sphinx.returncode != 0:
         sys.stderr.write("Command '%s' failed with status code: %d\n" % (' '.join(cmd), sphinx.returncode))

test/sanity/code-smell/package-data.py

@@ -172,14 +172,15 @@ def clean_repository(file_list):
 
 def create_sdist(tmp_dir):
     """Create an sdist in the repository"""
-    create = subprocess.Popen(
+    create = subprocess.run(
         ['make', 'snapshot', 'SDIST_DIR=%s' % tmp_dir],
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        universal_newlines=True,
+        stdin=subprocess.DEVNULL,
+        capture_output=True,
+        text=True,
+        check=False,
     )
 
-    stderr = create.communicate()[1]
+    stderr = create.stderr
 
     if create.returncode != 0:
         raise Exception('make snapshot failed:\n%s' % stderr)
@@ -220,15 +221,16 @@ def extract_sdist(sdist_path, tmp_dir):
 
 def install_sdist(tmp_dir, sdist_dir):
     """Install the extracted sdist into the temporary directory"""
-    install = subprocess.Popen(
+    install = subprocess.run(
         ['python', 'setup.py', 'install', '--root=%s' % tmp_dir],
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        universal_newlines=True,
+        stdin=subprocess.DEVNULL,
+        capture_output=True,
+        text=True,
         cwd=os.path.join(tmp_dir, sdist_dir),
+        check=False,
     )
 
-    stdout, stderr = install.communicate()
+    stdout, stderr = install.stdout, install.stderr
 
     if install.returncode != 0:
         raise Exception('sdist install failed:\n%s' % stderr)

test/units/test_no_tty.py

@@ -0,0 +1,7 @@
+import sys
+
+
+def test_no_tty():
+    assert not sys.stdin.isatty()
+    assert not sys.stdout.isatty()
+    assert not sys.stderr.isatty()