From 41bb466684284088b5daf6f94b8b0252fb8595bd Mon Sep 17 00:00:00 2001
From: Martin Hoyer <mhoyer@redhat.com>
Date: Thu, 26 Sep 2024 19:01:33 +0200
Subject: [PATCH 1/6] Initial check-result implementation

Failing 'check' should affect test result by default.
respect_checks option needs to be implemented.
---
 tests/execute/result/check_results.sh       | 75 +++++++++++++++++++++
 tests/execute/result/check_results/main.fmf | 31 +++++++++
 tests/execute/result/main.fmf               |  3 +
 tmt/result.py                               | 62 +++++++++++++----
 tmt/steps/execute/internal.py               | 11 +--
 5 files changed, 165 insertions(+), 17 deletions(-)
 create mode 100644 tests/execute/result/check_results.sh
 create mode 100644 tests/execute/result/check_results/main.fmf

diff --git a/tests/execute/result/check_results.sh b/tests/execute/result/check_results.sh
new file mode 100644
index 0000000000..8fcabdfb1f
--- /dev/null
+++ b/tests/execute/result/check_results.sh
@@ -0,0 +1,75 @@
+#!/bin/bash
+# vim: dict+=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k
+. /usr/share/beakerlib/beakerlib.sh || exit 1
+
+rlJournalStart
+    rlPhaseStartSetup
+        rlRun "run=\$(mktemp -d)" 0 "Create run directory"
+        rlRun "pushd check_results"
+        rlRun "set -o pipefail"
+    rlPhaseEnd
+
+    rlPhaseStartTest "Check results are respected"
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-pass execute -vv report -vv 2>&1 >/dev/null" 0
+        rlAssertGrep "pass /test/check-pass" $rlRun_LOG
+        rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
+        rlAssertGrep "pass dmesg (after-test check)" $rlRun_LOG
+
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-fail execute -vv report -vv 2>&1 >/dev/null" 1
+        rlAssertGrep "fail /test/check-fail" $rlRun_LOG
+        rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
+        rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
+        rlAssertGrep "check failed" $rlRun_LOG
+
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-ignore execute -vv report -vv 2>&1 >/dev/null" 0
+        rlAssertGrep "pass /test/check-ignore" $rlRun_LOG
+        rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
+        rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
+        rlAssertNotGrep "check failed" $rlRun_LOG
+    rlPhaseEnd
+
+    rlPhaseStartTest "Verify results.yaml content"
+        results_file="${run}/default/plan/execute/results.yaml"
+        rlAssertExists "$results_file"
+
+        rlRun -s "yq e '.[0]' $results_file"
+        rlAssertGrep "name: /test/check-pass" $rlRun_LOG
+        rlAssertGrep "result: pass" $rlRun_LOG
+        rlAssertGrep "check:" $rlRun_LOG
+        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: pass" $rlRun_LOG
+        rlAssertGrep "  event: before-test" $rlRun_LOG
+        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: pass" $rlRun_LOG
+        rlAssertGrep "  event: after-test" $rlRun_LOG
+
+        rlRun -s "yq e '.[1]' $results_file"
+        rlAssertGrep "name: /test/check-fail" $rlRun_LOG
+        rlAssertGrep "result: fail" $rlRun_LOG
+        rlAssertGrep "note: check failed" $rlRun_LOG
+        rlAssertGrep "check:" $rlRun_LOG
+        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: pass" $rlRun_LOG
+        rlAssertGrep "  event: before-test" $rlRun_LOG
+        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: fail" $rlRun_LOG
+        rlAssertGrep "  event: after-test" $rlRun_LOG
+
+        rlRun -s "yq e '.[2]' $results_file"
+        rlAssertGrep "name: /test/check-ignore" $rlRun_LOG
+        rlAssertGrep "result: pass" $rlRun_LOG
+        rlAssertNotGrep "note: check failed" $rlRun_LOG
+        rlAssertGrep "check:" $rlRun_LOG
+        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: pass" $rlRun_LOG
+        rlAssertGrep "  event: before-test" $rlRun_LOG
+        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: fail" $rlRun_LOG
+        rlAssertGrep "  event: after-test" $rlRun_LOG
+    rlPhaseEnd
+
+    rlPhaseStartCleanup
+        rlRun "popd"
+        rlRun "rm -r ${run}" 0 "Remove run directory"
+    rlPhaseEnd
+rlJournalEnd
diff --git a/tests/execute/result/check_results/main.fmf b/tests/execute/result/check_results/main.fmf
new file mode 100644
index 0000000000..6b1b76fecd
--- /dev/null
+++ b/tests/execute/result/check_results/main.fmf
@@ -0,0 +1,31 @@
+summary: Tests for check results behavior
+description: Verify that check results, including after-test checks, are correctly handled
+
+/test/check-pass:
+    summary: Test with passing checks
+    test: echo "Test passed"
+    framework: shell
+    duration: 1m
+    check:
+        - how: dmesg
+
+/test/check-fail:
+    summary: Test with failing dmesg check
+    test: |
+        echo "Test passed"
+        echo "Call Trace:" >> /dev/kmsg
+    framework: shell
+    duration: 1m
+    check:
+        - how: dmesg
+
+/test/check-ignore:
+    summary: Test with failing dmesg check but ignored
+    test: |
+        echo "Test passed"
+        echo "Call Trace:" >> /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 e5d21e3e30..0e9a6729c1 100644
--- a/tests/execute/result/main.fmf
+++ b/tests/execute/result/main.fmf
@@ -10,3 +10,6 @@
 /special:
     summary: Test special characters generated to tmt-report-results.yaml
     test: ./special.sh
+/check_results:
+    summary: Test behavior of check results, including after-test checks
+    test: ./check_results.sh
diff --git a/tmt/result.py b/tmt/result.py
index 546346afa6..b83d0ce25c 100644
--- a/tmt/result.py
+++ b/tmt/result.py
@@ -261,6 +261,11 @@ class Result(BaseResult):
         serialize=lambda path: None if path is None else str(path),
         unserialize=lambda value: None if value is None else Path(value)
         )
+    respect_checks: bool = field(
+        default=True,
+        serialize=lambda value: value,
+        unserialize=lambda value: value
+        )
 
     @classmethod
     def from_test_invocation(
@@ -270,7 +275,8 @@ def from_test_invocation(
             result: ResultOutcome,
             note: Optional[str] = None,
             ids: Optional[ResultIds] = None,
-            log: Optional[list[Path]] = None) -> 'Result':
+            log: Optional[list[Path]] = None,
+            respect_checks: bool = True) -> 'Result':
         """
         Create a result from a test invocation.
 
@@ -287,6 +293,7 @@ def from_test_invocation(
         :param ids: additional test IDs. They will be added to IDs extracted
             from the test.
         :param log: optional list of test logs.
+        :param respect_checks: whether to respect or ignore check results.
         """
 
         # Saving identifiable information for each test case so we can match them
@@ -318,7 +325,12 @@ def from_test_invocation(
             ids=ids,
             log=log or [],
             guest=ResultGuestData.from_test_invocation(invocation=invocation),
-            data_path=invocation.relative_test_data_path)
+            data_path=invocation.relative_test_data_path,
+            respect_checks=respect_checks,
+            check=invocation.check_results)
+
+        for check in _result.check:
+            print(f"  - Name: {check.name}, Result: {check.result}, Event: {check.event}")
 
         return _result.interpret_result(invocation.test.result)
 
