Implement check result key, checks affect test result by default (#…

…3239)

* Implement check result key feature
* Add check-result spec and release note
* Move and modify check schema
* Fix doc generation

Co-authored-by: Miloš Prchlík <mprchlik@redhat.com>
Co-authored-by: Petr Šplíchal <psplicha@redhat.com>

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

docs/scripts/generate-plugins.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 
 import dataclasses
+import enum
 import sys
 import textwrap
 from typing import Any
@@ -56,7 +57,7 @@ def _is_inherited(
 
     # TODO: for now, it's a list, but inspecting the actual tree of classes
     # would be more generic. It's good enough for now.
-    return field.name in ('name', 'where', 'order', 'summary', 'enabled')
+    return field.name in ('name', 'where', 'order', 'summary', 'enabled', 'result')
 
 
 def container_ignored_fields(container: ContainerClass) -> list[str]:
@@ -106,6 +107,12 @@ def container_intrinsic_fields(container: ContainerClass) -> list[str]:
     return field_names
 
 
+def is_enum(value: Any) -> bool:
+    """ Find out whether a given value is an enum member """
+
+    return isinstance(value, enum.Enum)
+
+
 def _create_step_plugin_iterator(registry: tmt.plugins.PluginRegistry[tmt.steps.Method]):
     """ Create iterator over plugins of a given registry """
 
@@ -184,6 +191,7 @@ def main() -> None:
         STEP=step_name,
         PLUGINS=plugin_generator,
         REVIEWED_PLUGINS=REVIEWED_PLUGINS,
+        is_enum=is_enum,
         container_fields=tmt.utils.container_fields,
         container_field=tmt.utils.container_field,
         container_ignored_fields=container_ignored_fields,

docs/templates/plugins.rst.j2

@@ -7,6 +7,8 @@
 {{ option }}: ``{{ metadata.metavar }}``
     {% elif metadata.default is boolean %}
 {{ option }}: ``true|false``
+    {% elif metadata.choices %}
+{{ option }}: ``{{ metadata.choices }}``
     {% else %}
 {{ option }}:
     {% endif %}
@@ -27,11 +29,13 @@
         {% elif actual_default is sequence %}
             {% if not actual_default %}
     Default: *not set*
-            {% else %}
+        {% else %}
     Default: {% for default_item in actual_default %}``{{ default_item.pattern | default(default_item) }}``{% if not loop.last %}, {% endif %}{% endfor %}
             {% endif %}
+        {% elif is_enum(actual_default) %}
+    Default: ``{{ actual_default.value }}``
         {% else %}
-            {% set _ = LOGGER.warn("%s/%s.%s: could not render default value, '%s'" | format(STEP, plugin_id, field_name, actual_default)) %}
+            {% set _ = LOGGER.warn("%s/%s.%s: could not render default value, '%s'" | format(STEP, plugin_id, field_name, actual_default), shift=0) %}
     Default: *could not render default value correctly*
         {% endif %}
     {% endif %}
@@ -71,7 +75,7 @@ The following keys are accepted by all plugins of the ``{{ STEP }}`` step.
     Please, be aware that the documentation below is a work in progress. We are
     working on fixing it, adding missing bits and generally making it better.
     Also, it was originally used for command line help only, therefore the
-    formatting is often suboptional.
+    formatting is often suboptimal.
 {% endif %}
 
 {% if PLUGIN.__doc__ %}

spec/tests/check.fmf

@@ -14,6 +14,29 @@ description: |
     panic detection, core dump collection or collection of system
     logs.
 
+    By default, the check results affect the overall test outcome.
+    To change this behaviour, use the ``result`` key, which accepts
+    the following values:
+
+        respect
+            The check result is respected and affects the overall
+            test result. This is the default.
+
+        xfail
+            The check result is expected to fail (pass becomes
+            fail and vice-versa).
+
+        info
+            The check result is treated as an informational
+            message and does not affect the overall test result.
+
+    .. warning::
+
+        Note that running one check multiple times for the same
+        test is not yet supported.
+
+    .. versionchanged:: 1.38.0 the ``result`` key added
+
     See :ref:`/plugins/test-checks` for the list of available checks.
 
 example:
@@ -37,7 +60,23 @@ example:
       - how: test-inspector
         enable: false
 
+  - |
+    # Expect the AVC check to fail
+    check:
+      - how: avc
+        result: xfail
+      - how: dmesg
+        result: respect
+
+  - |
+    # Treat the dmesg check as informational only
+    check:
+      - how: dmesg
+        result: info
+
 link:
   - implemented-by: /tmt/checks
+  - implemented-by: /tmt/result.py
   - verified-by: /tests/test/check/avc
   - verified-by: /tests/test/check/dmesg
+  - verified-by: /tests/execute/result/check

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
 

tests/execute/result/basic.sh

@@ -15,7 +15,7 @@ run()
     if [ -z "${orig}" ]; then # No original result provided
         rlAssertGrep "${res} ${tn}$" $rlRun_LOG
     else
-        rlAssertGrep "${res} ${tn} (original result: ${orig})$" $rlRun_LOG
+        rlAssertGrep "${res} ${tn} (.*original test result: ${orig}.*)$" $rlRun_LOG
     fi
 
     echo
@@ -38,7 +38,7 @@ rlJournalStart
         run   "errr"   "/test/error"          ""         2
         run   "pass"   "/test/xfail-fail"     "fail"     0
         run   "fail"   "/test/xfail-pass"     "pass"     1
-        run   "errr"   "/test/xfail-error"    "error"    2
+        run   "errr"   "/test/xfail-error"    ""         2
         run   "pass"   "/test/always-pass"    "fail"     0
         run   "info"   "/test/always-info"    "pass"     0
         run   "warn"   "/test/always-warn"    "pass"     1
@@ -58,18 +58,18 @@ rlJournalStart
                 rlAssertGrep "$line" "$rlRun_LOG" -F
             fi
         done <<-EOF
-00:00:00 errr /test/always-error (on default-0) (original result: pass) [1/12]
-00:00:00 fail /test/always-fail (on default-0) (original result: pass) [2/12]
-00:00:00 info /test/always-info (on default-0) (original result: pass) [3/12]
-00:00:00 pass /test/always-pass (on default-0) (original result: fail) [4/12]
-00:00:00 warn /test/always-warn (on default-0) (original result: pass) [5/12]
+00:00:00 errr /test/always-error (on default-0) (test result overridden: error, original test result: pass) [1/12]
+00:00:00 fail /test/always-fail (on default-0) (test result overridden: fail, original test result: pass) [2/12]
+00:00:00 info /test/always-info (on default-0) (test result overridden: info, original test result: pass) [3/12]
+00:00:00 pass /test/always-pass (on default-0) (test result overridden: pass, original test result: fail) [4/12]
+00:00:00 warn /test/always-warn (on default-0) (test result overridden: warn, original test result: pass) [5/12]
 00:00:00 errr /test/error (on default-0) [6/12]
 00:00:01 errr /test/error-timeout (on default-0) (timeout) [7/12]
 00:00:00 fail /test/fail (on default-0) [8/12]
 00:00:00 pass /test/pass (on default-0) [9/12]
-00:00:00 errr /test/xfail-error (on default-0) (original result: error) [10/12]
-00:00:00 pass /test/xfail-fail (on default-0) (original result: fail) [11/12]
-00:00:00 fail /test/xfail-pass (on default-0) (original result: pass) [12/12]
+00:00:00 errr /test/xfail-error (on default-0) [10/12]
+00:00:00 pass /test/xfail-fail (on default-0) (test failed as expected, original test result: fail) [11/12]
+00:00:00 fail /test/xfail-pass (on default-0) (test was expected to fail, original test result: pass) [12/12]
 EOF
     rlPhaseEnd
 
@@ -78,7 +78,8 @@ EOF
         rlRun -s "tmt run --id \${run} --scratch --until execute tests -n /xfail-with-reboot provision --how container execute -v 2>&1 >/dev/null"
         EXPECTED=$(cat <<EOF
             00:00:00 /test/xfail-with-reboot [1/1]
-            00:00:00 pass /test/xfail-with-reboot (on default-0) (original result: fail) [1/1]
+            00:00:00 pass /test/xfail-with-reboot (on default-0) (test failed as expected, original test result: fail) [1/1]
+
 EOF
 )
     rlAssertEquals "Output matches the expectation" "$EXPECTED" "$(grep /test/xfail-with-reboot $rlRun_LOG)"

tests/execute/result/check.sh

@@ -0,0 +1,29 @@
+#!/bin/bash
+. /usr/share/beakerlib/beakerlib.sh || exit 1
+
+rlJournalStart
+    rlPhaseStartSetup
+        rlRun "run=\$(mktemp -d)" 0 "Create run directory"
+        rlRun "PROVISION_HOW=${PROVISION_HOW:-virtual}"
+        rlRun "pushd check"
+        rlRun "set -o pipefail"
+    rlPhaseEnd
+
+    rlPhaseStartTest "Check Results"
+        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 (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 "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
+
+    rlPhaseStartCleanup
+        rlRun "popd"
+        rlRun "rm -r ${run}" 0 "Remove run directory"
+    rlPhaseEnd
+rlJournalEnd

tests/execute/result/check/.fmf/version

@@ -0,0 +1 @@
+1

tests/execute/result/check/test.fmf

@@ -0,0 +1,89 @@
+summary: Tests for check results behaviour
+description: Verify that check results are correctly interpreted and affect test results
+framework: shell
+duration: 1m
+
+/check-pass:
+    summary: Test with passing checks
+    description: |
+        Expected outcome: PASS (test passes, check passes)
+    test: echo "Test passed"
+    check:
+        - 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: |
+        Expected outcome: FAIL (test passes, but check fails and is respected)
+    test: echo "Fail Test Check Pattern" | tee /dev/kmsg
+    check:
+        - how: dmesg
+          failure-pattern: Fail Test Check Pattern
+
+/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" | tee /dev/kmsg
+    check:
+        - how: dmesg
+          failure-pattern: Fail Test Check Pattern
+          result: 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"
+    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)
+    test: echo "Fail Test Check Pattern" | tee /dev/kmsg
+    check:
+        - how: dmesg
+          failure-pattern: Fail Test Check Pattern
+          result: xfail
+
+/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" | tee /dev/kmsg
+    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" | tee /dev/kmsg
+    result: pass
+    check:
+        - how: dmesg
+          failure-pattern: Fail Test Check Pattern
+          result: respect

tests/execute/result/main.fmf

@@ -13,3 +13,10 @@
 /subresults:
     summary: Multiple calls to tmt-report-result should generate tmt subresults
     test: ./subresults.sh
+/check:
+    summary: Test check result interpretations
+    test: ./check.sh
+    tag+:
+      - provision-only
+      - provision-local
+      - provision-virtual

tests/report/html/test.sh

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

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
 

tests/test/check/test-dmesg.sh

@@ -65,7 +65,7 @@ rlJournalStart
             rlRun "dump_before=$segfault/checks/dmesg-before-test.txt"
             rlRun "dump_after=$segfault/checks/dmesg-after-test.txt"
 
-            rlRun "tmt run --id $run --scratch -a -vv provision -h $PROVISION_HOW test -n /dmesg/segfault"
+            rlRun "tmt run --id $run --scratch -a -vv provision -h $PROVISION_HOW test -n /dmesg/segfault" "1"
             rlRun "cat $results"
 
             assert_check_result "dmesg as a before-test should pass" "pass" "before-test" "segfault"
@@ -84,7 +84,7 @@ rlJournalStart
             rlRun "dump_before=$custom_patterns/checks/dmesg-before-test.txt"
             rlRun "dump_after=$custom_patterns/checks/dmesg-after-test.txt"
 
-            rlRun "tmt run --id $run --scratch -a -vv provision -h $PROVISION_HOW test -n /dmesg/custom-patterns"
+            rlRun "tmt run --id $run --scratch -a -vv provision -h $PROVISION_HOW test -n /dmesg/custom-patterns" "1"
             rlRun "cat $results"
 
             assert_check_result "dmesg as a before-test should fail" "fail" "before-test" "custom-patterns"