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.

Sign up for GitHub

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

report: support multiple results reported in one test #2236

Closed
wants to merge 1 commit into from
Closed

report: support multiple results reported in one test #2236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion tmt/result.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class Result(tmt.utils.SerializableContainer):
""" Describes what tmt knows about a single test result """

name: str
classname: str
serialnumber: int = 0
fmf_id: Optional['tmt.base.FmfId'] = field(
default=cast(Optional['tmt.base.FmfId'], None),
Expand Down Expand Up @@ -115,6 +116,7 @@ def from_test(
*,
test: 'tmt.base.Test',
result: ResultOutcome,
subresultname: Optional[str] = None,
note: Optional[str] = None,
ids: Optional[Dict[str, Optional[str]]] = None,
log: Optional[List[Path]] = None,
Expand Down Expand Up @@ -156,8 +158,12 @@ def from_test(
guest_data = ResultGuestData(name=guest.name, role=guest.role) if guest is not None \
else ResultGuestData()

if not subresultname:
subresultname = test.name

_result = Result(
name=test.name,
classname=test.name,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see there's only one use of classname, it's set to test.name, is it the correct move? I'm not very fluent in junit, can you help me understand what's the purpose of classname field? I'm slightly worried it's a junit feature leaking into a generic Result class, and maybe we could find a better name for the attribute, because, well, Result.classname is surprisingly not holding a class name :)

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for reviewing!
While junit format origins from Java, classname is refer to a test class which could contain a few steps.
If there is only one step, set "classname" to None and use "name" to identify the result is enough.
If there are multiple sub-results within one test, like steps in a Java class, we need to use "classname" to id the testcase, and use "name" to id the steps.
I see your points of class name :) Python class :)
OK, I'll try to find a better way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, thank you. It seems to me tmt has no use for classname as you described it, now, tmt's internal execute plugin does not recognize classes like pytest or Java frameworks - I guess if we ever get a pytest framework support, this feature would be needed there as well.

Having support for subresults makes sense to me, that's sure. First, I'd definitely keep name around, your patch is dropping it, replacing it with classname - name represents a test name as known to tmt, the one defined in fmf data, that's a crucial piece of information and I don't see it going away. New fields are fine, dropping name not so much :)

Second, right now, there's just one tmt test, and with custom results, https://tmt.readthedocs.io/en/stable/spec/tests.html#result, a test can generate "virtual" results nested under the test name, e.g. test /tests/foo can produce results with name set to /tests/foo/bar and /tests/foo/baz. That's similar to your subresults, but without any formal field for the structure. subresult field looks like a very good addition then, we can use it already for these "virtual" custom results - name would be set as it is now, plus we would set subresult to whatever the custom result brings in. This would be worth its own PR, BTW, it'd be a useful feature on its own.

But, I'm not sure the subresult concept in this sense fits the test/class/method relationship. I suppose that under a single tmt test, several Python/Java classes can be executed, each with multiple test methods, giving us plenty of (class, method) tuples. I would maybe try to add classname and methodname fields right away, but I'm not so sure we should fill them with values that are not classname or methodname, e.g. test.name. I'm not very fond of encoding multiple pieces of info and a single field, if we wish to store classname and methodname, if recognized by the test, then let's have two explicit fields for it :) It would be excellent if we could set these two fields via tmt-report-result explicitly, not masking them as a subresult or test name. In other words, adding fields rather than changing the semantics of existing ones. What do you think?

name=subresultname,
serialnumber=test.serialnumber,
fmf_id=test.fmf_id,
result=result,
Expand Down
90 changes: 65 additions & 25 deletions tmt/steps/execute/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import copy
import dataclasses
import datetime
import os
from dataclasses import dataclass
from typing import TYPE_CHECKING, Any, List, Optional, Tuple, Type, cast

