From d73aab174d78d95108558efd9e9944c161fded6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Mon, 14 Oct 2024 21:19:27 +0200 Subject: [PATCH 1/4] Add a helper for saving command output in a file This will be a very needed functionality: plugin runs a command, and its output needs to be saved in a dedicated file, usualy because the output represents some interesting, standalone piece of information. For example, an AVC denial report generated by `ausearch`. If it would be the only such actor, it would be fine to keep the implementation in the `avc` plugin. But, with the advent of saving results of `prepare` and `finish` phases, the question is, if I get `results.yaml` for a `prepare` step, would it even contain anything besides `pass` or `error`? Well, it can contain logs of commands like `ansible-playbook` or shell scripts. And suddenly we have several plugins that may need to save some walls of texts, usually produced by a command, in files, and then link those files to results and present them to user. And because of that, it makes sense to provide a helper function that would produce unified output for all of them? Why should `avc` separate stdout and stderr by two lines and `prepare/shell` with just one? Not on my watch. --- tests/unit/test_utils.py | 61 ++++++++++++++++++++ tmt/checks/avc.py | 20 ++----- tmt/checks/dmesg.py | 2 +- tmt/checks/watchdog.py | 4 +- tmt/utils/__init__.py | 118 ++++++++++++++++++++++++++++++++++----- 5 files changed, 172 insertions(+), 33 deletions(-) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index f5b96ad3f6..e099c54457 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1789,3 +1789,64 @@ def test_create_link_relation(self, mock_config_tree, mock_add_simple_link) -> N # Load the test object again with the link present test = tmt.Tree(logger=self.logger, path=self.tmp).tests(names=['tmp/test'])[0] assert test.link.get('verifies')[0].target == 'https://issues.redhat.com/browse/TT-262' + + +def test_render_command_report_output(): + assert '\n'.join(tmt.utils.render_command_report( + label='foo', + command=ShellScript('/bar/baz'), + output=tmt.utils.CommandOutput( + stdout='This is some stdout', + stderr='This is some stderr' + ) + )) == """## foo + +# /bar/baz + +# stdout (1 lines) +#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +This is some stdout +#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +# stderr (1 lines) +#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +This is some stderr +#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +""" + + +def test_render_command_report_exception(): + assert '\n'.join(tmt.utils.render_command_report( + label='foo', + command=ShellScript('/bar/baz'), + exc=tmt.utils.RunError( + 'foo failed', + ShellScript('/bar/baz').to_shell_command(), + 1, + stdout='This is some stdout', + stderr='This is some stderr' + ) + )) == """## foo + +# /bar/baz + +# stdout (1 lines) +#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +This is some stdout +#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +# stderr (1 lines) +#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +This is some stderr +#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +""" + + +def test_render_command_report_minimal(): + print(list(tmt.utils.render_command_report( + label='foo' + ))) + assert '\n'.join(tmt.utils.render_command_report( + label='foo' + )) == """## foo +""" diff --git a/tmt/checks/avc.py b/tmt/checks/avc.py index 1fdfad5d8d..96694440ee 100644 --- a/tmt/checks/avc.py +++ b/tmt/checks/avc.py @@ -13,7 +13,7 @@ Path, ShellScript, format_timestamp, - render_run_exception_streams, + render_command_report, ) if TYPE_CHECKING: @@ -117,21 +117,13 @@ def _output_logger( def _report_success(label: str, output: tmt.utils.CommandOutput) -> list[str]: """ Format successful command output for the report """ - return [ - f'# {label}', - output.stdout or '', - '' - ] + return list(render_command_report(label=label, output=output)) def _report_failure(label: str, exc: tmt.utils.RunError) -> list[str]: """ Format failed command output for the report """ - return [ - f'# {label}', - "\n".join(render_run_exception_streams(exc.stdout, exc.stderr, verbose=1)), - '' - ] + return list(render_command_report(label=label, exc=exc)) def create_ausearch_timestamp( @@ -243,11 +235,7 @@ def create_final_report( got_ausearch = True got_denials = True - report += [ - '# ausearch', - "\n".join(render_run_exception_streams(output.stdout, output.stderr, verbose=1)), - '' - ] + report += list(render_command_report(label='ausearch', output=output)) else: if exc.returncode == 1 and exc.stderr and '' in exc.stderr.strip(): diff --git a/tmt/checks/dmesg.py b/tmt/checks/dmesg.py index 540b167a3e..c0bf460a26 100644 --- a/tmt/checks/dmesg.py +++ b/tmt/checks/dmesg.py @@ -106,7 +106,7 @@ def _save_dmesg( except tmt.utils.RunError as exc: outcome = ResultOutcome.ERROR - output = "\n".join(render_run_exception_streams(exc.stdout, exc.stderr, verbose=1)) + output = "\n".join(render_run_exception_streams(exc.output, verbose=1)) else: outcome = ResultOutcome.PASS diff --git a/tmt/checks/watchdog.py b/tmt/checks/watchdog.py index 9d61560b69..3f72e1b364 100644 --- a/tmt/checks/watchdog.py +++ b/tmt/checks/watchdog.py @@ -252,7 +252,7 @@ def _handle_output(ping_output: str) -> None: _handle_output(exc.stdout or '') else: - _handle_output('\n'.join(render_run_exception_streams(exc.stdout, exc.stderr))) + _handle_output('\n'.join(render_run_exception_streams(exc.output))) def do_ssh_ping( self, @@ -337,7 +337,7 @@ def _success(ncat_output: str) -> None: _fail_connection_refused(exc.stderr or '') else: - _fail_unknown('\n'.join(render_run_exception_streams(exc.stdout, exc.stderr))) + _fail_unknown('\n'.join(render_run_exception_streams(exc.output))) logger.debug( f'failed {guest_context.ssh_ping_failures}' diff --git a/tmt/utils/__init__.py b/tmt/utils/__init__.py index 8977473ed1..a7b070d9bf 100644 --- a/tmt/utils/__init__.py +++ b/tmt/utils/__init__.py @@ -2289,6 +2289,21 @@ def __init__( # `finish`), save a logger for later. self.logger = caller._logger if isinstance(caller, Common) else None + @functools.cached_property + def output(self) -> CommandOutput: + """ + Captured output of the command. + + .. note:: + + This field contains basically the same info as :py:attr:`stdout` + and :py:attr:`stderr`, but it's bundled into a single object. + This is how command output is passed between functions, therefore + the exception should offer it as well. + """ + + return CommandOutput(self.stdout, self.stderr) + class MetadataError(GeneralError): """ General metadata error """ @@ -2446,29 +2461,104 @@ def from_env(cls) -> 'TracebackVerbosity': def render_run_exception_streams( - stdout: Optional[str], - stderr: Optional[str], - verbose: int = 0) -> Iterator[str]: + output: CommandOutput, + verbose: int = 0, + comment_sign: str = '#') -> Iterator[str]: """ Render run exception output streams for printing """ - for name, output in (('stdout', stdout), ('stderr', stderr)): - if not output: + for name, content in (('stdout', output.stdout), ('stderr', output.stderr)): + if not content: continue - output_lines = output.strip().split('\n') + content_lines = content.strip().split('\n') # Show all lines in verbose mode, limit to maximum otherwise if verbose > 0: - line_summary = f"{len(output_lines)}" + line_summary = f"{len(content_lines)}" else: - line_summary = f"{min(len(output_lines), OUTPUT_LINES)}/{len(output_lines)}" - output_lines = output_lines[-OUTPUT_LINES:] + line_summary = f"{min(len(content_lines), OUTPUT_LINES)}/{len(content_lines)}" + content_lines = content_lines[-OUTPUT_LINES:] - yield f'{name} ({line_summary} lines)' - yield OUTPUT_WIDTH * '~' - yield from output_lines - yield OUTPUT_WIDTH * '~' + yield f'{comment_sign} {name} ({line_summary} lines)' + yield comment_sign + (OUTPUT_WIDTH - 1) * '~' + yield from content_lines + yield comment_sign + (OUTPUT_WIDTH - 1) * '~' yield '' +@overload +def render_command_report( + *, + label: str, + command: Optional[Union[ShellScript, Command]] = None, + output: CommandOutput, + exc: None = None) -> Iterator[str]: + pass + + +@overload +def render_command_report( + *, + label: str, + command: Optional[Union[ShellScript, Command]] = None, + output: None = None, + exc: RunError) -> Iterator[str]: + pass + + +def render_command_report( + *, + label: str, + command: Optional[Union[ShellScript, Command]] = None, + output: Optional[CommandOutput] = None, + exc: Optional[RunError] = None, + comment_sign: str = '#') -> Iterator[str]: + """ + Format a command output for a report file. + + To provide unified look of various files reporting command outputs, + this helper would combine its arguments and emit lines the caller + may then write to a file. The following template is used: + + .. code-block:: + + ## ${label} + + # ${command} + + # stdout (N lines) + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ... + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + # stderr (N lines) + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ... + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + :param label: a string describing the intent of the command. It is + useful for user who reads the report file eventually. + :param command: command that was executed. + :param output: if set, it contains output of the command. It has + higher priority than ``exc``. + :param exc: if set, it represents a failed command, and input stored + in it is rendered. + :param comment_sign: a character to mark lines with comments that + document the report. + """ + + yield f'{comment_sign}{comment_sign} {label}' + yield '' + + if command: + yield f'{comment_sign} {command.to_element()}' + yield '' + + if output is not None: + yield from render_run_exception_streams(output, verbose=1) + + elif exc is not None: + yield from render_run_exception_streams(exc.output, verbose=1) + + def render_run_exception(exception: RunError) -> Iterator[str]: """ Render detailed output upon command execution errors for printing """ @@ -2480,7 +2570,7 @@ def render_run_exception(exception: RunError) -> Iterator[str]: else: verbose = 0 - yield from render_run_exception_streams(exception.stdout, exception.stderr, verbose=verbose) + yield from render_run_exception_streams(exception.output, verbose=verbose) def render_exception_stack( From 8a1d26ef04ef8cf32659599dbb98fd13f176fce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Thu, 17 Oct 2024 12:25:00 +0200 Subject: [PATCH 2/4] squash: fix test? --- tests/unit/test_utils.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index e099c54457..2041629413 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1792,6 +1792,8 @@ def test_create_link_relation(self, mock_config_tree, mock_add_simple_link) -> N def test_render_command_report_output(): + delimiter = (tmt.utils.OUTPUT_WIDTH - 1) * '~' + assert '\n'.join(tmt.utils.render_command_report( label='foo', command=ShellScript('/bar/baz'), @@ -1799,23 +1801,25 @@ def test_render_command_report_output(): stdout='This is some stdout', stderr='This is some stderr' ) - )) == """## foo + )) == f"""## foo # /bar/baz # stdout (1 lines) -#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +#{delimiter} This is some stdout -#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +#{delimiter} # stderr (1 lines) -#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +#{delimiter} This is some stderr -#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +#{delimiter} """ def test_render_command_report_exception(): + delimiter = (tmt.utils.OUTPUT_WIDTH - 1) * '~' + assert '\n'.join(tmt.utils.render_command_report( label='foo', command=ShellScript('/bar/baz'), @@ -1826,19 +1830,19 @@ def test_render_command_report_exception(): stdout='This is some stdout', stderr='This is some stderr' ) - )) == """## foo + )) == f"""## foo # /bar/baz # stdout (1 lines) -#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +#{delimiter} This is some stdout -#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +#{delimiter} # stderr (1 lines) -#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +#{delimiter} This is some stderr -#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +#{delimiter} """ From 21f189652b11f7c2762246accca6d34dcaf4a863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Sat, 2 Nov 2024 10:06:44 +0100 Subject: [PATCH 3/4] squash: one extra space --- tests/unit/test_utils.py | 20 ++++++++++---------- tmt/utils/__init__.py | 8 +++++--- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 2041629413..22655866a0 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1792,7 +1792,7 @@ def test_create_link_relation(self, mock_config_tree, mock_add_simple_link) -> N def test_render_command_report_output(): - delimiter = (tmt.utils.OUTPUT_WIDTH - 1) * '~' + delimiter = (tmt.utils.OUTPUT_WIDTH - 2) * '~' assert '\n'.join(tmt.utils.render_command_report( label='foo', @@ -1806,19 +1806,19 @@ def test_render_command_report_output(): # /bar/baz # stdout (1 lines) -#{delimiter} +# {delimiter} This is some stdout -#{delimiter} +# {delimiter} # stderr (1 lines) -#{delimiter} +# {delimiter} This is some stderr -#{delimiter} +# {delimiter} """ def test_render_command_report_exception(): - delimiter = (tmt.utils.OUTPUT_WIDTH - 1) * '~' + delimiter = (tmt.utils.OUTPUT_WIDTH - 2) * '~' assert '\n'.join(tmt.utils.render_command_report( label='foo', @@ -1835,14 +1835,14 @@ def test_render_command_report_exception(): # /bar/baz # stdout (1 lines) -#{delimiter} +# {delimiter} This is some stdout -#{delimiter} +# {delimiter} # stderr (1 lines) -#{delimiter} +# {delimiter} This is some stderr -#{delimiter} +# {delimiter} """ diff --git a/tmt/utils/__init__.py b/tmt/utils/__init__.py index a7b070d9bf..952971dd4e 100644 --- a/tmt/utils/__init__.py +++ b/tmt/utils/__init__.py @@ -2477,10 +2477,12 @@ def render_run_exception_streams( line_summary = f"{min(len(content_lines), OUTPUT_LINES)}/{len(content_lines)}" content_lines = content_lines[-OUTPUT_LINES:] - yield f'{comment_sign} {name} ({line_summary} lines)' - yield comment_sign + (OUTPUT_WIDTH - 1) * '~' + line_intro = f'{comment_sign} ' + + yield line_intro + f'{name} ({line_summary} lines)' + yield line_intro + (OUTPUT_WIDTH - 2) * '~' yield from content_lines - yield comment_sign + (OUTPUT_WIDTH - 1) * '~' + yield line_intro + (OUTPUT_WIDTH - 2) * '~' yield '' From 7fa0e2848fe70b277b6747f137adba7f65145e36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Sat, 2 Nov 2024 13:25:33 +0100 Subject: [PATCH 4/4] squash: fix docstring --- tmt/utils/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tmt/utils/__init__.py b/tmt/utils/__init__.py index 952971dd4e..c1b258ba8d 100644 --- a/tmt/utils/__init__.py +++ b/tmt/utils/__init__.py @@ -2527,14 +2527,14 @@ def render_command_report( # ${command} # stdout (N lines) - #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ... - #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # stderr (N lines) - #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ... - #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ :param label: a string describing the intent of the command. It is useful for user who reads the report file eventually.