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

Darrick J. Wong
2022-12-19T17:39:40Z

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 ' \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 ' \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 ' \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
>