Change result note key from a string to a list of strings (#3254)

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.

Co-authored-by: Petr Šplíchal <psplicha@redhat.com>

docs/releases.rst

@@ -22,6 +22,9 @@ support for submitting jobs on behalf of a group through the
 ``beaker-job-group`` key. The submitting user must be a member of
 the given job group.
 
+The ``note`` field of tmt :ref:`/spec/plans/results` changes from
+a string to a list of strings, to better accommodate multiple notes.
+
 
 tmt-1.40.0
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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:
@@ -256,6 +259,9 @@ description: |
     .. versionchanged:: 1.37
        original result of test, subtest and check is stored in ``original-result`` key.
 
+    .. versionchanged:: 1.41
+       ``note`` field changed from a string to a list of strings.
+
     __ https://github.com/teemtee/tmt/blob/main/tmt/schemas/results.yaml
 
 example:
@@ -269,7 +275,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 +291,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 +303,8 @@ example:
       log:
         - pass_log
       duration: 00:11:22
-      note: good result
+      note:
+        - good result
       ids:
         extra-nitrate: some-nitrate-id
 
@@ -305,7 +314,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 +335,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 +346,12 @@ example:
           event: after-test
           result: pass
           log: []
-          note:
+          note: []
         - name: kernel-panic
           event: after-test
           result: pass
           log: []
-          note:
+          note: []
 
   - |
     # syntax: json
@@ -351,7 +362,7 @@ example:
         "result": "pass",
         "log": ["pass_log"],
         "duration": "00:11:22",
-        "note": "good result"
+        "note": ["good result"]
       }
     ]
 

tests/execute/restraint/tmt-abort/test.sh

@@ -14,8 +14,11 @@ rlJournalStart
         rlAssertNotGrep "This test should not be executed." $rlRun_LOG
         rlAssertNotGrep "This should not be executed either." $rlRun_LOG
 
+
         rlAssertGrep "result: error" "${run}/plan/execute/results.yaml"
-        rlAssertGrep "note: aborted" "${run}/plan/execute/results.yaml"
+        rlAssertEquals "results should record the test aborted" \
+            "$(yq -r '.[] | .note | join(", ")' ${run}/plan/execute/results.yaml)" \
+            "beakerlib: State 'started', aborted"
     rlPhaseEnd
 
     rlPhaseStartCleanup

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

tests/report/html/test.sh

@@ -37,13 +37,14 @@ rlJournalStart
             grep -B 1 "/test/$test_name_suffix</td>" $HTML | tee $tmp/$test_name_suffix
             rlAssertGrep 'class="result error">error</td>' $tmp/$test_name_suffix -F
             sed -e "/name\">\/test\/$test_name_suffix/,/\/tr/!d" $HTML | tee $tmp/$test_name_suffix-note
-            rlAssertGrep '<td class="note">timeout</td>' $tmp/$test_name_suffix-note -F
+            rlAssertGrep '<li class="note">timeout</li>' $tmp/$test_name_suffix-note -F
 
             test_name_suffix=xfail
             grep -B 1 "/test/$test_name_suffix</td>" $HTML | tee $tmp/$test_name_suffix
             rlAssertGrep 'class="result pass">pass</td>' $tmp/$test_name_suffix -F
             sed -e "/name\">\/test\/$test_name_suffix/,/\/tr/!d" $HTML | tee $tmp/$test_name_suffix-note
-            rlAssertGrep '<td class="note">test failed as expected, original test result: fail</td>' $tmp/$test_name_suffix-note -F
+            rlAssertGrep '<li class="note">test failed as expected</li>' $tmp/$test_name_suffix-note -F
+            rlAssertGrep '<li class="note">original test result: fail</li>' $tmp/$test_name_suffix-note -F
         rlPhaseEnd
 
         if [ "$option" = "" ]; then

tests/unit/test_results.py

@@ -1,4 +1,3 @@
-from typing import Optional
 from unittest.mock import MagicMock
 
 import pytest
@@ -100,14 +99,14 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code:
             ResultInterpret.RESPECT,
             {"check1": CheckResultInterpret.RESPECT},
             ResultOutcome.PASS,
