diff --git a/docs/releases.rst b/docs/releases.rst index b051a1f667..16937b712c 100644 --- a/docs/releases.rst +++ b/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 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/spec/plans/results.fmf b/spec/plans/results.fmf index 732ae155c6..c1a4b044eb 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: @@ -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"] } ] diff --git a/tests/execute/restraint/tmt-abort/test.sh b/tests/execute/restraint/tmt-abort/test.sh index 3080efdb9c..a4ca7cff0b 100755 --- a/tests/execute/restraint/tmt-abort/test.sh +++ b/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 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/tests/report/html/test.sh b/tests/report/html/test.sh index 6600d7fad8..e0bcb1a4cc 100755 --- a/tests/report/html/test.sh +++ b/tests/report/html/test.sh @@ -37,13 +37,14 @@ rlJournalStart grep -B 1 "/test/$test_name_suffix" $HTML | tee $tmp/$test_name_suffix rlAssertGrep 'class="result error">error' $tmp/$test_name_suffix -F sed -e "/name\">\/test\/$test_name_suffix/,/\/tr/!d" $HTML | tee $tmp/$test_name_suffix-note - rlAssertGrep 'timeout' $tmp/$test_name_suffix-note -F + rlAssertGrep '
  • timeout
    • ' $tmp/$test_name_suffix-note -F test_name_suffix=xfail grep -B 1 "/test/$test_name_suffix" $HTML | tee $tmp/$test_name_suffix rlAssertGrep 'class="result pass">pass' $tmp/$test_name_suffix -F sed -e "/name\">\/test\/$test_name_suffix/,/\/tr/!d" $HTML | tee $tmp/$test_name_suffix-note - rlAssertGrep 'test failed as expected, original test result: fail' $tmp/$test_name_suffix-note -F + rlAssertGrep '
  • test failed as expected
    • ' $tmp/$test_name_suffix-note -F + rlAssertGrep '
  • original test result: fail
    • ' $tmp/$test_name_suffix-note -F rlPhaseEnd if [ "$option" = "" ]; then diff --git a/tests/unit/test_results.py b/tests/unit/test_results.py index 53540a8595..a0ddeb69aa 100644 --- a/tests/unit/test_results.py +++ b/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 diff --git a/tmt/frameworks/beakerlib.py b/tmt/frameworks/beakerlib.py index 62ce52f6c8..2b627461e3 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 @@ -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()) diff --git a/tmt/frameworks/shell.py b/tmt/frameworks/shell.py index d5042f3fe4..785f5735da 100644 --- a/tmt/frameworks/shell.py +++ b/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, diff --git a/tmt/result.py b/tmt/result.py index be495387b4..2b94cc42de 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -176,7 +176,9 @@ 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), + unserialize=lambda value: [] if value is None else value) log: list[Path] = field( default_factory=cast(Callable[[], list[Path]], list), serialize=lambda logs: [str(log) for log in logs], @@ -200,10 +202,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 +303,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 +347,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 +362,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 +384,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 +436,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 +449,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 +521,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 5176c1a93d..5d13b295a8 100644 --- a/tmt/schemas/common.yaml +++ b/tmt/schemas/common.yaml @@ -492,7 +492,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 103af4fced..f38263eeff 100644 --- a/tmt/steps/execute/__init__.py +++ b/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() diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index 92a5cd4aed..a7f2a52ec3 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -561,13 +561,14 @@ 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, ' - 'you may want to set restart-max-count larger') + result.note.append( + 'crashed too many times, ' + 'you may want to set restart-max-count larger') # Handle reboot, abort, exit-first if invocation.reboot_requested: @@ -580,12 +581,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) diff --git a/tmt/steps/report/html/template.html.j2 b/tmt/steps/report/html/template.html.j2 index bb65387806..82d2779753 100644 --- a/tmt/steps/report/html/template.html.j2 +++ b/tmt/steps/report/html/template.html.j2 @@ -117,6 +117,11 @@ width: 22ex; } + #results ul { + margin: 0px; + padding-inline-start: 16px; + } + .context-dimension { display: inline-block; text-align: center; @@ -266,7 +271,17 @@ Context: {% endfor %} data - {% if result.note %}{{ result.note | e }}{% else %}-{% endif %} + + {% if result.note %} + + {% else %} + - + {% endif %} + {% if result.check %}