ansible-test - Fix TTY and output handling. (#78350)

changelogs/fragments/ansible-test-tty-output-handling.yml

@@ -0,0 +1,7 @@
+bugfixes:
+  - ansible-test - The ``shell`` command no longer requests a TTY when using delegation unless an interactive shell is being used.
+                   An interactive shell is the default behavior when no command is given to pass to the shell.
+  - ansible-test - The ``shell`` command no longer redirects all output to stdout when running a provided command.
+                   Any command output written to stderr will be mixed with the stderr output from ansible-test.
+  - ansible-test - Delegation for commands which generate output for programmatic consumption no longer redirect all output to stdout.
+                   The affected commands and options are ``shell``, ``sanity --lint``, ``sanity --list-tests``, ``integration --list-targets``, ``coverage analyze``

test/integration/targets/ansible-test-sanity-lint/aliases

@@ -0,0 +1,4 @@
+shippable/posix/group1  # runs in the distro test containers
+shippable/generic/group1  # runs in the default test container
+context/controller
+needs/target/collection

test/integration/targets/ansible-test-sanity-lint/expected.txt

@@ -0,0 +1 @@
+plugins/modules/python-wrong-shebang.py:1:1: expected module shebang "b'#!/usr/bin/python'" but found: b'#!invalid'

test/integration/targets/ansible-test-sanity-lint/runme.sh

@@ -0,0 +1,47 @@
+#!/usr/bin/env bash
+# Make sure that `ansible-test sanity --lint` outputs the correct format to stdout, even when delegation is used.
+
+set -eu
+
+# Create test scenarios at runtime that do not pass sanity tests.
+# This avoids the need to create ignore entries for the tests.
+
+mkdir -p ansible_collections/ns/col/plugins/modules
+
+(
+  cd ansible_collections/ns/col/plugins/modules
+
+  echo '#!invalid' > python-wrong-shebang.py # expected module shebang "b'#!/usr/bin/python'" but found: b'#!invalid'
+)
+
+source ../collection/setup.sh
+
+set -x
+
+###
+### Run the sanity test with the `--lint` option.
+###
+
+# Use the `--venv` option to verify that delegation preserves the output streams.
+ansible-test sanity --test shebang --color --failure-ok --lint --venv "${@}" 1> actual-stdout.txt 2> actual-stderr.txt
+diff -u "${TEST_DIR}/expected.txt" actual-stdout.txt
+grep -f "${TEST_DIR}/expected.txt" actual-stderr.txt
+
+# Run without delegation to verify direct output uses the correct streams.
+ansible-test sanity --test shebang --color --failure-ok --lint        "${@}" 1> actual-stdout.txt 2> actual-stderr.txt
+diff -u "${TEST_DIR}/expected.txt" actual-stdout.txt
+grep -f "${TEST_DIR}/expected.txt" actual-stderr.txt
+
+###
+### Run the sanity test without the `--lint` option.
+###
+
+# Use the `--venv` option to verify that delegation preserves the output streams.
+ansible-test sanity --test shebang --color --failure-ok --venv "${@}" 1> actual-stdout.txt 2> actual-stderr.txt
+grep -f "${TEST_DIR}/expected.txt" actual-stdout.txt
+[ ! -s actual-stderr.txt ]
+
+# Run without delegation to verify direct output uses the correct streams.
+ansible-test sanity --test shebang --color --failure-ok "${@}"        1> actual-stdout.txt 2> actual-stderr.txt
+grep -f "${TEST_DIR}/expected.txt" actual-stdout.txt
+[ ! -s actual-stderr.txt ]

test/integration/targets/ansible-test-shell/aliases

@@ -0,0 +1,4 @@
+shippable/posix/group1  # runs in the distro test containers
+shippable/generic/group1  # runs in the default test container
+context/controller
+needs/target/collection

test/integration/targets/ansible-test-shell/expected-stderr.txt

@@ -0,0 +1 @@
+stderr

test/integration/targets/ansible-test-shell/expected-stdout.txt

@@ -0,0 +1 @@
+stdout

test/integration/targets/ansible-test-shell/runme.sh

@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+# Make sure that `ansible-test shell` outputs to the correct stream.
+
+set -eu
+
+source ../collection/setup.sh
+
+set -x
+
+# Try `shell` with delegation.
+
+ansible-test shell --venv -- \
+  python -c 'import sys; print("stdout"); print("stderr", file=sys.stderr)' 1> actual-stdout.txt 2> actual-stderr.txt
+
+cat actual-stdout.txt
+cat actual-stderr.txt
+
+diff -u "${TEST_DIR}/expected-stdout.txt" actual-stdout.txt
+grep -f "${TEST_DIR}/expected-stderr.txt" actual-stderr.txt
+
+# Try `shell` without delegation.
+
+ansible-test shell -- \
+  python -c 'import sys; print("stdout"); print("stderr", file=sys.stderr)' 1> actual-stdout.txt 2> actual-stderr.txt
+
+cat actual-stdout.txt
+cat actual-stderr.txt
+
+diff -u "${TEST_DIR}/expected-stdout.txt" actual-stdout.txt
+grep -f "${TEST_DIR}/expected-stderr.txt" actual-stderr.txt

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

@@ -29,10 +29,6 @@
 
 class CoverageAnalyzeTargetsConfig(CoverageAnalyzeConfig):
     """Configuration for the `coverage analyze targets` command."""
-    def __init__(self, args):  # type: (t.Any) -> None
-        super().__init__(args)
-
-        self.display_stderr = True
 
 
 def make_report(target_indexes, arcs, lines):  # type: (TargetIndexes, Arcs, Lines) -> t.Dict[str, t.Any]

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

@@ -7,6 +7,7 @@
 
 from ...util import (
     ApplicationError,
+    OutputStream,
     display,
 )
 
@@ -82,7 +83,10 @@ def command_shell(args):  # type: (ShellConfig) -> None
         return
 
     if args.cmd:
-        con.run(args.cmd, capture=False, interactive=False, force_stdout=True)
+        # Running a command is assumed to be non-interactive. Only a shell (no command) is interactive.
+        # If we want to support interactive commands in the future, we'll need an `--interactive` command line option.
+        # Command stderr output is allowed to mix with our own output, which is all sent to stderr.
+        con.run(args.cmd, capture=False, interactive=False, output_stream=OutputStream.ORIGINAL)
         return
 
     if isinstance(con, SshConnection) and args.raw:

test/lib/ansible_test/_internal/config.py

@@ -217,7 +217,7 @@ def __init__(self, args):  # type: (t.Any) -> None
         self.cmd = args.cmd  # type: t.List[str]
         self.raw = args.raw  # type: bool
         self.check_layout = self.delegate  # allow shell to be used without a valid layout as long as no delegation is required
-        self.interactive = True
+        self.interactive = sys.stdin.isatty() and not args.cmd  # delegation should only be interactive when stdin is a TTY and no command was given
         self.export = args.export  # type: t.Optional[str]
         self.display_stderr = True
 

test/lib/ansible_test/_internal/connections.py

@@ -16,6 +16,7 @@
 
 from .util import (
     Display,
+    OutputStream,
     SubprocessError,
     retry,
 )
@@ -50,7 +51,7 @@ def run(self,
             data=None,  # type: t.Optional[str]
             stdin=None,  # type: t.Optional[t.IO[bytes]]
             stdout=None,  # type: t.Optional[t.IO[bytes]]
-            force_stdout=False,  # type: bool
+            output_stream=None,  # type: t.Optional[OutputStream]
             ):  # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]]
         """Run the specified command and return the result."""
 
@@ -98,7 +99,7 @@ def run(self,
             data=None,  # type: t.Optional[str]
             stdin=None,  # type: t.Optional[t.IO[bytes]]
             stdout=None,  # type: t.Optional[t.IO[bytes]]
-            force_stdout=False,  # type: bool
+            output_stream=None,  # type: t.Optional[OutputStream]
             ):  # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]]
         """Run the specified command and return the result."""
         return run_command(
@@ -109,7 +110,7 @@ def run(self,
             stdin=stdin,
             stdout=stdout,
             interactive=interactive,
-            force_stdout=force_stdout,
+            output_stream=output_stream,
         )
 
 
@@ -140,7 +141,7 @@ def run(self,
             data=None,  # type: t.Optional[str]
             stdin=None,  # type: t.Optional[t.IO[bytes]]
             stdout=None,  # type: t.Optional[t.IO[bytes]]
-            force_stdout=False,  # type: bool
+            output_stream=None,  # type: t.Optional[OutputStream]
             ):  # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]]
         """Run the specified command and return the result."""
         options = list(self.options)
@@ -174,7 +175,7 @@ def error_callback(ex):  # type: (SubprocessError) -> None
                 stdin=stdin,
                 stdout=stdout,
                 interactive=interactive,
-                force_stdout=force_stdout,
+                output_stream=output_stream,
                 error_callback=error_callback,
             )
 
@@ -222,7 +223,7 @@ def run(self,
             data=None,  # type: t.Optional[str]
             stdin=None,  # type: t.Optional[t.IO[bytes]]
             stdout=None,  # type: t.Optional[t.IO[bytes]]
-            force_stdout=False,  # type: bool
+            output_stream=None,  # type: t.Optional[OutputStream]
             ):  # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]]
         """Run the specified command and return the result."""
         options = []
@@ -243,7 +244,7 @@ def run(self,
             stdin=stdin,
             stdout=stdout,
             interactive=interactive,
-            force_stdout=force_stdout,
+            output_stream=output_stream,
         )
 
     def inspect(self):  # type: () -> DockerInspect

test/lib/ansible_test/_internal/delegation.py

@@ -27,6 +27,7 @@
     ANSIBLE_BIN_PATH,
     ANSIBLE_LIB_ROOT,
     ANSIBLE_TEST_ROOT,
+    OutputStream,
 )
 
 from .util_common import (
@@ -191,7 +192,12 @@ def delegate_command(args, host_state, exclude, require):  # type: (EnvironmentC
         success = False
 
         try:
-            con.run(insert_options(command, options), capture=False, interactive=args.interactive)
+            # When delegating, preserve the original separate stdout/stderr streams, but only when the following conditions are met:
+            # 1) Display output is being sent to stderr. This indicates the output on stdout must be kept separate from stderr.
+            # 2) The delegation is non-interactive. Interactive mode, which generally uses a TTY, is not compatible with intercepting stdout/stderr.
+            # The downside to having separate streams is that individual lines of output from each are more likely to appear out-of-order.
+            output_stream = OutputStream.ORIGINAL if args.display_stderr and not args.interactive else None
+            con.run(insert_options(command, options), capture=False, interactive=args.interactive, output_stream=output_stream)
             success = True
         finally:
             if host_delegation:

test/lib/ansible_test/_internal/docker_util.py

@@ -20,6 +20,7 @@
     find_executable,
     SubprocessError,
     cache,
+    OutputStream,
 )
 
 from .util_common import (
@@ -516,7 +517,7 @@ def docker_exec(
         stdin=None,  # type: t.Optional[t.IO[bytes]]
         stdout=None,  # type: t.Optional[t.IO[bytes]]
         interactive=False,  # type: bool
-        force_stdout=False,  # type: bool
+        output_stream=None,  # type: t.Optional[OutputStream]
         data=None,  # type: t.Optional[str]
 ):  # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]]
     """Execute the given command in the specified container."""
@@ -527,7 +528,7 @@ def docker_exec(
         options.append('-i')
 
     return docker_command(args, ['exec'] + options + [container_id] + cmd, capture=capture, stdin=stdin, stdout=stdout, interactive=interactive,
-                          force_stdout=force_stdout, data=data)
+                          output_stream=output_stream, data=data)
 
 
 def docker_info(args):  # type: (CommonConfig) -> t.Dict[str, t.Any]
@@ -549,7 +550,7 @@ def docker_command(
         stdin=None,  # type: t.Optional[t.IO[bytes]]
         stdout=None,  # type: t.Optional[t.IO[bytes]]
         interactive=False,  # type: bool
-        force_stdout=False,  # type: bool
+        output_stream=None,  # type: t.Optional[OutputStream]
         always=False,  # type: bool
         data=None,  # type: t.Optional[str]
 ):  # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]]
@@ -561,7 +562,7 @@ def docker_command(
         command.append('--remote')
 
     return run_command(args, command + cmd, env=env, capture=capture, stdin=stdin, stdout=stdout, interactive=interactive, always=always,
-                       force_stdout=force_stdout, data=data)
+                       output_stream=output_stream, data=data)
 
 
 def docker_environment():  # type: () -> t.Dict[str, str]