From 60c2d37e1a38f839fff8a0b090c00329417e4491 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Fri, 25 Oct 2024 16:43:47 +0200 Subject: [PATCH 1/4] Implement check `result` key, checks affect test result by default (#3239) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Implement check result key feature * Add check-result spec and release note * Move and modify check schema * Fix doc generation Co-authored-by: Miloš Prchlík Co-authored-by: Petr Šplíchal --- docs/releases.rst | 8 ++ docs/scripts/generate-plugins.py | 10 +- docs/templates/plugins.rst.j2 | 10 +- spec/tests/check.fmf | 39 ++++++ tests/discover/tests.sh | 2 +- tests/execute/result/basic.sh | 23 ++-- tests/execute/result/check.sh | 29 ++++ tests/execute/result/check/.fmf/version | 1 + tests/execute/result/check/test.fmf | 89 ++++++++++++ tests/execute/result/main.fmf | 7 + tests/report/html/test.sh | 2 +- tests/test/check/test-avc.sh | 2 +- tests/test/check/test-dmesg.sh | 4 +- tests/unit/test_results.py | 171 +++++++++++++++++++++++- tmt/checks/__init__.py | 34 ++++- tmt/result.py | 142 ++++++++++++++++---- tmt/schemas/common.yaml | 12 -- tmt/schemas/test.yaml | 21 ++- tmt/steps/execute/internal.py | 11 +- 19 files changed, 550 insertions(+), 67 deletions(-) create mode 100755 tests/execute/result/check.sh create mode 100644 tests/execute/result/check/.fmf/version create mode 100644 tests/execute/result/check/test.fmf diff --git a/docs/releases.rst b/docs/releases.rst index e38921075b..ff05c0e467 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -12,6 +12,14 @@ A new :ref:`test-runner` section has been added to the tmt :ref:`guide`. It describes some important differences between running tests on a :ref:`user-system` and scheduling test jobs in +Test checks affect the overall test result by default. The +:ref:`/spec/tests/check` specification now supports a new +``result`` key for individual checks. This attribute allows users +to control how the result of each check affects the overall test +result. Please note that tests, which were previously passing +with failing checks will now fail by default, unless the ``xfail`` +or ``info`` is added. + Each execution of ``tmt-report-result`` command inside a shell test will now create a tmt subresult. The main result outcome is reduced from all subresults outcomes. If ``tmt-report-result`` is diff --git a/docs/scripts/generate-plugins.py b/docs/scripts/generate-plugins.py index f44b03a5f9..9368f7311d 100755 --- a/docs/scripts/generate-plugins.py +++ b/docs/scripts/generate-plugins.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 import dataclasses +import enum import sys import textwrap from typing import Any @@ -56,7 +57,7 @@ def _is_inherited( # TODO: for now, it's a list, but inspecting the actual tree of classes # would be more generic. It's good enough for now. - return field.name in ('name', 'where', 'order', 'summary', 'enabled') + return field.name in ('name', 'where', 'order', 'summary', 'enabled', 'result') def container_ignored_fields(container: ContainerClass) -> list[str]: @@ -106,6 +107,12 @@ def container_intrinsic_fields(container: ContainerClass) -> list[str]: return field_names +def is_enum(value: Any) -> bool: + """ Find out whether a given value is an enum member """ + + return isinstance(value, enum.Enum) + + def _create_step_plugin_iterator(registry: tmt.plugins.PluginRegistry[tmt.steps.Method]): """ Create iterator over plugins of a given registry """ @@ -184,6 +191,7 @@ def main() -> None: STEP=step_name, PLUGINS=plugin_generator, REVIEWED_PLUGINS=REVIEWED_PLUGINS, + is_enum=is_enum, container_fields=tmt.utils.container_fields, container_field=tmt.utils.container_field, container_ignored_fields=container_ignored_fields, diff --git a/docs/templates/plugins.rst.j2 b/docs/templates/plugins.rst.j2 index 74137aa787..995c9b4d09 100644 --- a/docs/templates/plugins.rst.j2 +++ b/docs/templates/plugins.rst.j2 @@ -7,6 +7,8 @@ {{ option }}: ``{{ metadata.metavar }}`` {% elif metadata.default is boolean %} {{ option }}: ``true|false`` + {% elif metadata.choices %} +{{ option }}: ``{{ metadata.choices }}`` {% else %} {{ option }}: {% endif %} @@ -27,11 +29,13 @@ {% elif actual_default is sequence %} {% if not actual_default %} Default: *not set* - {% else %} + {% else %} Default: {% for default_item in actual_default %}``{{ default_item.pattern | default(default_item) }}``{% if not loop.last %}, {% endif %}{% endfor %} {% endif %} + {% elif is_enum(actual_default) %} + Default: ``{{ actual_default.value }}`` {% else %} - {% set _ = LOGGER.warn("%s/%s.%s: could not render default value, '%s'" | format(STEP, plugin_id, field_name, actual_default)) %} + {% set _ = LOGGER.warn("%s/%s.%s: could not render default value, '%s'" | format(STEP, plugin_id, field_name, actual_default), shift=0) %} Default: *could not render default value correctly* {% endif %} {% endif %} @@ -71,7 +75,7 @@ The following keys are accepted by all plugins of the ``{{ STEP }}`` step. Please, be aware that the documentation below is a work in progress. We are working on fixing it, adding missing bits and generally making it better. Also, it was originally used for command line help only, therefore the - formatting is often suboptional. + formatting is often suboptimal. {% endif %} {% if PLUGIN.__doc__ %} diff --git a/spec/tests/check.fmf b/spec/tests/check.fmf index 102eb4771c..491b0510ed 100644 --- a/spec/tests/check.fmf +++ b/spec/tests/check.fmf @@ -14,6 +14,29 @@ description: | panic detection, core dump collection or collection of system logs. + By default, the check results affect the overall test outcome. + To change this behaviour, use the ``result`` key, which accepts + the following values: + + respect + The check result is respected and affects the overall + test result. This is the default. + + xfail + The check result is expected to fail (pass becomes + fail and vice-versa). + + info + The check result is treated as an informational + message and does not affect the overall test result. + + .. warning:: + + Note that running one check multiple times for the same + test is not yet supported. + + .. versionchanged:: 1.38.0 the ``result`` key added + See :ref:`/plugins/test-checks` for the list of available checks. example: @@ -37,7 +60,23 @@ example: - how: test-inspector enable: false + - | + # Expect the AVC check to fail + check: + - how: avc + result: xfail + - how: dmesg + result: respect + + - | + # Treat the dmesg check as informational only + check: + - how: dmesg + result: info + link: - implemented-by: /tmt/checks + - implemented-by: /tmt/result.py - verified-by: /tests/test/check/avc - verified-by: /tests/test/check/dmesg + - verified-by: /tests/execute/result/check diff --git a/tests/discover/tests.sh b/tests/discover/tests.sh index 9d766e6930..03ea940cef 100755 --- a/tests/discover/tests.sh +++ b/tests/discover/tests.sh @@ -14,7 +14,7 @@ rlJournalStart rlRun -s "tmt run --id $workdir -vvv $plan" rlAssertGrep "package: 1 package requested" "$rlRun_LOG" -F rlAssertGrep "test: Concise summary" "$rlRun_LOG" -F - rlAssertGrep '00:00:00 pass /first (on default-0) (original result: fail) [1/2]' "$rlRun_LOG" -F + rlAssertGrep '00:00:00 pass /first (on default-0) (test failed as expected, original test result: fail) [1/2]' "$rlRun_LOG" -F rlAssertGrep '00:00:00 pass /second (on default-0) [2/2]' "$rlRun_LOG" -F rlPhaseEnd diff --git a/tests/execute/result/basic.sh b/tests/execute/result/basic.sh index f3ea6e4fae..4704ecab87 100755 --- a/tests/execute/result/basic.sh +++ b/tests/execute/result/basic.sh @@ -15,7 +15,7 @@ run() if [ -z "${orig}" ]; then # No original result provided rlAssertGrep "${res} ${tn}$" $rlRun_LOG else - rlAssertGrep "${res} ${tn} (original result: ${orig})$" $rlRun_LOG + rlAssertGrep "${res} ${tn} (.*original test result: ${orig}.*)$" $rlRun_LOG fi echo @@ -38,7 +38,7 @@ rlJournalStart run "errr" "/test/error" "" 2 run "pass" "/test/xfail-fail" "fail" 0 run "fail" "/test/xfail-pass" "pass" 1 - run "errr" "/test/xfail-error" "error" 2 + run "errr" "/test/xfail-error" "" 2 run "pass" "/test/always-pass" "fail" 0 run "info" "/test/always-info" "pass" 0 run "warn" "/test/always-warn" "pass" 1 @@ -58,18 +58,18 @@ rlJournalStart rlAssertGrep "$line" "$rlRun_LOG" -F fi done <<-EOF -00:00:00 errr /test/always-error (on default-0) (original result: pass) [1/12] -00:00:00 fail /test/always-fail (on default-0) (original result: pass) [2/12] -00:00:00 info /test/always-info (on default-0) (original result: pass) [3/12] -00:00:00 pass /test/always-pass (on default-0) (original result: fail) [4/12] -00:00:00 warn /test/always-warn (on default-0) (original result: pass) [5/12] +00:00:00 errr /test/always-error (on default-0) (test result overridden: error, original test result: pass) [1/12] +00:00:00 fail /test/always-fail (on default-0) (test result overridden: fail, original test result: pass) [2/12] +00:00:00 info /test/always-info (on default-0) (test result overridden: info, original test result: pass) [3/12] +00:00:00 pass /test/always-pass (on default-0) (test result overridden: pass, original test result: fail) [4/12] +00:00:00 warn /test/always-warn (on default-0) (test result overridden: warn, original test result: pass) [5/12] 00:00:00 errr /test/error (on default-0) [6/12] 00:00:01 errr /test/error-timeout (on default-0) (timeout) [7/12] 00:00:00 fail /test/fail (on default-0) [8/12] 00:00:00 pass /test/pass (on default-0) [9/12] -00:00:00 errr /test/xfail-error (on default-0) (original result: error) [10/12] -00:00:00 pass /test/xfail-fail (on default-0) (original result: fail) [11/12] -00:00:00 fail /test/xfail-pass (on default-0) (original result: pass) [12/12] +00:00:00 errr /test/xfail-error (on default-0) [10/12] +00:00:00 pass /test/xfail-fail (on default-0) (test failed as expected, original test result: fail) [11/12] +00:00:00 fail /test/xfail-pass (on default-0) (test was expected to fail, original test result: pass) [12/12] EOF rlPhaseEnd @@ -78,7 +78,8 @@ EOF rlRun -s "tmt run --id \${run} --scratch --until execute tests -n /xfail-with-reboot provision --how container execute -v 2>&1 >/dev/null" EXPECTED=$(cat <" $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 'original result: fail' $tmp/$test_name_suffix-note -F + rlAssertGrep 'test failed as expected, original test result: fail' $tmp/$test_name_suffix-note -F rlPhaseEnd if [ "$option" = "" ]; then diff --git a/tests/test/check/test-avc.sh b/tests/test/check/test-avc.sh index a79663d57d..5eaacc20cc 100755 --- a/tests/test/check/test-avc.sh +++ b/tests/test/check/test-avc.sh @@ -18,7 +18,7 @@ rlJournalStart rlPhaseEnd rlPhaseStartTest "Run /avc tests with $PROVISION_HOW" - rlRun "tmt -c provision_method=$PROVISION_HOW run --id $run --scratch -a -vvv provision -h $PROVISION_HOW test -n /avc" + rlRun "tmt -c provision_method=$PROVISION_HOW run --id $run --scratch -a -vvv provision -h $PROVISION_HOW test -n /avc" "1" rlRun "cat $results" rlPhaseEnd diff --git a/tests/test/check/test-dmesg.sh b/tests/test/check/test-dmesg.sh index 0886d6473e..ff454bb6f3 100755 --- a/tests/test/check/test-dmesg.sh +++ b/tests/test/check/test-dmesg.sh @@ -65,7 +65,7 @@ rlJournalStart rlRun "dump_before=$segfault/checks/dmesg-before-test.txt" rlRun "dump_after=$segfault/checks/dmesg-after-test.txt" - rlRun "tmt run --id $run --scratch -a -vv provision -h $PROVISION_HOW test -n /dmesg/segfault" + rlRun "tmt run --id $run --scratch -a -vv provision -h $PROVISION_HOW test -n /dmesg/segfault" "1" rlRun "cat $results" assert_check_result "dmesg as a before-test should pass" "pass" "before-test" "segfault" @@ -84,7 +84,7 @@ rlJournalStart rlRun "dump_before=$custom_patterns/checks/dmesg-before-test.txt" rlRun "dump_after=$custom_patterns/checks/dmesg-after-test.txt" - rlRun "tmt run --id $run --scratch -a -vv provision -h $PROVISION_HOW test -n /dmesg/custom-patterns" + rlRun "tmt run --id $run --scratch -a -vv provision -h $PROVISION_HOW test -n /dmesg/custom-patterns" "1" rlRun "cat $results" assert_check_result "dmesg as a before-test should fail" "fail" "before-test" "custom-patterns" diff --git a/tests/unit/test_results.py b/tests/unit/test_results.py index 15e57f55ff..53540a8595 100644 --- a/tests/unit/test_results.py +++ b/tests/unit/test_results.py @@ -1,9 +1,18 @@ +from typing import Optional from unittest.mock import MagicMock import pytest +from tmt.checks import CheckEvent from tmt.cli import TmtExitCode -from tmt.result import ResultOutcome, results_to_exit_code +from tmt.result import ( + CheckResult, + CheckResultInterpret, + Result, + ResultInterpret, + ResultOutcome, + results_to_exit_code, + ) @pytest.mark.parametrize( @@ -76,3 +85,163 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code: int) -> None: assert results_to_exit_code([MagicMock(result=outcome) for outcome in outcomes]) \ == expected_exit_code + + +@pytest.mark.parametrize( + ('result_outcome', + 'interpret', + 'interpret_checks', + 'expected_outcome', + 'expected_note_contains'), + [ + # Test RESPECT interpretation + ( + ResultOutcome.PASS, + 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 + ), + + # Test XFAIL interpretation + ( + ResultOutcome.FAIL, + ResultInterpret.XFAIL, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.PASS, + "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 INFO interpretation + ( + ResultOutcome.FAIL, + ResultInterpret.INFO, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.INFO, + "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 WARN interpretation + ( + ResultOutcome.PASS, + ResultInterpret.WARN, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.WARN, + "test result overridden: warn, original test result: pass" + ), + + # Test ERROR interpretation + ( + ResultOutcome.PASS, + ResultInterpret.ERROR, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.ERROR, + "test result overridden: error, original test result: pass" + ), + + # Test CUSTOM interpretation (should not modify result) + ( + ResultOutcome.FAIL, + ResultInterpret.CUSTOM, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.FAIL, + None + ), + ], + ids=[ + "respect-pass", "respect-fail", + "xfail-fail", "xfail-pass", + "info-fail", "info-pass", + "warn-pass", + "error-pass", + "custom-fail" + ] + ) +def test_result_interpret_all_cases( + result_outcome: ResultOutcome, + interpret: ResultInterpret, + interpret_checks: dict[str, CheckResultInterpret], + expected_outcome: ResultOutcome, + expected_note_contains: Optional[str] + ) -> None: + """Test all possible combinations of result interpretations""" + result = Result( + name="test-case", + result=result_outcome, + check=[ + CheckResult(name="check1", result=result_outcome, event=CheckEvent.BEFORE_TEST) + ] + ) + + interpreted = result.interpret_result(interpret, interpret_checks) + assert interpreted.result == expected_outcome + + if expected_note_contains: + assert interpreted.note is not None + assert expected_note_contains in interpreted.note + else: + assert interpreted.note is None + + +def test_result_interpret_check_phases() -> None: + """Test the interpretation of check results with different phases""" + result = Result( + name="test-case", + check=[ + CheckResult(name="check1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), + CheckResult(name="check1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST), + CheckResult(name="check2", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST) + ] + ) + + # Test with mixed interpretations + interpret_checks = { + "check1": CheckResultInterpret.RESPECT, + "check2": CheckResultInterpret.INFO + } + + interpreted = result.interpret_result(ResultInterpret.RESPECT, interpret_checks) + assert interpreted.note is not None + assert "check 'check1' failed" in interpreted.note + assert "check 'check2' is informational" in interpreted.note + + # Verify individual check results were interpreted + assert interpreted.check[0].result == ResultOutcome.PASS # check1 BEFORE_TEST + assert interpreted.check[1].result == ResultOutcome.FAIL # check1 AFTER_TEST + assert interpreted.check[2].result == ResultOutcome.PASS # check2 BEFORE_TEST (INFO) + + +def test_result_interpret_edge_cases() -> None: + """Test edge cases in result interpretation""" + # Test with no checks + result = Result(name="test-case", result=ResultOutcome.FAIL) + interpreted = result.interpret_result(ResultInterpret.RESPECT, {}) + assert interpreted.result == ResultOutcome.FAIL + assert interpreted.note is None + + # 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 diff --git a/tmt/checks/__init__.py b/tmt/checks/__init__.py index 905c19f72f..0fcafb3e38 100644 --- a/tmt/checks/__init__.py +++ b/tmt/checks/__init__.py @@ -67,6 +67,7 @@ def find_plugin(name: str) -> 'CheckPluginClass': class _RawCheck(TypedDict): how: str enabled: bool + result: str class CheckEvent(enum.Enum): @@ -83,6 +84,22 @@ def from_spec(cls, spec: str) -> 'CheckEvent': raise tmt.utils.SpecificationError(f"Invalid test check event '{spec}'.") +class CheckResultInterpret(enum.Enum): + INFO = 'info' + RESPECT = 'respect' + XFAIL = 'xfail' + + @classmethod + def from_spec(cls, spec: str) -> 'CheckResultInterpret': + try: + return CheckResultInterpret(spec) + except ValueError as err: + raise ValueError(f"Invalid check result interpretation '{spec}'.") from err + + def to_spec(self) -> str: + return self.value + + @dataclasses.dataclass class Check( SpecBasedContainer[_RawCheck, _RawCheck], @@ -100,6 +117,12 @@ class Check( default=True, is_flag=True, help='Whether the check is enabled or not.') + result: CheckResultInterpret = field( + default=CheckResultInterpret.RESPECT, + help='How to interpret the check result.', + serialize=lambda result: result.value, + unserialize=CheckResultInterpret.from_spec, + choices=[value.value for value in CheckResultInterpret.__members__.values()]) @functools.cached_property def plugin(self) -> 'CheckPluginClass': @@ -113,14 +136,21 @@ def from_spec( # type: ignore[override] logger: tmt.log.Logger) -> 'Check': data = cls(how=raw_data['how']) data._load_keys(cast(dict[str, Any], raw_data), cls.__name__, logger) + if raw_data.get("result"): + data.result = CheckResultInterpret.from_spec(raw_data["result"]) return data def to_spec(self) -> _RawCheck: - return cast(_RawCheck, { + spec = cast(_RawCheck, { tmt.utils.key_to_option(key): value for key, value in self.items() }) + spec["result"] = self.result.to_spec() + return spec + + def to_minimal_spec(self) -> _RawCheck: + return self.to_spec() def go( self, @@ -228,7 +258,7 @@ def normalize_test_check( if isinstance(raw_test_check, str): try: return CheckPlugin.delegate( - raw_data={'how': raw_test_check, 'enabled': True}, + raw_data={'how': raw_test_check, 'enabled': True, 'result': 'respect'}, logger=logger) except Exception as exc: diff --git a/tmt/result.py b/tmt/result.py index 1a451202aa..be495387b4 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -10,7 +10,7 @@ import tmt.identifier import tmt.log import tmt.utils -from tmt.checks import CheckEvent +from tmt.checks import CheckEvent, CheckResultInterpret from tmt.utils import GeneralError, Path, SerializableContainer, field if TYPE_CHECKING: @@ -37,6 +37,31 @@ def from_spec(cls, spec: str) -> 'ResultOutcome': except ValueError: raise tmt.utils.SpecificationError(f"Invalid partial custom result '{spec}'.") + @staticmethod + def reduce(outcomes: list['ResultOutcome']) -> 'ResultOutcome': + """ + Reduce several result outcomes into a single outcome + + Convert multiple outcomes into a single one by picking the + worst. This is used when aggregating several test or check + results to present a single value to the user. + """ + + outcomes_by_severity = ( + ResultOutcome.ERROR, + ResultOutcome.FAIL, + ResultOutcome.WARN, + ResultOutcome.PASS, + ResultOutcome.INFO, + ResultOutcome.SKIP, + ) + + for outcome in outcomes_by_severity: + if outcome in outcomes: + return outcome + + raise GeneralError("No result outcome found to reduce.") + # Cannot subclass enums :/ # https://docs.python.org/3/library/enum.html#restricted-enum-subclassing @@ -324,11 +349,65 @@ def from_test_invocation( log=log or [], guest=ResultGuestData.from_test_invocation(invocation=invocation), data_path=invocation.relative_test_data_path, - subresult=subresult or []) + subresult=subresult or [], + check=invocation.check_results or []) + + interpret_checks = {check.how: check.result for check in invocation.test.check} + + 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, + interpret_checks: dict[str, CheckResultInterpret]) -> ResultOutcome: + """ + Aggregate all checks of given name and interpret the outcome + + :param check_name: name of the check to be aggregated + :param interpret_checks: mapping of check:how and its result interpret + :returns: :py:class:`ResultOutcome` instance with the interpreted result + """ + + # Reduce all check outcomes into a single worst outcome + reduced_outcome = ResultOutcome.reduce( + [check.result for check in self.check if check.name == check_name]) + + # Now let's handle the interpretation + interpret = interpret_checks[check_name] + interpreted_outcome = reduced_outcome + + if interpret == CheckResultInterpret.RESPECT: + if interpreted_outcome == ResultOutcome.FAIL: + self.append_note(f"check '{check_name}' failed") - return _result.interpret_result(invocation.test.result) + elif interpret == CheckResultInterpret.INFO: + interpreted_outcome = ResultOutcome.INFO + self.append_note(f"check '{check_name}' is informational") - def interpret_result(self, interpret: ResultInterpret) -> 'Result': + 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") + + if reduced_outcome == ResultOutcome.FAIL: + interpreted_outcome = ResultOutcome.PASS + self.append_note(f"check '{check_name}' failed as expected") + + return interpreted_outcome + + def interpret_result( + self, + interpret: ResultInterpret, + interpret_checks: dict[str, CheckResultInterpret] + ) -> 'Result': """ Interpret result according to a given interpretation instruction. @@ -336,37 +415,50 @@ def interpret_result(self, interpret: ResultInterpret) -> 'Result': attributes, following the ``interpret`` value. :param interpret: how to interpret current result. + :param interpret_checks: mapping of check:how and its result interpret :returns: :py:class:`Result` instance containing the updated result. """ - if interpret in (ResultInterpret.RESPECT, ResultInterpret.CUSTOM): + if interpret not in ResultInterpret: + raise tmt.utils.SpecificationError( + f"Invalid result '{interpret.value}' in test '{self.name}'.") + + if interpret == ResultInterpret.CUSTOM: return self - # Extend existing note or set a new one - if self.note: - self.note += f', original result: {self.result.value}' + # Interpret check results (aggregated by the check name) + check_outcomes: list[ResultOutcome] = [] + for check_name in tmt.utils.uniq([check.name for check in self.check]): + check_outcomes.append(self.interpret_check_result(check_name, interpret_checks)) - elif self.note is None: - self.note = f'original result: {self.result.value}' + # Aggregate check results with the main test result + self.result = ResultOutcome.reduce([self.result, *check_outcomes]) - else: - raise tmt.utils.SpecificationError( - f"Test result note '{self.note}' must be a string.") + # 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}") + + # 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}") + return self + + # Handle the expected fail if interpret == ResultInterpret.XFAIL: - # Swap just fail<-->pass, keep the rest as is (info, warn, - # error) - self.result = { - ResultOutcome.FAIL: ResultOutcome.PASS, - ResultOutcome.PASS: ResultOutcome.FAIL - }.get(self.result, self.result) - - elif ResultInterpret.is_result_outcome(interpret): - self.result = ResultOutcome(interpret.value) - else: - raise tmt.utils.SpecificationError( - f"Invalid result '{interpret.value}' in test '{self.name}'.") + if self.result == ResultOutcome.PASS: + self.result = ResultOutcome.FAIL + self.append_note("test was expected to fail") + + elif self.result == ResultOutcome.FAIL: + self.result = ResultOutcome.PASS + self.append_note("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}") return self diff --git a/tmt/schemas/common.yaml b/tmt/schemas/common.yaml index 0a160b4867..8c38b8313d 100644 --- a/tmt/schemas/common.yaml +++ b/tmt/schemas/common.yaml @@ -493,15 +493,3 @@ definitions: type: string # yamllint disable-line rule:line-length pattern: "^\\d{2,}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$" - - check: - type: object - properties: - how: - type: string - - enabled: - type: boolean - - required: - - how diff --git a/tmt/schemas/test.yaml b/tmt/schemas/test.yaml index d64923ee5f..1e265bbded 100644 --- a/tmt/schemas/test.yaml +++ b/tmt/schemas/test.yaml @@ -54,12 +54,12 @@ properties: check: anyOf: - type: string - - $ref: "/schemas/common#/definitions/check" + - $ref: "#/definitions/check" - type: array items: anyOf: - type: string - - $ref: "/schemas/common#/definitions/check" + - $ref: "#/definitions/check" # https://tmt.readthedocs.io/en/stable/spec/core.html#id id: @@ -145,3 +145,20 @@ patternProperties: required: - test + +definitions: + check: + type: object + properties: + how: + type: string + enabled: + type: boolean + result: + type: string + enum: + - respect + - xfail + - info + required: + - how diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index 7f8332fe20..3ca34a76d2 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -454,17 +454,18 @@ def _save_process( # losing logs if the guest becomes later unresponsive. guest.pull(source=self.step.plan.data_directory) - # Extract test results and store them in the invocation. Note - # that these results will be overwritten with a fresh set of - # results after a successful reboot in the middle of a test. - invocation.results = self.extract_results(invocation, logger) - + # Run after-test checks before extracting results invocation.check_results += self.run_checks_after_test( invocation=invocation, environment=environment, logger=logger ) + # Extract test results and store them in the invocation. Note + # that these results will be overwritten with a fresh set of + # results after a successful reboot in the middle of a test. + invocation.results = self.extract_results(invocation, logger) + if invocation.is_guest_healthy: # Fetch #2: after-test checks might have produced remote files as well, # we need to fetch them too. From cc75eb6bf245b3bcba1da68732f252b95a3f0970 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Fri, 25 Oct 2024 11:59:19 +0200 Subject: [PATCH 2/4] Release 1.38.0 - Added Luigi and Otto to the contributors - Reordered release notes according to this key: - New and possibly breaking features - New features - Bug fixes - Documentation updates and others Signed-off-by: Miroslav Vadkerti --- docs/overview.rst | 2 +- docs/releases.rst | 62 +++++++++++++++++++++++------------------------ 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/docs/overview.rst b/docs/overview.rst index b878270fe5..5917d09188 100644 --- a/docs/overview.rst +++ b/docs/overview.rst @@ -777,7 +777,7 @@ James Molet, Cristian Le, Lili Nie, Martin Čermák, Michael Vogt, Qinghua Cheng, Michael Engel, Anatoli Babenia, Colin Walters, Link Dupont, Mario Casquero, Martin Klusoň, Pavel Holica, Otto Šabart, Ismail Ibrahim Quwarah, Sergei Petrosian, Tom -Koscielniak and Han Han. +Koscielniak, Han Han and Luigi Pellecchia. Copyright diff --git a/docs/releases.rst b/docs/releases.rst index ff05c0e467..c604a106e9 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -8,10 +8,6 @@ tmt-1.38.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -A new :ref:`test-runner` section has been added to the tmt -:ref:`guide`. It describes some important differences between -running tests on a :ref:`user-system` and scheduling test jobs in - Test checks affect the overall test result by default. The :ref:`/spec/tests/check` specification now supports a new ``result`` key for individual checks. This attribute allows users @@ -20,13 +16,12 @@ result. Please note that tests, which were previously passing with failing checks will now fail by default, unless the ``xfail`` or ``info`` is added. -Each execution of ``tmt-report-result`` command inside a shell -test will now create a tmt subresult. The main result outcome is -reduced from all subresults outcomes. If ``tmt-report-result`` is -not called during the test, the shell test framework behavior -remains the same - the test script exit code still has an impact -on the main test result. See also -:ref:`/stories/features/report-result`. +In order to prevent dangerous commands to be unintentionally run +on user's system, the :ref:`/plugins/provision/local` provision +plugin now requires to be executed with the ``--feeling-safe`` +option or with the environment variable ``TMT_FEELING_SAFE`` set +to ``True``. See the :ref:`/stories/features/feeling-safe` section +for more details and motivation behind this change. The beakerlib test framework tests now generate tmt subresults. The behavior is very similar to the shell test framework with @@ -43,26 +38,13 @@ enabled. There is only one exception - if the ``result: restraint`` option is set to a beakerlib test, the phase subresults get converted as normal tmt custom results. -The :ref:`/plugins/report/junit` report plugin now removes all -invalid XML characters from the final JUnit XML. - -In order to prevent dangerous commands to be unintentionally run -on user's system, the :ref:`/plugins/provision/local` provision -plugin now requires to be executed with the ``--feeling-safe`` -option or with the environment variable ``TMT_FEELING_SAFE`` set -to ``True``. See the :ref:`/stories/features/feeling-safe` section -for more details and motivation behind this change. - -The :ref:`/plugins/discover/fmf` discover plugin now supports -a new ``adjust-tests`` key which allows modifying metadata of all -discovered tests. This can be useful especially when fetching -tests from remote repositories where the user does not have write -access. - -A race condition in the -:ref:`/plugins/provision/virtual.testcloud` plugin has been fixed, -thus multihost tests using this provision method should now work -reliably without unexpected connection failures. +Each execution of ``tmt-report-result`` command inside a shell +test will now create a tmt subresult. The main result outcome is +reduced from all subresults outcomes. If ``tmt-report-result`` is +not called during the test, the shell test framework behavior +remains the same - the test script exit code still has an impact +on the main test result. See also +:ref:`/stories/features/report-result`. Support for RHEL-like operating systems in `Image Mode`__ has been added. The destination directory of the scripts added by ``tmt`` @@ -71,6 +53,12 @@ all others the ``/usr/local/bin`` destination directory is used. A new environment variable ``TMT_SCRIPTS_DIR`` is available to override the default locations. +The :ref:`/plugins/discover/fmf` discover plugin now supports +a new ``adjust-tests`` key which allows modifying metadata of all +discovered tests. This can be useful especially when fetching +tests from remote repositories where the user does not have write +access. + __ https://www.redhat.com/en/technologies/linux-platforms/enterprise-linux/image-mode The ``tmt link`` command now supports providing multiple links by @@ -80,6 +68,18 @@ for example usage. The :ref:`/plugins/provision/beaker` provision plugin gains support for :ref:`cpu.stepping` hardware requirement. +The :ref:`/plugins/report/junit` report plugin now removes all +invalid XML characters from the final JUnit XML. + +A new :ref:`test-runner` section has been added to the tmt +:ref:`guide`. It describes some important differences between +running tests on a :ref:`user-system` and scheduling test jobs in + +A race condition in the +:ref:`/plugins/provision/virtual.testcloud` plugin has been fixed, +thus multihost tests using this provision method should now work +reliably without unexpected connection failures. + tmt-1.37.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 45863cf64393635cfe38b7e796d5f369c01748d2 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Fri, 25 Oct 2024 19:08:47 +0200 Subject: [PATCH 3/4] Remove xfail for feeling safe Because 1.38 is out Signed-off-by: Miroslav Vadkerti --- tests/core/feeling-safe/main.fmf | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/core/feeling-safe/main.fmf b/tests/core/feeling-safe/main.fmf index de75be0ff6..95384cf5e8 100644 --- a/tests/core/feeling-safe/main.fmf +++ b/tests/core/feeling-safe/main.fmf @@ -2,6 +2,3 @@ summary: Check whether the safe/unsafe mode can be controlled link: - verifies: /stories/features/feeling-safe - -# Until tmt-1.38 is released this will be failing -result: xfail From ce2c8bea6fbecf3b64e1efb7be26b7df385ea625 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Mon, 28 Oct 2024 10:59:51 +0100 Subject: [PATCH 4/4] Fix missing Testing Farm link in release notes (#3320) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some bad rebase caused the sentence to be not finished. Not sure where that happened, but let's fix it :) Signed-off-by: Miroslav Vadkerti Co-authored-by: Petr Šplíchal --- docs/releases.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/releases.rst b/docs/releases.rst index c604a106e9..8f28b6979a 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -74,6 +74,7 @@ invalid XML characters from the final JUnit XML. A new :ref:`test-runner` section has been added to the tmt :ref:`guide`. It describes some important differences between running tests on a :ref:`user-system` and scheduling test jobs in +:ref:`testing-farm`. A race condition in the :ref:`/plugins/provision/virtual.testcloud` plugin has been fixed,