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
@@ -95,7 +95,7 @@ def extract_results(
         subresults = [result.to_subresult() for result in results]
 
         # 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():
@@ -108,7 +108,7 @@ def extract_results(
             beakerlib_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,
@@ -133,7 +133,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,
@@ -147,15 +147,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

@@ -85,7 +85,7 @@ def _process_results_reduce(
             invocation=invocation,
             result=actual_outcome,
             log=test_logs,
-            note=note,
+            note=[note] if note else [],
             subresult=[result.to_subresult() for result in results])]
 
     @classmethod
@@ -108,7 +108,7 @@ def extract_results(
         :returns: list of results.
         """
         assert invocation.return_code is not None
-        note = None
+        note: list[str] = []
 
         # Handle the `tmt-report-result` command results as a single test with assigned tmt
         # subresults.
@@ -124,11 +124,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

@@ -176,7 +176,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],
@@ -200,10 +201,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):
@@ -297,7 +302,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,
             subresult: Optional[list[SubResult]] = None) -> 'Result':
@@ -341,7 +346,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,
@@ -356,13 +361,6 @@ def from_test_invocation(
 
         return _result.interpret_result(invocation.test.result, interpret_checks)
 
-    def append_note(self, note: str) -> None:
-        """ Append text to result note """
-        if self.note:
-            self.note += f", {note}"
-        else:
-            self.note = note
-
     def interpret_check_result(
             self,
             check_name: str,
@@ -385,21 +383,21 @@ def interpret_check_result(
 
         if interpret == CheckResultInterpret.RESPECT:
             if interpreted_outcome == ResultOutcome.FAIL:
-                self.append_note(f"check '{check_name}' failed")
+                self.note.append(f"check '{check_name}' failed")
 
         elif interpret == CheckResultInterpret.INFO:
             interpreted_outcome = ResultOutcome.INFO
-            self.append_note(f"check '{check_name}' is informational")
+            self.note.append(f"check '{check_name}' is informational")
 
         elif interpret == CheckResultInterpret.XFAIL:
 
             if reduced_outcome == ResultOutcome.PASS:
                 interpreted_outcome = ResultOutcome.FAIL
-                self.append_note(f"check '{check_name}' did not fail as expected")
+                self.note.append(f"check '{check_name}' did not fail as expected")
 
             if reduced_outcome == ResultOutcome.FAIL:
                 interpreted_outcome = ResultOutcome.PASS
-                self.append_note(f"check '{check_name}' failed as expected")
+                self.note.append(f"check '{check_name}' failed as expected")
 
         return interpreted_outcome
 
@@ -437,11 +435,11 @@ def interpret_result(
         # Override result with result outcome provided by user
         if interpret not in (ResultInterpret.RESPECT, ResultInterpret.XFAIL):
             self.result = ResultOutcome(interpret.value)
-            self.append_note(f"test result overridden: {self.result.value}")
+            self.note.append(f"test result overridden: {self.result.value}")
 
             # Add original result to note if the result has changed
             if self.result != self.original_result:
-                self.append_note(f"original test result: {self.original_result.value}")
+                self.note.append(f"original test result: {self.original_result.value}")
 
             return self
 
@@ -450,15 +448,15 @@ def interpret_result(
 
             if self.result == ResultOutcome.PASS:
                 self.result = ResultOutcome.FAIL
-                self.append_note("test was expected to fail")
+                self.note.append("test was expected to fail")
 
             elif self.result == ResultOutcome.FAIL:
                 self.result = ResultOutcome.PASS
-                self.append_note("test failed as expected")
+                self.note.append("test failed as expected")
 
         # Add original result to note if the result has changed
         if self.result != self.original_result:
-            self.append_note(f"original test result: {self.original_result.value}")
+            self.note.append(f"original test result: {self.original_result.value}")
 
         return self
 
@@ -522,7 +520,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

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

tmt/steps/execute/__init__.py

@@ -824,10 +824,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
 
@@ -878,13 +875,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()