From 3c0f414feba8aeab180b0fde854b2c388d0e0df7 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 | 6 ++ spec/plans/results.fmf | 3 + stories/features/report-result.fmf | 10 +- tests/execute/result/main.fmf | 3 + tests/execute/result/subresults.sh | 23 +++++ tests/execute/result/subresults/.fmf/version | 1 + .../subresults/beaker-phases-subresults.sh | 26 +++++ tests/execute/result/subresults/main.fmf | 8 ++ tests/execute/result/subresults/test.fmf | 32 +++++++ tmt/result.py | 14 ++- tmt/schemas/results.yaml | 10 ++ tmt/steps/execute/__init__.py | 95 +++++++++++++------ 12 files changed, 192 insertions(+), 39 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 163b1de6fc..c250b4327f 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -4,6 +4,12 @@ Releases ====================== +tmt-1.37 +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Each execution of ``tmt-report-result`` command inside a test will now create a +tmt subresult which gets assigned to the test. + tmt-1.36 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 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/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..517470d0f5 --- /dev/null +++ b/tests/execute/result/subresults.sh @@ -0,0 +1,23 @@ +#!/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 "Verbose execute prints result" + # TODO: fix the return code? + rlRun "tmt run --id $run_dir --scratch -v 2>&1 >/dev/null | tee output" 1 + rlPhaseEnd + + rlPhaseStartCleanup + rlRun "rm output" + 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..4c742c11c4 --- /dev/null +++ b/tests/execute/result/subresults/beaker-phases-subresults.sh @@ -0,0 +1,26 @@ +#!/bin/bash +. /usr/share/beakerlib/beakerlib.sh || exit 1 + + +rlJournalStart + rlPhaseStartSetup + rlRun "tmp=\$(mktemp -d)" 0 "Create tmp directory" + rlRun "pushd $tmp" + rlRun "set -o pipefail" + rlPhaseEnd + + rlPhaseStartTest + rlRun "echo mytest-pass | tee output" 0 "Check output" + rlAssertGrep "mytest-pass" "output" + rlPhaseEnd + + rlPhaseStartTest + rlRun "echo mytest-fail | tee output" 0 "Check output" + rlAssertGrep "asdf-asdf" "output" + rlPhaseEnd + + rlPhaseStartCleanup + 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..480fb5874f --- /dev/null +++ b/tests/execute/result/subresults/main.fmf @@ -0,0 +1,8 @@ +/plan: + discover: + how: fmf + provision: + #how: local + 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..90e1b86aa5 --- /dev/null +++ b/tests/execute/result/subresults/test.fmf @@ -0,0 +1,32 @@ +/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 + + # Explicitly set the TESTID to non-empty value. Also, set the + # BEAKERLIB_COMMAND_REPORT_RESULT to `tmt-report-result` explicitly. The + # command in this variable gets called with every `rlPhaseEnd`. + # + # Refs: + # - https://github.com/teemtee/tmt/issues/2826#issue-2225993479 + # - https://github.com/teemtee/tmt/issues/2826#issuecomment-2085014960 + environment: + TESTID: 12345678 + BEAKERLIB_COMMAND_REPORT_RESULT: tmt-report-result + + # Also, explicitly set the framework + framework: beakerlib + + test: ./beaker-phases-subresults.sh diff --git a/tmt/result.py b/tmt/result.py index 39bae5f8ce..294ead3f80 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=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..ecdc14b9de 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. """ - # 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 + 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)