Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save the subresults for tmt-report-result #3200

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/releases.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
3 changes: 3 additions & 0 deletions spec/plans/results.fmf
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 4 additions & 6 deletions stories/features/report-result.fmf
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 11 additions & 4 deletions tests/execute/filesubmit/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
3 changes: 3 additions & 0 deletions tests/execute/result/main.fmf
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@
/special:
summary: Test special characters generated to tmt-report-results.yaml
test: ./special.sh
/subresults:
summary: Multiple calls to tmt-report-result should generate tmt subresults
test: ./subresults.sh
63 changes: 63 additions & 0 deletions tests/execute/result/subresults.sh
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions tests/execute/result/subresults/.fmf/version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
33 changes: 33 additions & 0 deletions tests/execute/result/subresults/beaker-phases-subresults.sh
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions tests/execute/result/subresults/main.fmf
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/plan:
discover:
how: fmf
provision:
how: container
execute:
how: tmt
21 changes: 21 additions & 0 deletions tests/execute/result/subresults/test.fmf
Original file line number Diff line number Diff line change
@@ -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
22 changes: 20 additions & 2 deletions tmt/frameworks/beakerlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -118,8 +127,17 @@ def extract_results(
# Finally we have a valid result
else:
actual_result = ResultOutcome.from_spec(result.lower())

# The beakerlib rlPhaseEnd calls the tmt-report-result and generates a
# tmt-report-results.yaml. Propagate these results as subresults to parent result.
# TODO: Somehow handle the test_logs and note from subresults
# TODO: Do we want to propagate the (reduced) actual_outcome from subresults? Probably not.
actual_outcome, test_logs, note, subresults = invocation.phase.extract_tmt_report_subresults(
invocation)

return [tmt.Result.from_test_invocation(
invocation=invocation,
result=actual_result,
note=note,
log=log)]
log=log,
subresult=subresults)]
13 changes: 11 additions & 2 deletions tmt/result.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,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):
Expand Down Expand Up @@ -238,7 +244,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.

Expand Down Expand Up @@ -286,7 +294,8 @@ 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,
subresult=subresult or [])

return _result.interpret_result(ResultInterpret(
invocation.test.result) if invocation.test.result else ResultInterpret.RESPECT)
Expand Down
10 changes: 10 additions & 0 deletions tmt/schemas/results.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ definitions:
check:
$ref: "#/definitions/check_results"

# TODO: Fixme?
data-path:
type: string

required:
- name
- result
Expand Down Expand Up @@ -135,6 +139,12 @@ items:
duration:
$ref: "#/definitions/duration"

# TODO: Fixme? e.g. using ref. otherwise I am getting the error:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@happz Aren't we missing the data-path in the result and subresult schemas? Otherwise, I'm still getting the "result format violation" errors.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Result schema is definitely missing data-path key, subresults shouldn't have one, IIUIC, data path is a path in which tmt stores test data and info and stuff, so there is no dedicated subpath for a subresult.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about the data-path in the subresult? I do not have much insight, but it seems the subresults generated by tmt-report-result are using this field when the tmt-report-results.yaml gets created:

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, that does not look right. TMT_TEST_EXE is ${TMT_TREE/tree/execute}, TMT_TEST_DATA is said to be "Path to the directory where test can store logs and other artifacts generated during its execution." and I can't imagine that ${TMT_TEST_DATA/$TMT_TREE/tree/execute} could be anything sane.

@psss WDYT? There is "test data" for the test, TMT_TEST_DATA points to it, but data paths for Beakerlib phases? tmt does not manage those, in custom results we do not even have any idea whether the test used any subresult data paths, so we don't touch them, this seems like a mistake to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do you think we should somehow limit the data-path using a pattern in the schema?

For a parent result, the value is always a relative path (data/guest/default-0/tests/example-1/data). For subresults, the data-path seems to always start with pattern: "^/" - it's not relative, but absolute where root starts at 'execute' directory, e.g. /data/guest/default-0/tests/example-1/data.

Isn't it a bug? Also, the subresult's data-path seems to be always the same as the parent data-path (without / prefix).

# warn: Result format violation: 0 - Additional properties are not allowed
# ('data-path' was unexpected)
data-path:
type: string

ids:
type: object
patternProperties:
Expand Down
Loading
Loading