| Message ID | 20220720164356.4078789-2-tytso@mit.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: more random fixes from Ted | expand |
On Wed, Jul 20, 2022 at 12:43:53PM -0400, Theodore Ts'o wrote: > The xunit-quiet format excludes the NNN.{full,dmesg,bad} files in > <system-out> and <system-err> nodes which are included in the xunit > report format. > > For test runners that save the entire results directory to preserve > all of the test artifacts, capturing the NNN.{full,dmesg,bad} in the > results.xml file is redundant. In addition, if the NNN.bad is too > large, it can cause the junitparser python library to refuse to parse > the XML file to prevent potential denial of service attacks[1]. A > simple way to avoid this problem is to simply to omit the <system-out> > and <system-err> nodes in the results.xml file. > > [1] https://gitlab.com/gitlab-org/gitlab/-/issues/268035 > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > Reviewed-by: David Disseldorp <ddiss@suse.de> Looks fine to me, though I wonder if we should document what these report formats actually do? I really dislike the "xunit" name since AFAICT it's really the junit xml format, not the xunit xml format, and this trips me up **every** single time I have to go look at the fstests reporting code. For this bit though, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > check | 2 +- > common/report | 21 ++++++++++++++------- > 2 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/check b/check > index 5f6d86b4..0b2f10ed 100755 > --- a/check > +++ b/check > @@ -75,7 +75,7 @@ check options > -I <n> iterate the test list <n> times, but stops iterating further in case of any test failure > -d dump test output to stdout > -b brief test summary > - -R fmt[,fmt] generate report in formats specified. Supported format: [xunit] > + -R fmt[,fmt] generate report in formats specified. Supported formats: xunit, xunit-quiet > --large-fs optimise scratch device for large filesystems > -s section run only specified section from config file > -S section exclude the specified section from the config file > diff --git a/common/report b/common/report > index 5ca41bc4..64f9c866 100644 > --- a/common/report > +++ b/common/report > @@ -71,11 +71,16 @@ _xunit_make_testcase_report() > local test_name="$2" > local test_status="$3" > local test_time="$4" > + local report_format="$5" > + local quiet > + > + if [ "$report_format" = xunit-quiet ]; then > + quiet=yes > + fi > > # TODO: other places may also win if no-section mode will be named like 'default/global' > if [ $sect_name == '-no-sections-' ]; then > sect_name='global' > - > fi > local report=$tmp.report.xunit.$sect_name.xml > > @@ -104,14 +109,16 @@ _xunit_make_testcase_report() > _err_msg="Test $test_name failed, reason unknown" > fi > echo -e "\t\t<failure message=\"$_err_msg\" type=\"TestFail\" />" >> $report > - if [ -s "$full_file" ]; then > + if [ -z "$quiet" -a -s "$full_file" ]; then > echo -e "\t\t<system-out>" >> $report > printf '<![CDATA[\n' >>$report > cat "$full_file" | tr -dc '[:print:][:space:]' | encode_xml >>$report > printf ']]>\n' >>$report > echo -e "\t\t</system-out>" >> $report > fi > - if [ -f "$dmesg_file" ]; then > + 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 > @@ -144,7 +151,7 @@ _make_section_report() > local sect_time="$5" > for report in $REPORT_LIST; do > case "$report" in > - "xunit") > + "xunit"|"xunit-quiet") > _xunit_make_section_report "$sect_name" "$tests_count" \ > "$bad_count" "$notrun_count" \ > "$sect_time" > @@ -164,9 +171,9 @@ _make_testcase_report() > local test_time="$4" > for report in $REPORT_LIST; do > case "$report" in > - "xunit") > + "xunit"|"xunit-quiet") > _xunit_make_testcase_report "$sect_name" "$test_seq" \ > - "$test_status" "$test_time" > + "$test_status" "$test_time" "$report" > ;; > *) > _dump_err "report format '$report' is not supported" > @@ -178,7 +185,7 @@ _make_testcase_report() > _assert_report_list() { > for report in $REPORT_LIST; do > case "$report" in > - "xunit") > + "xunit"|"xunit-quiet") > ;; > *) > _fatal "report format '$report' is not supported" > -- > 2.31.0 >
diff --git a/check b/check index 5f6d86b4..0b2f10ed 100755 --- a/check +++ b/check @@ -75,7 +75,7 @@ check options -I <n> iterate the test list <n> times, but stops iterating further in case of any test failure -d dump test output to stdout -b brief test summary - -R fmt[,fmt] generate report in formats specified. Supported format: [xunit] + -R fmt[,fmt] generate report in formats specified. Supported formats: xunit, xunit-quiet --large-fs optimise scratch device for large filesystems -s section run only specified section from config file -S section exclude the specified section from the config file diff --git a/common/report b/common/report index 5ca41bc4..64f9c866 100644 --- a/common/report +++ b/common/report @@ -71,11 +71,16 @@ _xunit_make_testcase_report() local test_name="$2" local test_status="$3" local test_time="$4" + local report_format="$5" + local quiet + + if [ "$report_format" = xunit-quiet ]; then + quiet=yes + fi # TODO: other places may also win if no-section mode will be named like 'default/global' if [ $sect_name == '-no-sections-' ]; then sect_name='global' - fi local report=$tmp.report.xunit.$sect_name.xml @@ -104,14 +109,16 @@ _xunit_make_testcase_report() _err_msg="Test $test_name failed, reason unknown" fi echo -e "\t\t<failure message=\"$_err_msg\" type=\"TestFail\" />" >> $report - if [ -s "$full_file" ]; then + if [ -z "$quiet" -a -s "$full_file" ]; then echo -e "\t\t<system-out>" >> $report printf '<![CDATA[\n' >>$report cat "$full_file" | tr -dc '[:print:][:space:]' | encode_xml >>$report printf ']]>\n' >>$report echo -e "\t\t</system-out>" >> $report fi - if [ -f "$dmesg_file" ]; then + 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 @@ -144,7 +151,7 @@ _make_section_report() local sect_time="$5" for report in $REPORT_LIST; do case "$report" in - "xunit") + "xunit"|"xunit-quiet") _xunit_make_section_report "$sect_name" "$tests_count" \ "$bad_count" "$notrun_count" \ "$sect_time" @@ -164,9 +171,9 @@ _make_testcase_report() local test_time="$4" for report in $REPORT_LIST; do case "$report" in - "xunit") + "xunit"|"xunit-quiet") _xunit_make_testcase_report "$sect_name" "$test_seq" \ - "$test_status" "$test_time" + "$test_status" "$test_time" "$report" ;; *) _dump_err "report format '$report' is not supported" @@ -178,7 +185,7 @@ _make_testcase_report() _assert_report_list() { for report in $REPORT_LIST; do case "$report" in - "xunit") + "xunit"|"xunit-quiet") ;; *) _fatal "report format '$report' is not supported"
The xunit-quiet format excludes the NNN.{full,dmesg,bad} files in <system-out> and <system-err> nodes which are included in the xunit report format. For test runners that save the entire results directory to preserve all of the test artifacts, capturing the NNN.{full,dmesg,bad} in the results.xml file is redundant. In addition, if the NNN.bad is too large, it can cause the junitparser python library to refuse to parse the XML file to prevent potential denial of service attacks[1]. A simple way to avoid this problem is to simply to omit the <system-out> and <system-err> nodes in the results.xml file. [1] https://gitlab.com/gitlab-org/gitlab/-/issues/268035 Signed-off-by: Theodore Ts'o <tytso@mit.edu> Reviewed-by: David Disseldorp <ddiss@suse.de> --- check | 2 +- common/report | 21 ++++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-)