Re: [RFC PATCH v2 2/6] report: pass through most details as function parameters

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jun 28, 2022 at 12:22:52AM +0200, David Disseldorp wrote:
> Report generation currently involves reaching into a whole bunch of
> globals for things like section name and start/end times. Pass these
> through as explicit function parameters to avoid unintentional breakage.
> 
> One minor fix included is the default xunit error message, which used
> $sequm instead of $seqnum.

I wish bash would just error out on undefined variables...

> Signed-off-by: David Disseldorp <ddiss@suse.de>

Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  check         | 14 +++++++----
>  common/report | 64 +++++++++++++++++++++++++++++----------------------
>  2 files changed, 47 insertions(+), 31 deletions(-)
> 
> diff --git a/check b/check
> index 2ea2920f..9d60df45 100755
> --- a/check
> +++ b/check
> @@ -429,7 +429,9 @@ _wrapup()
>  	if $showme && $needwrap; then
>  		if $do_report; then
>  			# $showme = all selected tests are notrun (no tries)
> -			_make_section_report "${#notrun[*]}" "0" "${#notrun[*]}"
> +			_make_section_report "$section" "${#notrun[*]}" "0" \
> +					     "${#notrun[*]}" \
> +					     "$((sect_stop - sect_start))"
>  		fi
>  		needwrap=false
>  	elif $needwrap; then
> @@ -490,7 +492,9 @@ _wrapup()
>  		fi
>  		echo "" >>$tmp.summary
>  		if $do_report; then
> -			_make_section_report "${#try[*]}" "${#bad[*]}" "${#notrun[*]}"
> +			_make_section_report "$section" "${#try[*]}" \
> +					     "${#bad[*]}" "${#notrun[*]}" \
> +					     "$((sect_stop - sect_start))"
>  		fi
>  		needwrap=false
>  	fi
> @@ -733,7 +737,8 @@ function run_section()
>  			bad+=("$seqnum")
>  		fi
>  		if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
> -			_make_testcase_report "$prev_seq" "$tc_status"
> +			_make_testcase_report "$section" "$seqnum" \
> +					      "$tc_status" "$((stop - start))"
>  		fi
>  
>  		prev_seq="$seq"
> @@ -937,7 +942,8 @@ function run_section()
>  		bad+=("$seqnum")
>  	fi
>  	if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
> -		_make_testcase_report "$prev_seq" "$tc_status"
> +		_make_testcase_report "$section" "$seqnum" "$tc_status" \
> +				      "$((stop - start))"
>  	fi
>  
>  	sect_stop=`_wallclock`
> diff --git a/common/report b/common/report
> index 2b8285d8..5ca41bc4 100644
> --- a/common/report
> +++ b/common/report
> @@ -33,11 +33,11 @@ _xunit_add_property()
>  _xunit_make_section_report()
>  {
>  	# xfstest:section ==> xunit:testsuite
> -	local tests_count="$1"
> -	local bad_count="$2"
> -	local notrun_count="$3"
> -	local sect_name=$section
> -	local sect_time=`expr $sect_stop - $sect_start`
> +	local sect_name="$1"
> +	local tests_count="$2"
> +	local bad_count="$3"
> +	local notrun_count="$4"
> +	local sect_time="$5"
>  
>  	if [ $sect_name == '-no-sections-' ]; then
>  		sect_name='global'
> @@ -67,12 +67,10 @@ _xunit_make_section_report()
>  
>  _xunit_make_testcase_report()
>  {
> -	local test_seq="$1"
> -	local test_status="$2"
> -	local test_time=`expr $stop - $start`
> -	local strip="$SRC_DIR/"
> -	local test_name=${test_seq#$strip}
> -	local sect_name=$section
> +	local sect_name="$1"
> +	local test_name="$2"
> +	local test_status="$3"
> +	local test_time="$4"
>  
>  	# TODO: other places may also win if no-section mode will be named like 'default/global'
>  	if [ $sect_name == '-no-sections-' ]; then
> @@ -86,8 +84,9 @@ _xunit_make_testcase_report()
>  	"pass")
>  		;;
>  	"notrun")
> -		if [ -f $seqres.notrun ]; then
> -			local msg=`cat $seqres.notrun | encode_xml`
> +		local notrun_file="${REPORT_DIR}/${test_name}.notrun"
> +		if [ -f "$notrun_file" ]; then
> +			local msg=`cat "$notrun_file" | encode_xml`
>  			echo -e "\t\t<skipped message=\"$msg\" />" >> $report
>  		else
>  			echo -e "\t\t<skipped/>" >> $report
> @@ -97,27 +96,31 @@ _xunit_make_testcase_report()
>  		echo -e "\t\t<skipped/>" >> $report
>  		;;
>  	"fail")
> +		local out_src="${SRC_DIR}/${test_name}.out"
> +		local full_file="${REPORT_DIR}/${test_name}.full"
> +		local dmesg_file="${REPORT_DIR}/${test_name}.dmesg"
> +		local outbad_file="${REPORT_DIR}/${test_name}.out.bad"
>  		if [ -z "$_err_msg" ]; then
> -			_err_msg="Test $sequm failed, reason unknown"
> +			_err_msg="Test $test_name failed, reason unknown"
>  		fi
>  		echo -e "\t\t<failure message=\"$_err_msg\" type=\"TestFail\" />" >> $report
> -		if [ -s $seqres.full ]; then
> +		if [ -s "$full_file" ]; then
>  			echo -e "\t\t<system-out>" >> $report
>  			printf	'<![CDATA[\n' >>$report
> -			cat $seqres.full | tr -dc '[:print:][:space:]' | encode_xml >>$report
> +			cat "$full_file" | tr -dc '[:print:][:space:]' | encode_xml >>$report
>  			printf ']]>\n'	>>$report
>  			echo -e "\t\t</system-out>" >> $report
>  		fi
> -		if [ -f $seqres.dmesg ]; then
> +		if [ -f "$dmesg_file" ]; then
>  			echo -e "\t\t<system-err>" >> $report
>  			printf	'<![CDATA[\n' >>$report
> -			cat $seqres.dmesg | tr -dc '[:print:][:space:]' | encode_xml >>$report
> +			cat "$dmesg_file" | tr -dc '[:print:][:space:]' | encode_xml >>$report
>  			printf ']]>\n'	>>$report
>  			echo -e "\t\t</system-err>" >> $report
> -		elif [ -s $seqres.out.bad ]; then
> +		elif [ -s "$outbad_file" ]; then
>  			echo -e "\t\t<system-err>" >> $report
>  			printf	'<![CDATA[\n' >>$report
> -			$diff $test_seq.out $seqres.out.bad | encode_xml >>$report
> +			$diff "$out_src" "$outbad_file" | encode_xml >>$report
>  			printf ']]>\n'	>>$report
>  			echo -e "\t\t</system-err>" >> $report
>  		fi
> @@ -134,13 +137,17 @@ _xunit_make_testcase_report()
>  #  Common report generator entry points
>  _make_section_report()
>  {
> -	local tests_count="$1"
> -	local bad_count="$2"
> -	local notrun_count="$3"
> +	local sect_name="$1"
> +	local tests_count="$2"
> +	local bad_count="$3"
> +	local notrun_count="$4"
> +	local sect_time="$5"
>  	for report in $REPORT_LIST; do
>  		case "$report" in
>  		"xunit")
> -			_xunit_make_section_report "$tests_count" "$bad_count" "$notrun_count"
> +			_xunit_make_section_report "$sect_name" "$tests_count" \
> +						   "$bad_count" "$notrun_count" \
> +						   "$sect_time"
>  			;;
>  		*)
>  			_dump_err "format '$report' is not supported"
> @@ -151,12 +158,15 @@ _make_section_report()
>  
>  _make_testcase_report()
>  {
> -	local test_seq="$1"
> -	local test_status="$2"
> +	local sect_name="$1"
> +	local test_seq="$2"
> +	local test_status="$3"
> +	local test_time="$4"
>  	for report in $REPORT_LIST; do
>  		case "$report" in
>  		"xunit")
> -			_xunit_make_testcase_report "$test_seq" "$test_status"
> +			_xunit_make_testcase_report "$sect_name" "$test_seq" \
> +						    "$test_status" "$test_time"
>  			;;
>  		*)
>  			_dump_err "report format '$report' is not supported"
> -- 
> 2.35.3
>