-            None
+            []
             ),
         (
             ResultOutcome.FAIL,
             ResultInterpret.RESPECT,
             {"check1": CheckResultInterpret.RESPECT},
             ResultOutcome.FAIL,
-            "check 'check1' failed"  # Note is set when check fails
+            ["check 'check1' failed"]  # Note is set when check fails
             ),
 
         # Test XFAIL interpretation
@@ -116,14 +115,14 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code:
             ResultInterpret.XFAIL,
             {"check1": CheckResultInterpret.RESPECT},
             ResultOutcome.PASS,
-            "check 'check1' failed, test failed as expected, original test result: fail"
+            ["check 'check1' failed", "test failed as expected", "original test result: fail"]
             ),
         (
             ResultOutcome.PASS,
             ResultInterpret.XFAIL,
             {"check1": CheckResultInterpret.RESPECT},
             ResultOutcome.FAIL,
-            "test was expected to fail, original test result: pass"
+            ["test was expected to fail", "original test result: pass"]
             ),
 
         # Test INFO interpretation
@@ -132,14 +131,14 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code:
             ResultInterpret.INFO,
             {"check1": CheckResultInterpret.RESPECT},
             ResultOutcome.INFO,
-            "check 'check1' failed, test result overridden: info, original test result: fail"
+            ["check 'check1' failed", "test result overridden: info", "original test result: fail"]
             ),
         (
             ResultOutcome.PASS,
             ResultInterpret.INFO,
             {"check1": CheckResultInterpret.RESPECT},
             ResultOutcome.INFO,
-            "test result overridden: info, original test result: pass"
+            ["test result overridden: info", "original test result: pass"]
             ),
 
         # Test WARN interpretation
@@ -148,7 +147,7 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code:
             ResultInterpret.WARN,
             {"check1": CheckResultInterpret.RESPECT},
             ResultOutcome.WARN,
-            "test result overridden: warn, original test result: pass"
+            ["test result overridden: warn", "original test result: pass"]
             ),
 
         # Test ERROR interpretation
@@ -157,7 +156,7 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code:
             ResultInterpret.ERROR,
             {"check1": CheckResultInterpret.RESPECT},
             ResultOutcome.ERROR,
-            "test result overridden: error, original test result: pass"
+            ["test result overridden: error", "original test result: pass"]
             ),
 
         # Test CUSTOM interpretation (should not modify result)
@@ -166,7 +165,7 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code:
             ResultInterpret.CUSTOM,
             {"check1": CheckResultInterpret.RESPECT},
             ResultOutcome.FAIL,
-            None
+            []
             ),
         ],
     ids=[
@@ -183,7 +182,7 @@ def test_result_interpret_all_cases(
         interpret: ResultInterpret,
         interpret_checks: dict[str, CheckResultInterpret],
         expected_outcome: ResultOutcome,
-        expected_note_contains: Optional[str]
+        expected_note_contains: list[str]
         ) -> None:
     """Test all possible combinations of result interpretations"""
     result = Result(
@@ -198,10 +197,11 @@ def test_result_interpret_all_cases(
     assert interpreted.result == expected_outcome
 
     if expected_note_contains:
-        assert interpreted.note is not None
-        assert expected_note_contains in interpreted.note
+        assert interpreted.note
+        for expected_note in expected_note_contains:
+            assert expected_note in interpreted.note
     else:
-        assert interpreted.note is None
+        assert not interpreted.note
 
 
 def test_result_interpret_check_phases() -> None:
@@ -238,10 +238,10 @@ def test_result_interpret_edge_cases() -> None:
     result = Result(name="test-case", result=ResultOutcome.FAIL)
     interpreted = result.interpret_result(ResultInterpret.RESPECT, {})
     assert interpreted.result == ResultOutcome.FAIL
-    assert interpreted.note is None
+    assert not interpreted.note
 
     # Test with empty check list
     result = Result(name="test-case", result=ResultOutcome.FAIL, check=[])
     interpreted = result.interpret_result(ResultInterpret.RESPECT, {})
     assert interpreted.result == ResultOutcome.FAIL
-    assert interpreted.note is None
+    assert not interpreted.note

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