Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow commands whose output is not logged unless they fail #2751

Merged
merged 1 commit into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import tmt.plugins
import tmt.steps.discover
import tmt.utils
from tmt.log import Logger
from tmt.utils import (
Command,
Common,
Expand All @@ -41,7 +42,7 @@
wait,
)

from . import MATCH, assert_log
from . import MATCH, assert_log, assert_not_log

run = Common(logger=tmt.log.Logger.create(verbose=0, debug=0, quiet=False)).run

Expand Down Expand Up @@ -700,6 +701,35 @@ def test_run_big(root_logger):
assert len(output.stdout) == 200000


def test_command_run_without_streaming(root_logger: Logger, caplog) -> None:
ShellScript('ls -al /').to_shell_command().run(
cwd=Path.cwd(),
stream_output=True,
logger=root_logger)

assert_log(caplog, message=MATCH('out: drwx.+? mnt'))

caplog.clear()

ShellScript('ls -al /').to_shell_command().run(
cwd=Path.cwd(),
stream_output=False,
logger=root_logger)

assert_not_log(caplog, message=MATCH('out: drwx.+? mnt'))

caplog.clear()

with pytest.raises(tmt.utils.RunError):
ShellScript('ls -al / /does/not/exist').to_shell_command().run(
cwd=Path.cwd(),
stream_output=False,
logger=root_logger)

assert_log(caplog, message=MATCH('out: drwx.+? mnt'))
assert_log(caplog, message=MATCH("err: ls: cannot access '/does/not/exist'"))


def test_get_distgit_handler():
for _wrong_remotes in [[], ["blah"]]:
with pytest.raises(tmt.utils.GeneralError):
Expand Down
2 changes: 2 additions & 0 deletions tmt/checks/watchdog.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ def _handle_output(ping_output: str) -> None:
'-c',
str(self.ping_packets),
invocation.guest.primary_address) .run(cwd=Path.cwd(),
stream_output=False,
logger=logger)

_handle_output(output.stdout or '')
Expand Down Expand Up @@ -327,6 +328,7 @@ def _success(ncat_output: str) -> None:
'-zv',
invocation.guest.primary_address,
str(invocation.guest.port or 22)) .run(cwd=Path.cwd(),
stream_output=False,
logger=logger)

_success(output.stderr or '')
Expand Down
25 changes: 21 additions & 4 deletions tmt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -965,14 +965,16 @@ def __init__(
*,
stream: Optional[IO[bytes]] = None,
logger: Optional[tmt.log.LoggingFunction] = None,
click_context: Optional[click.Context] = None) -> None:
click_context: Optional[click.Context] = None,
stream_output: bool = True) -> None:
super().__init__(daemon=True)

self.stream = stream
self.output: list[str] = []
self.log_header = log_header
self.logger = logger
self.click_context = click_context
self.stream_output = stream_output

def run(self) -> None:
if self.stream is None:
Expand All @@ -986,7 +988,7 @@ def run(self) -> None:

for _line in self.stream:
line = _line.decode('utf-8', errors='replace')
if line != '':
if self.stream_output and line != '':
self.logger(
self.log_header,
line.rstrip('\n'),
Expand Down Expand Up @@ -1155,6 +1157,7 @@ def run(
friendly_command: Optional[str] = None,
log: Optional[tmt.log.LoggingFunction] = None,
silent: bool = False,
stream_output: bool = True,
caller: Optional['Common'] = None,
logger: tmt.log.Logger) -> CommandOutput:
"""
Expand Down Expand Up @@ -1182,6 +1185,9 @@ def run(
default, ``logger.debug`` is used.
:param silent: if set, logging of steps taken by this function would be
reduced.
:param stream_output: if set, command output would be streamed
live into the log. When unset, the output would be logged
only when the command fails.
:param caller: optional "parent" of the command execution, used for better
linked exceptions.
:param logger: logger to use for logging.
Expand Down Expand Up @@ -1277,7 +1283,8 @@ def _spawn_process() -> subprocess.Popen[bytes]:
'out',
stream=process.stdout,
logger=output_logger,
click_context=click.get_current_context(silent=True))
click_context=click.get_current_context(silent=True),
stream_output=stream_output)

if join:
stderr_logger: StreamLogger = UnusedStreamLogger('err')
Expand All @@ -1287,7 +1294,8 @@ def _spawn_process() -> subprocess.Popen[bytes]:
'err',
stream=process.stderr,
logger=output_logger,
click_context=click.get_current_context(silent=True))
click_context=click.get_current_context(silent=True),
stream_output=stream_output)

stdout_logger.start()
stderr_logger.start()
Expand Down Expand Up @@ -1347,6 +1355,15 @@ def log_event(msg: str) -> None:
if process.returncode != 0:
logger.debug(f"Command returned '{process.returncode}'.", level=3)

if not stream_output:
if stdout is not None:
for line in stdout.splitlines():
output_logger('out', value=line, color='yellow', level=3)

if stderr is not None:
for line in stderr.splitlines():
output_logger('err', value=line, color='yellow', level=3)

raise RunError(
f"Command '{friendly_command or str(self)}' returned {process.returncode}.",
self,
Expand Down
Loading