Expand Down Expand Up @@ -265,42 +266,81 @@ def load_tmt_report_results(self, test: "tmt.Test", guest: Guest) -> List["tmt.R
or an empty list if the file does not exist.
"""
report_result_path = self.data_path(test, guest, full=True) \
/ tmt.steps.execute.TEST_DATA \
/ TMT_REPORT_RESULT_SCRIPT.created_file
/ tmt.steps.execute.TEST_DATA

# Nothing to do if there's no result file
if not report_result_path.exists():
self.debug(f"tmt-report-results file '{report_result_path}' does not exist.")
return []

# Check the test result
self.debug(f"tmt-report-results file '{report_result_path} detected.")
self.debug(f"tmt-report-results file '{report_result_path}' detected.")

with open(report_result_path) as result_file:
result_list = [line for line in result_file.readlines() if "TESTRESULT" in line]
if not result_list:
raise tmt.utils.ExecuteError(
f"Test result not found in result file '{report_result_path}'.")
result = result_list[0].split("=")[1].strip()
if not report_result_path.glob(r'*restraint-result.*'):
self.debug(f"tmt-report-results file '{report_result_path}' is empty.")
return []

# Map the restraint result to the corresponding tmt value
actual_result = ResultOutcome.ERROR
note: Optional[str] = None
all_results = []
for f in sorted(report_result_path.glob(r'*restraint-result.*'), \
key=os.path.getmtime):
self.debug(f"tmt-report-results file '{f}' detected.")
with open(f) as result_file:
result_list = [line for line in result_file.readlines() if "TESTRESULT" in line]
with open(f) as result_file:
log_list = [line for line in result_file.readlines() if "OUTPUTFILE" in line]
with open(f) as result_file:
name_list = [line for line in result_file.readlines() if "TESTNAME" in line]

if not result_list:
raise tmt.utils.ExecuteError(
f"Test result not found in result file '{f}'.")
else:
if len(result_list[0].split("=")) == 2:
result = result_list[0].split("=")[1].strip()

try:
actual_result = ResultOutcome(result.lower())
except ValueError:
if result == 'SKIP':
actual_result = ResultOutcome.INFO
if not log_list:
self.debug(f"Test log not found in result file '{f}'.")
logfile=[self.data_path(test, guest, TEST_OUTPUT_FILENAME)]
else:
note = f"invalid test result '{result}' in result file"

return [tmt.Result.from_test(
test=test,
result=actual_result,
log=[self.data_path(test, guest, TEST_OUTPUT_FILENAME)],
note=note,
guest=guest)]
if len(log_list[0].split("=")) == 2:
logname = log_list[0].split("=")[1].strip()
if logname == '':
logfile = \
[self.data_path(test, guest, TEST_OUTPUT_FILENAME)]
else:
logfile = [self.data_path(test, guest, full=True) \
/ tmt.steps.execute.TEST_DATA / logname]

if not name_list:
self.debug(f"Test name not found in result file '{f}'.")
resultname = test.name
else:
if len(name_list[0].split("=")) == 2:
resultname = name_list[0].split("=")[1].strip()
else:
resultname = test.name

# Map the restraint result to the corresponding tmt value
actual_result = ResultOutcome.ERROR
note: Optional[str] = None

try:
actual_result = ResultOutcome(result.lower())
except ValueError:
if result == 'SKIP':
actual_result = ResultOutcome.INFO
else:
note = f"invalid test result '{result}' in result file"

all_results.append(tmt.Result.from_test(
test=test,
subresultname=resultname,
result=actual_result,
log=logfile,
note=note,
guest=guest))

return all_results

def load_custom_results(self, test: "tmt.Test", guest: Guest) -> List["tmt.Result"]:
"""
Expand Down
111 changes: 58 additions & 53 deletions tmt/steps/execute/scripts/tmt-report-result
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
#!/bin/bash
set -o errexit -o pipefail -o noclobber -o nounset

server=
outputFile=
disablePlugin=
port=
message=
help=False

die() { echo "$*" >&2; exit 2; } # complain to STDERR and exit with error
needs_arg() { if [ -z "$OPTARG" ]; then die "No arg for --$OPT option"; fi; }

Expand Down Expand Up @@ -38,12 +31,41 @@ check_opt_args () {
shift $((OPTIND-1)) # remove parsed options and args from $@ list
position=$((OPTIND-1))
}
# Options wrapped in quotes to ensure -t/--message argument is parsed as a string phrase
# rather than just the first string until a whitespace is encountered.

write_report_file () {
#rm -f $REPORT_RESULT_OUTPUTFILE
if [ -e $REPORT_RESULT_CNT_FILE ]
then
local cnt=$(cat $REPORT_RESULT_CNT_FILE | wc -l)
else
local cnt=0
fi
echo "SERVER=$server" >> $REPORT_RESULT_OUTPUTFILE.$cnt
echo "PORT=$port" >> $REPORT_RESULT_OUTPUTFILE.$cnt
echo "MESSAGE=$message" >> $REPORT_RESULT_OUTPUTFILE.$cnt
echo "OUTPUTFILE=$resultfile" >> $REPORT_RESULT_OUTPUTFILE.$cnt
echo "DISABLEPLUGIN=$disablePlugin" >> $REPORT_RESULT_OUTPUTFILE.$cnt
echo "TESTNAME=$TESTNAME" >> $REPORT_RESULT_OUTPUTFILE.$cnt
echo "TESTNAME=$TESTNAME" >> $REPORT_RESULT_CNT_FILE
echo "TESTRESULT=$TESTRESULT" >> $REPORT_RESULT_OUTPUTFILE.$cnt
echo "METRIC=$METRIC" >> $REPORT_RESULT_OUTPUTFILE.$cnt
}

server=
outputFile=
disablePlugin=
port=
message=
help=False

# Options wrapped in quotes to ensure -t/--message argument is parsed as a
# string phrase rather than just the first string until a whitespace
# is encountered.
check_opt_args "$@"
shift $position # remove parsed options and args from $@ list
# Return help options when command issued with no options or arguments.
if [ $# -lt 2 ] || [ $help == True ]; then
if ([ $# -lt 2 ] || [ $help == True ]) && [[ $0 =~ "rstrnt-report-result" ]]
then
echo "Usage:"
echo " rstrnt-report-result [OPTION?] TASK_PATH RESULT [SCORE]"
echo ""
Expand All @@ -62,12 +84,27 @@ if [ $# -lt 2 ] || [ $help == True ]; then
echo " -o, --outputfile=FILE Log to upload with result, \$OUTPUTFILE is used by default"
echo " -p, --disable-plugin=PLUGIN don't run plugin on server side"
echo " --no-plugins don't run any plugins on server side"
echo ""
exit 1
fi
if ([ $# -lt 2 ] || [ $help == True ]) && [[ $0 =~ "rhts-report-result" ]]
then
echo "Usage: rhts-report-result TESTNAME TESTRESULT LOGFILE [METRIC]"
echo "where TESTNAME is the name of the test result being reported"
echo " TESTRESULT should be PASS or FAIL"
echo " LOGFILE is the name of the log file to be uploaded"
echo " METRIC is an optional numeric metric (e.g. a performance figure)"
exit 1
fi
TESTNAME=$1
TESTRESULT=$2
shift 2 #$((OPTIND-1)) # remove parsed options and args from $@ list

# the third postion parameter of the rhts helper is log file
if [[ $0 =~ "rhts-report-result" ]]
then
outputFile=$1
shift 1
fi
METRIC=
if [ $# -gt 0 ];then
value1=$1
Expand All @@ -79,48 +116,16 @@ if [ $# -gt 0 ];then
check_opt_args "$@"
fi

write_report_file () {
rm -f $REPORT_RESULT_OUTPUTFILE
echo "SERVER=$server" >> $REPORT_RESULT_OUTPUTFILE
echo "PORT=$port" >> $REPORT_RESULT_OUTPUTFILE
echo "MESSAGE=$message" >> $REPORT_RESULT_OUTPUTFILE
echo "OUTPUTFILE=$outputFile" >> $REPORT_RESULT_OUTPUTFILE
echo "DISABLEPLUGIN=$disablePlugin" >> $REPORT_RESULT_OUTPUTFILE
echo "TESTNAME=$TESTNAME" >> $REPORT_RESULT_OUTPUTFILE
echo "TESTRESULT=$TESTRESULT" >> $REPORT_RESULT_OUTPUTFILE
echo "METRIC=$METRIC" >> $REPORT_RESULT_OUTPUTFILE
}

if [ -n "$outputFile" ] && [ -f $outputFile ]
then
resultfile=$(mktemp -u result.XXXXXX)
cp -f "$outputFile" "$TMT_TEST_DATA"/$resultfile
else
resultfile=""
fi

declare -A RESULTS_HIERARCHY=( ["FAIL"]=4 ["WARN"]=3 ["PASS"]=2 ["SKIP"]=1 )
# Create the output file in which to store the results.
REPORT_RESULT_OUTPUTFILE="$TMT_TEST_DATA/restraint-result"
if [ -e "$REPORT_RESULT_OUTPUTFILE" ]; then
# Check the existing output file.
# If new result is higher on the hierarchy
# or no result is contained in the file
# then overwrite the file with the new
# report contents.
fileResult=$(/usr/bin/grep "TESTRESULT" $REPORT_RESULT_OUTPUTFILE |
cut -d = -f 2)
if [ ! $fileResult ]; then
write_report_file;
exit 0
elif [ ! $TESTRESULT ]; then
exit 0
fi
fileHierarchy="${RESULTS_HIERARCHY[$fileResult]}"
# Compare the current result hierarchy against
# the hierarchy result retrieved from the file.
# Overwrite the file if the current hierachy is higher.
thisResultHierarchy="${RESULTS_HIERARCHY[$TESTRESULT]}"
if [ $thisResultHierarchy -gt $fileHierarchy ]; then
write_report_file
exit 0
fi

else
# Create the output file with the report contents.
write_report_file
exit 0
fi
REPORT_RESULT_CNT_FILE="$TMT_TEST_DATA/restraint-result-cnt"
write_report_file
exit 0
2 changes: 1 addition & 1 deletion tmt/steps/report/junit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def make_junit_xml(report: "tmt.steps.report.ReportPlugin") -> JunitTestSuite:
main_log = None
case = junit_xml.TestCase(
result.name,
classname=None,
classname=result.classname,
elapsed_sec=duration_to_seconds(result.duration),
stdout=main_log)
# Map tmt OUTCOME to JUnit states
Expand Down