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

Qu Wenruo
2022-12-19T22:55:05Z

On 2022/12/20 01:39, Darrick J. Wong wrote:
> 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} ?

I guess this would cover a lot of work of xunit-quiet?

>
>> - 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?

Well, before you mentioning this, I didn't even notice we don't save
out.bad if we have dmesg...

And we need some standard on what should be put into system-out and what
should go system-err.

Personally speaking, out.bad should go system-out, while dmesg and full
should go system-err.

Another thing is, if we have multiple system-err elements, should we add
some properties for the system-err elements to distinguish .full and .dmesg?

Thanks,
Qu

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