From 713c0e69e4a311ed061d977ee389cebd08e01626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Tue, 1 Oct 2024 17:21:54 +0200 Subject: [PATCH] Change result note key from a string to a list of strings This better represents its real nature: a collection of various notes, there may be more than one note or topic tmt needed to share with user depending on what happened while the test was running. --- spec/plans/results.fmf | 34 +++++++++++++++--------- tests/execute/result/custom/results.json | 8 ++++-- tests/execute/result/custom/results.yaml | 6 +++-- tmt/frameworks/beakerlib.py | 14 +++++----- tmt/frameworks/shell.py | 6 ++--- tmt/result.py | 25 ++++++++--------- tmt/schemas/common.yaml | 4 ++- tmt/steps/execute/__init__.py | 13 ++++----- tmt/steps/execute/internal.py | 8 +++--- 9 files changed, 64 insertions(+), 54 deletions(-) diff --git a/spec/plans/results.fmf b/spec/plans/results.fmf index 732ae155c6..b58dd7a7e1 100644 --- a/spec/plans/results.fmf +++ b/spec/plans/results.fmf @@ -33,8 +33,9 @@ description: | # String, the original outcome of the test execution. original-result: "pass"|"fail"|"info"|"warn"|"error"|"skip" - # String, optional comment to report with the result. - note: "Things were great." + # List of strings, optional comments to report with the result. + note: + - "Things were great." # List of strings, paths to log files. log: @@ -88,8 +89,9 @@ description: | # String, the original outcome of the check execution. original-result: "pass"|"fail"|"info"|"warn"|"error"|"skip" - # String, optional comment to report with the result. - note: "Things were great." + # List of strings, optional comments to report with the result. + note: + - "Things were great." # List of strings, paths to logs files. log: @@ -126,8 +128,9 @@ description: | # String, the original outcome of the phase execution. original-result: "pass"|"fail"|"info"|"warn"|"error"|"skip" - # String, optional comment to report with the result. - note: "Things were great." + # List of strings, optional comments to report with the result. + note: + - "Things were great." # List of strings, paths to log files. log: @@ -269,7 +272,8 @@ example: start-time: "2023-03-10T09:44:14.439120+00:00" end-time: "2023-03-10T09:44:24.242227+00:00" duration: 00:00:09 - note: good result + note: + - good result ids: extra-nitrate: some-nitrate-id guest: @@ -284,7 +288,8 @@ example: start-time: "2023-03-10T09:44:14.439120+00:00" end-time: "2023-03-10T09:44:24.242227+00:00" duration: 00:00:09 - note: fail result + note: + - fail result guest: name: default-0 @@ -295,7 +300,8 @@ example: log: - pass_log duration: 00:11:22 - note: good result + note: + - good result ids: extra-nitrate: some-nitrate-id @@ -305,7 +311,8 @@ example: - fail_log - another_log duration: 00:22:33 - note: fail result + note: + - fail result - | # Example of a perfectly valid, yet stingy custom results file @@ -325,7 +332,8 @@ example: start-time: "2023-03-10T09:44:14.439120+00:00" end-time: "2023-03-10T09:44:24.242227+00:00" duration: 00:00:09 - note: good result + note: + - good result ids: extra-nitrate: some-nitrate-id guest: @@ -335,12 +343,12 @@ example: event: after-test result: pass log: [] - note: + note: [] - name: kernel-panic event: after-test result: pass log: [] - note: + note: [] - | # syntax: json diff --git a/tests/execute/result/custom/results.json b/tests/execute/result/custom/results.json index c13bd60af0..dcb344f06c 100644 --- a/tests/execute/result/custom/results.json +++ b/tests/execute/result/custom/results.json @@ -2,7 +2,9 @@ { "name": "/test/passing", "result": "pass", - "note": "good result", + "note": [ + "good result" + ], "log": [ "pass_log" ], @@ -28,7 +30,9 @@ "another-id": "bar2" }, "duration": "00:23:34", - "note": "fail result", + "note": [ + "fail result" + ], "serial-number": 2, "guest": { "name": "client-1", diff --git a/tests/execute/result/custom/results.yaml b/tests/execute/result/custom/results.yaml index eb21489ad8..3857338081 100644 --- a/tests/execute/result/custom/results.yaml +++ b/tests/execute/result/custom/results.yaml @@ -1,6 +1,7 @@ - name: /test/passing result: pass - note: good result + note: + - good result log: - pass_log ids: @@ -20,7 +21,8 @@ some-id: foo2 another-id: bar2 duration: 00:22:33 - note: fail result + note: + - fail result serial-number: 2 guest: name: client-1 diff --git a/tmt/frameworks/beakerlib.py b/tmt/frameworks/beakerlib.py index a722f0b82b..3259a8301b 100644 --- a/tmt/frameworks/beakerlib.py +++ b/tmt/frameworks/beakerlib.py @@ -1,5 +1,5 @@ import re -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING import tmt.base import tmt.log @@ -56,7 +56,7 @@ def extract_results( logger: tmt.log.Logger) -> list[tmt.result.Result]: """ Check result of a beakerlib test """ # Initialize data, prepare log paths - note: Optional[str] = None + note: list[str] = [] log: list[Path] = [] for filename in [tmt.steps.execute.TEST_OUTPUT_FILENAME, 'journal.txt']: if (invocation.path / filename).is_file(): @@ -69,7 +69,7 @@ def extract_results( results = invocation.phase.read(beakerlib_results_filepath, level=3) except tmt.utils.FileError: logger.debug(f"Unable to read '{beakerlib_results_filepath}'.", level=3) - note = 'beakerlib: TestResults FileError' + note.append('beakerlib: TestResults FileError') return [tmt.Result.from_test_invocation( invocation=invocation, @@ -93,7 +93,7 @@ def extract_results( logger.debug( f"No '{missing_piece}' found in '{beakerlib_results_filepath}'{hint}.", level=3) - note = 'beakerlib: Result/State missing' + note.append('beakerlib: Result/State missing') return [tmt.Result.from_test_invocation( invocation=invocation, result=ResultOutcome.ERROR, @@ -106,15 +106,15 @@ def extract_results( # Check if it was killed by timeout (set by tmt executor) actual_result = ResultOutcome.ERROR if invocation.return_code == tmt.utils.ProcessExitCodes.TIMEOUT: - note = 'timeout' + note.append('timeout') invocation.phase.timeout_hint(invocation) elif tmt.utils.ProcessExitCodes.is_pidfile(invocation.return_code): - note = 'pidfile locking' + note.append('pidfile locking') # Test results should be in complete state elif state != 'complete': - note = f"beakerlib: State '{state}'" + note.append(f"beakerlib: State '{state}'") # Finally we have a valid result else: actual_result = ResultOutcome.from_spec(result.lower()) diff --git a/tmt/frameworks/shell.py b/tmt/frameworks/shell.py index 2dd22fa3da..40e9ad04ef 100644 --- a/tmt/frameworks/shell.py +++ b/tmt/frameworks/shell.py @@ -30,7 +30,7 @@ def extract_results( logger: tmt.log.Logger) -> list[tmt.result.Result]: """ Check result of a shell test """ assert invocation.return_code is not None - note = None + note: list[str] = [] try: # Process the exit code and prepare the log path @@ -39,11 +39,11 @@ def extract_results( result = ResultOutcome.ERROR # Add note about the exceeded duration if invocation.return_code == tmt.utils.ProcessExitCodes.TIMEOUT: - note = 'timeout' + note.append('timeout') invocation.phase.timeout_hint(invocation) elif tmt.utils.ProcessExitCodes.is_pidfile(invocation.return_code): - note = 'pidfile locking' + note.append('pidfile locking') return [tmt.Result.from_test_invocation( invocation=invocation, diff --git a/tmt/result.py b/tmt/result.py index 546346afa6..af8d6f1c4c 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -152,7 +152,8 @@ class BaseResult(SerializableContainer): serialize=lambda result: result.value, unserialize=ResultOutcome.from_spec ) - note: Optional[str] = None + note: list[str] = field( + default_factory=cast(Callable[[], list[str]], list)) log: list[Path] = field( default_factory=cast(Callable[[], list[Path]], list), serialize=lambda logs: [str(log) for log in logs], @@ -176,10 +177,14 @@ def show(self) -> str: ] if self.note: - components.append(f'({self.note})') + components.append(f'({self.printable_note})') return ' '.join(components) + @property + def printable_note(self) -> str: + return ', '.join(self.note) + @dataclasses.dataclass class CheckResult(BaseResult): @@ -268,7 +273,7 @@ def from_test_invocation( *, invocation: 'tmt.steps.execute.TestInvocation', result: ResultOutcome, - note: Optional[str] = None, + note: Optional[list[str]] = None, ids: Optional[ResultIds] = None, log: Optional[list[Path]] = None) -> 'Result': """ @@ -311,7 +316,7 @@ def from_test_invocation( fmf_id=invocation.test.fmf_id, context=invocation.phase.step.plan._fmf_context, result=result, - note=note, + note=note or [], start_time=invocation.start_time, end_time=invocation.end_time, duration=invocation.real_duration, @@ -337,15 +342,7 @@ def interpret_result(self, interpret: ResultInterpret) -> 'Result': return self # Extend existing note or set a new one - if self.note: - self.note += f', original result: {self.result.value}' - - elif self.note is None: - self.note = f'original result: {self.result.value}' - - else: - raise tmt.utils.SpecificationError( - f"Test result note '{self.note}' must be a string.") + self.note.append(f'original result: {self.result.value}') if interpret == ResultInterpret.XFAIL: # Swap just fail<-->pass, keep the rest as is (info, warn, @@ -417,7 +414,7 @@ def show(self, display_guest: bool = True) -> str: components.append(f'(on {format_guest_full_name(self.guest.name, self.guest.role)})') if self.note: - components.append(f'({self.note})') + components.append(f'({self.printable_note})') return ' '.join(components) diff --git a/tmt/schemas/common.yaml b/tmt/schemas/common.yaml index 220a5d1282..28c26339aa 100644 --- a/tmt/schemas/common.yaml +++ b/tmt/schemas/common.yaml @@ -483,7 +483,9 @@ definitions: - restraint result_note: - type: string + type: array + items: + type: string result_log: type: array diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index a5f18da10c..650c2071e8 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -686,7 +686,7 @@ def _process_results_reduce( # The original one may be left unset - malformed results file, # for example, provides no usable original outcome. actual_outcome: ResultOutcome - note: Optional[str] = None + note: list[str] = [] try: outcomes = [ @@ -696,7 +696,7 @@ def _process_results_reduce( except tmt.utils.SpecificationError as exc: actual_outcome = ResultOutcome.ERROR - note = exc.message + note.append(exc.message) else: hierarchy = [ @@ -763,10 +763,7 @@ def _process_results_partials( else: if not partial_result.name.startswith('/'): - if partial_result.note and isinstance(partial_result.note, str): - partial_result.note += ", custom test result name should start with '/'" - else: - partial_result.note = "custom test result name should start with '/'" + partial_result.note.append("custom test result name should start with '/'") partial_result.name = '/' + partial_result.name partial_result.name = test.name + partial_result.name @@ -817,13 +814,13 @@ def extract_custom_results(self, invocation: TestInvocation) -> list["tmt.Result if not collection.file_exists: return [tmt.Result.from_test_invocation( invocation=invocation, - note=f"custom results file not found in '{invocation.test_data_path}'", + note=[f"custom results file not found in '{invocation.test_data_path}'"], result=ResultOutcome.ERROR)] if not collection.results: return [tmt.Result.from_test_invocation( invocation=invocation, - note="no custom results were provided", + note=["no custom results were provided"], result=ResultOutcome.ERROR)] collection.validate() diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index 595d7b68e8..204bfa9be9 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -554,12 +554,12 @@ def _run_tests( except tmt.utils.RebootTimeoutError: for result in invocation.results: result.result = ResultOutcome.ERROR - result.note = 'reboot timeout' + result.note.append('reboot timeout') else: for result in invocation.results: result.result = ResultOutcome.ERROR - result.note = 'crashed too many times' + result.note.append('crashed too many times') # Handle reboot, abort, exit-first if invocation.reboot_requested: @@ -572,12 +572,12 @@ def _run_tests( except tmt.utils.RebootTimeoutError: for result in invocation.results: result.result = ResultOutcome.ERROR - result.note = 'reboot timeout' + result.note.append('reboot timeout') if invocation.abort_requested: for result in invocation.results: # In case of aborted all results in list will be aborted - result.note = 'aborted' + result.note.append('aborted') self._results.extend(invocation.results)