@@ -333,7 +345,41 @@ def interpret_result(self, interpret: ResultInterpret) -> 'Result':
         :returns: :py:class:`Result` instance containing the updated result.
         """
 
-        if interpret in (ResultInterpret.RESPECT, ResultInterpret.CUSTOM):
+        if interpret == ResultInterpret.CUSTOM:
+            return self
+
+        # Check for failed checks
+        checks_failed = any(check.result == ResultOutcome.FAIL for check in self.check)
+
+        if interpret == ResultInterpret.RESPECT:
+            if self.respect_checks and checks_failed:
+                self.result = ResultOutcome.FAIL
+                if self.note:
+                    self.note += ', check failed'
+                else:
+                    self.note = 'check failed'
+            return self
+
+        # Handle XFAIL
+        if interpret == ResultInterpret.XFAIL:
+            if self.result == ResultOutcome.PASS and checks_failed:
+                self.result = ResultOutcome.FAIL
+                if self.note:
+                    self.note += ', check failed'
+                else:
+                    self.note = 'check failed'
+            elif self.result == ResultOutcome.FAIL:
+                self.result = ResultOutcome.PASS
+                if self.note:
+                    self.note += ', expected failure'
+                else:
+                    self.note = 'expected failure'
+            elif self.result == ResultOutcome.PASS:
+                self.result = ResultOutcome.FAIL
+                if self.note:
+                    self.note += ', unexpected pass'
+                else:
+                    self.note = 'unexpected pass'
             return self
 
         # Extend existing note or set a new one
@@ -347,15 +393,7 @@ def interpret_result(self, interpret: ResultInterpret) -> 'Result':
             raise tmt.utils.SpecificationError(
                 f"Test result note '{self.note}' must be a string.")
 
-        if interpret == ResultInterpret.XFAIL:
-            # Swap just fail<-->pass, keep the rest as is (info, warn,
-            # error)
-            self.result = {
-                ResultOutcome.FAIL: ResultOutcome.PASS,
-                ResultOutcome.PASS: ResultOutcome.FAIL
-                }.get(self.result, self.result)
-
-        elif ResultInterpret.is_result_outcome(interpret):
+        if ResultInterpret.is_result_outcome(interpret):
             self.result = ResultOutcome(interpret.value)
 
         else:
diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py
index 595d7b68e8..bdee6cac0d 100644
--- a/tmt/steps/execute/internal.py
+++ b/tmt/steps/execute/internal.py
@@ -453,17 +453,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 730fc74f183465983cca86ae80b5bcfc03e82874 Mon Sep 17 00:00:00 2001
From: Martin Hoyer <mhoyer@redhat.com>
Date: Mon, 30 Sep 2024 13:36:12 +0200
Subject: [PATCH 2/6] Add check-result key

---
 docs/releases.rst                           |  6 ++++
 spec/tests/result.fmf                       | 13 ++++++++
 tests/execute/result/check_results.sh       | 35 +++++++++++++++++----
 tests/execute/result/check_results/main.fmf | 26 ++++++++++++---
 tmt/base.py                                 |  1 +
 tmt/result.py                               | 18 +++++------
 tmt/schemas/test.yaml                       |  8 +++++
 7 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/docs/releases.rst b/docs/releases.rst
index c270cb6e0b..b5d5b3ce57 100644
--- a/docs/releases.rst
+++ b/docs/releases.rst
@@ -73,6 +73,12 @@ now wrapped in double quotes, and any double quotes within the
 values are escaped to ensure that the resulting file is always
 valid YAML.
 
++A new ``check-result`` key has been added to the test specification. This key
++allows users to configure whether "check" failure will affect the test result.
++The key accepts two options: 'respect' (default) and 'skip'. When set to
++'respect', check failures will affect the test result as before. When set to
++'skip', check failures will not affect the overall test result.
+
 
 tmt-1.36.1
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/spec/tests/result.fmf b/spec/tests/result.fmf
index 4a842e5ec9..c721dee435 100644
--- a/spec/tests/result.fmf
+++ b/spec/tests/result.fmf
@@ -32,6 +32,15 @@ description: |
 
         .. versionadded:: 1.35
 
+    Additionally, the `check-result` attribute can be used to specify
+    how check results should be interpreted:
+
+    respect
+        check results are respected (test fails if any check fails) - default value
+    skip
+        check results are ignored (test result is not affected by check failures)
+
+        .. versionadded:: 1.37
 
 example:
   - |
@@ -42,6 +51,10 @@ example:
     # Look for $TMT_TEST_DATA/results.yaml (or results.json) with custom results
     result: custom
 
+  - |
+    # Ignore check failures
+    check-result: skip
+
 link:
   - implemented-by: /tmt/base.py
   - verified-by: /tests/execute/result
diff --git a/tests/execute/result/check_results.sh b/tests/execute/result/check_results.sh
index 8fcabdfb1f..8e51ef81ca 100644
--- a/tests/execute/result/check_results.sh
+++ b/tests/execute/result/check_results.sh
@@ -14,15 +14,22 @@ rlJournalStart
         rlAssertGrep "pass /test/check-pass" $rlRun_LOG
         rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
         rlAssertGrep "pass dmesg (after-test check)" $rlRun_LOG
+        echo $rlRun_LOG
 
-        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-fail execute -vv report -vv 2>&1 >/dev/null" 1
-        rlAssertGrep "fail /test/check-fail" $rlRun_LOG
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-fail-respect execute -vv report -vv 2>&1 >/dev/null" 1
+        rlAssertGrep "fail /test/check-fail-respect" $rlRun_LOG
         rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
         rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
         rlAssertGrep "check failed" $rlRun_LOG
 
-        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-ignore execute -vv report -vv 2>&1 >/dev/null" 0
-        rlAssertGrep "pass /test/check-ignore" $rlRun_LOG
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-fail-skip execute -vv report -vv 2>&1 >/dev/null" 0
+        rlAssertGrep "pass /test/check-fail-skip" $rlRun_LOG
+        rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
+        rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
+        rlAssertNotGrep "check failed" $rlRun_LOG
+
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-skip execute -vv report -vv 2>&1 >/dev/null" 0
+        rlAssertGrep "pass /test/check-skip" $rlRun_LOG
         rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
         rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
         rlAssertNotGrep "check failed" $rlRun_LOG
@@ -42,9 +49,10 @@ rlJournalStart
         rlAssertGrep "- name: dmesg" $rlRun_LOG
         rlAssertGrep "  result: pass" $rlRun_LOG
         rlAssertGrep "  event: after-test" $rlRun_LOG
+        rlAssertGrep "check_result: respect" $rlRun_LOG
 
         rlRun -s "yq e '.[1]' $results_file"
-        rlAssertGrep "name: /test/check-fail" $rlRun_LOG
+        rlAssertGrep "name: /test/check-fail-respect" $rlRun_LOG
         rlAssertGrep "result: fail" $rlRun_LOG
         rlAssertGrep "note: check failed" $rlRun_LOG
         rlAssertGrep "check:" $rlRun_LOG
@@ -54,9 +62,23 @@ rlJournalStart
         rlAssertGrep "- name: dmesg" $rlRun_LOG
         rlAssertGrep "  result: fail" $rlRun_LOG
         rlAssertGrep "  event: after-test" $rlRun_LOG
+        rlAssertGrep "check_result: respect" $rlRun_LOG
 
         rlRun -s "yq e '.[2]' $results_file"
-        rlAssertGrep "name: /test/check-ignore" $rlRun_LOG
+        rlAssertGrep "name: /test/check-fail-skip" $rlRun_LOG
+        rlAssertGrep "result: pass" $rlRun_LOG
+        rlAssertNotGrep "note: check failed" $rlRun_LOG
+        rlAssertGrep "check:" $rlRun_LOG
+        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: pass" $rlRun_LOG
+        rlAssertGrep "  event: before-test" $rlRun_LOG
+        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: fail" $rlRun_LOG
+        rlAssertGrep "  event: after-test" $rlRun_LOG
+        rlAssertGrep "check_result: skip" $rlRun_LOG
+
+        rlRun -s "yq e '.[3]' $results_file"
+        rlAssertGrep "name: /test/check-skip" $rlRun_LOG
         rlAssertGrep "result: pass" $rlRun_LOG
         rlAssertNotGrep "note: check failed" $rlRun_LOG
         rlAssertGrep "check:" $rlRun_LOG
@@ -66,6 +88,7 @@ rlJournalStart
         rlAssertGrep "- name: dmesg" $rlRun_LOG
         rlAssertGrep "  result: fail" $rlRun_LOG
         rlAssertGrep "  event: after-test" $rlRun_LOG
+        rlAssertGrep "check_result: respect" $rlRun_LOG
     rlPhaseEnd
 
     rlPhaseStartCleanup
diff --git a/tests/execute/result/check_results/main.fmf b/tests/execute/result/check_results/main.fmf
index 6b1b76fecd..c2c4d9117a 100644
--- a/tests/execute/result/check_results/main.fmf
+++ b/tests/execute/result/check_results/main.fmf
@@ -8,9 +8,11 @@ description: Verify that check results, including after-test checks, are correct
     duration: 1m
     check:
         - how: dmesg
+    check-result: respect
+    # Expected outcome: PASS (test passes, check passes)
 
-/test/check-fail:
-    summary: Test with failing dmesg check
+/test/check-fail-respect:
+    summary: Test with failing dmesg check (respect)
     test: |
         echo "Test passed"
         echo "Call Trace:" >> /dev/kmsg
@@ -18,9 +20,23 @@ description: Verify that check results, including after-test checks, are correct
     duration: 1m
     check:
         - how: dmesg
+    check-result: respect
+    # Expected outcome: FAIL (test passes, but check fails and is respected)
 
-/test/check-ignore:
-    summary: Test with failing dmesg check but ignored
+/test/check-fail-skip:
+    summary: Test with failing dmesg check (skip)
+    test: |
+        echo "Test passed"
+        echo "Call Trace:" >> /dev/kmsg
+    framework: shell
+    duration: 1m
+    check:
+        - how: dmesg
+    check-result: skip
+    # Expected outcome: PASS (test passes, check fails but is ignored)
+
+/test/check-skip:
+    summary: Test with failing dmesg check but ignored due to result interpretation
     test: |
         echo "Test passed"
         echo "Call Trace:" >> /dev/kmsg
@@ -29,3 +45,5 @@ description: Verify that check results, including after-test checks, are correct
     result: pass
     check:
         - how: dmesg
+    check-result: respect
+    # Expected outcome: PASS (test passes, check fails but is overridden by 'result: pass')
diff --git a/tmt/base.py b/tmt/base.py
index 874c1b70f5..86e49f0e8d 100644
--- a/tmt/base.py
+++ b/tmt/base.py
@@ -1126,6 +1126,7 @@ class Test(
         'order',
         'result',
         'check',
+        'check-result',
         'restart_on_exit_code',
         'restart_max_count',
         'restart_with_reboot',
diff --git a/tmt/result.py b/tmt/result.py
index b83d0ce25c..09a2b14065 100644
--- a/tmt/result.py
+++ b/tmt/result.py
@@ -261,11 +261,6 @@ class Result(BaseResult):
         serialize=lambda path: None if path is None else str(path),
         unserialize=lambda value: None if value is None else Path(value)
         )
-    respect_checks: bool = field(
-        default=True,
-        serialize=lambda value: value,
-        unserialize=lambda value: value
-        )
 
     @classmethod
     def from_test_invocation(
@@ -275,8 +270,7 @@ def from_test_invocation(
             result: ResultOutcome,
             note: Optional[str] = None,
             ids: Optional[ResultIds] = None,
-            log: Optional[list[Path]] = None,
-            respect_checks: bool = True) -> 'Result':
+            log: Optional[list[Path]] = None) -> 'Result':
         """
         Create a result from a test invocation.
 
