Re: [PATCH] fstests: report: always save the dmesg as system-err if KEEP_DMESG is set

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

On Fri, Dec 16, 2022 at 02:51:21PM +0800, Qu Wenruo wrote:
> When KEEP_DMESG is set to "yes", we will always save the dmesg of any
> test case (no matter if it passed or not) into "$seqnum.dmesg".
> 
> But this KEEP_DMESG behavior doesn't affect xunit report.
> 
> This patch will make xunit report to follow KEEP_DMESG setting.
> Since error is checked by testcase.failure attribute, this new
> <system-err> section should not cause the existing parsers to treat
> passed cases as errors.
> 
> KEEP_DMESG is only followed if all the following conditions are met:
> 
> - KEEP_DMESG is set to yes

Feel free to push back against the proliferation of config variables,
but perhaps this ought to be REPORT_DMESG={always,never,auto} ?

> - Using xunit reporting
>   xunit-quite won't save the dmesg for passed test cases.

          ^^^^^ "quiet"?

> This extra saved dmesg would definitely boost the xml size, but if the
> end user wants to save all the dmesg (for later verification), then I'd
> say it's a unavoidable cost.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  common/report | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/common/report b/common/report
> index 64f9c866..4a747f8d 100644
> --- a/common/report
> +++ b/common/report
> @@ -87,6 +87,19 @@ _xunit_make_testcase_report()
>  	echo -e "\t<testcase classname=\"xfstests.$sect_name\" name=\"$test_name\" time=\"$test_time\">" >> $report
>  	case $test_status in
>  	"pass")
> +		# If we have KEEP_DMESG and want full output, also save the
> +		# dmesg into the passed result
> +		if [ "$KEEP_DMESG" == yes -a "$quiet" != "yes" ]; then
> +			local dmesg_file="${REPORT_DIR}/${test_name}.dmesg"
> +			if [ -f "$dmesg_file" ]; then
> +				echo -e "\t\t<system-err>" >> $report
> +				printf	'<![CDATA[\n' >>$report
> +				cat "$dmesg_file" | tr -dc '[:print:][:space:]' | \
> +					encode_xml >>$report
> +				printf ']]>\n'	>>$report
> +				echo -e "\t\t</system-err>" >> $report

Hmm.  Does the junit xml schema[1] (that's what fstests implements, even
if we call it xunit, and even though there's a separate xunit[2] format
that is not the same!) actually allow us to have multiple <system-err>
elements?

For that matter, I look at things like this:

	if [ -n "$quiet" ]; then
	   :
	elif [ -f "$dmesg_file" ]; then
		echo -e "\t\t<system-err>" >> $report
		printf	'<![CDATA[\n' >>$report
		cat "$dmesg_file" | tr -dc '[:print:][:space:]' | encode_xml >>$report
		printf ']]>\n'	>>$report
		echo -e "\t\t</system-err>" >> $report
	elif [ -s "$outbad_file" ]; then
		echo -e "\t\t<system-err>" >> $report
		printf	'<![CDATA[\n' >>$report
		$diff "$out_src" "$outbad_file" | encode_xml >>$report
		printf ']]>\n'	>>$report
		echo -e "\t\t</system-err>" >> $report
	fi

and wonder why we prioritize recording dmesg output over .out.bad when
we could do both?

--D

[1] https://raw.githubusercontent.com/windyroad/JUnit-Schema/master/JUnit.xsd
[2] https://xunit.net/docs/format-xml-v2

> +			fi
> +		fi
>  		;;
>  	"notrun")
>  		local notrun_file="${REPORT_DIR}/${test_name}.notrun"
> -- 
> 2.38.0
>