From c872c2d51be073d9a8eef4b6c86e9c159583d8bb Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 9 Oct 2024 19:29:38 +0200 Subject: [PATCH 01/35] Implement check result key feature --- tests/execute/result/check_results.sh | 61 +++++++++++++++ tests/execute/result/check_results/test.fmf | 83 +++++++++++++++++++++ tests/execute/result/main.fmf | 3 + tmt/checks/__init__.py | 22 +++++- tmt/result.py | 75 +++++++++++++------ tmt/steps/execute/internal.py | 11 +-- 6 files changed, 225 insertions(+), 30 deletions(-) create mode 100755 tests/execute/result/check_results.sh create mode 100644 tests/execute/result/check_results/test.fmf diff --git a/tests/execute/result/check_results.sh b/tests/execute/result/check_results.sh new file mode 100755 index 0000000000..1df66f4dda --- /dev/null +++ b/tests/execute/result/check_results.sh @@ -0,0 +1,61 @@ +#!/bin/bash +. /usr/share/beakerlib/beakerlib.sh || exit 1 + +rlJournalStart + rlPhaseStartSetup + rlRun "tmp=\$(mktemp -d)" 0 "Creating tmp directory" + rlRun "pushd $tmp" + rlRun "set -o pipefail" + rlPhaseEnd + + rlPhaseStartTest "Test with passing checks" + rlRun -s "tmt run -vvv test --name /test/check-pass" 0 + rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG + rlAssertGrep "pass dmesg (after-test check)" $rlRun_LOG + rlPhaseEnd + + rlPhaseStartTest "Test with failing dmesg check (respect)" + rlRun -s "tmt run -vvv test --name /test/check-fail-respect" 2 + rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG + rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG + rlPhaseEnd + + rlPhaseStartTest "Test with failing dmesg check (info)" + rlRun -s "tmt run -vvv test --name /test/check-fail-info" 0 + rlAssertGrep "info dmesg (before-test check)" $rlRun_LOG + rlAssertGrep "info dmesg (after-test check)" $rlRun_LOG + rlPhaseEnd + + rlPhaseStartTest "Test with passing dmesg check (xfail)" + rlRun -s "tmt run -vvv test --name /test/check-xfail-pass" 2 + rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG + rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG + rlPhaseEnd + + rlPhaseStartTest "Test with failing dmesg check (xfail)" + rlRun -s "tmt run -vvv test --name /test/check-xfail-fail" 0 + rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG + rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG + rlPhaseEnd + + rlPhaseStartTest "Test with multiple checks with different result interpretations" + rlRun -s "tmt run -vvv test --name /test/check-multiple" 2 + rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG + rlAssertGrep "fail dmesg (before-test check)" $rlRun_LOG + rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG + rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG + rlAssertGrep "info dmesg (after-test check)" $rlRun_LOG + rlAssertGrep "info dmesg (after-test check)" $rlRun_LOG + rlPhaseEnd + + rlPhaseStartTest "Test with failing dmesg check but overridden by test result" + rlRun -s "tmt run -vvv test --name /test/check-override" 0 + rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG + rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG + rlPhaseEnd + + rlPhaseStartCleanup + rlRun "popd" + rlRun "rm -r $tmp" 0 "Removing tmp directory" + rlPhaseEnd +rlJournalEnd diff --git a/tests/execute/result/check_results/test.fmf b/tests/execute/result/check_results/test.fmf new file mode 100644 index 0000000000..1ca7081503 --- /dev/null +++ b/tests/execute/result/check_results/test.fmf @@ -0,0 +1,83 @@ +summary: Tests for check results behaviour +description: Verify that check results are correctly interpreted and affect test results + +/test/check-pass: + summary: Test with passing checks + test: echo "Test passed" + framework: shell + duration: 1m + check: + - how: dmesg + result: respect + # Expected outcome: PASS (test passes, check passes) + +/test/check-fail-respect: + summary: Test with failing dmesg check (respect) + test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + framework: shell + duration: 1m + check: + - how: dmesg + failure-pattern: Fail Test Check Pattern + result: respect + # Expected outcome: FAIL (test passes, but check fails and is respected) + +/test/check-fail-info: + summary: Test with failing dmesg check (info) + test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + framework: shell + duration: 1m + check: + - how: dmesg + failure-pattern: Fail Test Check Pattern + result: info + # Expected outcome: PASS (test passes, check fails, but should be just info) + +/test/check-xfail-pass: + summary: Test with passing dmesg check (xfail) + test: echo "Test passed" + framework: shell + duration: 1m + check: + - how: dmesg + result: xfail + # Expected outcome: FAIL (test passes, check passes but xfail expects it to fail) + +/test/check-xfail-fail: + summary: Test with failing dmesg check (xfail) + test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + framework: shell + duration: 1m + check: + - how: dmesg + failure-pattern: Fail Test Check Pattern + result: xfail + # Expected outcome: PASS (test passes, check fails but xfail expects it to fail) + +/test/check-multiple: + summary: Test with multiple checks with different result interpretations + test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + framework: shell + duration: 1m + check: + - how: dmesg + failure-pattern: Fail Test Check Pattern + result: respect + - how: dmesg + result: xfail + - how: dmesg + failure-pattern: Fail Test Check Pattern + result: info + # Expected outcome: FAIL (first dmesg check fails and is respected, second dmesg check passes but xfail expects it to fail, third failing dmesg check is just info) + +/test/check-override: + summary: Test with failing dmesg check but overridden by test result + test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + framework: shell + duration: 1m + result: pass + check: + - how: dmesg + failure-pattern: Fail Test Check Pattern + result: respect + # Expected outcome: PASS (test passes, check fails but is overridden by 'result: pass') diff --git a/tests/execute/result/main.fmf b/tests/execute/result/main.fmf index 85b9755478..873a841765 100644 --- a/tests/execute/result/main.fmf +++ b/tests/execute/result/main.fmf @@ -13,3 +13,6 @@ /subresults: summary: Multiple calls to tmt-report-result should generate tmt subresults test: ./subresults.sh +/check_results_interpret: + summary: Test check result interpretations + test: ./check_results.sh diff --git a/tmt/checks/__init__.py b/tmt/checks/__init__.py index 905c19f72f..e8c0360714 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,19 @@ 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: + raise ValueError(f"Invalid check result interpretation '{spec}'.") + + @dataclasses.dataclass class Check( SpecBasedContainer[_RawCheck, _RawCheck], @@ -100,6 +114,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 + ) @functools.cached_property def plugin(self) -> 'CheckPluginClass': @@ -228,7 +248,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..08e3b42cb3 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: @@ -189,6 +189,20 @@ class CheckResult(BaseResult): serialize=lambda event: event.value, unserialize=CheckEvent.from_spec) + def interpret_check_result(self, interpret: CheckResultInterpret) -> 'CheckResult': + """Interpret check result according to the check_result interpretation.""" + + if interpret == CheckResultInterpret.RESPECT: + return self + if interpret == CheckResultInterpret.INFO: + self.result = ResultOutcome.INFO + elif interpret == CheckResultInterpret.XFAIL: + self.result = { + ResultOutcome.FAIL: ResultOutcome.PASS, + ResultOutcome.PASS: ResultOutcome.FAIL + }.get(self.result, self.result) + return self + def to_subcheck(self) -> 'SubCheckResult': """ Convert check to a tmt SubCheckResult """ @@ -324,11 +338,18 @@ 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) + return _result.interpret_result(invocation.test.result, interpret_checks) - def interpret_result(self, interpret: ResultInterpret) -> 'Result': + def interpret_result( + self, + interpret: ResultInterpret, + interpret_checks: dict[str, CheckResultInterpret] + ) -> 'Result': """ Interpret result according to a given interpretation instruction. @@ -339,35 +360,41 @@ def interpret_result(self, interpret: ResultInterpret) -> 'Result': :returns: :py:class:`Result` instance containing the updated result. """ - if interpret in (ResultInterpret.RESPECT, ResultInterpret.CUSTOM): - return self + if interpret not in (ResultInterpret): + raise tmt.utils.SpecificationError( + f"Invalid result '{interpret.value}' in test '{self.name}'." + ) - # Extend existing note or set a new one - if self.note: - self.note += f', original result: {self.result.value}' + if interpret in (ResultInterpret.CUSTOM, ResultInterpret.RESTRAINT): + # Interpret check results first, in case there is "info" + self.check = [ + check_result.interpret_check_result(interpret_checks[check_result.name]) + for check_result in self.check + ] - elif self.note is None: - self.note = f'original result: {self.result.value}' + return self - else: - raise tmt.utils.SpecificationError( - f"Test result note '{self.note}' must be a string.") + # Keeping "hardcoded" PASS, FAIL, ERROR... + if interpret not in (ResultInterpret.XFAIL, ResultInterpret.RESPECT): + return self + + failed_checks = [ + check_result + for check_result in self.check + if check_result.result == ResultOutcome.FAIL + ] + if failed_checks: + self.result = ResultOutcome.FAIL + check_note = ", ".join([f"Check '{check.name}' failed" for check in failed_checks]) + self.note = f"{self.note}, {check_note}" if self.note else check_note if interpret == ResultInterpret.XFAIL: - # Swap just fail<-->pass, keep the rest as is (info, warn, - # error) + # Swap fail<-->pass self.result = { ResultOutcome.FAIL: ResultOutcome.PASS, - ResultOutcome.PASS: ResultOutcome.FAIL + 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}'.") - return self def to_subresult(self) -> 'SubResult': 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 897e01e115454fb72328d31dce475d78dd1baf0f Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 16 Oct 2024 13:03:03 +0200 Subject: [PATCH 02/35] Add check-result spec and release note --- docs/releases.rst | 18 ++++++++++++++- spec/tests/check-result.fmf | 45 +++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 spec/tests/check-result.fmf diff --git a/docs/releases.rst b/docs/releases.rst index e38921075b..dce55f4f17 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -4,7 +4,6 @@ Releases ====================== - tmt-1.38.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -73,6 +72,23 @@ The :ref:`/plugins/provision/beaker` provision plugin gains support for :ref:`cpu.stepping` hardware requirement. +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. The following values +are supported: + +respect (default) + The check result is respected and affects the overall test result. +xfail + The check result is expected to fail (pass becomes fail and vice-versa). +info + The check result is always treated as "INFO" and does not affect 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. + + tmt-1.37.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/spec/tests/check-result.fmf b/spec/tests/check-result.fmf new file mode 100644 index 0000000000..c410c71053 --- /dev/null +++ b/spec/tests/check-result.fmf @@ -0,0 +1,45 @@ +summary: Specify how check result should be interpreted + +story: + As a tester I want to control how the result of individual checks + affects the overall test result, allowing for expected failures + or informational checks. + +description: | + Checks can have different impacts on the overall test result. + The 'result' attribute allows testers to specify how each check's + result should be interpreted. The following values are supported: + + respect + check result is respected and affects the overall test result + (default value) + xfail + expected fail (pass when check fails, fail when check passes) + info + check result is always treated as "INFO" and does not affect + the overall test result + + .. versionadded:: 1.38.0 + +example: + - | + # Use the default behavior (respect) + check: + - how: avc + + - | + # Expect the AVC check to fail + check: + - how: avc + result: xfail + + - | + # Treat the dmesg check as informational only + check: + - how: dmesg + result: info + +link: + - implemented-by: /tmt/checks/__init__.py + - implemented-by: /tmt/result.py + - verified-by: /tests/execute/result/check_results From f75ec1cc6e631b97627f29bfeae206c16643254d Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 16 Oct 2024 13:03:52 +0200 Subject: [PATCH 03/35] Move and modify check schema --- tmt/schemas/common.yaml | 12 ------------ tmt/schemas/test.yaml | 21 +++++++++++++++++++-- 2 files changed, 19 insertions(+), 14 deletions(-) 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 From bd7aac88e95f160a72aab19a252ffd6ee453fed3 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 16 Oct 2024 15:41:33 +0200 Subject: [PATCH 04/35] Fix doc generation --- docs/scripts/generate-plugins.py | 2 +- docs/templates/plugins.rst.j2 | 4 +- spec/tests/check-result.fmf | 45 ------------------- spec/tests/check.fmf | 23 ++++++++++ .../execute/result/check_results/.fmf/version | 1 + tests/execute/result/main.fmf | 2 +- 6 files changed, 29 insertions(+), 48 deletions(-) delete mode 100644 spec/tests/check-result.fmf create mode 100644 tests/execute/result/check_results/.fmf/version diff --git a/docs/scripts/generate-plugins.py b/docs/scripts/generate-plugins.py index f44b03a5f9..a37d83c13f 100755 --- a/docs/scripts/generate-plugins.py +++ b/docs/scripts/generate-plugins.py @@ -56,7 +56,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]: diff --git a/docs/templates/plugins.rst.j2 b/docs/templates/plugins.rst.j2 index 74137aa787..5b99149ca8 100644 --- a/docs/templates/plugins.rst.j2 +++ b/docs/templates/plugins.rst.j2 @@ -30,8 +30,10 @@ {% else %} Default: {% for default_item in actual_default %}``{{ default_item.pattern | default(default_item) }}``{% if not loop.last %}, {% endif %}{% endfor %} {% endif %} + {% elif actual_default.__class__.__name__ == 'CheckResultInterpret' %} + 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 %} diff --git a/spec/tests/check-result.fmf b/spec/tests/check-result.fmf deleted file mode 100644 index c410c71053..0000000000 --- a/spec/tests/check-result.fmf +++ /dev/null @@ -1,45 +0,0 @@ -summary: Specify how check result should be interpreted - -story: - As a tester I want to control how the result of individual checks - affects the overall test result, allowing for expected failures - or informational checks. - -description: | - Checks can have different impacts on the overall test result. - The 'result' attribute allows testers to specify how each check's - result should be interpreted. The following values are supported: - - respect - check result is respected and affects the overall test result - (default value) - xfail - expected fail (pass when check fails, fail when check passes) - info - check result is always treated as "INFO" and does not affect - the overall test result - - .. versionadded:: 1.38.0 - -example: - - | - # Use the default behavior (respect) - check: - - how: avc - - - | - # Expect the AVC check to fail - check: - - how: avc - result: xfail - - - | - # Treat the dmesg check as informational only - check: - - how: dmesg - result: info - -link: - - implemented-by: /tmt/checks/__init__.py - - implemented-by: /tmt/result.py - - verified-by: /tests/execute/result/check_results diff --git a/spec/tests/check.fmf b/spec/tests/check.fmf index 102eb4771c..1721bb17d3 100644 --- a/spec/tests/check.fmf +++ b/spec/tests/check.fmf @@ -13,6 +13,13 @@ description: | the user can run into. Another useful checks would be kernel 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 apart from + the default ``respect`` can be changed to ``xfail``, which + expects the check to fail, or ``info``, which changes the check + result to 'INFO'. + + .. versionchanged:: 1.38.0 ``result`` key added See :ref:`/plugins/test-checks` for the list of available checks. @@ -37,7 +44,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_results diff --git a/tests/execute/result/check_results/.fmf/version b/tests/execute/result/check_results/.fmf/version new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/execute/result/check_results/.fmf/version @@ -0,0 +1 @@ +1 diff --git a/tests/execute/result/main.fmf b/tests/execute/result/main.fmf index 873a841765..ac2605ee71 100644 --- a/tests/execute/result/main.fmf +++ b/tests/execute/result/main.fmf @@ -13,6 +13,6 @@ /subresults: summary: Multiple calls to tmt-report-result should generate tmt subresults test: ./subresults.sh -/check_results_interpret: +/check_results: summary: Test check result interpretations test: ./check_results.sh From 855cd0ed15a5b42639e005c1f5ed4c08a40bd114 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Wed, 16 Oct 2024 16:52:05 +0200 Subject: [PATCH 05/35] Add is_enum to plugins doc generate script --- docs/scripts/generate-plugins.py | 8 ++++++++ docs/templates/plugins.rst.j2 | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/scripts/generate-plugins.py b/docs/scripts/generate-plugins.py index a37d83c13f..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 @@ -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 5b99149ca8..5969cc4cbd 100644 --- a/docs/templates/plugins.rst.j2 +++ b/docs/templates/plugins.rst.j2 @@ -30,7 +30,7 @@ {% else %} Default: {% for default_item in actual_default %}``{{ default_item.pattern | default(default_item) }}``{% if not loop.last %}, {% endif %}{% endfor %} {% endif %} - {% elif actual_default.__class__.__name__ == 'CheckResultInterpret' %} + {% 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), shift=0) %} From 8207ca6183392937f74cfdf79575873c86ea1009 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Fri, 18 Oct 2024 18:04:03 +0200 Subject: [PATCH 06/35] Fix result interpretation, notes, tests --- tests/execute/result/basic.sh | 4 +- tests/execute/result/check_results.sh | 78 +++++++++------------ tests/execute/result/check_results/test.fmf | 43 ++++++------ tmt/result.py | 35 ++++++--- 4 files changed, 83 insertions(+), 77 deletions(-) diff --git a/tests/execute/result/basic.sh b/tests/execute/result/basic.sh index f3ea6e4fae..dbf636ad55 100755 --- a/tests/execute/result/basic.sh +++ b/tests/execute/result/basic.sh @@ -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 @@ -67,7 +67,7 @@ rlJournalStart 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 errr /test/xfail-error (on default-0) [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] EOF diff --git a/tests/execute/result/check_results.sh b/tests/execute/result/check_results.sh index 1df66f4dda..6ea418a935 100755 --- a/tests/execute/result/check_results.sh +++ b/tests/execute/result/check_results.sh @@ -1,61 +1,53 @@ #!/bin/bash . /usr/share/beakerlib/beakerlib.sh || exit 1 -rlJournalStart - rlPhaseStartSetup - rlRun "tmp=\$(mktemp -d)" 0 "Creating tmp directory" - rlRun "pushd $tmp" - rlRun "set -o pipefail" - rlPhaseEnd +run() +{ + res=$1 # expected final result of test + tn=$2 # test name + ret=$3 # tmt return code - rlPhaseStartTest "Test with passing checks" - rlRun -s "tmt run -vvv test --name /test/check-pass" 0 - rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG - rlAssertGrep "pass dmesg (after-test check)" $rlRun_LOG - rlPhaseEnd + rlRun -s "tmt run -a --scratch --id \${run} test --name ${tn} provision --how local report -v 2>&1 >/dev/null | grep report -A2 | tail -n 1" \ + ${ret} "Result: ${res}, Test name: ${tn}, tmt return code: ${ret}" - rlPhaseStartTest "Test with failing dmesg check (respect)" - rlRun -s "tmt run -vvv test --name /test/check-fail-respect" 2 - rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG - rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG - rlPhaseEnd + rlAssertGrep "${res} ${tn}" $rlRun_LOG - rlPhaseStartTest "Test with failing dmesg check (info)" - rlRun -s "tmt run -vvv test --name /test/check-fail-info" 0 - rlAssertGrep "info dmesg (before-test check)" $rlRun_LOG - rlAssertGrep "info dmesg (after-test check)" $rlRun_LOG - rlPhaseEnd + echo +} - rlPhaseStartTest "Test with passing dmesg check (xfail)" - rlRun -s "tmt run -vvv test --name /test/check-xfail-pass" 2 - rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG - rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG +rlJournalStart + rlPhaseStartSetup + rlRun "run=\$(mktemp -d)" 0 "Create run directory" + rlRun "pushd check_results" + rlRun "set -o pipefail" rlPhaseEnd - rlPhaseStartTest "Test with failing dmesg check (xfail)" - rlRun -s "tmt run -vvv test --name /test/check-xfail-fail" 0 - rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG - rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG + rlPhaseStartTest "Check Results Tests" + run "pass" "/test/check-pass" 0 + run "fail" "/test/check-fail-respect" 1 + run "pass" "/test/check-fail-info" 0 + run "fail" "/test/check-xfail-pass" 1 + run "pass" "/test/check-xfail-fail" 0 + run "pass" "/test/check-override" 0 rlPhaseEnd - rlPhaseStartTest "Test with multiple checks with different result interpretations" - rlRun -s "tmt run -vvv test --name /test/check-multiple" 2 - rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG - rlAssertGrep "fail dmesg (before-test check)" $rlRun_LOG - rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG - rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG - rlAssertGrep "info dmesg (after-test check)" $rlRun_LOG - rlAssertGrep "info dmesg (after-test check)" $rlRun_LOG - rlPhaseEnd + rlPhaseStartTest "Verbose execute prints result" + rlRun -s "tmt run --id \${run} --scratch --until execute tests --filter tag:-cherry_pick provision --how local execute -v 2>&1 >/dev/null" "1" - rlPhaseStartTest "Test with failing dmesg check but overridden by test result" - rlRun -s "tmt run -vvv test --name /test/check-override" 0 - rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG - rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG + while read -r line; do + rlAssertGrep "$line" "$rlRun_LOG" -F + done <<-EOF +00:00:00 pass /test/check-fail-info (on default-0) [1/6] +00:00:00 fail /test/check-fail-respect (on default-0) (Check 'dmesg' failed, original result: pass) [2/6] +00:00:00 pass /test/check-override (on default-0) [3/6] +00:00:00 pass /test/check-pass (on default-0) [4/6] +00:00:00 pass /test/check-xfail-fail (on default-0) [5/6] +00:00:00 fail /test/check-xfail-pass (on default-0) (Check 'dmesg' failed, original result: pass) [6/6] +EOF rlPhaseEnd rlPhaseStartCleanup rlRun "popd" - rlRun "rm -r $tmp" 0 "Removing tmp directory" + rlRun "rm -r ${run}" 0 "Remove run directory" rlPhaseEnd rlJournalEnd diff --git a/tests/execute/result/check_results/test.fmf b/tests/execute/result/check_results/test.fmf index 1ca7081503..4591a61381 100644 --- a/tests/execute/result/check_results/test.fmf +++ b/tests/execute/result/check_results/test.fmf @@ -1,7 +1,7 @@ summary: Tests for check results behaviour description: Verify that check results are correctly interpreted and affect test results -/test/check-pass: +/check-pass: summary: Test with passing checks test: echo "Test passed" framework: shell @@ -11,7 +11,7 @@ description: Verify that check results are correctly interpreted and affect test result: respect # Expected outcome: PASS (test passes, check passes) -/test/check-fail-respect: +/check-fail-respect: summary: Test with failing dmesg check (respect) test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg framework: shell @@ -22,7 +22,7 @@ description: Verify that check results are correctly interpreted and affect test result: respect # Expected outcome: FAIL (test passes, but check fails and is respected) -/test/check-fail-info: +/check-fail-info: summary: Test with failing dmesg check (info) test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg framework: shell @@ -33,7 +33,7 @@ description: Verify that check results are correctly interpreted and affect test result: info # Expected outcome: PASS (test passes, check fails, but should be just info) -/test/check-xfail-pass: +/check-xfail-pass: summary: Test with passing dmesg check (xfail) test: echo "Test passed" framework: shell @@ -43,7 +43,7 @@ description: Verify that check results are correctly interpreted and affect test result: xfail # Expected outcome: FAIL (test passes, check passes but xfail expects it to fail) -/test/check-xfail-fail: +/check-xfail-fail: summary: Test with failing dmesg check (xfail) test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg framework: shell @@ -54,23 +54,24 @@ description: Verify that check results are correctly interpreted and affect test result: xfail # Expected outcome: PASS (test passes, check fails but xfail expects it to fail) -/test/check-multiple: - summary: Test with multiple checks with different result interpretations - test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg - framework: shell - duration: 1m - check: - - how: dmesg - failure-pattern: Fail Test Check Pattern - result: respect - - how: dmesg - result: xfail - - how: dmesg - failure-pattern: Fail Test Check Pattern - result: info - # Expected outcome: FAIL (first dmesg check fails and is respected, second dmesg check passes but xfail expects it to fail, third failing dmesg check is just info) +# TODO: handle multiple checks with same 'name'/'how' +#/check-multiple: +# summary: Test with multiple checks with different result interpretations +# test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg +# framework: shell +# duration: 1m +# check: +# - how: dmesg +# failure-pattern: Fail Test Check Pattern +# result: respect +# - how: dmesg +# result: xfail +# - how: dmesg +# failure-pattern: Fail Test Check Pattern +# result: info +# # Expected outcome: FAIL (first dmesg check fails and is respected, second dmesg check passes but xfail expects it to fail, third failing dmesg check is just info) -/test/check-override: +/check-override: summary: Test with failing dmesg check but overridden by test result test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg framework: shell diff --git a/tmt/result.py b/tmt/result.py index 08e3b42cb3..795849ea07 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -196,11 +196,13 @@ def interpret_check_result(self, interpret: CheckResultInterpret) -> 'CheckResul return self if interpret == CheckResultInterpret.INFO: self.result = ResultOutcome.INFO - elif interpret == CheckResultInterpret.XFAIL: + + elif interpret == CheckResultInterpret.XFAIL and self.event != CheckEvent.BEFORE_TEST: self.result = { ResultOutcome.FAIL: ResultOutcome.PASS, ResultOutcome.PASS: ResultOutcome.FAIL }.get(self.result, self.result) + return self def to_subcheck(self) -> 'SubCheckResult': @@ -360,22 +362,27 @@ def interpret_result( :returns: :py:class:`Result` instance containing the updated result. """ - if interpret not in (ResultInterpret): + if interpret not in ResultInterpret: raise tmt.utils.SpecificationError( f"Invalid result '{interpret.value}' in test '{self.name}'." ) - if interpret in (ResultInterpret.CUSTOM, ResultInterpret.RESTRAINT): - # Interpret check results first, in case there is "info" - self.check = [ - check_result.interpret_check_result(interpret_checks[check_result.name]) - for check_result in self.check - ] + original_result = self.result - return self + # Interpret check results + self.check = [ + check_result.interpret_check_result(CheckResultInterpret(interpret_checks[check_result.name])) + for check_result in self.check + ] + + if interpret not in (ResultInterpret.RESPECT, ResultInterpret.XFAIL): + self.result = ResultOutcome(interpret.value) + + # Add original result to note if the result has changed + if self.result != original_result: + orig_note = f"original result: {original_result.value}" + self.note = f"{self.note}, {orig_note}" if self.note else orig_note - # Keeping "hardcoded" PASS, FAIL, ERROR... - if interpret not in (ResultInterpret.XFAIL, ResultInterpret.RESPECT): return self failed_checks = [ @@ -388,6 +395,7 @@ def interpret_result( self.result = ResultOutcome.FAIL check_note = ", ".join([f"Check '{check.name}' failed" for check in failed_checks]) self.note = f"{self.note}, {check_note}" if self.note else check_note + if interpret == ResultInterpret.XFAIL: # Swap fail<-->pass self.result = { @@ -395,6 +403,11 @@ def interpret_result( ResultOutcome.PASS: ResultOutcome.FAIL, }.get(self.result, self.result) + # Add original result to note if the result has changed + if self.result != original_result: + orig_note = f"original result: {original_result.value}" + self.note = f"{self.note}, {orig_note}" if self.note else orig_note + return self def to_subresult(self) -> 'SubResult': From cca13d1b878293980c3ac96cc33c9f090cc89ea3 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 21 Oct 2024 14:37:27 +0200 Subject: [PATCH 07/35] Do not interpret any result with CUSTOM interpret --- tmt/result.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tmt/result.py b/tmt/result.py index 795849ea07..56999cd8b1 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -367,6 +367,9 @@ def interpret_result( f"Invalid result '{interpret.value}' in test '{self.name}'." ) + if interpret == ResultInterpret.CUSTOM: + return self + original_result = self.result # Interpret check results From d5d7629d03d0e04b95905135932ad018ecbb53c3 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 21 Oct 2024 15:35:50 +0200 Subject: [PATCH 08/35] Addressing comments --- tmt/checks/__init__.py | 7 +++---- tmt/result.py | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tmt/checks/__init__.py b/tmt/checks/__init__.py index e8c0360714..5f9920d25a 100644 --- a/tmt/checks/__init__.py +++ b/tmt/checks/__init__.py @@ -93,8 +93,8 @@ class CheckResultInterpret(enum.Enum): def from_spec(cls, spec: str) -> 'CheckResultInterpret': try: return CheckResultInterpret(spec) - except ValueError: - raise ValueError(f"Invalid check result interpretation '{spec}'.") + except ValueError as err: + raise ValueError(f"Invalid check result interpretation '{spec}'.") from err @dataclasses.dataclass @@ -118,8 +118,7 @@ class Check( default=CheckResultInterpret.RESPECT, help='How to interpret the check result.', serialize=lambda result: result.value, - unserialize=CheckResultInterpret.from_spec - ) + unserialize=CheckResultInterpret.from_spec) @functools.cached_property def plugin(self) -> 'CheckPluginClass': diff --git a/tmt/result.py b/tmt/result.py index 56999cd8b1..adb1963a27 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -190,7 +190,7 @@ class CheckResult(BaseResult): unserialize=CheckEvent.from_spec) def interpret_check_result(self, interpret: CheckResultInterpret) -> 'CheckResult': - """Interpret check result according to the check_result interpretation.""" + """Interpret check result according to the "check_result" interpretation.""" if interpret == CheckResultInterpret.RESPECT: return self From 0c8031e10dfa5c98e2f5377044a38f66dc34cdaf Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 21 Oct 2024 15:39:13 +0200 Subject: [PATCH 09/35] Add note for running one check multiple times --- spec/tests/check.fmf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/tests/check.fmf b/spec/tests/check.fmf index 1721bb17d3..bd9be05598 100644 --- a/spec/tests/check.fmf +++ b/spec/tests/check.fmf @@ -18,6 +18,8 @@ description: | the default ``respect`` can be changed to ``xfail``, which expects the check to fail, or ``info``, which changes the check result to 'INFO'. + Note that running one check multiple times for the same test is + not yet supported." .. versionchanged:: 1.38.0 ``result`` key added From d0b5f7ed7ac2894b5a804c86e85ad6d5d1ffdf83 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 21 Oct 2024 16:42:03 +0200 Subject: [PATCH 10/35] Add description to check-result tests --- tests/execute/result/check_results/test.fmf | 44 ++++++++++----------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/tests/execute/result/check_results/test.fmf b/tests/execute/result/check_results/test.fmf index 4591a61381..166f10d772 100644 --- a/tests/execute/result/check_results/test.fmf +++ b/tests/execute/result/check_results/test.fmf @@ -3,16 +3,17 @@ description: Verify that check results are correctly interpreted and affect test /check-pass: summary: Test with passing checks + description: Expected outcome: PASS (test passes, check passes) test: echo "Test passed" framework: shell duration: 1m check: - how: dmesg result: respect - # Expected outcome: PASS (test passes, check passes) /check-fail-respect: summary: Test with failing dmesg check (respect) + description: Expected outcome: FAIL (test passes, but check fails and is respected) test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg framework: shell duration: 1m @@ -20,10 +21,10 @@ description: Verify that check results are correctly interpreted and affect test - how: dmesg failure-pattern: Fail Test Check Pattern result: respect - # Expected outcome: FAIL (test passes, but check fails and is respected) /check-fail-info: summary: Test with failing dmesg check (info) + description: Expected outcome: PASS (test passes, check fails, but should be just info) test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg framework: shell duration: 1m @@ -31,20 +32,20 @@ description: Verify that check results are correctly interpreted and affect test - how: dmesg failure-pattern: Fail Test Check Pattern result: info - # Expected outcome: PASS (test passes, check fails, but should be just info) /check-xfail-pass: summary: Test with passing dmesg check (xfail) + description: Expected outcome: FAIL (test passes, check passes but xfail expects it to fail) test: echo "Test passed" framework: shell duration: 1m check: - how: dmesg result: xfail - # Expected outcome: FAIL (test passes, check passes but xfail expects it to fail) /check-xfail-fail: summary: Test with failing dmesg check (xfail) + description: Expected outcome: PASS (test passes, check fails but xfail expects it to fail) test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg framework: shell duration: 1m @@ -52,27 +53,27 @@ description: Verify that check results are correctly interpreted and affect test - how: dmesg failure-pattern: Fail Test Check Pattern result: xfail - # Expected outcome: PASS (test passes, check fails but xfail expects it to fail) -# TODO: handle multiple checks with same 'name'/'how' -#/check-multiple: -# summary: Test with multiple checks with different result interpretations -# test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg -# framework: shell -# duration: 1m -# check: -# - how: dmesg -# failure-pattern: Fail Test Check Pattern -# result: respect -# - how: dmesg -# result: xfail -# - how: dmesg -# failure-pattern: Fail Test Check Pattern -# result: info -# # Expected outcome: FAIL (first dmesg check fails and is respected, second dmesg check passes but xfail expects it to fail, third failing dmesg check is just info) +/check-multiple: + summary: Test with multiple checks with different result interpretations + description: Expected outcome: FAIL (first dmesg check fails and is respected, second dmesg check passes but xfail expects it to fail, third failing dmesg check is just info) + test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + framework: shell + duration: 1m + check: + - how: dmesg + failure-pattern: Fail Test Check Pattern + result: respect + - how: dmesg + result: xfail + - how: dmesg + failure-pattern: Fail Test Check Pattern + result: info + enabled: false # TODO: handle multiple checks with same 'name'/'how' /check-override: summary: Test with failing dmesg check but overridden by test result + description: Expected outcome: PASS (test passes, check fails but is overridden by 'result: pass') test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg framework: shell duration: 1m @@ -81,4 +82,3 @@ description: Verify that check results are correctly interpreted and affect test - how: dmesg failure-pattern: Fail Test Check Pattern result: respect - # Expected outcome: PASS (test passes, check fails but is overridden by 'result: pass') From 04e7aed156031e991ee8d33255888459b989fc29 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 21 Oct 2024 16:43:49 +0200 Subject: [PATCH 11/35] fixup! Addressing comments --- tmt/result.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmt/result.py b/tmt/result.py index adb1963a27..8ca174fda1 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -190,7 +190,7 @@ class CheckResult(BaseResult): unserialize=CheckEvent.from_spec) def interpret_check_result(self, interpret: CheckResultInterpret) -> 'CheckResult': - """Interpret check result according to the "check_result" interpretation.""" + """Interpret check result according to the "check result" interpretation.""" if interpret == CheckResultInterpret.RESPECT: return self From 84ffa1102cbf3e956fbf45acd2e9772db443048f Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 21 Oct 2024 16:47:43 +0200 Subject: [PATCH 12/35] Move the result definition list to spec --- docs/releases.rst | 12 +----------- spec/tests/check.fmf | 15 +++++++++++---- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/docs/releases.rst b/docs/releases.rst index dce55f4f17..549c6ff8f1 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -74,17 +74,7 @@ for :ref:`cpu.stepping` hardware requirement. 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. The following values -are supported: - -respect (default) - The check result is respected and affects the overall test result. -xfail - The check result is expected to fail (pass becomes fail and vice-versa). -info - The check result is always treated as "INFO" and does not affect the - overall test result. - +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. diff --git a/spec/tests/check.fmf b/spec/tests/check.fmf index bd9be05598..2ea096b169 100644 --- a/spec/tests/check.fmf +++ b/spec/tests/check.fmf @@ -14,10 +14,17 @@ 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 apart from - the default ``respect`` can be changed to ``xfail``, which - expects the check to fail, or ``info``, which changes the check - result to 'INFO'. + To change this behaviour, use the 'result' key, which accepts + the following values: + + respect (default) + The check result is respected and affects the overall test result. + xfail + The check result is expected to fail (pass becomes fail and vice-versa). + info + The check result is always treated as "INFO" and does not affect the + overall test result. + Note that running one check multiple times for the same test is not yet supported." From 32917bd72510c12f53531d99057cdbcbcf1b743e Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 21 Oct 2024 17:28:08 +0200 Subject: [PATCH 13/35] Addressing comments for tests, spec --- spec/tests/check.fmf | 25 +++++++------ .../result/{check_results.sh => check.sh} | 4 +-- .../{check_results => check}/.fmf/version | 0 .../result/{check_results => check}/test.fmf | 35 ++++++++----------- tests/execute/result/main.fmf | 4 +-- 5 files changed, 32 insertions(+), 36 deletions(-) rename tests/execute/result/{check_results.sh => check.sh} (93%) rename tests/execute/result/{check_results => check}/.fmf/version (100%) rename tests/execute/result/{check_results => check}/test.fmf (65%) diff --git a/spec/tests/check.fmf b/spec/tests/check.fmf index 2ea096b169..ab0e243ac5 100644 --- a/spec/tests/check.fmf +++ b/spec/tests/check.fmf @@ -13,22 +13,25 @@ description: | the user can run into. Another useful checks would be kernel 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 + To change this behaviour, use the ``result`` key, which accepts the following values: - respect (default) - The check result is respected and affects the overall test result. - xfail - The check result is expected to fail (pass becomes fail and vice-versa). - info - The check result is always treated as "INFO" and does not affect the - overall test result. + respect (default) + The check result is respected and affects the overall test result. + + xfail + The check result is expected to fail (pass becomes fail and vice-versa). + + info + The check result is always treated as "INFO" and does not affect the + overall test result. Note that running one check multiple times for the same test is - not yet supported." + not yet supported. - .. versionchanged:: 1.38.0 ``result`` key added + .. versionchanged:: 1.38.0 the ``result`` key added See :ref:`/plugins/test-checks` for the list of available checks. @@ -72,4 +75,4 @@ link: - implemented-by: /tmt/result.py - verified-by: /tests/test/check/avc - verified-by: /tests/test/check/dmesg - - verified-by: /tests/execute/result/check_results + - verified-by: /tests/execute/result/check diff --git a/tests/execute/result/check_results.sh b/tests/execute/result/check.sh similarity index 93% rename from tests/execute/result/check_results.sh rename to tests/execute/result/check.sh index 6ea418a935..abcd40e4f9 100755 --- a/tests/execute/result/check_results.sh +++ b/tests/execute/result/check.sh @@ -18,7 +18,7 @@ run() rlJournalStart rlPhaseStartSetup rlRun "run=\$(mktemp -d)" 0 "Create run directory" - rlRun "pushd check_results" + rlRun "pushd check" rlRun "set -o pipefail" rlPhaseEnd @@ -32,7 +32,7 @@ rlJournalStart rlPhaseEnd rlPhaseStartTest "Verbose execute prints result" - rlRun -s "tmt run --id \${run} --scratch --until execute tests --filter tag:-cherry_pick provision --how local execute -v 2>&1 >/dev/null" "1" + rlRun -s "tmt run --id \${run} --scratch --until execute tests provision --how local execute -v 2>&1 >/dev/null" "1" while read -r line; do rlAssertGrep "$line" "$rlRun_LOG" -F diff --git a/tests/execute/result/check_results/.fmf/version b/tests/execute/result/check/.fmf/version similarity index 100% rename from tests/execute/result/check_results/.fmf/version rename to tests/execute/result/check/.fmf/version diff --git a/tests/execute/result/check_results/test.fmf b/tests/execute/result/check/test.fmf similarity index 65% rename from tests/execute/result/check_results/test.fmf rename to tests/execute/result/check/test.fmf index 166f10d772..2df185f2df 100644 --- a/tests/execute/result/check_results/test.fmf +++ b/tests/execute/result/check/test.fmf @@ -1,22 +1,25 @@ summary: Tests for check results behaviour description: Verify that check results are correctly interpreted and affect test results +framework: shell +duration: 1m +tag+: + - provision-only + - provision-local + - provision-virtual + /check-pass: summary: Test with passing checks - description: Expected outcome: PASS (test passes, check passes) + description: "Expected outcome: PASS (test passes, check passes)" test: echo "Test passed" - framework: shell - duration: 1m check: - how: dmesg result: respect /check-fail-respect: summary: Test with failing dmesg check (respect) - description: Expected outcome: FAIL (test passes, but check fails and is respected) + description: "Expected outcome: FAIL (test passes, but check fails and is respected)" test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg - framework: shell - duration: 1m check: - how: dmesg failure-pattern: Fail Test Check Pattern @@ -24,10 +27,8 @@ description: Verify that check results are correctly interpreted and affect test /check-fail-info: summary: Test with failing dmesg check (info) - description: Expected outcome: PASS (test passes, check fails, but should be just info) + description: "Expected outcome: PASS (test passes, check fails, but should be just info)" test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg - framework: shell - duration: 1m check: - how: dmesg failure-pattern: Fail Test Check Pattern @@ -35,20 +36,16 @@ description: Verify that check results are correctly interpreted and affect test /check-xfail-pass: summary: Test with passing dmesg check (xfail) - description: Expected outcome: FAIL (test passes, check passes but xfail expects it to fail) + description: "Expected outcome: FAIL (test passes, check passes but xfail expects it to fail)" test: echo "Test passed" - framework: shell - duration: 1m check: - how: dmesg result: xfail /check-xfail-fail: summary: Test with failing dmesg check (xfail) - description: Expected outcome: PASS (test passes, check fails but xfail expects it to fail) + description: "Expected outcome: PASS (test passes, check fails but xfail expects it to fail)" test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg - framework: shell - duration: 1m check: - how: dmesg failure-pattern: Fail Test Check Pattern @@ -56,10 +53,8 @@ description: Verify that check results are correctly interpreted and affect test /check-multiple: summary: Test with multiple checks with different result interpretations - description: Expected outcome: FAIL (first dmesg check fails and is respected, second dmesg check passes but xfail expects it to fail, third failing dmesg check is just info) + description: "Expected outcome: FAIL (first dmesg check fails and is respected, second dmesg check passes but xfail expects it to fail, third failing dmesg check is just info)" test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg - framework: shell - duration: 1m check: - how: dmesg failure-pattern: Fail Test Check Pattern @@ -73,10 +68,8 @@ description: Verify that check results are correctly interpreted and affect test /check-override: summary: Test with failing dmesg check but overridden by test result - description: Expected outcome: PASS (test passes, check fails but is overridden by 'result: pass') + description: "Expected outcome: PASS (test passes, check fails but is overridden by 'result: pass')" test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg - framework: shell - duration: 1m result: pass check: - how: dmesg diff --git a/tests/execute/result/main.fmf b/tests/execute/result/main.fmf index ac2605ee71..ebc158314c 100644 --- a/tests/execute/result/main.fmf +++ b/tests/execute/result/main.fmf @@ -13,6 +13,6 @@ /subresults: summary: Multiple calls to tmt-report-result should generate tmt subresults test: ./subresults.sh -/check_results: +/check: summary: Test check result interpretations - test: ./check_results.sh + test: ./check.sh From 1ecc935ad0642216d05180b8a4dc9f23ce61d3cf Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Tue, 22 Oct 2024 15:27:51 +0200 Subject: [PATCH 14/35] [PoC] use metadata.choices for the enums --- docs/templates/plugins.rst.j2 | 4 +++- tmt/checks/__init__.py | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/templates/plugins.rst.j2 b/docs/templates/plugins.rst.j2 index 5969cc4cbd..bd51c57db5 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,7 +29,7 @@ {% 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) %} diff --git a/tmt/checks/__init__.py b/tmt/checks/__init__.py index 5f9920d25a..52f0639347 100644 --- a/tmt/checks/__init__.py +++ b/tmt/checks/__init__.py @@ -118,7 +118,8 @@ class Check( default=CheckResultInterpret.RESPECT, help='How to interpret the check result.', serialize=lambda result: result.value, - unserialize=CheckResultInterpret.from_spec) + unserialize=CheckResultInterpret.from_spec, + choices=[value.value for value in CheckResultInterpret.__members__.values()]) @functools.cached_property def plugin(self) -> 'CheckPluginClass': From 83e16e36de94e63513c75703347b4946ddbc5868 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Tue, 22 Oct 2024 15:46:58 +0200 Subject: [PATCH 15/35] Improve xfail check result handling --- tests/execute/result/check.sh | 6 ++++++ tmt/result.py | 15 +++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/execute/result/check.sh b/tests/execute/result/check.sh index abcd40e4f9..6ee51a18b5 100755 --- a/tests/execute/result/check.sh +++ b/tests/execute/result/check.sh @@ -46,6 +46,12 @@ rlJournalStart EOF rlPhaseEnd + rlPhaseStartTest "Verify before test check xfail fail is interpreted as pass" + rlRun "echo 'Fail Test Check Pattern' | sudo tee /dev/kmsg" + rlRun -s "tmt run -a --scratch test --name /test/check-xfail-fail provision --how local report -v 2>&1 >/dev/null | grep report -A3" + rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG + rlPhaseEnd + rlPhaseStartCleanup rlRun "popd" rlRun "rm -r ${run}" 0 "Remove run directory" diff --git a/tmt/result.py b/tmt/result.py index 8ca174fda1..9f27b622de 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -196,12 +196,15 @@ def interpret_check_result(self, interpret: CheckResultInterpret) -> 'CheckResul return self if interpret == CheckResultInterpret.INFO: self.result = ResultOutcome.INFO - - elif interpret == CheckResultInterpret.XFAIL and self.event != CheckEvent.BEFORE_TEST: - self.result = { - ResultOutcome.FAIL: ResultOutcome.PASS, - ResultOutcome.PASS: ResultOutcome.FAIL - }.get(self.result, self.result) + elif interpret == CheckResultInterpret.XFAIL: + mapping = ( + # if 'xfail' BEFORE_TEST check passed, use WARN, otherwise switch PASS<->FAIL + {ResultOutcome.FAIL: ResultOutcome.PASS, ResultOutcome.PASS: ResultOutcome.WARN} + if self.event == CheckEvent.BEFORE_TEST + else + {ResultOutcome.FAIL: ResultOutcome.PASS, ResultOutcome.PASS: ResultOutcome.FAIL} + ) + self.result = mapping.get(self.result, self.result) return self From e0970eea966af37bf2581dc86fd6efe6ee2f617d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Wed, 23 Oct 2024 10:28:55 +0200 Subject: [PATCH 16/35] A minor spec wording adjustment --- spec/tests/check.fmf | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/spec/tests/check.fmf b/spec/tests/check.fmf index ab0e243ac5..19d73ffef1 100644 --- a/spec/tests/check.fmf +++ b/spec/tests/check.fmf @@ -18,15 +18,17 @@ description: | To change this behaviour, use the ``result`` key, which accepts the following values: - respect (default) - The check result is respected and affects the overall test result. + 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). + The check result is expected to fail (pass becomes + fail and vice-versa). info - The check result is always treated as "INFO" and does not affect the - overall test result. + The check result is treated as an informational + message and does not affect the overall test result. Note that running one check multiple times for the same test is not yet supported. From 6389548e60cfdf7dba1727341c2e506fdfff512a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Wed, 23 Oct 2024 11:08:11 +0200 Subject: [PATCH 17/35] Small release notes adjustment, fix a typo --- docs/releases.rst | 13 +++++++------ docs/templates/plugins.rst.j2 | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/releases.rst b/docs/releases.rst index 549c6ff8f1..98240b6b7a 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -4,6 +4,7 @@ Releases ====================== + tmt-1.38.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -71,12 +72,12 @@ for example usage. The :ref:`/plugins/provision/beaker` provision plugin gains support for :ref:`cpu.stepping` hardware requirement. - -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. +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. tmt-1.37.0 diff --git a/docs/templates/plugins.rst.j2 b/docs/templates/plugins.rst.j2 index bd51c57db5..995c9b4d09 100644 --- a/docs/templates/plugins.rst.j2 +++ b/docs/templates/plugins.rst.j2 @@ -75,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__ %} From 0e36df156ea910e6fa62748890d0bc54af1a527a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Wed, 23 Oct 2024 18:09:58 +0200 Subject: [PATCH 18/35] Add missing coversions to and from spec --- tmt/checks/__init__.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tmt/checks/__init__.py b/tmt/checks/__init__.py index 52f0639347..19c33d4e6a 100644 --- a/tmt/checks/__init__.py +++ b/tmt/checks/__init__.py @@ -96,6 +96,9 @@ def from_spec(cls, spec: str) -> 'CheckResultInterpret': 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( @@ -133,14 +136,18 @@ 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 go( self, From 7bc797316af488794e7acad3b7d8d5c31d964ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Wed, 23 Oct 2024 19:18:38 +0200 Subject: [PATCH 19/35] Define the `to_minimal_spec()` method as well This one is used in test export. --- tmt/checks/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tmt/checks/__init__.py b/tmt/checks/__init__.py index 19c33d4e6a..0fcafb3e38 100644 --- a/tmt/checks/__init__.py +++ b/tmt/checks/__init__.py @@ -149,6 +149,9 @@ def to_spec(self) -> _RawCheck: spec["result"] = self.result.to_spec() return spec + def to_minimal_spec(self) -> _RawCheck: + return self.to_spec() + def go( self, *, From b7bdfa4d6ea19c4d76f3c2b27173aabb1916810b Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 23 Oct 2024 20:05:41 +0200 Subject: [PATCH 20/35] Improve tests --- tests/execute/result/check.sh | 67 +++++++++++------------------ tests/execute/result/check/test.fmf | 20 ++++----- tests/execute/result/main.fmf | 4 ++ 3 files changed, 38 insertions(+), 53 deletions(-) diff --git a/tests/execute/result/check.sh b/tests/execute/result/check.sh index 6ee51a18b5..d1d8fbf222 100755 --- a/tests/execute/result/check.sh +++ b/tests/execute/result/check.sh @@ -1,55 +1,40 @@ #!/bin/bash . /usr/share/beakerlib/beakerlib.sh || exit 1 -run() -{ - res=$1 # expected final result of test - tn=$2 # test name - ret=$3 # tmt return code - - rlRun -s "tmt run -a --scratch --id \${run} test --name ${tn} provision --how local report -v 2>&1 >/dev/null | grep report -A2 | tail -n 1" \ - ${ret} "Result: ${res}, Test name: ${tn}, tmt return code: ${ret}" - - rlAssertGrep "${res} ${tn}" $rlRun_LOG - - echo -} - rlJournalStart rlPhaseStartSetup rlRun "run=\$(mktemp -d)" 0 "Create run directory" + rlRun "PROVISION_HOW=${PROVISION_HOW:-virtual}" rlRun "pushd check" rlRun "set -o pipefail" + # Write pattern for tests that need pre-existing dmesg content + rlRun "echo 'Fail Test Check Pattern' | sudo tee /dev/kmsg" rlPhaseEnd - rlPhaseStartTest "Check Results Tests" - run "pass" "/test/check-pass" 0 - run "fail" "/test/check-fail-respect" 1 - run "pass" "/test/check-fail-info" 0 - run "fail" "/test/check-xfail-pass" 1 - run "pass" "/test/check-xfail-fail" 0 - run "pass" "/test/check-override" 0 - rlPhaseEnd - - rlPhaseStartTest "Verbose execute prints result" - rlRun -s "tmt run --id \${run} --scratch --until execute tests provision --how local execute -v 2>&1 >/dev/null" "1" - - while read -r line; do - rlAssertGrep "$line" "$rlRun_LOG" -F - done <<-EOF -00:00:00 pass /test/check-fail-info (on default-0) [1/6] -00:00:00 fail /test/check-fail-respect (on default-0) (Check 'dmesg' failed, original result: pass) [2/6] -00:00:00 pass /test/check-override (on default-0) [3/6] -00:00:00 pass /test/check-pass (on default-0) [4/6] -00:00:00 pass /test/check-xfail-fail (on default-0) [5/6] -00:00:00 fail /test/check-xfail-pass (on default-0) (Check 'dmesg' failed, original result: pass) [6/6] + rlPhaseStartTest "Check Results" + rlRun -s "tmt run -a --id \${run} --scratch tests provision --how $PROVISION_HOW report -v 2>&1 >/dev/null | grep report -A19" "1" + + rlAssertGrep "$(cat <<-EOF +pass /test/check-fail-info + info dmesg (before-test check) + info dmesg (after-test check) +fail /test/check-fail-respect (Check 'dmesg' failed, original result: pass) + pass dmesg (before-test check) + fail dmesg (after-test check) +pass /test/check-override + pass dmesg (before-test check) + fail dmesg (after-test check) +pass /test/check-pass + pass dmesg (before-test check) + pass dmesg (after-test check) +pass /test/check-xfail-fail + warn dmesg (before-test check) + pass dmesg (after-test check) +fail /test/check-xfail-pass (Check 'dmesg' failed, original result: pass) + warn dmesg (before-test check) + fail dmesg (after-test check) EOF - rlPhaseEnd - - rlPhaseStartTest "Verify before test check xfail fail is interpreted as pass" - rlRun "echo 'Fail Test Check Pattern' | sudo tee /dev/kmsg" - rlRun -s "tmt run -a --scratch test --name /test/check-xfail-fail provision --how local report -v 2>&1 >/dev/null | grep report -A3" - rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG +)" "$rlRun_LOG" -F rlPhaseEnd rlPhaseStartCleanup diff --git a/tests/execute/result/check/test.fmf b/tests/execute/result/check/test.fmf index 2df185f2df..d0f49ea72f 100644 --- a/tests/execute/result/check/test.fmf +++ b/tests/execute/result/check/test.fmf @@ -2,11 +2,6 @@ summary: Tests for check results behaviour description: Verify that check results are correctly interpreted and affect test results framework: shell duration: 1m -tag+: - - provision-only - - provision-local - - provision-virtual - /check-pass: summary: Test with passing checks @@ -19,16 +14,15 @@ tag+: /check-fail-respect: summary: Test with failing dmesg check (respect) description: "Expected outcome: FAIL (test passes, but check fails and is respected)" - test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + test: echo "Fail Test Check Pattern" | tee /dev/kmsg check: - how: dmesg failure-pattern: Fail Test Check Pattern - result: respect /check-fail-info: summary: Test with failing dmesg check (info) description: "Expected outcome: PASS (test passes, check fails, but should be just info)" - test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + test: echo "Fail Test Check Pattern" | tee /dev/kmsg check: - how: dmesg failure-pattern: Fail Test Check Pattern @@ -45,7 +39,7 @@ tag+: /check-xfail-fail: summary: Test with failing dmesg check (xfail) description: "Expected outcome: PASS (test passes, check fails but xfail expects it to fail)" - test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + test: echo "Fail Test Check Pattern" | tee /dev/kmsg check: - how: dmesg failure-pattern: Fail Test Check Pattern @@ -53,8 +47,10 @@ tag+: /check-multiple: summary: Test with multiple checks with different result interpretations - description: "Expected outcome: FAIL (first dmesg check fails and is respected, second dmesg check passes but xfail expects it to fail, third failing dmesg check is just info)" - test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + description: | + Expected outcome: FAIL (first dmesg check fails and is respected, second dmesg check + passes but xfail expects it to fail, third failing dmesg check is just info)" + test: echo "Fail Test Check Pattern" | tee /dev/kmsg check: - how: dmesg failure-pattern: Fail Test Check Pattern @@ -69,7 +65,7 @@ tag+: /check-override: summary: Test with failing dmesg check but overridden by test result description: "Expected outcome: PASS (test passes, check fails but is overridden by 'result: pass')" - test: echo "Fail Test Check Pattern" | sudo tee /dev/kmsg + test: echo "Fail Test Check Pattern" | tee /dev/kmsg result: pass check: - how: dmesg diff --git a/tests/execute/result/main.fmf b/tests/execute/result/main.fmf index ebc158314c..51968ffdfe 100644 --- a/tests/execute/result/main.fmf +++ b/tests/execute/result/main.fmf @@ -16,3 +16,7 @@ /check: summary: Test check result interpretations test: ./check.sh + tag+: + - provision-only + - provision-local + - provision-virtual From f4cc82a93fa4f200a78e30101f74e6ad1cd640b5 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 23 Oct 2024 20:06:13 +0200 Subject: [PATCH 21/35] Add missing parameter in docstring --- tmt/result.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tmt/result.py b/tmt/result.py index 9f27b622de..5600bc83be 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -362,6 +362,7 @@ def interpret_result( attributes, following the ``interpret`` value. :param interpret: how to interpret current result. + :param interpret_checks: mapping of check:how and it's result interpret :returns: :py:class:`Result` instance containing the updated result. """ From d7c37c8e71421bec9eaa24af2c69b2754d424b62 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Wed, 23 Oct 2024 22:21:11 +0200 Subject: [PATCH 22/35] Fix avc, dmesg tests expecting exit code 0 --- tests/test/check/test-avc.sh | 2 +- tests/test/check/test-dmesg.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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..6e10ce7a19 100755 --- a/tests/test/check/test-dmesg.sh +++ b/tests/test/check/test-dmesg.sh @@ -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" From 22963077e0d059afdee6a30b4d3b784fccf131d4 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Thu, 24 Oct 2024 10:40:15 +0200 Subject: [PATCH 23/35] Addressing comments --- tests/execute/result/check/test.fmf | 18 ++++++++++++------ tmt/result.py | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/execute/result/check/test.fmf b/tests/execute/result/check/test.fmf index d0f49ea72f..482f305978 100644 --- a/tests/execute/result/check/test.fmf +++ b/tests/execute/result/check/test.fmf @@ -5,7 +5,8 @@ duration: 1m /check-pass: summary: Test with passing checks - description: "Expected outcome: PASS (test passes, check passes)" + description: | + Expected outcome: PASS (test passes, check passes) test: echo "Test passed" check: - how: dmesg @@ -13,7 +14,8 @@ duration: 1m /check-fail-respect: summary: Test with failing dmesg check (respect) - description: "Expected outcome: FAIL (test passes, but check fails and is respected)" + description: | + Expected outcome: FAIL (test passes, but check fails and is respected) test: echo "Fail Test Check Pattern" | tee /dev/kmsg check: - how: dmesg @@ -21,7 +23,8 @@ duration: 1m /check-fail-info: summary: Test with failing dmesg check (info) - description: "Expected outcome: PASS (test passes, check fails, but should be just info)" + description: | + Expected outcome: PASS (test passes, check fails, but should be just info) test: echo "Fail Test Check Pattern" | tee /dev/kmsg check: - how: dmesg @@ -30,7 +33,8 @@ duration: 1m /check-xfail-pass: summary: Test with passing dmesg check (xfail) - description: "Expected outcome: FAIL (test passes, check passes but xfail expects it to fail)" + description: | + Expected outcome: FAIL (test passes, check passes but xfail expects it to fail) test: echo "Test passed" check: - how: dmesg @@ -38,7 +42,8 @@ duration: 1m /check-xfail-fail: summary: Test with failing dmesg check (xfail) - description: "Expected outcome: PASS (test passes, check fails but xfail expects it to fail)" + description: | + Expected outcome: PASS (test passes, check fails but xfail expects it to fail) test: echo "Fail Test Check Pattern" | tee /dev/kmsg check: - how: dmesg @@ -64,7 +69,8 @@ duration: 1m /check-override: summary: Test with failing dmesg check but overridden by test result - description: "Expected outcome: PASS (test passes, check fails but is overridden by 'result: pass')" + description: | + Expected outcome: PASS (test passes, check fails but is overridden by 'result: pass') test: echo "Fail Test Check Pattern" | tee /dev/kmsg result: pass check: diff --git a/tmt/result.py b/tmt/result.py index 5600bc83be..9c490f579c 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -190,7 +190,7 @@ class CheckResult(BaseResult): unserialize=CheckEvent.from_spec) def interpret_check_result(self, interpret: CheckResultInterpret) -> 'CheckResult': - """Interpret check result according to the "check result" interpretation.""" + """Interpret check result according to the check result interpretation.""" if interpret == CheckResultInterpret.RESPECT: return self From 28302540efea5f2334c5137b27ba4f2f24c36beb Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Thu, 24 Oct 2024 12:41:49 +0200 Subject: [PATCH 24/35] fixup! Fix avc, dmesg tests expecting exit code 0 --- tests/test/check/test-dmesg.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test/check/test-dmesg.sh b/tests/test/check/test-dmesg.sh index 6e10ce7a19..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" From f71767a3a256ac29f4ee6b931b95029a3e76938c Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Thu, 24 Oct 2024 12:45:49 +0200 Subject: [PATCH 25/35] Addressing yet more valuable comments --- tests/execute/result/check.sh | 2 -- tmt/result.py | 14 ++++++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/execute/result/check.sh b/tests/execute/result/check.sh index d1d8fbf222..51acf283da 100755 --- a/tests/execute/result/check.sh +++ b/tests/execute/result/check.sh @@ -7,8 +7,6 @@ rlJournalStart rlRun "PROVISION_HOW=${PROVISION_HOW:-virtual}" rlRun "pushd check" rlRun "set -o pipefail" - # Write pattern for tests that need pre-existing dmesg content - rlRun "echo 'Fail Test Check Pattern' | sudo tee /dev/kmsg" rlPhaseEnd rlPhaseStartTest "Check Results" diff --git a/tmt/result.py b/tmt/result.py index 9c490f579c..52dbd58636 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -374,11 +374,9 @@ def interpret_result( if interpret == ResultInterpret.CUSTOM: return self - original_result = self.result - # Interpret check results self.check = [ - check_result.interpret_check_result(CheckResultInterpret(interpret_checks[check_result.name])) + check_result.interpret_check_result(interpret_checks[check_result.name]) for check_result in self.check ] @@ -386,8 +384,8 @@ def interpret_result( self.result = ResultOutcome(interpret.value) # Add original result to note if the result has changed - if self.result != original_result: - orig_note = f"original result: {original_result.value}" + if self.result != self.original_result: + orig_note = f"original result: {self.original_result.value}" self.note = f"{self.note}, {orig_note}" if self.note else orig_note return self @@ -400,7 +398,7 @@ def interpret_result( if failed_checks: self.result = ResultOutcome.FAIL - check_note = ", ".join([f"Check '{check.name}' failed" for check in failed_checks]) + check_note = ", ".join([f"check '{check.name}' failed" for check in failed_checks]) self.note = f"{self.note}, {check_note}" if self.note else check_note if interpret == ResultInterpret.XFAIL: @@ -411,8 +409,8 @@ def interpret_result( }.get(self.result, self.result) # Add original result to note if the result has changed - if self.result != original_result: - orig_note = f"original result: {original_result.value}" + if self.result != self.original_result: + orig_note = f"original result: {self.original_result.value}" self.note = f"{self.note}, {orig_note}" if self.note else orig_note return self From 96c03df5a33f25d004c45ae9b0ab1c77adb962d9 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Thu, 24 Oct 2024 15:20:08 +0200 Subject: [PATCH 26/35] Aggregating check results, attempt 1 --- tests/execute/result/check.sh | 44 ++++++++++++----------- tmt/result.py | 66 +++++++++++++++++++++++++---------- 2 files changed, 70 insertions(+), 40 deletions(-) diff --git a/tests/execute/result/check.sh b/tests/execute/result/check.sh index 51acf283da..29aeeda428 100755 --- a/tests/execute/result/check.sh +++ b/tests/execute/result/check.sh @@ -12,27 +12,29 @@ rlJournalStart rlPhaseStartTest "Check Results" rlRun -s "tmt run -a --id \${run} --scratch tests provision --how $PROVISION_HOW report -v 2>&1 >/dev/null | grep report -A19" "1" - rlAssertGrep "$(cat <<-EOF -pass /test/check-fail-info - info dmesg (before-test check) - info dmesg (after-test check) -fail /test/check-fail-respect (Check 'dmesg' failed, original result: pass) - pass dmesg (before-test check) - fail dmesg (after-test check) -pass /test/check-override - pass dmesg (before-test check) - fail dmesg (after-test check) -pass /test/check-pass - pass dmesg (before-test check) - pass dmesg (after-test check) -pass /test/check-xfail-fail - warn dmesg (before-test check) - pass dmesg (after-test check) -fail /test/check-xfail-pass (Check 'dmesg' failed, original result: pass) - warn dmesg (before-test check) - fail dmesg (after-test check) -EOF -)" "$rlRun_LOG" -F + rlAssertGrep "pass /test/check-fail-info" "$rlRun_LOG" + rlAssertGrep " info dmesg (before-test check)" "$rlRun_LOG" + rlAssertGrep " info dmesg (after-test check)" "$rlRun_LOG" + + rlAssertGrep "fail /test/check-fail-respect (check 'dmesg' failed, original result: pass)" "$rlRun_LOG" + rlAssertGrep " pass dmesg (before-test check)" "$rlRun_LOG" + rlAssertGrep " fail dmesg (after-test check)" "$rlRun_LOG" + + rlAssertGrep "pass /test/check-override" "$rlRun_LOG" + rlAssertGrep " pass dmesg (before-test check)" "$rlRun_LOG" + rlAssertGrep " fail dmesg (after-test check)" "$rlRun_LOG" + + rlAssertGrep "pass /test/check-pass" "$rlRun_LOG" + rlAssertGrep " pass dmesg (before-test check)" "$rlRun_LOG" + rlAssertGrep " pass dmesg (after-test check)" "$rlRun_LOG" + + rlAssertGrep "pass /test/check-xfail-fail" "$rlRun_LOG" + rlAssertGrep " fail dmesg (before-test check)" "$rlRun_LOG" + rlAssertGrep " pass dmesg (after-test check)" "$rlRun_LOG" + + rlAssertGrep "fail /test/check-xfail-pass (check 'dmesg' failed, original result: pass)" "$rlRun_LOG" + rlAssertGrep " fail dmesg (before-test check)" "$rlRun_LOG" + rlAssertGrep " fail dmesg (after-test check)" "$rlRun_LOG" rlPhaseEnd rlPhaseStartCleanup diff --git a/tmt/result.py b/tmt/result.py index 52dbd58636..5cfbd9c09c 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -1,6 +1,7 @@ import dataclasses import enum import re +from collections import defaultdict from typing import TYPE_CHECKING, Any, Callable, Optional, cast import click @@ -197,13 +198,9 @@ def interpret_check_result(self, interpret: CheckResultInterpret) -> 'CheckResul if interpret == CheckResultInterpret.INFO: self.result = ResultOutcome.INFO elif interpret == CheckResultInterpret.XFAIL: - mapping = ( - # if 'xfail' BEFORE_TEST check passed, use WARN, otherwise switch PASS<->FAIL - {ResultOutcome.FAIL: ResultOutcome.PASS, ResultOutcome.PASS: ResultOutcome.WARN} - if self.event == CheckEvent.BEFORE_TEST - else - {ResultOutcome.FAIL: ResultOutcome.PASS, ResultOutcome.PASS: ResultOutcome.FAIL} - ) + mapping = { + ResultOutcome.FAIL: ResultOutcome.PASS, + ResultOutcome.PASS: ResultOutcome.FAIL} self.result = mapping.get(self.result, self.result) return self @@ -374,12 +371,27 @@ def interpret_result( if interpret == ResultInterpret.CUSTOM: return self - # Interpret check results - self.check = [ - check_result.interpret_check_result(interpret_checks[check_result.name]) - for check_result in self.check - ] + # Group check phases by the check name (how) + check_groups: dict[str, list[CheckResult]] = defaultdict(list) + for check_result in self.check: + check_groups[check_result.name].append(check_result) + + # Process each group of check results + failed_checks: list[str] = [] + for how, group in check_groups.items(): + # Apply interpretation to each check result in the group + interpreted_results = [ + check_result.interpret_check_result(interpret_checks[how]) + for check_result in group + ] + # Reduce the group to a single result + reduced_outcome = aggregate_check_results(interpreted_results, interpret_checks[how]) + + if reduced_outcome == ResultOutcome.FAIL: + failed_checks.append(how) + + # Check results are interpreted, deal with test results that are not affected by checks if interpret not in (ResultInterpret.RESPECT, ResultInterpret.XFAIL): self.result = ResultOutcome(interpret.value) @@ -390,15 +402,9 @@ def interpret_result( return self - failed_checks = [ - check_result - for check_result in self.check - if check_result.result == ResultOutcome.FAIL - ] - if failed_checks: self.result = ResultOutcome.FAIL - check_note = ", ".join([f"check '{check.name}' failed" for check in failed_checks]) + check_note = ", ".join([f"check '{check}' failed" for check in failed_checks]) self.note = f"{self.note}, {check_note}" if self.note else check_note if interpret == ResultInterpret.XFAIL: @@ -559,3 +565,25 @@ def results_to_exit_code(results: list[Result]) -> int: return TmtExitCode.SUCCESS raise GeneralError("Unhandled combination of test result.") + + +def aggregate_check_results(results: list['CheckResult'], + interpret: CheckResultInterpret) -> ResultOutcome: + """ + Reduce multiple check results to a single outcome based on interpretation. + + :param results: List of check results to reduce + :param interpret: How to interpret the results + :returns: A single ResultOutcome representing the aggregated result + """ + if not results: + return ResultOutcome.PASS + + # For xfail, if any result is PASS(i.e. failed), the overall result is PASS + if interpret == CheckResultInterpret.XFAIL: + return ResultOutcome.PASS if any( + r.result == ResultOutcome.PASS for r in results) else ResultOutcome.FAIL + + # For all other cases, if any result is FAIL, the overall result is FAIL + return ResultOutcome.FAIL if any( + r.result == ResultOutcome.FAIL for r in results) else ResultOutcome.PASS From 308f78a2501461b83f505c21eef94c3a9217cdc2 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Thu, 24 Oct 2024 16:03:08 +0200 Subject: [PATCH 27/35] Unit tests attempt 1 --- tests/unit/test_results.py | 268 ++++++++++++++++++++++++++++++++++++- 1 file changed, 267 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_results.py b/tests/unit/test_results.py index 15e57f55ff..af38cfb62a 100644 --- a/tests/unit/test_results.py +++ b/tests/unit/test_results.py @@ -1,9 +1,19 @@ +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, + aggregate_check_results, + results_to_exit_code, + ) @pytest.mark.parametrize( @@ -76,3 +86,259 @@ 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( + ('check_results', 'interpret', 'expected_outcome'), + [ + # Empty check results should pass + ([], CheckResultInterpret.RESPECT, ResultOutcome.PASS), + ([], CheckResultInterpret.INFO, ResultOutcome.PASS), + ([], CheckResultInterpret.XFAIL, ResultOutcome.PASS), + + # Single check with single phase + ( + [CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST)], + CheckResultInterpret.RESPECT, + ResultOutcome.PASS + ), + ( + [CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST)], + CheckResultInterpret.RESPECT, + ResultOutcome.FAIL + ), + + # Single check with both phases + ( + [ + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.AFTER_TEST) + ], + CheckResultInterpret.RESPECT, + ResultOutcome.PASS + ), + ( + [ + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), + CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST) + ], + CheckResultInterpret.RESPECT, + ResultOutcome.FAIL + ), + + # Multiple different checks + ( + [ + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), + CheckResult(name="test2", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST) + ], + CheckResultInterpret.RESPECT, + ResultOutcome.PASS + ), + ( + [ + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), + CheckResult(name="test2", result=ResultOutcome.FAIL, event=CheckEvent.BEFORE_TEST) + ], + CheckResultInterpret.RESPECT, + ResultOutcome.FAIL + ), + + # Check with XFAIL interpretation + ( + [ + CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.BEFORE_TEST), + CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST) + ], + CheckResultInterpret.XFAIL, + ResultOutcome.FAIL + ), + ( + [ + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), + CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST) + ], + CheckResultInterpret.XFAIL, + ResultOutcome.PASS + ), + + # Check with INFO interpretation + ( + [ + CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.BEFORE_TEST), + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.AFTER_TEST) + ], + CheckResultInterpret.INFO, + ResultOutcome.FAIL + ), + ], + ids=[ + "empty-respect", "empty-info", "empty-xfail", + "single-phase-pass", "single-phase-fail", + "both-phases-pass", "both-phases-mixed", + "different-checks-pass", "different-checks-mixed", + "xfail-both-fail", "xfail-mixed", + "info-mixed" + ] + ) +def test_aggregate_check_results( + check_results: list[CheckResult], + interpret: CheckResultInterpret, + expected_outcome: ResultOutcome + ) -> None: + assert aggregate_check_results(check_results, interpret) == expected_outcome + + +@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, + "original result: fail" + ), + ( + ResultOutcome.PASS, + ResultInterpret.XFAIL, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.FAIL, + "original result: pass" + ), + + # Test INFO interpretation + ( + ResultOutcome.FAIL, + ResultInterpret.INFO, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.INFO, + "original result: fail" + ), + ( + ResultOutcome.PASS, + ResultInterpret.INFO, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.INFO, + "original result: pass" + ), + + # Test WARN interpretation + ( + ResultOutcome.PASS, + ResultInterpret.WARN, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.WARN, + "original result: pass" + ), + + # Test ERROR interpretation + ( + ResultOutcome.PASS, + ResultInterpret.ERROR, + {"check1": CheckResultInterpret.RESPECT}, + ResultOutcome.ERROR, + "original 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'" not in interpreted.note # check2 passed + + +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 From 03877baad84fb2b00477109668fa6f3f11442bd1 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Thu, 24 Oct 2024 16:45:38 +0200 Subject: [PATCH 28/35] Aggregating check results first --- tests/unit/test_results.py | 27 ++++++++++++++++++++------- tmt/result.py | 23 ++++++++++++----------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/tests/unit/test_results.py b/tests/unit/test_results.py index af38cfb62a..0fc72ca313 100644 --- a/tests/unit/test_results.py +++ b/tests/unit/test_results.py @@ -91,7 +91,7 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code: @pytest.mark.parametrize( ('check_results', 'interpret', 'expected_outcome'), [ - # Empty check results should pass + # Empty results default to PASS ([], CheckResultInterpret.RESPECT, ResultOutcome.PASS), ([], CheckResultInterpret.INFO, ResultOutcome.PASS), ([], CheckResultInterpret.XFAIL, ResultOutcome.PASS), @@ -151,15 +151,15 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code: CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST) ], CheckResultInterpret.XFAIL, - ResultOutcome.FAIL + ResultOutcome.PASS ), ( [ CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), - CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST) + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.AFTER_TEST) ], CheckResultInterpret.XFAIL, - ResultOutcome.PASS + ResultOutcome.FAIL ), # Check with INFO interpretation @@ -169,7 +169,15 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code: CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.AFTER_TEST) ], CheckResultInterpret.INFO, - ResultOutcome.FAIL + ResultOutcome.INFO + ), + ( + [ + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), + CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.AFTER_TEST) + ], + CheckResultInterpret.INFO, + ResultOutcome.INFO ), ], ids=[ @@ -177,8 +185,8 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code: "single-phase-pass", "single-phase-fail", "both-phases-pass", "both-phases-mixed", "different-checks-pass", "different-checks-mixed", - "xfail-both-fail", "xfail-mixed", - "info-mixed" + "xfail-both-fail", "xfail-pass", + "info-mixed", "info-pass" ] ) def test_aggregate_check_results( @@ -328,6 +336,11 @@ def test_result_interpret_check_phases() -> None: assert "check 'check1' failed" in interpreted.note assert "check 'check2'" not in interpreted.note # check2 passed + # 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.INFO # check2 BEFORE_TEST (INFO) + def test_result_interpret_edge_cases() -> None: """Test edge cases in result interpretation""" diff --git a/tmt/result.py b/tmt/result.py index 5cfbd9c09c..625beda02f 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -379,18 +379,16 @@ def interpret_result( # Process each group of check results failed_checks: list[str] = [] for how, group in check_groups.items(): - # Apply interpretation to each check result in the group - interpreted_results = [ - check_result.interpret_check_result(interpret_checks[how]) - for check_result in group - ] - - # Reduce the group to a single result - reduced_outcome = aggregate_check_results(interpreted_results, interpret_checks[how]) - + reduced_outcome = aggregate_check_results(group, interpret_checks[how]) if reduced_outcome == ResultOutcome.FAIL: failed_checks.append(how) + # Interpret individual checks + self.check = [ + check_result.interpret_check_result(interpret_checks[check_result.name]) + for check_result in self.check + ] + # Check results are interpreted, deal with test results that are not affected by checks if interpret not in (ResultInterpret.RESPECT, ResultInterpret.XFAIL): self.result = ResultOutcome(interpret.value) @@ -579,10 +577,13 @@ def aggregate_check_results(results: list['CheckResult'], if not results: return ResultOutcome.PASS - # For xfail, if any result is PASS(i.e. failed), the overall result is PASS + # For xfail, if any result is FAIL, the overall result is PASS if interpret == CheckResultInterpret.XFAIL: return ResultOutcome.PASS if any( - r.result == ResultOutcome.PASS for r in results) else ResultOutcome.FAIL + r.result == ResultOutcome.FAIL for r in results) else ResultOutcome.FAIL + + if interpret == CheckResultInterpret.INFO: + return ResultOutcome.INFO # For all other cases, if any result is FAIL, the overall result is FAIL return ResultOutcome.FAIL if any( From 21181dd69b03e6114a7fc85d9bd918331401cc16 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Thu, 24 Oct 2024 17:31:28 +0200 Subject: [PATCH 29/35] Aggregate check results attempt 2 --- tests/execute/result/check.sh | 17 -------- tests/unit/test_results.py | 5 +-- tmt/result.py | 75 +++++++++++++---------------------- 3 files changed, 30 insertions(+), 67 deletions(-) diff --git a/tests/execute/result/check.sh b/tests/execute/result/check.sh index 29aeeda428..c18941eb3f 100755 --- a/tests/execute/result/check.sh +++ b/tests/execute/result/check.sh @@ -13,28 +13,11 @@ rlJournalStart rlRun -s "tmt run -a --id \${run} --scratch tests provision --how $PROVISION_HOW report -v 2>&1 >/dev/null | grep report -A19" "1" rlAssertGrep "pass /test/check-fail-info" "$rlRun_LOG" - rlAssertGrep " info dmesg (before-test check)" "$rlRun_LOG" - rlAssertGrep " info dmesg (after-test check)" "$rlRun_LOG" - rlAssertGrep "fail /test/check-fail-respect (check 'dmesg' failed, original result: pass)" "$rlRun_LOG" - rlAssertGrep " pass dmesg (before-test check)" "$rlRun_LOG" - rlAssertGrep " fail dmesg (after-test check)" "$rlRun_LOG" - rlAssertGrep "pass /test/check-override" "$rlRun_LOG" - rlAssertGrep " pass dmesg (before-test check)" "$rlRun_LOG" - rlAssertGrep " fail dmesg (after-test check)" "$rlRun_LOG" - rlAssertGrep "pass /test/check-pass" "$rlRun_LOG" - rlAssertGrep " pass dmesg (before-test check)" "$rlRun_LOG" - rlAssertGrep " pass dmesg (after-test check)" "$rlRun_LOG" - rlAssertGrep "pass /test/check-xfail-fail" "$rlRun_LOG" - rlAssertGrep " fail dmesg (before-test check)" "$rlRun_LOG" - rlAssertGrep " pass dmesg (after-test check)" "$rlRun_LOG" - rlAssertGrep "fail /test/check-xfail-pass (check 'dmesg' failed, original result: pass)" "$rlRun_LOG" - rlAssertGrep " fail dmesg (before-test check)" "$rlRun_LOG" - rlAssertGrep " fail dmesg (after-test check)" "$rlRun_LOG" rlPhaseEnd rlPhaseStartCleanup diff --git a/tests/unit/test_results.py b/tests/unit/test_results.py index 0fc72ca313..b26e68ea15 100644 --- a/tests/unit/test_results.py +++ b/tests/unit/test_results.py @@ -11,7 +11,6 @@ Result, ResultInterpret, ResultOutcome, - aggregate_check_results, results_to_exit_code, ) @@ -194,7 +193,7 @@ def test_aggregate_check_results( interpret: CheckResultInterpret, expected_outcome: ResultOutcome ) -> None: - assert aggregate_check_results(check_results, interpret) == expected_outcome + assert Result.aggregate_check_results(check_results, interpret) == expected_outcome @pytest.mark.parametrize( @@ -339,7 +338,7 @@ def test_result_interpret_check_phases() -> None: # 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.INFO # check2 BEFORE_TEST (INFO) + assert interpreted.check[2].result == ResultOutcome.PASS # check2 BEFORE_TEST (INFO) def test_result_interpret_edge_cases() -> None: diff --git a/tmt/result.py b/tmt/result.py index 625beda02f..bf254c2eb9 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -190,21 +190,6 @@ class CheckResult(BaseResult): serialize=lambda event: event.value, unserialize=CheckEvent.from_spec) - def interpret_check_result(self, interpret: CheckResultInterpret) -> 'CheckResult': - """Interpret check result according to the check result interpretation.""" - - if interpret == CheckResultInterpret.RESPECT: - return self - if interpret == CheckResultInterpret.INFO: - self.result = ResultOutcome.INFO - elif interpret == CheckResultInterpret.XFAIL: - mapping = { - ResultOutcome.FAIL: ResultOutcome.PASS, - ResultOutcome.PASS: ResultOutcome.FAIL} - self.result = mapping.get(self.result, self.result) - - return self - def to_subcheck(self) -> 'SubCheckResult': """ Convert check to a tmt SubCheckResult """ @@ -282,6 +267,33 @@ class Result(BaseResult): unserialize=lambda value: None if value is None else Path(value) ) + @staticmethod + def aggregate_check_results(results: list[CheckResult], + interpret: CheckResultInterpret) -> ResultOutcome: + """ + Reduce multiple check results to a single outcome based on interpretation. + + This method aggregates multiple check results into a single outcome following + specific interpretation rules: + - For XFAIL: If any result is FAIL, returns PASS; otherwise returns FAIL + - For INFO: Always returns INFO + - For RESPECT: Returns FAIL if any check failed, otherwise PASS + """ + if not results: + return ResultOutcome.PASS + + # For xfail, if any result is FAIL, the overall result is PASS + if interpret == CheckResultInterpret.XFAIL: + return ResultOutcome.PASS if any( + r.result == ResultOutcome.FAIL for r in results) else ResultOutcome.FAIL + + if interpret == CheckResultInterpret.INFO: + return ResultOutcome.INFO + + # For all other cases, if any result is FAIL, the overall result is FAIL + return ResultOutcome.FAIL if any( + r.result == ResultOutcome.FAIL for r in results) else ResultOutcome.PASS + @classmethod def from_test_invocation( cls, @@ -379,16 +391,10 @@ def interpret_result( # Process each group of check results failed_checks: list[str] = [] for how, group in check_groups.items(): - reduced_outcome = aggregate_check_results(group, interpret_checks[how]) + reduced_outcome = self.aggregate_check_results(group, interpret_checks[how]) if reduced_outcome == ResultOutcome.FAIL: failed_checks.append(how) - # Interpret individual checks - self.check = [ - check_result.interpret_check_result(interpret_checks[check_result.name]) - for check_result in self.check - ] - # Check results are interpreted, deal with test results that are not affected by checks if interpret not in (ResultInterpret.RESPECT, ResultInterpret.XFAIL): self.result = ResultOutcome(interpret.value) @@ -563,28 +569,3 @@ def results_to_exit_code(results: list[Result]) -> int: return TmtExitCode.SUCCESS raise GeneralError("Unhandled combination of test result.") - - -def aggregate_check_results(results: list['CheckResult'], - interpret: CheckResultInterpret) -> ResultOutcome: - """ - Reduce multiple check results to a single outcome based on interpretation. - - :param results: List of check results to reduce - :param interpret: How to interpret the results - :returns: A single ResultOutcome representing the aggregated result - """ - if not results: - return ResultOutcome.PASS - - # For xfail, if any result is FAIL, the overall result is PASS - if interpret == CheckResultInterpret.XFAIL: - return ResultOutcome.PASS if any( - r.result == ResultOutcome.FAIL for r in results) else ResultOutcome.FAIL - - if interpret == CheckResultInterpret.INFO: - return ResultOutcome.INFO - - # For all other cases, if any result is FAIL, the overall result is FAIL - return ResultOutcome.FAIL if any( - r.result == ResultOutcome.FAIL for r in results) else ResultOutcome.PASS From 4b1972a2b7b4f1e2c1e8bf2406e8ef51296add9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Fri, 25 Oct 2024 12:16:23 +0200 Subject: [PATCH 30/35] Aggregate check results attempt 3 --- tmt/result.py | 121 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 93 insertions(+), 28 deletions(-) diff --git a/tmt/result.py b/tmt/result.py index bf254c2eb9..bcf93fb735 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -1,7 +1,6 @@ import dataclasses import enum import re -from collections import defaultdict from typing import TYPE_CHECKING, Any, Callable, Optional, cast import click @@ -38,9 +37,35 @@ 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 + + class ResultInterpret(enum.Enum): # These are "inherited" from ResultOutcome PASS = 'pass' @@ -359,6 +384,53 @@ 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, + 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 it's 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") + + elif interpret == CheckResultInterpret.INFO: + interpreted_outcome = ResultOutcome.INFO + self.append_note(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") + + 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, @@ -377,51 +449,44 @@ def interpret_result( if interpret not in ResultInterpret: raise tmt.utils.SpecificationError( - f"Invalid result '{interpret.value}' in test '{self.name}'." - ) + f"Invalid result '{interpret.value}' in test '{self.name}'.") if interpret == ResultInterpret.CUSTOM: return self - # Group check phases by the check name (how) - check_groups: dict[str, list[CheckResult]] = defaultdict(list) - for check_result in self.check: - check_groups[check_result.name].append(check_result) + # 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)) - # Process each group of check results - failed_checks: list[str] = [] - for how, group in check_groups.items(): - reduced_outcome = self.aggregate_check_results(group, interpret_checks[how]) - if reduced_outcome == ResultOutcome.FAIL: - failed_checks.append(how) + # Aggregate check results with the main test result + self.result = ResultOutcome.reduce([self.result, *check_outcomes]) - # Check results are interpreted, deal with test results that are not affected by checks + # 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: - orig_note = f"original result: {self.original_result.value}" - self.note = f"{self.note}, {orig_note}" if self.note else orig_note + self.append_note(f"original test result: {self.original_result.value}") return self - if failed_checks: - self.result = ResultOutcome.FAIL - check_note = ", ".join([f"check '{check}' failed" for check in failed_checks]) - self.note = f"{self.note}, {check_note}" if self.note else check_note - + # Handle the expected fail if interpret == ResultInterpret.XFAIL: - # Swap fail<-->pass - self.result = { - ResultOutcome.FAIL: ResultOutcome.PASS, - ResultOutcome.PASS: ResultOutcome.FAIL, - }.get(self.result, self.result) + + if self.result == ResultOutcome.PASS: + self.result = ResultOutcome.FAIL + self.append_note("test was expected to fail") + + if 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: - orig_note = f"original result: {self.original_result.value}" - self.note = f"{self.note}, {orig_note}" if self.note else orig_note + self.append_note(f"original test result: {self.original_result.value}") return self From 3f75c53f6bc137d2eb7436189ae83adb39883dff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Fri, 25 Oct 2024 13:05:20 +0200 Subject: [PATCH 31/35] Adjust tests accordingly --- tests/execute/result/check.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/execute/result/check.sh b/tests/execute/result/check.sh index c18941eb3f..17fbfc458c 100755 --- a/tests/execute/result/check.sh +++ b/tests/execute/result/check.sh @@ -10,14 +10,15 @@ rlJournalStart rlPhaseEnd rlPhaseStartTest "Check Results" - rlRun -s "tmt run -a --id \${run} --scratch tests provision --how $PROVISION_HOW report -v 2>&1 >/dev/null | grep report -A19" "1" + rlRun "tmt run -av --id $run provision --how $PROVISION_HOW" 1 + rlRun -s "tmt run --id $run report -v" 1 - rlAssertGrep "pass /test/check-fail-info" "$rlRun_LOG" - rlAssertGrep "fail /test/check-fail-respect (check 'dmesg' failed, original result: pass)" "$rlRun_LOG" - rlAssertGrep "pass /test/check-override" "$rlRun_LOG" + rlAssertGrep "pass /test/check-fail-info (check 'dmesg' is informational)" "$rlRun_LOG" + rlAssertGrep "fail /test/check-fail-respect (check 'dmesg' failed, original test result: pass)" "$rlRun_LOG" + rlAssertGrep "pass /test/check-override (check 'dmesg' failed, test result overridden: pass)" "$rlRun_LOG" rlAssertGrep "pass /test/check-pass" "$rlRun_LOG" - rlAssertGrep "pass /test/check-xfail-fail" "$rlRun_LOG" - rlAssertGrep "fail /test/check-xfail-pass (check 'dmesg' failed, original result: pass)" "$rlRun_LOG" + rlAssertGrep "pass /test/check-xfail-fail (check 'dmesg' failed as expected)" "$rlRun_LOG" + rlAssertGrep "fail /test/check-xfail-pass (check 'dmesg' did not fail as expected, original test result: pass)" "$rlRun_LOG" rlPhaseEnd rlPhaseStartCleanup From 45889d71f79aace129528f9a35fa5b9a9aa348b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Fri, 25 Oct 2024 13:49:07 +0200 Subject: [PATCH 32/35] Adjust tests accordingly --- tests/execute/result/check.sh | 1 + tests/execute/result/check/test.fmf | 10 ++++++++++ tests/unit/test_results.py | 14 +++++++------- tmt/result.py | 5 ++--- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/tests/execute/result/check.sh b/tests/execute/result/check.sh index 17fbfc458c..426ceaabec 100755 --- a/tests/execute/result/check.sh +++ b/tests/execute/result/check.sh @@ -17,6 +17,7 @@ rlJournalStart rlAssertGrep "fail /test/check-fail-respect (check 'dmesg' failed, original test result: pass)" "$rlRun_LOG" rlAssertGrep "pass /test/check-override (check 'dmesg' failed, test result overridden: pass)" "$rlRun_LOG" rlAssertGrep "pass /test/check-pass" "$rlRun_LOG" + rlAssertGrep "fail /test/check-pass-test-xfail (test was expected to fail, original test result: pass)" "$rlRun_LOG" rlAssertGrep "pass /test/check-xfail-fail (check 'dmesg' failed as expected)" "$rlRun_LOG" rlAssertGrep "fail /test/check-xfail-pass (check 'dmesg' did not fail as expected, original test result: pass)" "$rlRun_LOG" rlPhaseEnd diff --git a/tests/execute/result/check/test.fmf b/tests/execute/result/check/test.fmf index 482f305978..375f17cf2b 100644 --- a/tests/execute/result/check/test.fmf +++ b/tests/execute/result/check/test.fmf @@ -12,6 +12,16 @@ duration: 1m - how: dmesg result: respect +/check-pass-test-xfail: + summary: Everything passing but failure expected + description: | + Expected outcome: FAIL (test passes, check passes, but fail is expected) + test: echo "Test passed" + check: + - how: dmesg + result: respect + result: xfail + /check-fail-respect: summary: Test with failing dmesg check (respect) description: | diff --git a/tests/unit/test_results.py b/tests/unit/test_results.py index b26e68ea15..d924367c16 100644 --- a/tests/unit/test_results.py +++ b/tests/unit/test_results.py @@ -225,14 +225,14 @@ def test_aggregate_check_results( ResultInterpret.XFAIL, {"check1": CheckResultInterpret.RESPECT}, ResultOutcome.PASS, - "original result: fail" + "check 'check1' failed, test failed as expected, original test result: fail" ), ( ResultOutcome.PASS, ResultInterpret.XFAIL, {"check1": CheckResultInterpret.RESPECT}, ResultOutcome.FAIL, - "original result: pass" + "test was expected to fail, original test result: pass" ), # Test INFO interpretation @@ -241,14 +241,14 @@ def test_aggregate_check_results( ResultInterpret.INFO, {"check1": CheckResultInterpret.RESPECT}, ResultOutcome.INFO, - "original result: fail" + "check 'check1' failed, test result overridden: info, original test result: fail" ), ( ResultOutcome.PASS, ResultInterpret.INFO, {"check1": CheckResultInterpret.RESPECT}, ResultOutcome.INFO, - "original result: pass" + "test result overridden: info, original test result: pass" ), # Test WARN interpretation @@ -257,7 +257,7 @@ def test_aggregate_check_results( ResultInterpret.WARN, {"check1": CheckResultInterpret.RESPECT}, ResultOutcome.WARN, - "original result: pass" + "test result overridden: warn, original test result: pass" ), # Test ERROR interpretation @@ -266,7 +266,7 @@ def test_aggregate_check_results( ResultInterpret.ERROR, {"check1": CheckResultInterpret.RESPECT}, ResultOutcome.ERROR, - "original result: pass" + "test result overridden: error, original test result: pass" ), # Test CUSTOM interpretation (should not modify result) @@ -333,7 +333,7 @@ def test_result_interpret_check_phases() -> None: interpreted = result.interpret_result(ResultInterpret.RESPECT, interpret_checks) assert interpreted.note is not None assert "check 'check1' failed" in interpreted.note - assert "check 'check2'" not in interpreted.note # check2 passed + assert "check 'check2' is informational" in interpreted.note # Verify individual check results were interpreted assert interpreted.check[0].result == ResultOutcome.PASS # check1 BEFORE_TEST diff --git a/tmt/result.py b/tmt/result.py index bcf93fb735..e26bea0076 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -62,10 +62,9 @@ def reduce(outcomes: list['ResultOutcome']) -> 'ResultOutcome': raise GeneralError("No result outcome found to reduce.") + # Cannot subclass enums :/ # https://docs.python.org/3/library/enum.html#restricted-enum-subclassing - - class ResultInterpret(enum.Enum): # These are "inherited" from ResultOutcome PASS = 'pass' @@ -480,7 +479,7 @@ def interpret_result( self.result = ResultOutcome.FAIL self.append_note("test was expected to fail") - if self.result == ResultOutcome.FAIL: + elif self.result == ResultOutcome.FAIL: self.result = ResultOutcome.PASS self.append_note("test failed as expected") From b6f152b709355fb5d374ef543184edf8bd7181f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Fri, 25 Oct 2024 14:19:27 +0200 Subject: [PATCH 33/35] Drop `aggregate_check_results` and related test --- tests/unit/test_results.py | 109 ------------------------------------- tmt/result.py | 27 --------- 2 files changed, 136 deletions(-) diff --git a/tests/unit/test_results.py b/tests/unit/test_results.py index d924367c16..53540a8595 100644 --- a/tests/unit/test_results.py +++ b/tests/unit/test_results.py @@ -87,115 +87,6 @@ def test_result_to_exit_code(outcomes: list[ResultOutcome], expected_exit_code: == expected_exit_code -@pytest.mark.parametrize( - ('check_results', 'interpret', 'expected_outcome'), - [ - # Empty results default to PASS - ([], CheckResultInterpret.RESPECT, ResultOutcome.PASS), - ([], CheckResultInterpret.INFO, ResultOutcome.PASS), - ([], CheckResultInterpret.XFAIL, ResultOutcome.PASS), - - # Single check with single phase - ( - [CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST)], - CheckResultInterpret.RESPECT, - ResultOutcome.PASS - ), - ( - [CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST)], - CheckResultInterpret.RESPECT, - ResultOutcome.FAIL - ), - - # Single check with both phases - ( - [ - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.AFTER_TEST) - ], - CheckResultInterpret.RESPECT, - ResultOutcome.PASS - ), - ( - [ - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), - CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST) - ], - CheckResultInterpret.RESPECT, - ResultOutcome.FAIL - ), - - # Multiple different checks - ( - [ - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), - CheckResult(name="test2", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST) - ], - CheckResultInterpret.RESPECT, - ResultOutcome.PASS - ), - ( - [ - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), - CheckResult(name="test2", result=ResultOutcome.FAIL, event=CheckEvent.BEFORE_TEST) - ], - CheckResultInterpret.RESPECT, - ResultOutcome.FAIL - ), - - # Check with XFAIL interpretation - ( - [ - CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.BEFORE_TEST), - CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.AFTER_TEST) - ], - CheckResultInterpret.XFAIL, - ResultOutcome.PASS - ), - ( - [ - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.AFTER_TEST) - ], - CheckResultInterpret.XFAIL, - ResultOutcome.FAIL - ), - - # Check with INFO interpretation - ( - [ - CheckResult(name="test1", result=ResultOutcome.FAIL, event=CheckEvent.BEFORE_TEST), - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.AFTER_TEST) - ], - CheckResultInterpret.INFO, - ResultOutcome.INFO - ), - ( - [ - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.BEFORE_TEST), - CheckResult(name="test1", result=ResultOutcome.PASS, event=CheckEvent.AFTER_TEST) - ], - CheckResultInterpret.INFO, - ResultOutcome.INFO - ), - ], - ids=[ - "empty-respect", "empty-info", "empty-xfail", - "single-phase-pass", "single-phase-fail", - "both-phases-pass", "both-phases-mixed", - "different-checks-pass", "different-checks-mixed", - "xfail-both-fail", "xfail-pass", - "info-mixed", "info-pass" - ] - ) -def test_aggregate_check_results( - check_results: list[CheckResult], - interpret: CheckResultInterpret, - expected_outcome: ResultOutcome - ) -> None: - assert Result.aggregate_check_results(check_results, interpret) == expected_outcome - - @pytest.mark.parametrize( ('result_outcome', 'interpret', diff --git a/tmt/result.py b/tmt/result.py index e26bea0076..a0d9f63310 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -291,33 +291,6 @@ class Result(BaseResult): unserialize=lambda value: None if value is None else Path(value) ) - @staticmethod - def aggregate_check_results(results: list[CheckResult], - interpret: CheckResultInterpret) -> ResultOutcome: - """ - Reduce multiple check results to a single outcome based on interpretation. - - This method aggregates multiple check results into a single outcome following - specific interpretation rules: - - For XFAIL: If any result is FAIL, returns PASS; otherwise returns FAIL - - For INFO: Always returns INFO - - For RESPECT: Returns FAIL if any check failed, otherwise PASS - """ - if not results: - return ResultOutcome.PASS - - # For xfail, if any result is FAIL, the overall result is PASS - if interpret == CheckResultInterpret.XFAIL: - return ResultOutcome.PASS if any( - r.result == ResultOutcome.FAIL for r in results) else ResultOutcome.FAIL - - if interpret == CheckResultInterpret.INFO: - return ResultOutcome.INFO - - # For all other cases, if any result is FAIL, the overall result is FAIL - return ResultOutcome.FAIL if any( - r.result == ResultOutcome.FAIL for r in results) else ResultOutcome.PASS - @classmethod def from_test_invocation( cls, From cd0527d4900f97c3667a414a6f34a951d7b26252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Fri, 25 Oct 2024 14:45:51 +0200 Subject: [PATCH 34/35] Add warning, fix typos, extend release note a bit --- docs/releases.rst | 15 ++++++++------- spec/tests/check.fmf | 6 ++++-- tmt/result.py | 4 ++-- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/docs/releases.rst b/docs/releases.rst index 98240b6b7a..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 @@ -72,13 +80,6 @@ for example usage. The :ref:`/plugins/provision/beaker` provision plugin gains support for :ref:`cpu.stepping` hardware requirement. -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. - tmt-1.37.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/spec/tests/check.fmf b/spec/tests/check.fmf index 19d73ffef1..491b0510ed 100644 --- a/spec/tests/check.fmf +++ b/spec/tests/check.fmf @@ -30,8 +30,10 @@ description: | The check result is treated as an informational message and does not affect the overall test result. - Note that running one check multiple times for the same test is - not yet supported. + .. warning:: + + Note that running one check multiple times for the same + test is not yet supported. .. versionchanged:: 1.38.0 the ``result`` key added diff --git a/tmt/result.py b/tmt/result.py index a0d9f63310..be495387b4 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -371,7 +371,7 @@ def interpret_check_result( 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 it's result interpret + :param interpret_checks: mapping of check:how and its result interpret :returns: :py:class:`ResultOutcome` instance with the interpreted result """ @@ -415,7 +415,7 @@ def interpret_result( attributes, following the ``interpret`` value. :param interpret: how to interpret current result. - :param interpret_checks: mapping of check:how and it's result interpret + :param interpret_checks: mapping of check:how and its result interpret :returns: :py:class:`Result` instance containing the updated result. """ From 64e2b0bf72c7309e985153e8068771de00781bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Fri, 25 Oct 2024 15:07:03 +0200 Subject: [PATCH 35/35] A couple of necessary test output check adjustments --- tests/discover/tests.sh | 2 +- tests/execute/result/basic.sh | 19 ++++++++++--------- tests/report/html/test.sh | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) 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 dbf636ad55..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 @@ -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) [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 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