@@ -293,7 +287,6 @@ def from_test_invocation(
         :param ids: additional test IDs. They will be added to IDs extracted
             from the test.
         :param log: optional list of test logs.
-        :param respect_checks: whether to respect or ignore check results.
         """
 
         # Saving identifiable information for each test case so we can match them
@@ -326,7 +319,6 @@ def from_test_invocation(
             log=log or [],
             guest=ResultGuestData.from_test_invocation(invocation=invocation),
             data_path=invocation.relative_test_data_path,
-            respect_checks=respect_checks,
             check=invocation.check_results)
 
         for check in _result.check:
@@ -334,7 +326,10 @@ def from_test_invocation(
 
         return _result.interpret_result(invocation.test.result)
 
-    def interpret_result(self, interpret: ResultInterpret) -> 'Result':
+    def interpret_result(
+            self,
+            interpret: ResultInterpret,
+            check_interpret: ResultInterpret) -> 'Result':
         """
         Interpret result according to a given interpretation instruction.
 
@@ -342,6 +337,7 @@ def interpret_result(self, interpret: ResultInterpret) -> 'Result':
         attributes, following the ``interpret`` value.
 
         :param interpret: how to interpret current result.
+        :param check_interpret: how to interpret check results.
         :returns: :py:class:`Result` instance containing the updated result.
         """
 
@@ -352,7 +348,7 @@ def interpret_result(self, interpret: ResultInterpret) -> 'Result':
         checks_failed = any(check.result == ResultOutcome.FAIL for check in self.check)
 
         if interpret == ResultInterpret.RESPECT:
-            if self.respect_checks and checks_failed:
+            if check_interpret == ResultInterpret.RESPECT and checks_failed:
                 self.result = ResultOutcome.FAIL
                 if self.note:
                     self.note += ', check failed'
diff --git a/tmt/schemas/test.yaml b/tmt/schemas/test.yaml
index d64923ee5f..2214a9e6d7 100644
--- a/tmt/schemas/test.yaml
+++ b/tmt/schemas/test.yaml
@@ -61,6 +61,14 @@ properties:
             - type: string
             - $ref: "/schemas/common#/definitions/check"
 
+  # https://tmt.readthedocs.io/en/stable/spec/tests.html#check-result
+  check-result:
+    type: string
+    enum:
+      - respect
+      - skip
+    default: respect
+
   # https://tmt.readthedocs.io/en/stable/spec/core.html#id
   id:
     $ref: "/schemas/core#/definitions/id"

From f91ea0573c4e8355cd784b998fe9e14efa5b9b83 Mon Sep 17 00:00:00 2001
From: Martin Hoyer <mhoyer@redhat.com>
Date: Mon, 30 Sep 2024 19:17:56 +0200
Subject: [PATCH 3/6] Refactor to test.check.result

---
 docs/releases.rst                           |  14 +--
 spec/tests/check.fmf                        |  29 +++++
 spec/tests/result.fmf                       |  13 ---
 tests/execute/result/check_results.sh       | 113 ++++++++++++++++----
 tests/execute/result/check_results/main.fmf |  70 ++++++++----
 tests/execute/result/main.fmf               |   2 +-
 tmt/base.py                                 |   1 -
 tmt/checks/__init__.py                      |  27 ++---
 tmt/checks/events.py                        |  19 ++++
 tmt/result.py                               |  86 +++++++++++++--
 tmt/schemas/test.yaml                       |  18 ++--
 11 files changed, 304 insertions(+), 88 deletions(-)
 create mode 100644 tmt/checks/events.py

diff --git a/docs/releases.rst b/docs/releases.rst
index b5d5b3ce57..8662c2d2ae 100644
--- a/docs/releases.rst
+++ b/docs/releases.rst
@@ -4,6 +4,14 @@
     Releases
 ======================
 
+tmt-1.38.0
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+A new ``result`` key has been added to the :ref:`test check</spec/tests/check>`
+specification. This key allows users to configure how check results are
+interpreted.
+
+
 tmt-1.37.0
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -73,12 +81,6 @@ now wrapped in double quotes, and any double quotes within the
 values are escaped to ensure that the resulting file is always
 valid YAML.
 
-+A new ``check-result`` key has been added to the test specification. This key
-+allows users to configure whether "check" failure will affect the test result.
-+The key accepts two options: 'respect' (default) and 'skip'. When set to
-+'respect', check failures will affect the test result as before. When set to
-+'skip', check failures will not affect the overall test result.
-
 
 tmt-1.36.1
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/spec/tests/check.fmf b/spec/tests/check.fmf
index 102eb4771c..825eabe273 100644
--- a/spec/tests/check.fmf
+++ b/spec/tests/check.fmf
@@ -16,6 +16,15 @@ description: |
 
     See :ref:`/plugins/test-checks` for the list of available checks.
 
+    The `result` key can be used to specify how the check result should be
+    interpreted. It accepts the following options:
+    - 'respect' (default): The check result affects the overall test result.
+    - 'ignore': The check result does not affect the overall test result.
+    - 'xfail': The check is expected to fail. If it passes, the test fails,
+      and if it fails, the test passes.
+    - 'pass', 'fail', 'info', 'warn', 'error': The check result is interpreted
+      as the specified outcome, regardless of the actual check result.
+
 example:
   - |
     # Enable a single check, AVC denial detection.
@@ -37,6 +46,26 @@ example:
       - how: test-inspector
         enable: false
 
+  - |
+    # Enable a check with a specific result interpretation
+    check:
+      - how: dmesg
+        result: xfail
+
+  - |
+    # Enable multiple checks with different result interpretations
+    check:
+      - how: dmesg
+        result: ignore
+      - how: avc
+        result: xfail
+
+  - |
+    # Enable a check with a specific outcome interpretation
+    check:
+      - how: dmesg
+        result: warn
+
 link:
   - implemented-by: /tmt/checks
   - verified-by: /tests/test/check/avc
diff --git a/spec/tests/result.fmf b/spec/tests/result.fmf
index c721dee435..8b84006ec4 100644
--- a/spec/tests/result.fmf
+++ b/spec/tests/result.fmf
@@ -32,16 +32,6 @@ description: |
 
         .. versionadded:: 1.35
 
-    Additionally, the `check-result` attribute can be used to specify
-    how check results should be interpreted:
-
-    respect
-        check results are respected (test fails if any check fails) - default value
-    skip
-        check results are ignored (test result is not affected by check failures)
-
-        .. versionadded:: 1.37
-
 example:
   - |
     # Plain swapping fail to pass and pass to fail result
@@ -51,9 +41,6 @@ example:
     # Look for $TMT_TEST_DATA/results.yaml (or results.json) with custom results
     result: custom
 
-  - |
-    # Ignore check failures
-    check-result: skip
 
 link:
   - implemented-by: /tmt/base.py
diff --git a/tests/execute/result/check_results.sh b/tests/execute/result/check_results.sh
index 8e51ef81ca..45dfca2503 100644
--- a/tests/execute/result/check_results.sh
+++ b/tests/execute/result/check_results.sh
@@ -14,7 +14,6 @@ rlJournalStart
         rlAssertGrep "pass /test/check-pass" $rlRun_LOG
         rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
         rlAssertGrep "pass dmesg (after-test check)" $rlRun_LOG
-        echo $rlRun_LOG
 
         rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-fail-respect execute -vv report -vv 2>&1 >/dev/null" 1
         rlAssertGrep "fail /test/check-fail-respect" $rlRun_LOG
@@ -22,17 +21,45 @@ rlJournalStart
         rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
         rlAssertGrep "check failed" $rlRun_LOG
 
-        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-fail-skip execute -vv report -vv 2>&1 >/dev/null" 0
-        rlAssertGrep "pass /test/check-fail-skip" $rlRun_LOG
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-fail-ignore execute -vv report -vv 2>&1 >/dev/null" 0
+        rlAssertGrep "pass /test/check-fail-ignore" $rlRun_LOG
         rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
         rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
         rlAssertNotGrep "check failed" $rlRun_LOG
 
-        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-skip execute -vv report -vv 2>&1 >/dev/null" 0
-        rlAssertGrep "pass /test/check-skip" $rlRun_LOG
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-xfail-pass execute -vv report -vv 2>&1 >/dev/null" 1
+        rlAssertGrep "fail /test/check-xfail-pass" $rlRun_LOG
+        rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
+        rlAssertGrep "pass dmesg (after-test check)" $rlRun_LOG
+        rlAssertGrep "unexpected pass" $rlRun_LOG
+
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-xfail-fail execute -vv report -vv 2>&1 >/dev/null" 0
+        rlAssertGrep "pass /test/check-xfail-fail" $rlRun_LOG
+        rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
+        rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
+        rlAssertGrep "expected failure" $rlRun_LOG
+
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-multiple execute -vv report -vv 2>&1 >/dev/null" 1
+        rlAssertGrep "fail /test/check-multiple" $rlRun_LOG
+        rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
+        rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
+        rlAssertGrep "fail dmesg (before-test check)" $rlRun_LOG
+        rlAssertGrep "check failed" $rlRun_LOG
+        rlAssertGrep "unexpected pass" $rlRun_LOG
+
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-override execute -vv report -vv 2>&1 >/dev/null" 0
+        rlAssertGrep "pass /test/check-override" $rlRun_LOG
         rlAssertGrep "pass dmesg (before-test check)" $rlRun_LOG
         rlAssertGrep "fail dmesg (after-test check)" $rlRun_LOG
         rlAssertNotGrep "check failed" $rlRun_LOG
+
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-warn execute -vv report -vv 2>&1 >/dev/null" 1
+        rlAssertGrep "warn /test/check-warn" $rlRun_LOG
+        rlAssertGrep "warn dmesg (after-test check)" $rlRun_LOG
+
+        rlRun -s "tmt run --id \${run} --scratch provision --how local test --name /test/check-info execute -vv report -vv 2>&1 >/dev/null" 0
+        rlAssertGrep "info /test/check-info" $rlRun_LOG
+        rlAssertGrep "info dmesg (after-test check)" $rlRun_LOG
     rlPhaseEnd
 
     rlPhaseStartTest "Verify results.yaml content"
@@ -43,52 +70,100 @@ rlJournalStart
         rlAssertGrep "name: /test/check-pass" $rlRun_LOG
         rlAssertGrep "result: pass" $rlRun_LOG
         rlAssertGrep "check:" $rlRun_LOG
-        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
         rlAssertGrep "  result: pass" $rlRun_LOG
         rlAssertGrep "  event: before-test" $rlRun_LOG
-        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
         rlAssertGrep "  result: pass" $rlRun_LOG
         rlAssertGrep "  event: after-test" $rlRun_LOG
-        rlAssertGrep "check_result: respect" $rlRun_LOG
 
         rlRun -s "yq e '.[1]' $results_file"
         rlAssertGrep "name: /test/check-fail-respect" $rlRun_LOG
         rlAssertGrep "result: fail" $rlRun_LOG
         rlAssertGrep "note: check failed" $rlRun_LOG
         rlAssertGrep "check:" $rlRun_LOG
-        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
         rlAssertGrep "  result: pass" $rlRun_LOG
         rlAssertGrep "  event: before-test" $rlRun_LOG
-        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
         rlAssertGrep "  result: fail" $rlRun_LOG
         rlAssertGrep "  event: after-test" $rlRun_LOG
-        rlAssertGrep "check_result: respect" $rlRun_LOG
 
         rlRun -s "yq e '.[2]' $results_file"
-        rlAssertGrep "name: /test/check-fail-skip" $rlRun_LOG
+        rlAssertGrep "name: /test/check-fail-ignore" $rlRun_LOG
         rlAssertGrep "result: pass" $rlRun_LOG
         rlAssertNotGrep "note: check failed" $rlRun_LOG
         rlAssertGrep "check:" $rlRun_LOG
-        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
         rlAssertGrep "  result: pass" $rlRun_LOG
         rlAssertGrep "  event: before-test" $rlRun_LOG
-        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
         rlAssertGrep "  result: fail" $rlRun_LOG
         rlAssertGrep "  event: after-test" $rlRun_LOG
-        rlAssertGrep "check_result: skip" $rlRun_LOG
 
         rlRun -s "yq e '.[3]' $results_file"
-        rlAssertGrep "name: /test/check-skip" $rlRun_LOG
+        rlAssertGrep "name: /test/check-xfail-pass" $rlRun_LOG
+        rlAssertGrep "result: fail" $rlRun_LOG
+        rlAssertGrep "note: unexpected pass" $rlRun_LOG
+        rlAssertGrep "check:" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: pass" $rlRun_LOG
+        rlAssertGrep "  event: before-test" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: pass" $rlRun_LOG
+        rlAssertGrep "  event: after-test" $rlRun_LOG
+
+        rlRun -s "yq e '.[4]' $results_file"
+        rlAssertGrep "name: /test/check-xfail-fail" $rlRun_LOG
+        rlAssertGrep "result: pass" $rlRun_LOG
+        rlAssertGrep "note: expected failure" $rlRun_LOG
+        rlAssertGrep "check:" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: pass" $rlRun_LOG
+        rlAssertGrep "  event: before-test" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: fail" $rlRun_LOG
+        rlAssertGrep "  event: after-test" $rlRun_LOG
+
+        rlRun -s "yq e '.[5]' $results_file"
+        rlAssertGrep "name: /test/check-multiple" $rlRun_LOG
+        rlAssertGrep "result: fail" $rlRun_LOG
+        rlAssertGrep "note: check failed, unexpected pass" $rlRun_LOG
+        rlAssertGrep "check:" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: fail" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: pass" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: fail" $rlRun_LOG
+
+        rlRun -s "yq e '.[6]' $results_file"
+        rlAssertGrep "name: /test/check-override" $rlRun_LOG
         rlAssertGrep "result: pass" $rlRun_LOG
         rlAssertNotGrep "note: check failed" $rlRun_LOG
         rlAssertGrep "check:" $rlRun_LOG
-        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
         rlAssertGrep "  result: pass" $rlRun_LOG
         rlAssertGrep "  event: before-test" $rlRun_LOG
-        rlAssertGrep "- name: dmesg" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
         rlAssertGrep "  result: fail" $rlRun_LOG
         rlAssertGrep "  event: after-test" $rlRun_LOG
-        rlAssertGrep "check_result: respect" $rlRun_LOG
+
+        rlRun -s "yq e '.[7]' $results_file"
+        rlAssertGrep "name: /test/check-warn" $rlRun_LOG
+        rlAssertGrep "result: warn" $rlRun_LOG
+        rlAssertGrep "check:" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: warn" $rlRun_LOG
+        rlAssertGrep "  event: after-test" $rlRun_LOG
+
+        rlRun -s "yq e '.[8]' $results_file"
+        rlAssertGrep "name: /test/check-info" $rlRun_LOG
+        rlAssertGrep "result: info" $rlRun_LOG
+        rlAssertGrep "check:" $rlRun_LOG
+        rlAssertGrep "- how: dmesg" $rlRun_LOG
+        rlAssertGrep "  result: info" $rlRun_LOG
+        rlAssertGrep "  event: after-test" $rlRun_LOG
     rlPhaseEnd
 
     rlPhaseStartCleanup
diff --git a/tests/execute/result/check_results/main.fmf b/tests/execute/result/check_results/main.fmf
index c2c4d9117a..de3aac7c15 100644
--- a/tests/execute/result/check_results/main.fmf
+++ b/tests/execute/result/check_results/main.fmf
@@ -1,4 +1,4 @@
-summary: Tests for check results behavior
+summary: Tests for check results behaviour
 description: Verify that check results, including after-test checks, are correctly handled
 
 /test/check-pass:
@@ -8,42 +8,76 @@ description: Verify that check results, including after-test checks, are correct
     duration: 1m
     check:
         - how: dmesg
-    check-result: respect
+          result: respect
     # Expected outcome: PASS (test passes, check passes)
 
 /test/check-fail-respect:
     summary: Test with failing dmesg check (respect)
-    test: |
-        echo "Test passed"
-        echo "Call Trace:" >> /dev/kmsg
+    test: echo "Test passed"
     framework: shell
     duration: 1m
     check:
         - how: dmesg
-    check-result: respect
+          failure-pattern: '.*'
+          result: respect
     # Expected outcome: FAIL (test passes, but check fails and is respected)
 
-/test/check-fail-skip:
-    summary: Test with failing dmesg check (skip)
-    test: |
-        echo "Test passed"
-        echo "Call Trace:" >> /dev/kmsg
+/test/check-fail-ignore:
+    summary: Test with failing dmesg check (ignore)
+    test: echo "Test passed"
     framework: shell
     duration: 1m
     check:
         - how: dmesg
-    check-result: skip
+          failure-pattern: '.*'
+          result: ignore
     # Expected outcome: PASS (test passes, check fails but is ignored)
 
-/test/check-skip:
-    summary: Test with failing dmesg check but ignored due to result interpretation
-    test: |
-        echo "Test passed"
-        echo "Call Trace:" >> /dev/kmsg
+/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 "Test passed"
+    framework: shell
+    duration: 1m
+    check:
+        - how: dmesg
+          failure-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 "Test passed"
+    framework: shell
+    duration: 1m
+    check:
+        - how: dmesg
+          failure-pattern: '.*'
+          result: respect
+        - how: dmesg
+          result: xfail
+        - how: dmesg
+          failure-pattern: '.*'
+          result: ignore
+    # 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 ignored)
+
+/test/check-override:
+    summary: Test with failing dmesg check but overridden by test result
+    test: echo "Test passed"
     framework: shell
     duration: 1m
     result: pass
     check:
         - how: dmesg
-    check-result: respect
+          failure-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 0e9a6729c1..93d35c2646 100644
--- a/tests/execute/result/main.fmf
+++ b/tests/execute/result/main.fmf
@@ -11,5 +11,5 @@
     summary: Test special characters generated to tmt-report-results.yaml
     test: ./special.sh
 /check_results:
-    summary: Test behavior of check results, including after-test checks
+    summary: Test behaviour of check results, including after-test checks
     test: ./check_results.sh
diff --git a/tmt/base.py b/tmt/base.py
index 86e49f0e8d..874c1b70f5 100644
--- a/tmt/base.py
+++ b/tmt/base.py
@@ -1126,7 +1126,6 @@ class Test(
         'order',
         'result',
         'check',
-        'check-result',
         'restart_on_exit_code',
         'restart_max_count',
         'restart_with_reboot',
diff --git a/tmt/checks/__init__.py b/tmt/checks/__init__.py
index 905c19f72f..e8d757a944 100644
--- a/tmt/checks/__init__.py
+++ b/tmt/checks/__init__.py
@@ -1,5 +1,4 @@
 import dataclasses
-import enum
 import functools
 from typing import TYPE_CHECKING, Any, Callable, Generic, Optional, TypedDict, TypeVar, cast
 
@@ -20,6 +19,8 @@
     from tmt.steps.execute import TestInvocation
     from tmt.steps.provision import Guest
 
+from tmt.checks.events import CheckEvent
+from tmt.result import CheckResultInterpret
 
 #: A type variable representing a :py:class:`Check` instances.
 CheckT = TypeVar('CheckT', bound='Check')
@@ -67,20 +68,7 @@ def find_plugin(name: str) -> 'CheckPluginClass':
 class _RawCheck(TypedDict):
     how: str
     enabled: bool
-
-
-class CheckEvent(enum.Enum):
-    """ Events in test runtime when a check can be executed """
-
-    BEFORE_TEST = 'before-test'
-    AFTER_TEST = 'after-test'
-
-    @classmethod
-    def from_spec(cls, spec: str) -> 'CheckEvent':
-        try:
-            return CheckEvent(spec)
-        except ValueError:
-            raise tmt.utils.SpecificationError(f"Invalid test check event '{spec}'.")
+    result: str
 
 
 @dataclasses.dataclass
@@ -100,6 +88,11 @@ class Check(
         default=True,
         is_flag=True,
         help='Whether the check is enabled or not.')
+    result: CheckResultInterpret = field(
+        default=CheckResultInterpret.RESPECT,
+        serialize=lambda result: result.value,
+        unserialize=CheckResultInterpret.from_spec,
+        help='How to interpret the check result.')
 
     @functools.cached_property
     def plugin(self) -> 'CheckPluginClass':
@@ -228,7 +221,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:
@@ -236,6 +229,8 @@ def normalize_test_check(
                 f"Cannot instantiate check from '{key_address}'.") from exc
 
     if isinstance(raw_test_check, dict):
+        if 'result' not in raw_test_check:
+            raw_test_check['result'] = 'respect'
         try:
             return CheckPlugin.delegate(
                 raw_data=cast(_RawCheck, raw_test_check),
diff --git a/tmt/checks/events.py b/tmt/checks/events.py
new file mode 100644
index 0000000000..df663361a0
--- /dev/null
+++ b/tmt/checks/events.py
@@ -0,0 +1,19 @@
+# TODO temporary solution for circular imports
+
+import enum
+
+import tmt.utils
+
+
+class CheckEvent(enum.Enum):
+    """ Events in test runtime when a check can be executed """
+
+    BEFORE_TEST = 'before-test'
+    AFTER_TEST = 'after-test'
+
+    @classmethod
+    def from_spec(cls, spec: str) -> 'CheckEvent':
+        try:
+            return CheckEvent(spec)
+        except ValueError:
+            raise tmt.utils.SpecificationError(f"Invalid test check event '{spec}'.")
diff --git a/tmt/result.py b/tmt/result.py
index 09a2b14065..98309c3da5 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.events import CheckEvent
 from tmt.utils import GeneralError, Path, SerializableContainer, field
 
 if TYPE_CHECKING:
@@ -18,6 +18,7 @@
     import tmt.steps.execute
     import tmt.steps.provision
 
+
 # Extra keys used for identification in Result class
 EXTRA_RESULT_IDENTIFICATION_KEYS = ['extra-nitrate', 'extra-task']
 
@@ -82,6 +83,40 @@ def normalize(
             f"Invalid result interpretation '{value}' at {key_address}.")
 
 
+class CheckResultInterpret(enum.Enum):
+    PASS = 'pass'
+    FAIL = 'fail'
+    INFO = 'info'
+    WARN = 'warn'
+    ERROR = 'error'
+    SKIP = 'skip'
+    RESPECT = 'respect'
+    XFAIL = 'xfail'
+    IGNORE = 'ignore'
+
+    @classmethod
+    def from_spec(cls, spec: str) -> 'CheckResultInterpret':
+        try:
+            return CheckResultInterpret(spec)
+        except ValueError:
+            raise tmt.utils.SpecificationError(f"Invalid check result interpretation '{spec}'.")
+
+    @classmethod
+    def normalize(
+            cls,
+            key_address: str,
+            value: Any,
+            logger: tmt.log.Logger) -> 'CheckResultInterpret':
+        if isinstance(value, CheckResultInterpret):
+            return value
+
+        if isinstance(value, str):
+            return cls.from_spec(value)
+
+        raise tmt.utils.SpecificationError(
+            f"Invalid check result interpretation '{value}' at {key_address}.")
+
+
 RESULT_OUTCOME_COLORS: dict[ResultOutcome, str] = {
     ResultOutcome.PASS: 'green',
     ResultOutcome.FAIL: 'red',
@@ -328,8 +363,7 @@ def from_test_invocation(
 
     def interpret_result(
             self,
-            interpret: ResultInterpret,
-            check_interpret: ResultInterpret) -> 'Result':
+            interpret: ResultInterpret) -> 'Result':
         """
         Interpret result according to a given interpretation instruction.
 
@@ -337,7 +371,6 @@ def interpret_result(
         attributes, following the ``interpret`` value.
 
         :param interpret: how to interpret current result.
-        :param check_interpret: how to interpret check results.
         :returns: :py:class:`Result` instance containing the updated result.
         """
 
@@ -348,7 +381,7 @@ def interpret_result(
         checks_failed = any(check.result == ResultOutcome.FAIL for check in self.check)
 
         if interpret == ResultInterpret.RESPECT:
-            if check_interpret == ResultInterpret.RESPECT and checks_failed:
+            if checks_failed:
                 self.result = ResultOutcome.FAIL
                 if self.note:
                     self.note += ', check failed'
@@ -356,7 +389,7 @@ def interpret_result(
                     self.note = 'check failed'
             return self
 
-        # Handle XFAIL
+        # Handle XFAIL for the overall test result
         if interpret == ResultInterpret.XFAIL:
             if self.result == ResultOutcome.PASS and checks_failed:
                 self.result = ResultOutcome.FAIL
@@ -378,6 +411,47 @@ def interpret_result(
                     self.note = 'unexpected pass'
             return self
 
+        # Handle individual check results
+        for check in self.check:
+            check_result = CheckResultInterpret(check.result.value)
+
+            if check_result == CheckResultInterpret.RESPECT:
+                if check.result == ResultOutcome.FAIL:
+                    self.result = ResultOutcome.FAIL
+                    if self.note:
+                        self.note += ', check failed'
+                    else:
+                        self.note = 'check failed'
+
+            elif check_result == CheckResultInterpret.XFAIL:
+                if check.result == ResultOutcome.PASS:
+                    self.result = ResultOutcome.FAIL
+                    if self.note:
+                        self.note += ', unexpected pass in check'
+                    else:
+                        self.note = 'unexpected pass in check'
+                elif check.result == ResultOutcome.FAIL:
+                    # Check failed as expected, don't change the overall result
+                    if self.note:
+                        self.note += ', expected failure in check'
+                    else:
+                        self.note = 'expected failure in check'
+
+            elif check_result == CheckResultInterpret.IGNORE:
+                continue
+
+            elif check_result in [CheckResultInterpret.PASS, CheckResultInterpret.FAIL,
+                                  CheckResultInterpret.INFO, CheckResultInterpret.WARN,
+                                  CheckResultInterpret.ERROR, CheckResultInterpret.SKIP]:
+                self.result = ResultOutcome(check_result.value)
+                if self.note:
+                    self.note += f', check result: {check.result.value}'
+                else:
+                    self.note = f'check result: {check.result.value}'
+            else:
+                raise tmt.utils.SpecificationError(
+                    f"Invalid result '{check_result.value}' in check for test '{self.name}'.")
+
         # Extend existing note or set a new one
         if self.note:
             self.note += f', original result: {self.result.value}'
diff --git a/tmt/schemas/test.yaml b/tmt/schemas/test.yaml
index 2214a9e6d7..1a365bc314 100644
--- a/tmt/schemas/test.yaml
+++ b/tmt/schemas/test.yaml
@@ -60,14 +60,16 @@ properties:
           anyOf:
             - type: string
             - $ref: "/schemas/common#/definitions/check"
-
-  # https://tmt.readthedocs.io/en/stable/spec/tests.html#check-result
-  check-result:
-    type: string
-    enum:
-      - respect
-      - skip
-    default: respect
+      - type: object
+        properties:
+          how:
+            type: string
+          enabled:
+            type: boolean
+          result:
+            $ref: "/schemas/common#/definitions/result_interpret"
+        required:
+          - how
 
   # https://tmt.readthedocs.io/en/stable/spec/core.html#id
   id:

From 0efce7c7b53882ddeb791ebdb8cfba667cf66f1b Mon Sep 17 00:00:00 2001
From: Martin Hoyer <mhoyer@redhat.com>
Date: Fri, 4 Oct 2024 15:03:15 +0200
Subject: [PATCH 4/6] fixup! Add check-result key

---
 spec/tests/result.fmf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/tests/result.fmf b/spec/tests/result.fmf
index 8b84006ec4..4a842e5ec9 100644
--- a/spec/tests/result.fmf
+++ b/spec/tests/result.fmf
@@ -32,6 +32,7 @@ description: |
 
         .. versionadded:: 1.35
 
+
 example:
   - |
     # Plain swapping fail to pass and pass to fail result
@@ -41,7 +42,6 @@ example:
     # Look for $TMT_TEST_DATA/results.yaml (or results.json) with custom results
     result: custom
 
-
 link:
   - implemented-by: /tmt/base.py
   - verified-by: /tests/execute/result

From 795d1759756438d6d1b6a559df79807b6142d3fb Mon Sep 17 00:00:00 2001
From: Martin Hoyer <mhoyer@redhat.com>
Date: Fri, 4 Oct 2024 16:40:00 +0200
Subject: [PATCH 5/6] Incorporating suggestions

---
 tmt/result.py | 122 ++++++++++++++++++++------------------------------
 1 file changed, 49 insertions(+), 73 deletions(-)

diff --git a/tmt/result.py b/tmt/result.py
index 98309c3da5..2a0b1ea3c3 100644
--- a/tmt/result.py
+++ b/tmt/result.py
@@ -356,86 +356,58 @@ def from_test_invocation(
             data_path=invocation.relative_test_data_path,
             check=invocation.check_results)
 
-        for check in _result.check:
-            print(f"  - Name: {check.name}, Result: {check.result}, Event: {check.event}")
+        result_interpretations = {
+            check.how: CheckResultInterpret(
+                check.result.value if isinstance(check.result, ResultOutcome) else check.result
+                )
+            for check in invocation.test.check
+            }
+
+        return _result.interpret_result(
+            invocation.test.result,
+            result_interpretations
+            )
 
-        return _result.interpret_result(invocation.test.result)
+    def _set_note(self, new_note: str) -> None:
+        """Helper function to set a note, handling the ', ...' and condition."""
+        if self.note:
+            self.note += f', {new_note}'
+        else:
+            self.note = new_note
 
     def interpret_result(
             self,
-            interpret: ResultInterpret) -> 'Result':
+            interpret_test: ResultInterpret,
+            interpret_checks: dict[str, CheckResultInterpret]) -> 'Result':
         """
         Interpret result according to a given interpretation instruction.
 
         Inspect and possibly modify :py:attr:`result` and :py:attr:`note`
         attributes, following the ``interpret`` value.
 
-        :param interpret: how to interpret current result.
+        :param interpret_test: how to interpret current test result.
+        :param interpret_checks: how to interpret individual check results.
         :returns: :py:class:`Result` instance containing the updated result.
         """
 
-        if interpret == ResultInterpret.CUSTOM:
-            return self
-
-        # Check for failed checks
-        checks_failed = any(check.result == ResultOutcome.FAIL for check in self.check)
-
-        if interpret == ResultInterpret.RESPECT:
-            if checks_failed:
-                self.result = ResultOutcome.FAIL
-                if self.note:
-                    self.note += ', check failed'
-                else:
-                    self.note = 'check failed'
+        if interpret_test == ResultInterpret.CUSTOM:
             return self
 
-        # Handle XFAIL for the overall test result
-        if interpret == ResultInterpret.XFAIL:
-            if self.result == ResultOutcome.PASS and checks_failed:
-                self.result = ResultOutcome.FAIL
-                if self.note:
-                    self.note += ', check failed'
-                else:
-                    self.note = 'check failed'
-            elif self.result == ResultOutcome.FAIL:
-                self.result = ResultOutcome.PASS
-                if self.note:
-                    self.note += ', expected failure'
-                else:
-                    self.note = 'expected failure'
-            elif self.result == ResultOutcome.PASS:
-                self.result = ResultOutcome.FAIL
-                if self.note:
-                    self.note += ', unexpected pass'
-                else:
-                    self.note = 'unexpected pass'
-            return self
-
-        # Handle individual check results
+        # First, handle individual check results
         for check in self.check:
-            check_result = CheckResultInterpret(check.result.value)
+            check_result = interpret_checks.get(check.name, CheckResultInterpret.RESPECT)
 
             if check_result == CheckResultInterpret.RESPECT:
                 if check.result == ResultOutcome.FAIL:
                     self.result = ResultOutcome.FAIL
-                    if self.note:
-                        self.note += ', check failed'
-                    else:
-                        self.note = 'check failed'
+                    self._set_note('check failed')
 
             elif check_result == CheckResultInterpret.XFAIL:
                 if check.result == ResultOutcome.PASS:
                     self.result = ResultOutcome.FAIL
-                    if self.note:
-                        self.note += ', unexpected pass in check'
-                    else:
-                        self.note = 'unexpected pass in check'
+                    self._set_note('unexpected pass in check')
                 elif check.result == ResultOutcome.FAIL:
-                    # Check failed as expected, don't change the overall result
-                    if self.note:
-                        self.note += ', expected failure in check'
-                    else:
-                        self.note = 'expected failure in check'
+                    self._set_note('expected failure in check')
 
             elif check_result == CheckResultInterpret.IGNORE:
                 continue
@@ -443,32 +415,36 @@ def interpret_result(
             elif check_result in [CheckResultInterpret.PASS, CheckResultInterpret.FAIL,
                                   CheckResultInterpret.INFO, CheckResultInterpret.WARN,
                                   CheckResultInterpret.ERROR, CheckResultInterpret.SKIP]:
-                self.result = ResultOutcome(check_result.value)
-                if self.note:
-                    self.note += f', check result: {check.result.value}'
-                else:
-                    self.note = f'check result: {check.result.value}'
+                # We don't promote check result to main result, but failed checks turn the
+                # main result to FAIL
+                if check_result == CheckResultInterpret.FAIL:
+                    self.result = ResultOutcome.FAIL
+                self._set_note(f'check result: {check.result.value}')
             else:
                 raise tmt.utils.SpecificationError(
-                    f"Invalid result '{check_result.value}' in check for test '{self.name}'.")
+                    f"Invalid result '{check_result}' in check for test '{self.name}'.")
 
-        # Extend existing note or set a new one
-        if self.note:
-            self.note += f', original result: {self.result.value}'
-
-        elif self.note is None:
-            self.note = f'original result: {self.result.value}'
+        # Now handle the overall test result
+        if interpret_test == ResultInterpret.RESPECT:
+            # This is already handled by the check results
+            pass
 
-        else:
-            raise tmt.utils.SpecificationError(
-                f"Test result note '{self.note}' must be a string.")
+        elif interpret_test == ResultInterpret.XFAIL:
+            if self.result == ResultOutcome.PASS:
+                self.result = ResultOutcome.FAIL
+                self._set_note('unexpected pass')
+            elif self.result == ResultOutcome.FAIL:
+                self.result = ResultOutcome.PASS
+                self._set_note('expected failure')
 
-        if ResultInterpret.is_result_outcome(interpret):
-            self.result = ResultOutcome(interpret.value)
+        elif ResultInterpret.is_result_outcome(interpret_test):
+            self.result = ResultOutcome(interpret_test.value)
 
         else:
             raise tmt.utils.SpecificationError(
-                f"Invalid result '{interpret.value}' in test '{self.name}'.")
+                f"Invalid result '{interpret_test}' in test '{self.name}'.")
+
+        self._set_note(f'original result: {self.original_result.value}')
 
         return self
 

From 7fa2cbc3721cae9a888d78af1c4905a5277a881d Mon Sep 17 00:00:00 2001
From: Martin Hoyer <mhoyer@redhat.com>
Date: Mon, 7 Oct 2024 14:00:38 +0200
Subject: [PATCH 6/6] hacking

---
 tmt/result.py | 101 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 74 insertions(+), 27 deletions(-)

diff --git a/tmt/result.py b/tmt/result.py
index 2a0b1ea3c3..8c365e3db0 100644
--- a/tmt/result.py
+++ b/tmt/result.py
@@ -375,6 +375,68 @@ def _set_note(self, new_note: str) -> None:
         else:
             self.note = new_note
 
+    def interpret_results_base(
+            self,
+            test_result: ResultOutcome,
+            test_interpret: ResultInterpret,
+            check_results: dict[str, ResultOutcome],
+            check_interprets: dict[str, CheckResultInterpret]
+            ) -> tuple[ResultOutcome, dict[str, ResultOutcome], Optional[str]]:
+        """
+        Interpret test and check results.
+
+        :param test_result: The initial test result.
+        :param test_interpret: How to interpret the test result.
+        :param check_results: A dictionary of check names and their results.
+        :param check_interprets: A dictionary of check names and their interpretation instructions.
+        :return: A tuple containing the final test result, final check results, optionally a note.
+        """
+        final_test_result = test_result
+        final_check_results = check_results.copy()
+        note = None
+
+        # First, interpret check results
+        for check_name, check_result in check_results.items():
+            check_interpret = check_interprets.get(check_name, CheckResultInterpret.RESPECT)
+
+            if check_interpret == CheckResultInterpret.RESPECT:
+                if check_result == ResultOutcome.FAIL:
+                    final_test_result = ResultOutcome.FAIL
+                    note = 'check failed'
+            elif check_interpret == CheckResultInterpret.XFAIL:
+                if check_result == ResultOutcome.PASS:
+                    final_check_results[check_name] = ResultOutcome.FAIL
+                    note = 'unexpected pass in check'
+                elif check_result == ResultOutcome.FAIL:
+                    final_check_results[check_name] = ResultOutcome.PASS
+                    note = 'expected failure in check'
+            elif check_interpret == CheckResultInterpret.IGNORE:
+                continue
+            elif check_interpret in (CheckResultInterpret.PASS, CheckResultInterpret.FAIL,
+                                     CheckResultInterpret.INFO, CheckResultInterpret.WARN,
+                                     CheckResultInterpret.ERROR, CheckResultInterpret.SKIP):
+                final_check_results[check_name] = ResultOutcome(check_interpret.value)
+                if check_interpret == CheckResultInterpret.FAIL:
+                    final_test_result = ResultOutcome.FAIL
+                note = f'check result: {check_result.value}'
+
+        # Now interpret the overall test result
+        if test_interpret == ResultInterpret.RESPECT:
+            pass  # Already handled by check results
+        elif test_interpret == ResultInterpret.XFAIL:
+            if final_test_result == ResultOutcome.PASS:
+                final_test_result = ResultOutcome.FAIL
+                note = 'unexpected pass'
+            elif final_test_result == ResultOutcome.FAIL:
+                final_test_result = ResultOutcome.PASS
+                note = 'expected failure'
+        elif ResultInterpret.is_result_outcome(test_interpret):
+            final_test_result = ResultOutcome(test_interpret.value)
+        else:
+            raise tmt.utils.SpecificationError(f"Invalid result interpretation '{test_interpret}'")
+
+        return final_test_result, final_check_results, note
+
     def interpret_result(
             self,
             interpret_test: ResultInterpret,
@@ -393,36 +455,21 @@ def interpret_result(
         if interpret_test == ResultInterpret.CUSTOM:
             return self
 
-        # First, handle individual check results
-        for check in self.check:
-            check_result = interpret_checks.get(check.name, CheckResultInterpret.RESPECT)
-
-            if check_result == CheckResultInterpret.RESPECT:
-                if check.result == ResultOutcome.FAIL:
-                    self.result = ResultOutcome.FAIL
-                    self._set_note('check failed')
+        check_results = {check.name: check.result for check in self.check}
 
-            elif check_result == CheckResultInterpret.XFAIL:
-                if check.result == ResultOutcome.PASS:
-                    self.result = ResultOutcome.FAIL
-                    self._set_note('unexpected pass in check')
-                elif check.result == ResultOutcome.FAIL:
-                    self._set_note('expected failure in check')
+        final_result, final_check_results, note = self.interpret_results_base(
+            self.result,
+            interpret_test,
+            check_results,
+            interpret_checks
+            )
 
-            elif check_result == CheckResultInterpret.IGNORE:
-                continue
+        self.result = final_result
+        for check in self.check:
+            check.result = final_check_results.get(check.name, check.result)
 
-            elif check_result in [CheckResultInterpret.PASS, CheckResultInterpret.FAIL,
-                                  CheckResultInterpret.INFO, CheckResultInterpret.WARN,
-                                  CheckResultInterpret.ERROR, CheckResultInterpret.SKIP]:
-                # We don't promote check result to main result, but failed checks turn the
-                # main result to FAIL
-                if check_result == CheckResultInterpret.FAIL:
-                    self.result = ResultOutcome.FAIL
-                self._set_note(f'check result: {check.result.value}')
-            else:
-                raise tmt.utils.SpecificationError(
-                    f"Invalid result '{check_result}' in check for test '{self.name}'.")
+        if note:
+            self._set_note(note)
 
         # Now handle the overall test result
         if interpret_test == ResultInterpret.RESPECT: