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

@@ -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

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",

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

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())

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,

tmt/result.py

@@ -128,7 +128,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],
@@ -152,10 +153,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):
@@ -244,7 +249,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':
         """
@@ -287,7 +292,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,
@@ -314,15 +319,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,
@@ -394,7 +391,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)
 

tmt/schemas/common.yaml

@@ -483,7 +483,9 @@ definitions:
       - restraint
 
   result_note:
-    type: string
+    type: array
+    items:
+      type: string
 
   result_log:
     type: array

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()

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)