From 39181f969fc6a39a9a26c5ce5994d8341a865f8e Mon Sep 17 00:00:00 2001 From: Otto Sabart Date: Fri, 6 Sep 2024 09:00:00 +0200 Subject: [PATCH] Save the subresults for tmt-report-result All the results generated by tmt-report-results become objects of `tmt.result.SubResult`. --- docs/releases.rst | 9 ++ spec/plans/results.fmf | 3 + stories/features/report-result.fmf | 10 +- tests/execute/filesubmit/test.sh | 15 ++- tests/execute/result/main.fmf | 3 + tests/execute/result/subresults.sh | 63 ++++++++++++ tests/execute/result/subresults/.fmf/version | 1 + .../subresults/beaker-phases-subresults.sh | 33 +++++++ tests/execute/result/subresults/main.fmf | 7 ++ tests/execute/result/subresults/test.fmf | 21 ++++ tmt/frameworks/beakerlib.py | 11 ++- tmt/result.py | 14 ++- tmt/schemas/results.yaml | 10 ++ tmt/steps/execute/__init__.py | 99 ++++++++++++------- 14 files changed, 253 insertions(+), 46 deletions(-) create mode 100755 tests/execute/result/subresults.sh create mode 100644 tests/execute/result/subresults/.fmf/version create mode 100755 tests/execute/result/subresults/beaker-phases-subresults.sh create mode 100644 tests/execute/result/subresults/main.fmf create mode 100644 tests/execute/result/subresults/test.fmf diff --git a/docs/releases.rst b/docs/releases.rst index e520c4f730..869ed10b1e 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -4,6 +4,15 @@ Releases ====================== +tmt-1.38.0 +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Each execution of ``tmt-report-result`` command inside a test will now create a +tmt subresult. The same behavior applies also for the beakerlib where the +``tmt-report-result`` gets called with every ``rlPhaseEnd`` macro. The +subresults are always assigned under the parent tmt result. + + tmt-1.37.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/spec/plans/results.fmf b/spec/plans/results.fmf index 9d2616dbc0..10c9762f00 100644 --- a/spec/plans/results.fmf +++ b/spec/plans/results.fmf @@ -141,6 +141,9 @@ description: | check: ... + # String, path to /data directory storing possible test artifacts + data-path: path/to/test/data + .. _/spec/plans/results/outcomes: The ``result`` key can have the following values: diff --git a/stories/features/report-result.fmf b/stories/features/report-result.fmf index 7cebc22877..a768a7095e 100644 --- a/stories/features/report-result.fmf +++ b/stories/features/report-result.fmf @@ -12,14 +12,12 @@ description: | guest and overwrite any existing scripts with the same name. The command can be called multiple times for a single test, - the final result will be the most severe rating. Available - values ordered by severity are SKIP, PASS, WARN and FAIL. - Note, that the only value currently processed by ``tmt`` is - the test result, other options are silently ignored. + all these calls will be saved as tmt subresults. The final result + will be the most severe rating. Available values ordered by severity are + SKIP, PASS, WARN and FAIL. When ``tmt-report-result`` is called, the return value of the - test itself is ignored, and only result saved by - ``tmt-report-result`` is consumed by tmt. + test itself is saved as a tmt subresult. __ https://restraint.readthedocs.io/en/latest/commands.html#rstrnt-report-result diff --git a/tests/execute/filesubmit/test.sh b/tests/execute/filesubmit/test.sh index 761ead432a..64f235c4f5 100755 --- a/tests/execute/filesubmit/test.sh +++ b/tests/execute/filesubmit/test.sh @@ -10,18 +10,25 @@ rlJournalStart # would be set by TMT_TEST_DATA tmt_test_data="default/plan/execute/data/guest/default-0/default-1/data" + tmt_results_yaml_file="$tmp/default/plan/execute/results.yaml" rlPhaseStartTest rlRun "tmt run -vfi $tmp -a provision -h container" FILE_PATH=$tmp/$tmt_test_data/this_file.txt - BUNDLE_PATH=$tmp/$tmt_test_data/tmp-bundle_name.tar.gz + + # The `TESTID` is set to same value as `TMT_TEST_SERIAL_NUMBER`. Get + # the serial number of a first test. + tmt_test_serial_number=$(yq -e '.[] | select(.name == "/") | .["serial-number"]' "$tmt_results_yaml_file") + + # The bundle name is suffixed with `TESTID` value + BUNDLE_PATH="$tmp/$tmt_test_data/tmp-bundle_name-${tmt_test_serial_number}.tar.gz" # File was submitted and has correct content - rlAssertExists $FILE_PATH - rlAssertGrep "YES" $FILE_PATH + rlAssertExists "$FILE_PATH" + rlAssertGrep "YES" "$FILE_PATH" # Bundle was submitted and has correct content - rlAssertExists $BUNDLE_PATH + rlAssertExists "$BUNDLE_PATH" rlRun "tar tzf $BUNDLE_PATH | grep /this_file.txt" \ 0 "Bundle contains an expected file" diff --git a/tests/execute/result/main.fmf b/tests/execute/result/main.fmf index 97bf9981ee..3a204c8cef 100644 --- a/tests/execute/result/main.fmf +++ b/tests/execute/result/main.fmf @@ -7,3 +7,6 @@ /repeated: summary: Repeated test should provide multiple results test: ./repeated.sh +/subresults: + summary: Multiple calls to tmt-report-result should generate tmt subresults + test: ./subresults.sh diff --git a/tests/execute/result/subresults.sh b/tests/execute/result/subresults.sh new file mode 100755 index 0000000000..c65f06cb42 --- /dev/null +++ b/tests/execute/result/subresults.sh @@ -0,0 +1,63 @@ +#!/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_dir=\$(mktemp -d)" 0 "Create run directory" + rlRun "pushd subresults" + rlRun "set -o pipefail" + rlPhaseEnd + + rlPhaseStartTest "Test the subresults were generated into results.yaml" + rlRun "tmt run --id $run_dir --scratch -v 2>&1 >/dev/null | tee output" 1 + + # Check the parent test outcomes + rlAssertGrep "fail /test/beakerlib (on default-0)" "output" + rlAssertGrep "fail /test/fail (on default-0)" "output" + rlAssertGrep "pass /test/pass (on default-0)" "output" + rlAssertGrep "total: 1 test passed and 2 tests failed" "output" + + # Check subtests outcomes + + ## The internal tests which is checking the TESTID and + ## BEAKERLIB_COMMAND_REPORT_RESULT variables must pass + rlAssertGrep "pass /test/beakerlib/Internal-test-of-environment-variable-values" "output" + + rlAssertGrep "pass /test/beakerlib/phase-setup" "output" + rlAssertGrep "pass /test/beakerlib/phase-test-pass" "output" + rlAssertGrep "fail /test/beakerlib/phase-test-fail" "output" + rlAssertGrep "pass /test/beakerlib/phase-cleanup" "output" + + rlAssertGrep "warn /test/fail/subtest/weird" "output" + rlAssertGrep "pass /test/pass/subtest/good2" "output" + + # Check the subresults get correctly saved in results.yaml + rlRun "results_file=${run_dir}/plan/execute/results.yaml" + + rlRun "yq -ey '.[] | select(.name == \"/test/beakerlib\") | .subresult' ${results_file} > subresults_beakerlib.yaml" + rlAssertGrep "name: /test/beakerlib/phase-setup" "subresults_beakerlib.yaml" + rlAssertGrep "name: /test/beakerlib/phase-test-pass" "subresults_beakerlib.yaml" + rlAssertGrep "name: /test/beakerlib/phase-test-fail" "subresults_beakerlib.yaml" + rlAssertGrep "name: /test/beakerlib/phase-cleanup" "subresults_beakerlib.yaml" + + rlRun "yq -ey '.[] | select(.name == \"/test/fail\") | .subresult' ${results_file} > subresults_fail.yaml" + rlAssertGrep "name: /test/fail/subtest/good" "subresults_fail.yaml" + rlAssertGrep "name: /test/fail/subtest/fail" "subresults_fail.yaml" + rlAssertGrep "name: /test/fail/subtest/weird" "subresults_fail.yaml" + + rlRun "yq -ey '.[] | select(.name == \"/test/pass\") | .subresult' ${results_file} > subresults_pass.yaml" + rlAssertGrep "name: /test/pass/subtest/good0" "subresults_pass.yaml" + rlAssertGrep "name: /test/pass/subtest/good1" "subresults_pass.yaml" + rlAssertGrep "name: /test/pass/subtest/good2" "subresults_pass.yaml" + rlPhaseEnd + + rlPhaseStartCleanup + rlRun "rm output" + rlRun "rm subresults_{beakerlib,fail,pass}.yaml" + rlRun "popd" + rlRun "rm -rf $run_dir" 0 "Remove run directory" + rlPhaseEnd + +rlJournalEnd diff --git a/tests/execute/result/subresults/.fmf/version b/tests/execute/result/subresults/.fmf/version new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/execute/result/subresults/.fmf/version @@ -0,0 +1 @@ +1 diff --git a/tests/execute/result/subresults/beaker-phases-subresults.sh b/tests/execute/result/subresults/beaker-phases-subresults.sh new file mode 100755 index 0000000000..58ceb1651a --- /dev/null +++ b/tests/execute/result/subresults/beaker-phases-subresults.sh @@ -0,0 +1,33 @@ +#!/bin/bash +. /usr/share/beakerlib/beakerlib.sh || exit 1 + + +rlJournalStart + rlPhaseStartSetup "phase-setup" + rlRun "tmp=\$(mktemp -d)" 0 "Create tmp directory" + rlRun "pushd $tmp" + rlRun "set -o pipefail" + rlPhaseEnd + + rlPhaseStartTest "Internal test of environment variable values" + rlRun "test -n \"\$TMT_TEST_SERIAL_NUMBER\" -o -n \"\$TESTID\"" 0 "Check the variables are not empty" + rlAssertEquals "TESTID must be set to TMT_TEST_SERIAL_NUMBER" "$TESTID" "$TMT_TEST_SERIAL_NUMBER" + + rlRun "[[ \$BEAKERLIB_COMMAND_REPORT_RESULT =~ tmt-report-result$ ]]" 0 "Check the variable contains path to a tmt-report-result script" + rlPhaseEnd + + rlPhaseStartTest "phase-test pass" + rlRun "echo mytest-pass | tee output" 0 "Check output" + rlAssertGrep "mytest-pass" "output" + rlPhaseEnd + + rlPhaseStartTest "phase-test fail" + rlRun "echo mytest-fail | tee output" 0 "Check output" + rlAssertGrep "asdf-asdf" "output" + rlPhaseEnd + + rlPhaseStartCleanup "phase-cleanup" + rlRun "popd" + rlRun "rm -r $tmp" 0 "Remove tmp directory" + rlPhaseEnd +rlJournalEnd diff --git a/tests/execute/result/subresults/main.fmf b/tests/execute/result/subresults/main.fmf new file mode 100644 index 0000000000..4beeae169e --- /dev/null +++ b/tests/execute/result/subresults/main.fmf @@ -0,0 +1,7 @@ +/plan: + discover: + how: fmf + provision: + how: container + execute: + how: tmt diff --git a/tests/execute/result/subresults/test.fmf b/tests/execute/result/subresults/test.fmf new file mode 100644 index 0000000000..301f862aa6 --- /dev/null +++ b/tests/execute/result/subresults/test.fmf @@ -0,0 +1,21 @@ +/pass: + summary: Basic test of subresults + test: | + tmt-report-result /subtest/good0 PASS + tmt-report-result /subtest/good1 PASS + tmt-report-result /subtest/good2 PASS + +/fail: + summary: Reduced outcome of subresults must be fail + test: | + tmt-report-result /subtest/good PASS + tmt-report-result /subtest/fail FAIL + tmt-report-result /subtest/weird WARN + +/beakerlib: + summary: Beakerlib rlPhaseEnd as tmt subresult + + # Also, explicitly set the framework + framework: beakerlib + + test: ./beaker-phases-subresults.sh diff --git a/tmt/frameworks/beakerlib.py b/tmt/frameworks/beakerlib.py index a722f0b82b..02bb8783ee 100644 --- a/tmt/frameworks/beakerlib.py +++ b/tmt/frameworks/beakerlib.py @@ -36,7 +36,16 @@ def get_environment_variables( return Environment({ 'BEAKERLIB_DIR': EnvVarValue(invocation.path), 'BEAKERLIB_COMMAND_SUBMIT_LOG': EnvVarValue( - f'bash {tmt.steps.execute.TMT_FILE_SUBMIT_SCRIPT.path}') + f'bash {tmt.steps.execute.TMT_FILE_SUBMIT_SCRIPT.path}'), + + # The command in this variable gets called with every `rlPhaseEnd` call in beakerlib. + 'BEAKERLIB_COMMAND_REPORT_RESULT': EnvVarValue( + f'bash {tmt.steps.execute.TMT_REPORT_RESULT_SCRIPT.path}'), + + # This variables must be set explicitly, otherwise the beakerlib `rlPhaseEnd` macro + # will not call the the command in `BEAKERLIB_COMMAND_REPORT_RESULT`. + # - https://github.com/beakerlib/beakerlib/blob/cfa801fb175fef1e47b8552d6cf6efcb51df7227/src/testing.sh#L1074 + 'TESTID': EnvVarValue(str(invocation.test.serial_number)), }) @classmethod diff --git a/tmt/result.py b/tmt/result.py index c807f5e817..f052c0a124 100644 --- a/tmt/result.py +++ b/tmt/result.py @@ -161,6 +161,12 @@ class SubResult(BaseResult): SubCheckResult.from_serialized(check) for check in serialized] ) + data_path: Optional[Path] = field( + default=cast(Optional[Path], None), + serialize=lambda path: None if path is None else str(path), + unserialize=lambda value: None if value is None else Path(value) + ) + @dataclasses.dataclass class PhaseResult(BaseResult): @@ -218,7 +224,9 @@ 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, + subresult: Optional[list[SubResult]] = None, + ) -> 'Result': """ Create a result from a test invocation. @@ -268,7 +276,9 @@ def from_test_invocation( ids=ids, log=log or [], guest=guest_data, - data_path=invocation.relative_test_data_path) + data_path=invocation.relative_test_data_path, + subresult=subresult or [], + ) return _result.interpret_result(ResultInterpret( invocation.test.result) if invocation.test.result else ResultInterpret.RESPECT) diff --git a/tmt/schemas/results.yaml b/tmt/schemas/results.yaml index 6668c51428..5abb82ce8c 100644 --- a/tmt/schemas/results.yaml +++ b/tmt/schemas/results.yaml @@ -81,6 +81,10 @@ definitions: check: $ref: "#/definitions/check_results" + # TODO: Fixme? + data-path: + type: string + required: - name - result @@ -135,6 +139,12 @@ items: duration: $ref: "#/definitions/duration" + # TODO: Fixme? e.g. using ref. otherwise I am getting the error: + # warn: Result format violation: 0 - Additional properties are not allowed + # ('data-path' was unexpected) + data-path: + type: string + ids: type: object patternProperties: diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index f1da4e7d52..de3bf8f200 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -664,48 +664,81 @@ def _load_tmt_report_results_file(self, invocation: TestInvocation) -> ResultCol return collection - def _process_results_reduce( + @staticmethod + def _subresult_outcomes_reduce(outcomes: list[ResultOutcome]) -> ResultOutcome: + """ + + Reduce given subresult outcomes to one main outcome. + + This is the default behavior applied to test subresults, all subresults will be reduced + to the worst outcome possible. + + :param outcomes: subresult outcomes to reduce. + :returns: parent result outcome. + """ + hierarchy = [ + ResultOutcome.SKIP, + ResultOutcome.PASS, + ResultOutcome.WARN, + ResultOutcome.FAIL] + outcome_indices = [hierarchy.index(outcome) for outcome in outcomes] + + # The actual outcome we decided is the best representing the subresults. + return hierarchy[max(outcome_indices)] + + def _process_subresults( self, invocation: TestInvocation, results: list['tmt.result.RawResult']) -> list['tmt.result.Result']: """ - Reduce given results to one outcome. - This is the default behavior applied to test results, all - results will be reduced to the worst outcome possible. + Load the subresults and assign them to their respective parent result defined by current + invocation. + + Also, reduce given subresult outcomes to one which is assigned to a parent result. + + This is the default behavior applied to test subresults, all subresult outcomes will be + reduced to the worst outcome possible. :param invocation: test invocation to which the results belong to. - :param results: results to reduce. - :returns: list of results. + :param results: results to load as subresults. + :returns: list of parent results with assigned subresults. """ - # The worst result outcome we can find among loaded results... + # The worst result outcome we can find among loaded results. original_outcome: Optional[ResultOutcome] = None - # ... and the actual outcome we decided is the best representing - # the results. - # The original one may be left unset - malformed results file, - # for example, provides no usable original outcome. + + # The actual outcome we decided is the best representing the results. + # The original one may be left unset - malformed results file, for example, provides no + # usable original outcome. actual_outcome: ResultOutcome note: Optional[str] = None + # Loaded tmt subresults + subresults: list[tmt.result.SubResult] = [] + + # Load the results as tmt SubResults try: - outcomes = [ - ResultOutcome.from_spec(result.get('result')) - for result in results - ] + for result in results: + subresult = tmt.result.SubResult.from_serialized(result) + + # Ensure that subresult name starts with '/' + if not subresult.name.startswith('/'): + subresult.note = "subresult name should start with '/'" + subresult.name = f'/{subresult.name}' + + # Change the name hierarchy, so the subresult name is under the parent result name + subresult.name = f'{invocation.test.name}{subresult.name}' + + # TODO: The subresult checks are not saved yet, will be implemented in the future + subresults.append(subresult) except tmt.utils.SpecificationError as exc: actual_outcome = ResultOutcome.ERROR note = exc.message - else: - hierarchy = [ - ResultOutcome.SKIP, - ResultOutcome.PASS, - ResultOutcome.WARN, - ResultOutcome.FAIL] - outcome_indices = [hierarchy.index(outcome) for outcome in outcomes] - actual_outcome = original_outcome = hierarchy[max(outcome_indices)] + actual_outcome = original_outcome = self._subresult_outcomes_reduce( + [r.result for r in subresults]) # Find a usable log - the first one matching our "interim" outcome. # We cannot use the "actual" outcome, because that one may not even @@ -715,14 +748,12 @@ def _process_results_reduce( test_logs = [invocation.relative_path / TEST_OUTPUT_FILENAME] if original_outcome is not None: - for result in results: - if result.get('result') != original_outcome.value: + for subresult in subresults: + if subresult.result != original_outcome: continue - result_logs: list[str] = result.get('log', []) - - if result_logs: - test_logs.append(invocation.relative_test_data_path / result_logs[0]) + if subresult.log: + test_logs.append(invocation.relative_test_data_path / subresult.log[0]) break @@ -730,7 +761,8 @@ def _process_results_reduce( invocation=invocation, result=actual_outcome, log=test_logs, - note=note)] + note=note, + subresult=subresults)] def _process_results_partials( self, @@ -834,7 +866,8 @@ def extract_tmt_report_results(self, invocation: TestInvocation) -> list["tmt.Re """ Extract results from the file generated by ``tmt-report-result`` script. - All recorded results are reduced to one result eventually. + All recorded results are saved as subresults and their outomes are reduced to one result + which is assigned to a parent result. """ collection = self._load_tmt_report_results_file(invocation) @@ -854,7 +887,7 @@ def extract_tmt_report_results(self, invocation: TestInvocation) -> list["tmt.Re collection.validate() - return self._process_results_reduce(invocation, collection.results) + return self._process_subresults(invocation, collection.results) def extract_tmt_report_results_restraint( self, @@ -904,7 +937,7 @@ def extract_results( invocation=invocation, default_log=invocation.relative_path / TEST_OUTPUT_FILENAME) - # Handle the 'tmt-report-result' command results as a single test + # Handle the 'tmt-report-result' command results and save them as tmt subresults if self._tmt_report_results_filepath(invocation).exists(): return self.extract_tmt_report_results(invocation)