* [RFC PATCH v2 0/6] check: add option to rerun failed tests
@ 2022-06-27 22:22 David Disseldorp
2022-06-27 22:22 ` [RFC PATCH v2 1/6] report: use array for REPORT_ENV_LIST David Disseldorp
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: David Disseldorp @ 2022-06-27 22:22 UTC (permalink / raw)
To: fstests, tytso
This RFC patchset adds support to loop on failed tests, as proposed by
Ted Ts'o in https://lwn.net/Articles/897061/:
add a mode that will immediately rerun a failed test 25 or 100 times
to establish a failure percentage.
Changes since previous version (v1):
- rebased atop upstream v2022.06.26
- added a few extra cleanup commits
- append details for every rerun to xunit output
+ provide aggregate stats via <testcase status=X>
- extend _stash_test_status to call report hook, as well as save failure
artifacts with a .rerun# suffix
Still flagged as RFC as I'd like to do a bit more testing and work out
whether the regular / xunit reporting output makes sense. Feedback
appreciated.
Diffstat:
check | 146 ++++++++++++++++++++++++++++++++++++++++++--------
common/report | 96 ++++++++++++++++-----------------
2 files changed, 170 insertions(+), 72 deletions(-)
Cheers, David
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 1/6] report: use array for REPORT_ENV_LIST
2022-06-27 22:22 [RFC PATCH v2 0/6] check: add option to rerun failed tests David Disseldorp
@ 2022-06-27 22:22 ` David Disseldorp
2022-06-28 14:54 ` Darrick J. Wong
2022-06-27 22:22 ` [RFC PATCH v2 2/6] report: pass through most details as function parameters David Disseldorp
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: David Disseldorp @ 2022-06-27 22:22 UTC (permalink / raw)
To: fstests, tytso; +Cc: David Disseldorp
There's no need for multiple assignments.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
common/report | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/common/report b/common/report
index 84d9e0a7..2b8285d8 100644
--- a/common/report
+++ b/common/report
@@ -4,26 +4,10 @@
# List of xfstests's enviroment variables to include reports
## TODO automate list population inside common/conf
-REPORT_ENV_LIST="$REPORT_ENV_LIST SECTION"
-REPORT_ENV_LIST="$REPORT_ENV_LIST FSTYP"
-REPORT_ENV_LIST="$REPORT_ENV_LIST PLATFORM"
-REPORT_ENV_LIST="$REPORT_ENV_LIST MKFS_OPTIONS"
-REPORT_ENV_LIST="$REPORT_ENV_LIST MOUNT_OPTIONS"
-
-REPORT_ENV_LIST="$REPORT_ENV_LIST HOST_OPTIONS"
-REPORT_ENV_LIST="$REPORT_ENV_LIST CHECK_OPTIONS"
-REPORT_ENV_LIST="$REPORT_ENV_LIST XFS_MKFS_OPTIONS"
-REPORT_ENV_LIST="$REPORT_ENV_LIST TIME_FACTOR"
-REPORT_ENV_LIST="$REPORT_ENV_LIST LOAD_FACTOR"
-
-REPORT_ENV_LIST="$REPORT_ENV_LIST TEST_DIR"
-REPORT_ENV_LIST="$REPORT_ENV_LIST TEST_DEV"
-REPORT_ENV_LIST="$REPORT_ENV_LIST SCRATCH_DEV"
-REPORT_ENV_LIST="$REPORT_ENV_LIST SCRATCH_MNT"
-
-REPORT_ENV_LIST="$REPORT_ENV_LIST OVL_UPPER"
-REPORT_ENV_LIST="$REPORT_ENV_LIST OVL_LOWER"
-REPORT_ENV_LIST="$REPORT_ENV_LIST OVL_WORK"
+REPORT_ENV_LIST=("SECTION" "FSTYP" "PLATFORM" "MKFS_OPTIONS" "MOUNT_OPTIONS" \
+ "HOST_OPTIONS" "CHECK_OPTIONS" "XFS_MKFS_OPTIONS" \
+ "TIME_FACTOR" "LOAD_FACTOR" "TEST_DIR" "TEST_DEV" \
+ "SCRATCH_DEV" "SCRATCH_MNT" "OVL_UPPER" "OVL_LOWER" "OVL_WORK")
encode_xml()
{
@@ -70,7 +54,7 @@ _xunit_make_section_report()
# Properties
echo -e "\t<properties>" >> $REPORT_DIR/result.xml
- for p in $REPORT_ENV_LIST;do
+ for p in "${REPORT_ENV_LIST[@]}"; do
_xunit_add_property "$p"
done
echo -e "\t</properties>" >> $REPORT_DIR/result.xml
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 2/6] report: pass through most details as function parameters
2022-06-27 22:22 [RFC PATCH v2 0/6] check: add option to rerun failed tests David Disseldorp
2022-06-27 22:22 ` [RFC PATCH v2 1/6] report: use array for REPORT_ENV_LIST David Disseldorp
@ 2022-06-27 22:22 ` David Disseldorp
2022-06-28 14:55 ` Darrick J. Wong
2022-06-27 22:22 ` [RFC PATCH v2 3/6] check: make a few variables local David Disseldorp
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: David Disseldorp @ 2022-06-27 22:22 UTC (permalink / raw)
To: fstests, tytso; +Cc: David Disseldorp
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.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 3/6] check: make a few variables local
2022-06-27 22:22 [RFC PATCH v2 0/6] check: add option to rerun failed tests David Disseldorp
2022-06-27 22:22 ` [RFC PATCH v2 1/6] report: use array for REPORT_ENV_LIST David Disseldorp
2022-06-27 22:22 ` [RFC PATCH v2 2/6] report: pass through most details as function parameters David Disseldorp
@ 2022-06-27 22:22 ` David Disseldorp
2022-06-28 14:56 ` Darrick J. Wong
2022-06-27 22:22 ` [RFC PATCH v2 4/6] check: append bad / notrun arrays in helper function David Disseldorp
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: David Disseldorp @ 2022-06-27 22:22 UTC (permalink / raw)
To: fstests, tytso; +Cc: David Disseldorp
The variables aren't used outside of function scope. Also convert one
timestamp output to use the helper.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
check | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/check b/check
index 9d60df45..f973dd28 100755
--- a/check
+++ b/check
@@ -176,10 +176,10 @@ get_all_tests()
# the function from that list.
trim_test_list()
{
- test_list="$*"
+ local test_list="$*"
rm -f $tmp.grep
- numsed=0
+ local numsed=0
for t in $test_list
do
if [ $numsed -gt 100 ]; then
@@ -204,7 +204,7 @@ _wallclock()
_timestamp()
{
- now=`date "+%T"`
+ local now=`date "+%T"`
echo -n " [$now]"
}
@@ -600,7 +600,7 @@ fi
function run_section()
{
- local section=$1
+ local section=$1 skip
OLD_FSTYP=$FSTYP
OLD_TEST_FS_MOUNT_OPTS=$TEST_FS_MOUNT_OPTS
@@ -817,7 +817,7 @@ function run_section()
rm -f core $seqres.notrun
start=`_wallclock`
- $timestamp && echo -n " ["`date "+%T"`"]"
+ $timestamp && _timestamp
[ ! -x $seq ] && chmod u+x $seq # ensure we can run it
$LOGGER_PROG "run xfstest $seqnum"
if [ -w /dev/kmsg ]; then
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 4/6] check: append bad / notrun arrays in helper function
2022-06-27 22:22 [RFC PATCH v2 0/6] check: add option to rerun failed tests David Disseldorp
` (2 preceding siblings ...)
2022-06-27 22:22 ` [RFC PATCH v2 3/6] check: make a few variables local David Disseldorp
@ 2022-06-27 22:22 ` David Disseldorp
2022-06-28 15:00 ` Darrick J. Wong
2022-06-27 22:22 ` [RFC PATCH v2 5/6] check: add -L <n> parameter to rerun failed tests David Disseldorp
2022-06-27 22:22 ` [RFC PATCH v2 6/6] check: stash full/dmesg/out.bad files on rerun David Disseldorp
5 siblings, 1 reply; 15+ messages in thread
From: David Disseldorp @ 2022-06-27 22:22 UTC (permalink / raw)
To: fstests, tytso; +Cc: David Disseldorp
Currently the @try, @bad and @notrun arrays are appended with seqnum at
different points in the main run_section() loop:
- @try: shortly prior to test script execution
- @notrun: on list (check -n), or after .notrun flagged test completion
- @bad: at the start of subsequent test loop and loop exit
For future loop-test-following-failure functionality it makes sense to
combine some of these steps. This change moves both @notrun and @bad
appends into a helper function.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
check | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/check b/check
index f973dd28..aa7dac2f 100755
--- a/check
+++ b/check
@@ -550,6 +550,27 @@ _expunge_test()
return 0
}
+# Retain in @bad / @notrun the result of previously run @test_seq. @try array
+# entries are added prior to execution.
+_stash_test_status() {
+ local test_seq="$1"
+ local test_status="$2"
+
+ case "$test_status" in
+ fail)
+ bad+=("$test_seq")
+ ;;
+ list|notrun)
+ notrun+=("$test_seq")
+ ;;
+ pass|expunge|init)
+ ;;
+ *)
+ echo "Unexpected test $test_seq status: $test_status"
+ ;;
+ esac
+}
+
# Can we run systemd scopes?
HAVE_SYSTEMD_SCOPES=
systemctl reset-failed "fstests-check" &>/dev/null
@@ -733,9 +754,7 @@ function run_section()
prev_seq=""
for seq in $list ; do
# Run report for previous test!
- if [ "$tc_status" == "fail" ]; then
- bad+=("$seqnum")
- fi
+ _stash_test_status "$seqnum" "$tc_status"
if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
_make_testcase_report "$section" "$seqnum" \
"$tc_status" "$((stop - start))"
@@ -788,7 +807,6 @@ function run_section()
start=0
stop=0
tc_status="list"
- notrun+=("$seqnum")
continue
fi
@@ -854,7 +872,6 @@ function run_section()
$timestamp && echo " [not run]" && \
echo -n " $seqnum -- "
cat $seqres.notrun
- notrun+=("$seqnum")
tc_status="notrun"
# Unmount the scratch fs so that we can wipe the scratch
@@ -938,9 +955,7 @@ function run_section()
done
# make sure we record the status of the last test we ran.
- if [ "$tc_status" == "fail" ]; then
- bad+=("$seqnum")
- fi
+ _stash_test_status "$seqnum" "$tc_status"
if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
_make_testcase_report "$section" "$seqnum" "$tc_status" \
"$((stop - start))"
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 5/6] check: add -L <n> parameter to rerun failed tests
2022-06-27 22:22 [RFC PATCH v2 0/6] check: add option to rerun failed tests David Disseldorp
` (3 preceding siblings ...)
2022-06-27 22:22 ` [RFC PATCH v2 4/6] check: append bad / notrun arrays in helper function David Disseldorp
@ 2022-06-27 22:22 ` David Disseldorp
2022-06-28 15:15 ` Darrick J. Wong
2022-06-27 22:22 ` [RFC PATCH v2 6/6] check: stash full/dmesg/out.bad files on rerun David Disseldorp
5 siblings, 1 reply; 15+ messages in thread
From: David Disseldorp @ 2022-06-27 22:22 UTC (permalink / raw)
To: fstests, tytso; +Cc: David Disseldorp
If check is run with -L <n>, then a failed test will be rerun <n> times
before proceeding to the next test. Following completion of the rerun
loop, aggregate pass/fail statistics are printed.
Rerun tests will be tracked as a single failure in overall pass/fail
metrics (via @try and @bad), with .out.bad, .dmesg and .full saved using
a .rerun# suffix.
Suggested-by: Theodore Ts'o <tytso@mit.edu>
Link: https://lwn.net/Articles/897061/
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
check | 94 ++++++++++++++++++++++++++++++++++++++++++++-------
common/report | 8 +++--
2 files changed, 88 insertions(+), 14 deletions(-)
diff --git a/check b/check
index aa7dac2f..726c83d9 100755
--- a/check
+++ b/check
@@ -26,6 +26,7 @@ do_report=false
DUMP_OUTPUT=false
iterations=1
istop=false
+loop_on_fail=0
# This is a global variable used to pass test failure text to reporting gunk
_err_msg=""
@@ -75,6 +76,7 @@ check options
--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
+ -L <n> loop tests <n> times following a failure, measuring aggregate pass/fail metrics
testlist options
-g group[,group...] include tests from these groups
@@ -333,6 +335,9 @@ while [ $# -gt 0 ]; do
;;
--large-fs) export LARGE_SCRATCH_DEV=yes ;;
--extra-space=*) export SCRATCH_DEV_EMPTY_SPACE=${r#*=} ;;
+ -L) [[ $2 =~ ^[0-9]+$ ]] || usage
+ loop_on_fail=$2; shift
+ ;;
-*) usage ;;
*) # not an argument, we've got tests now.
@@ -555,6 +560,18 @@ _expunge_test()
_stash_test_status() {
local test_seq="$1"
local test_status="$2"
+ local test_time="$3"
+ local loop_num="$4"
+ local report_msg="$5"
+
+ if $do_report && [[ ! $test_status =~ ^(init|expunge)$ ]]; then
+ _make_testcase_report "$section" "$test_seq" \
+ "$test_status" "$test_time" \
+ "$report_msg"
+ fi
+
+ # only stash result for first failure (triggering loop)
+ ((loop_num > 1)) && return
case "$test_status" in
fail)
@@ -610,6 +627,38 @@ _run_seq() {
fi
}
+# Check whether the last test should be rerun according to loop-on-error state
+# and return "0" if so, otherwise return "1".
+_ix_inc() {
+ local test_status="$1"
+ local loop_len="$2"
+
+ if ((!loop_on_fail)); then
+ echo 1
+ return
+ fi
+
+ if [ "$test_status" == "fail" ] && ((!loop_len)); then
+ echo 0 # initial failure of this test, start loop-on-fail
+ elif ((loop_len > 0)) && ((loop_len < loop_on_fail)); then
+ echo 0 # continue loop following initial failure
+ else
+ echo 1 # completed or not currently in a failure loop
+ fi
+}
+
+_failure_loop_dump_stats() {
+ awk "BEGIN {
+ n=split(\"$*\", arr);"'
+ for (i = 1; i <= n; i++)
+ stats[arr[i]]++;
+ printf("aggregate results across %d runs: ", n);
+ for (x in stats)
+ printf("%s=%d (%.1f%%)", (i-- > n ? x : ", " x),
+ stats[x], 100 * stats[x] / n);
+ }'
+}
+
_detect_kmemleak
_prepare_test_list
@@ -750,14 +799,29 @@ function run_section()
seqres="$check"
_check_test_fs
- local tc_status="init"
+ local tc_status="init" ix agg_msg
prev_seq=""
- for seq in $list ; do
+ local -a _list=( $list ) loop_status=()
+ for ((ix = 0; ix < ${#_list[*]};
+ ix += $(_ix_inc "$tc_status" "${#loop_status[*]}"))); do
+ seq="${_list[$ix]}"
+
+ if [ "$seq" == "$prev_seq" ]; then
+ loop_status+=("$tc_status")
+ elif ((${#loop_status[*]})); then
+ # leaving rerun-on-failure loop
+ loop_status+=("$tc_status")
+ agg_msg=$(_failure_loop_dump_stats "${loop_status[@]}")
+ echo "$seqnum $agg_msg"
+ fi
+
# Run report for previous test!
- _stash_test_status "$seqnum" "$tc_status"
- if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
- _make_testcase_report "$section" "$seqnum" \
- "$tc_status" "$((stop - start))"
+ _stash_test_status "$seqnum" "$tc_status" "$((stop - start))" \
+ "${#loop_status[*]}" "$agg_msg"
+
+ if [ -n "$agg_msg" ]; then
+ loop_status=()
+ agg_msg=""
fi
prev_seq="$seq"
@@ -827,7 +891,9 @@ function run_section()
fi
# record that we really tried to run this test.
- try+=("$seqnum")
+ if ((!${#loop_status[*]})); then
+ try+=("$seqnum")
+ fi
awk 'BEGIN {lasttime=" "} \
$1 == "'$seqnum'" {lasttime=" " $2 "s ... "; exit} \
@@ -954,13 +1020,17 @@ function run_section()
fi
done
- # make sure we record the status of the last test we ran.
- _stash_test_status "$seqnum" "$tc_status"
- if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
- _make_testcase_report "$section" "$seqnum" "$tc_status" \
- "$((stop - start))"
+ if ((${#loop_status[*]})); then
+ # leaving rerun-on-failure loop
+ loop_status+=("$tc_status")
+ agg_msg=$(_failure_loop_dump_stats "${loop_status[@]}")
+ echo "$seqnum $agg_msg"
fi
+ # Run report for previous test!
+ _stash_test_status "$seqnum" "$tc_status" "$((stop - start))" \
+ "${#loop_status[*]}" "$agg_msg"
+
sect_stop=`_wallclock`
interrupt=false
_wrapup
diff --git a/common/report b/common/report
index 5ca41bc4..cede4987 100644
--- a/common/report
+++ b/common/report
@@ -71,6 +71,7 @@ _xunit_make_testcase_report()
local test_name="$2"
local test_status="$3"
local test_time="$4"
+ local test_md="$5"
# TODO: other places may also win if no-section mode will be named like 'default/global'
if [ $sect_name == '-no-sections-' ]; then
@@ -79,7 +80,8 @@ _xunit_make_testcase_report()
fi
local report=$tmp.report.xunit.$sect_name.xml
- echo -e "\t<testcase classname=\"xfstests.$sect_name\" name=\"$test_name\" time=\"$test_time\">" >> $report
+ [ -n "$test_md" ] && test_md=" status=\"$(echo "$test_md"|encode_xml)\""
+ echo -e "\t<testcase classname=\"xfstests.$sect_name\" name=\"$test_name\" time=\"$test_time\"${test_md}>" >> $report
case $test_status in
"pass")
;;
@@ -162,11 +164,13 @@ _make_testcase_report()
local test_seq="$2"
local test_status="$3"
local test_time="$4"
+ local test_md="$5"
for report in $REPORT_LIST; do
case "$report" in
"xunit")
_xunit_make_testcase_report "$sect_name" "$test_seq" \
- "$test_status" "$test_time"
+ "$test_status" \
+ "$test_time" "$test_md"
;;
*)
_dump_err "report format '$report' is not supported"
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 6/6] check: stash full/dmesg/out.bad files on rerun
2022-06-27 22:22 [RFC PATCH v2 0/6] check: add option to rerun failed tests David Disseldorp
` (4 preceding siblings ...)
2022-06-27 22:22 ` [RFC PATCH v2 5/6] check: add -L <n> parameter to rerun failed tests David Disseldorp
@ 2022-06-27 22:22 ` David Disseldorp
2022-06-28 15:16 ` Darrick J. Wong
5 siblings, 1 reply; 15+ messages in thread
From: David Disseldorp @ 2022-06-27 22:22 UTC (permalink / raw)
To: fstests, tytso; +Cc: David Disseldorp
These files would otherwise be overwritten.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
check | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/check b/check
index 726c83d9..baf336da 100755
--- a/check
+++ b/check
@@ -570,8 +570,17 @@ _stash_test_status() {
"$report_msg"
fi
- # only stash result for first failure (triggering loop)
- ((loop_num > 1)) && return
+ if ((loop_num > 0)); then
+ # retain files which would be overwritten in subsequent reruns
+ for i in "${REPORT_DIR}/${test_seq}.full" \
+ "${REPORT_DIR}/${test_seq}.dmesg" \
+ "${REPORT_DIR}/${test_seq}.out.bad"; do
+ [ -f "$i" ] || continue
+ cp "$i" "${i}.rerun$((loop_num - 1))"
+ done
+ # only stash result for first failure (triggering loop)
+ ((loop_num != 1)) && return
+ fi
case "$test_status" in
fail)
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC PATCH v2 1/6] report: use array for REPORT_ENV_LIST
2022-06-27 22:22 ` [RFC PATCH v2 1/6] report: use array for REPORT_ENV_LIST David Disseldorp
@ 2022-06-28 14:54 ` Darrick J. Wong
0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2022-06-28 14:54 UTC (permalink / raw)
To: David Disseldorp; +Cc: fstests, tytso
On Tue, Jun 28, 2022 at 12:22:51AM +0200, David Disseldorp wrote:
> There's no need for multiple assignments.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> ---
> common/report | 26 +++++---------------------
> 1 file changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/common/report b/common/report
> index 84d9e0a7..2b8285d8 100644
> --- a/common/report
> +++ b/common/report
> @@ -4,26 +4,10 @@
>
> # List of xfstests's enviroment variables to include reports
> ## TODO automate list population inside common/conf
> -REPORT_ENV_LIST="$REPORT_ENV_LIST SECTION"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST FSTYP"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST PLATFORM"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST MKFS_OPTIONS"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST MOUNT_OPTIONS"
> -
> -REPORT_ENV_LIST="$REPORT_ENV_LIST HOST_OPTIONS"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST CHECK_OPTIONS"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST XFS_MKFS_OPTIONS"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST TIME_FACTOR"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST LOAD_FACTOR"
> -
> -REPORT_ENV_LIST="$REPORT_ENV_LIST TEST_DIR"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST TEST_DEV"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST SCRATCH_DEV"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST SCRATCH_MNT"
> -
> -REPORT_ENV_LIST="$REPORT_ENV_LIST OVL_UPPER"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST OVL_LOWER"
> -REPORT_ENV_LIST="$REPORT_ENV_LIST OVL_WORK"
> +REPORT_ENV_LIST=("SECTION" "FSTYP" "PLATFORM" "MKFS_OPTIONS" "MOUNT_OPTIONS" \
> + "HOST_OPTIONS" "CHECK_OPTIONS" "XFS_MKFS_OPTIONS" \
> + "TIME_FACTOR" "LOAD_FACTOR" "TEST_DIR" "TEST_DEV" \
> + "SCRATCH_DEV" "SCRATCH_MNT" "OVL_UPPER" "OVL_LOWER" "OVL_WORK")
Cheers for the end of a stringbuilder pattern,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
>
> encode_xml()
> {
> @@ -70,7 +54,7 @@ _xunit_make_section_report()
>
> # Properties
> echo -e "\t<properties>" >> $REPORT_DIR/result.xml
> - for p in $REPORT_ENV_LIST;do
> + for p in "${REPORT_ENV_LIST[@]}"; do
> _xunit_add_property "$p"
> done
> echo -e "\t</properties>" >> $REPORT_DIR/result.xml
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH v2 2/6] report: pass through most details as function parameters
2022-06-27 22:22 ` [RFC PATCH v2 2/6] report: pass through most details as function parameters David Disseldorp
@ 2022-06-28 14:55 ` Darrick J. Wong
0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2022-06-28 14:55 UTC (permalink / raw)
To: David Disseldorp; +Cc: fstests, tytso
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH v2 3/6] check: make a few variables local
2022-06-27 22:22 ` [RFC PATCH v2 3/6] check: make a few variables local David Disseldorp
@ 2022-06-28 14:56 ` Darrick J. Wong
0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2022-06-28 14:56 UTC (permalink / raw)
To: David Disseldorp; +Cc: fstests, tytso
On Tue, Jun 28, 2022 at 12:22:53AM +0200, David Disseldorp wrote:
> The variables aren't used outside of function scope. Also convert one
> timestamp output to use the helper.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
Looks good,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> check | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/check b/check
> index 9d60df45..f973dd28 100755
> --- a/check
> +++ b/check
> @@ -176,10 +176,10 @@ get_all_tests()
> # the function from that list.
> trim_test_list()
> {
> - test_list="$*"
> + local test_list="$*"
>
> rm -f $tmp.grep
> - numsed=0
> + local numsed=0
> for t in $test_list
> do
> if [ $numsed -gt 100 ]; then
> @@ -204,7 +204,7 @@ _wallclock()
>
> _timestamp()
> {
> - now=`date "+%T"`
> + local now=`date "+%T"`
> echo -n " [$now]"
> }
>
> @@ -600,7 +600,7 @@ fi
>
> function run_section()
> {
> - local section=$1
> + local section=$1 skip
>
> OLD_FSTYP=$FSTYP
> OLD_TEST_FS_MOUNT_OPTS=$TEST_FS_MOUNT_OPTS
> @@ -817,7 +817,7 @@ function run_section()
> rm -f core $seqres.notrun
>
> start=`_wallclock`
> - $timestamp && echo -n " ["`date "+%T"`"]"
> + $timestamp && _timestamp
> [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
> $LOGGER_PROG "run xfstest $seqnum"
> if [ -w /dev/kmsg ]; then
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH v2 4/6] check: append bad / notrun arrays in helper function
2022-06-27 22:22 ` [RFC PATCH v2 4/6] check: append bad / notrun arrays in helper function David Disseldorp
@ 2022-06-28 15:00 ` Darrick J. Wong
0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2022-06-28 15:00 UTC (permalink / raw)
To: David Disseldorp; +Cc: fstests, tytso
On Tue, Jun 28, 2022 at 12:22:54AM +0200, David Disseldorp wrote:
> Currently the @try, @bad and @notrun arrays are appended with seqnum at
> different points in the main run_section() loop:
> - @try: shortly prior to test script execution
> - @notrun: on list (check -n), or after .notrun flagged test completion
> - @bad: at the start of subsequent test loop and loop exit
>
> For future loop-test-following-failure functionality it makes sense to
> combine some of these steps. This change moves both @notrun and @bad
> appends into a helper function.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> ---
> check | 31 +++++++++++++++++++++++--------
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/check b/check
> index f973dd28..aa7dac2f 100755
> --- a/check
> +++ b/check
> @@ -550,6 +550,27 @@ _expunge_test()
> return 0
> }
>
> +# Retain in @bad / @notrun the result of previously run @test_seq. @try array
> +# entries are added prior to execution.
> +_stash_test_status() {
> + local test_seq="$1"
> + local test_status="$2"
> +
> + case "$test_status" in
> + fail)
> + bad+=("$test_seq")
> + ;;
> + list|notrun)
> + notrun+=("$test_seq")
> + ;;
> + pass|expunge|init)
> + ;;
> + *)
> + echo "Unexpected test $test_seq status: $test_status"
> + ;;
> + esac
> +}
> +
> # Can we run systemd scopes?
> HAVE_SYSTEMD_SCOPES=
> systemctl reset-failed "fstests-check" &>/dev/null
> @@ -733,9 +754,7 @@ function run_section()
> prev_seq=""
> for seq in $list ; do
> # Run report for previous test!
> - if [ "$tc_status" == "fail" ]; then
> - bad+=("$seqnum")
> - fi
> + _stash_test_status "$seqnum" "$tc_status"
> if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
> _make_testcase_report "$section" "$seqnum" \
> "$tc_status" "$((stop - start))"
> @@ -788,7 +807,6 @@ function run_section()
> start=0
> stop=0
> tc_status="list"
> - notrun+=("$seqnum")
> continue
> fi
>
> @@ -854,7 +872,6 @@ function run_section()
> $timestamp && echo " [not run]" && \
> echo -n " $seqnum -- "
> cat $seqres.notrun
> - notrun+=("$seqnum")
Ahh, it took me a minute to figure out that removing this line was ok
because we'll either jump back to the top of the loop or fall out below.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> tc_status="notrun"
>
> # Unmount the scratch fs so that we can wipe the scratch
> @@ -938,9 +955,7 @@ function run_section()
> done
>
> # make sure we record the status of the last test we ran.
> - if [ "$tc_status" == "fail" ]; then
> - bad+=("$seqnum")
> - fi
> + _stash_test_status "$seqnum" "$tc_status"
> if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
> _make_testcase_report "$section" "$seqnum" "$tc_status" \
> "$((stop - start))"
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH v2 5/6] check: add -L <n> parameter to rerun failed tests
2022-06-27 22:22 ` [RFC PATCH v2 5/6] check: add -L <n> parameter to rerun failed tests David Disseldorp
@ 2022-06-28 15:15 ` Darrick J. Wong
2022-06-28 22:34 ` David Disseldorp
0 siblings, 1 reply; 15+ messages in thread
From: Darrick J. Wong @ 2022-06-28 15:15 UTC (permalink / raw)
To: David Disseldorp; +Cc: fstests, tytso
On Tue, Jun 28, 2022 at 12:22:55AM +0200, David Disseldorp wrote:
> If check is run with -L <n>, then a failed test will be rerun <n> times
> before proceeding to the next test. Following completion of the rerun
> loop, aggregate pass/fail statistics are printed.
>
> Rerun tests will be tracked as a single failure in overall pass/fail
> metrics (via @try and @bad), with .out.bad, .dmesg and .full saved using
> a .rerun# suffix.
>
> Suggested-by: Theodore Ts'o <tytso@mit.edu>
> Link: https://lwn.net/Articles/897061/
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> ---
> check | 94 ++++++++++++++++++++++++++++++++++++++++++++-------
> common/report | 8 +++--
> 2 files changed, 88 insertions(+), 14 deletions(-)
>
> diff --git a/check b/check
> index aa7dac2f..726c83d9 100755
> --- a/check
> +++ b/check
> @@ -26,6 +26,7 @@ do_report=false
> DUMP_OUTPUT=false
> iterations=1
> istop=false
> +loop_on_fail=0
>
> # This is a global variable used to pass test failure text to reporting gunk
> _err_msg=""
> @@ -75,6 +76,7 @@ check options
> --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
> + -L <n> loop tests <n> times following a failure, measuring aggregate pass/fail metrics
>
> testlist options
> -g group[,group...] include tests from these groups
> @@ -333,6 +335,9 @@ while [ $# -gt 0 ]; do
> ;;
> --large-fs) export LARGE_SCRATCH_DEV=yes ;;
> --extra-space=*) export SCRATCH_DEV_EMPTY_SPACE=${r#*=} ;;
> + -L) [[ $2 =~ ^[0-9]+$ ]] || usage
> + loop_on_fail=$2; shift
> + ;;
>
> -*) usage ;;
> *) # not an argument, we've got tests now.
> @@ -555,6 +560,18 @@ _expunge_test()
> _stash_test_status() {
> local test_seq="$1"
> local test_status="$2"
> + local test_time="$3"
> + local loop_num="$4"
> + local report_msg="$5"
> +
> + if $do_report && [[ ! $test_status =~ ^(init|expunge)$ ]]; then
> + _make_testcase_report "$section" "$test_seq" \
> + "$test_status" "$test_time" \
> + "$report_msg"
> + fi
> +
> + # only stash result for first failure (triggering loop)
> + ((loop_num > 1)) && return
>
> case "$test_status" in
> fail)
> @@ -610,6 +627,38 @@ _run_seq() {
> fi
> }
>
> +# Check whether the last test should be rerun according to loop-on-error state
> +# and return "0" if so, otherwise return "1".
Er... this echoes 0 and 1, and the return value of the function is
neither checked nor set to anything other than zero, right?
I'm also not sure what 'ix' stands for?
> +_ix_inc() {
> + local test_status="$1"
> + local loop_len="$2"
> +
> + if ((!loop_on_fail)); then
> + echo 1
> + return
> + fi
> +
> + if [ "$test_status" == "fail" ] && ((!loop_len)); then
> + echo 0 # initial failure of this test, start loop-on-fail
> + elif ((loop_len > 0)) && ((loop_len < loop_on_fail)); then
> + echo 0 # continue loop following initial failure
> + else
> + echo 1 # completed or not currently in a failure loop
> + fi
> +}
> +
> +_failure_loop_dump_stats() {
> + awk "BEGIN {
> + n=split(\"$*\", arr);"'
> + for (i = 1; i <= n; i++)
> + stats[arr[i]]++;
> + printf("aggregate results across %d runs: ", n);
> + for (x in stats)
> + printf("%s=%d (%.1f%%)", (i-- > n ? x : ", " x),
> + stats[x], 100 * stats[x] / n);
> + }'
> +}
> +
> _detect_kmemleak
> _prepare_test_list
>
> @@ -750,14 +799,29 @@ function run_section()
> seqres="$check"
> _check_test_fs
>
> - local tc_status="init"
> + local tc_status="init" ix agg_msg
> prev_seq=""
> - for seq in $list ; do
> + local -a _list=( $list ) loop_status=()
> + for ((ix = 0; ix < ${#_list[*]};
> + ix += $(_ix_inc "$tc_status" "${#loop_status[*]}"))); do
> + seq="${_list[$ix]}"
> +
> + if [ "$seq" == "$prev_seq" ]; then
> + loop_status+=("$tc_status")
> + elif ((${#loop_status[*]})); then
> + # leaving rerun-on-failure loop
> + loop_status+=("$tc_status")
> + agg_msg=$(_failure_loop_dump_stats "${loop_status[@]}")
> + echo "$seqnum $agg_msg"
> + fi
> +
> # Run report for previous test!
> - _stash_test_status "$seqnum" "$tc_status"
> - if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
> - _make_testcase_report "$section" "$seqnum" \
> - "$tc_status" "$((stop - start))"
> + _stash_test_status "$seqnum" "$tc_status" "$((stop - start))" \
> + "${#loop_status[*]}" "$agg_msg"
> +
> + if [ -n "$agg_msg" ]; then
> + loop_status=()
> + agg_msg=""
> fi
>
> prev_seq="$seq"
> @@ -827,7 +891,9 @@ function run_section()
> fi
>
> # record that we really tried to run this test.
> - try+=("$seqnum")
> + if ((!${#loop_status[*]})); then
> + try+=("$seqnum")
> + fi
>
> awk 'BEGIN {lasttime=" "} \
> $1 == "'$seqnum'" {lasttime=" " $2 "s ... "; exit} \
> @@ -954,13 +1020,17 @@ function run_section()
> fi
> done
>
> - # make sure we record the status of the last test we ran.
> - _stash_test_status "$seqnum" "$tc_status"
> - if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
> - _make_testcase_report "$section" "$seqnum" "$tc_status" \
> - "$((stop - start))"
> + if ((${#loop_status[*]})); then
> + # leaving rerun-on-failure loop
> + loop_status+=("$tc_status")
> + agg_msg=$(_failure_loop_dump_stats "${loop_status[@]}")
> + echo "$seqnum $agg_msg"
> fi
>
> + # Run report for previous test!
> + _stash_test_status "$seqnum" "$tc_status" "$((stop - start))" \
> + "${#loop_status[*]}" "$agg_msg"
> +
> sect_stop=`_wallclock`
> interrupt=false
> _wrapup
> diff --git a/common/report b/common/report
> index 5ca41bc4..cede4987 100644
> --- a/common/report
> +++ b/common/report
> @@ -71,6 +71,7 @@ _xunit_make_testcase_report()
> local test_name="$2"
> local test_status="$3"
> local test_time="$4"
> + local test_md="$5"
...or what "md" here means.
>
> # TODO: other places may also win if no-section mode will be named like 'default/global'
> if [ $sect_name == '-no-sections-' ]; then
> @@ -79,7 +80,8 @@ _xunit_make_testcase_report()
> fi
> local report=$tmp.report.xunit.$sect_name.xml
>
> - echo -e "\t<testcase classname=\"xfstests.$sect_name\" name=\"$test_name\" time=\"$test_time\">" >> $report
> + [ -n "$test_md" ] && test_md=" status=\"$(echo "$test_md"|encode_xml)\""
AFAICT the junit xml schema defines a @status attribute for <testcase>.
https://raw.githubusercontent.com/windyroad/JUnit-Schema/master/JUnit.xsd
And yes, it's confusing that fstests namespaced all this code with
'xunit', since there's a separate xunit test reporting schema too!
That said -- I'm not sure what's supposed to end up in this attribute?
It looks like it's supposed to capture the aggregation report?
But then I wonder, why not stick to adding a separate <testcase> element
for each test run, even the repeated ones? And let the xml consumer
compute aggregation statistics from the elements?
--D
> + echo -e "\t<testcase classname=\"xfstests.$sect_name\" name=\"$test_name\" time=\"$test_time\"${test_md}>" >> $report
> case $test_status in
> "pass")
> ;;
> @@ -162,11 +164,13 @@ _make_testcase_report()
> local test_seq="$2"
> local test_status="$3"
> local test_time="$4"
> + local test_md="$5"
> for report in $REPORT_LIST; do
> case "$report" in
> "xunit")
> _xunit_make_testcase_report "$sect_name" "$test_seq" \
> - "$test_status" "$test_time"
> + "$test_status" \
> + "$test_time" "$test_md"
> ;;
> *)
> _dump_err "report format '$report' is not supported"
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH v2 6/6] check: stash full/dmesg/out.bad files on rerun
2022-06-27 22:22 ` [RFC PATCH v2 6/6] check: stash full/dmesg/out.bad files on rerun David Disseldorp
@ 2022-06-28 15:16 ` Darrick J. Wong
2022-06-28 22:36 ` David Disseldorp
0 siblings, 1 reply; 15+ messages in thread
From: Darrick J. Wong @ 2022-06-28 15:16 UTC (permalink / raw)
To: David Disseldorp; +Cc: fstests, tytso
On Tue, Jun 28, 2022 at 12:22:56AM +0200, David Disseldorp wrote:
> These files would otherwise be overwritten.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
Why not fold this into the previous patch?
--D
> ---
> check | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/check b/check
> index 726c83d9..baf336da 100755
> --- a/check
> +++ b/check
> @@ -570,8 +570,17 @@ _stash_test_status() {
> "$report_msg"
> fi
>
> - # only stash result for first failure (triggering loop)
> - ((loop_num > 1)) && return
> + if ((loop_num > 0)); then
> + # retain files which would be overwritten in subsequent reruns
> + for i in "${REPORT_DIR}/${test_seq}.full" \
> + "${REPORT_DIR}/${test_seq}.dmesg" \
> + "${REPORT_DIR}/${test_seq}.out.bad"; do
> + [ -f "$i" ] || continue
> + cp "$i" "${i}.rerun$((loop_num - 1))"
> + done
> + # only stash result for first failure (triggering loop)
> + ((loop_num != 1)) && return
> + fi
>
> case "$test_status" in
> fail)
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH v2 5/6] check: add -L <n> parameter to rerun failed tests
2022-06-28 15:15 ` Darrick J. Wong
@ 2022-06-28 22:34 ` David Disseldorp
0 siblings, 0 replies; 15+ messages in thread
From: David Disseldorp @ 2022-06-28 22:34 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: fstests, tytso
Thanks for the review, Darrick...
On Tue, 28 Jun 2022 08:15:07 -0700, Darrick J. Wong wrote:
> On Tue, Jun 28, 2022 at 12:22:55AM +0200, David Disseldorp wrote:
> > If check is run with -L <n>, then a failed test will be rerun <n> times
> > before proceeding to the next test. Following completion of the rerun
> > loop, aggregate pass/fail statistics are printed.
> >
> > Rerun tests will be tracked as a single failure in overall pass/fail
> > metrics (via @try and @bad), with .out.bad, .dmesg and .full saved using
> > a .rerun# suffix.
> >
> > Suggested-by: Theodore Ts'o <tytso@mit.edu>
> > Link: https://lwn.net/Articles/897061/
> > Signed-off-by: David Disseldorp <ddiss@suse.de>
> > ---
> > check | 94 ++++++++++++++++++++++++++++++++++++++++++++-------
> > common/report | 8 +++--
> > 2 files changed, 88 insertions(+), 14 deletions(-)
> >
> > diff --git a/check b/check
> > index aa7dac2f..726c83d9 100755
> > --- a/check
> > +++ b/check
> > @@ -26,6 +26,7 @@ do_report=false
> > DUMP_OUTPUT=false
> > iterations=1
> > istop=false
> > +loop_on_fail=0
> >
> > # This is a global variable used to pass test failure text to reporting gunk
> > _err_msg=""
> > @@ -75,6 +76,7 @@ check options
> > --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
> > + -L <n> loop tests <n> times following a failure, measuring aggregate pass/fail metrics
> >
> > testlist options
> > -g group[,group...] include tests from these groups
> > @@ -333,6 +335,9 @@ while [ $# -gt 0 ]; do
> > ;;
> > --large-fs) export LARGE_SCRATCH_DEV=yes ;;
> > --extra-space=*) export SCRATCH_DEV_EMPTY_SPACE=${r#*=} ;;
> > + -L) [[ $2 =~ ^[0-9]+$ ]] || usage
> > + loop_on_fail=$2; shift
> > + ;;
> >
> > -*) usage ;;
> > *) # not an argument, we've got tests now.
> > @@ -555,6 +560,18 @@ _expunge_test()
> > _stash_test_status() {
> > local test_seq="$1"
> > local test_status="$2"
> > + local test_time="$3"
> > + local loop_num="$4"
> > + local report_msg="$5"
> > +
> > + if $do_report && [[ ! $test_status =~ ^(init|expunge)$ ]]; then
> > + _make_testcase_report "$section" "$test_seq" \
> > + "$test_status" "$test_time" \
> > + "$report_msg"
> > + fi
> > +
> > + # only stash result for first failure (triggering loop)
> > + ((loop_num > 1)) && return
> >
> > case "$test_status" in
> > fail)
> > @@ -610,6 +627,38 @@ _run_seq() {
> > fi
> > }
> >
> > +# Check whether the last test should be rerun according to loop-on-error state
> > +# and return "0" if so, otherwise return "1".
>
> Er... this echoes 0 and 1, and the return value of the function is
> neither checked nor set to anything other than zero, right?
Right. run_section() test list iteration uses this helper to
conditionally increment the test index, depending on whether or not a
rerun (following test failure) is required.
> I'm also not sure what 'ix' stands for?
Index... I figured 'i' would get stomped on and didn't come up with a
better name at the time :)
> > +_ix_inc() {
> > + local test_status="$1"
> > + local loop_len="$2"
> > +
> > + if ((!loop_on_fail)); then
> > + echo 1
> > + return
> > + fi
> > +
> > + if [ "$test_status" == "fail" ] && ((!loop_len)); then
> > + echo 0 # initial failure of this test, start loop-on-fail
> > + elif ((loop_len > 0)) && ((loop_len < loop_on_fail)); then
> > + echo 0 # continue loop following initial failure
> > + else
> > + echo 1 # completed or not currently in a failure loop
> > + fi
> > +}
> > +
> > +_failure_loop_dump_stats() {
> > + awk "BEGIN {
> > + n=split(\"$*\", arr);"'
> > + for (i = 1; i <= n; i++)
> > + stats[arr[i]]++;
> > + printf("aggregate results across %d runs: ", n);
> > + for (x in stats)
> > + printf("%s=%d (%.1f%%)", (i-- > n ? x : ", " x),
> > + stats[x], 100 * stats[x] / n);
> > + }'
> > +}
> > +
> > _detect_kmemleak
> > _prepare_test_list
> >
> > @@ -750,14 +799,29 @@ function run_section()
> > seqres="$check"
> > _check_test_fs
> >
> > - local tc_status="init"
> > + local tc_status="init" ix agg_msg
> > prev_seq=""
> > - for seq in $list ; do
> > + local -a _list=( $list ) loop_status=()
> > + for ((ix = 0; ix < ${#_list[*]};
> > + ix += $(_ix_inc "$tc_status" "${#loop_status[*]}"))); do
Here is the _ix_inc caller ^^. It's a bit convoluted so I could probably
turn it into a while loop and put the increment logic here. Not sure how
much simpler it'd be though.
> > + seq="${_list[$ix]}"
> > +
> > + if [ "$seq" == "$prev_seq" ]; then
> > + loop_status+=("$tc_status")
> > + elif ((${#loop_status[*]})); then
> > + # leaving rerun-on-failure loop
> > + loop_status+=("$tc_status")
> > + agg_msg=$(_failure_loop_dump_stats "${loop_status[@]}")
> > + echo "$seqnum $agg_msg"
> > + fi
> > +
> > # Run report for previous test!
> > - _stash_test_status "$seqnum" "$tc_status"
> > - if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
> > - _make_testcase_report "$section" "$seqnum" \
> > - "$tc_status" "$((stop - start))"
> > + _stash_test_status "$seqnum" "$tc_status" "$((stop - start))" \
> > + "${#loop_status[*]}" "$agg_msg"
> > +
> > + if [ -n "$agg_msg" ]; then
> > + loop_status=()
> > + agg_msg=""
> > fi
> >
> > prev_seq="$seq"
> > @@ -827,7 +891,9 @@ function run_section()
> > fi
> >
> > # record that we really tried to run this test.
> > - try+=("$seqnum")
> > + if ((!${#loop_status[*]})); then
> > + try+=("$seqnum")
> > + fi
> >
> > awk 'BEGIN {lasttime=" "} \
> > $1 == "'$seqnum'" {lasttime=" " $2 "s ... "; exit} \
> > @@ -954,13 +1020,17 @@ function run_section()
> > fi
> > done
> >
> > - # make sure we record the status of the last test we ran.
> > - _stash_test_status "$seqnum" "$tc_status"
> > - if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
> > - _make_testcase_report "$section" "$seqnum" "$tc_status" \
> > - "$((stop - start))"
> > + if ((${#loop_status[*]})); then
> > + # leaving rerun-on-failure loop
> > + loop_status+=("$tc_status")
> > + agg_msg=$(_failure_loop_dump_stats "${loop_status[@]}")
> > + echo "$seqnum $agg_msg"
> > fi
> >
> > + # Run report for previous test!
> > + _stash_test_status "$seqnum" "$tc_status" "$((stop - start))" \
> > + "${#loop_status[*]}" "$agg_msg"
> > +
> > sect_stop=`_wallclock`
> > interrupt=false
> > _wrapup
> > diff --git a/common/report b/common/report
> > index 5ca41bc4..cede4987 100644
> > --- a/common/report
> > +++ b/common/report
> > @@ -71,6 +71,7 @@ _xunit_make_testcase_report()
> > local test_name="$2"
> > local test_status="$3"
> > local test_time="$4"
> > + local test_md="$5"
>
> ...or what "md" here means.
Test (result) metadata.
> > # TODO: other places may also win if no-section mode will be named like 'default/global'
> > if [ $sect_name == '-no-sections-' ]; then
> > @@ -79,7 +80,8 @@ _xunit_make_testcase_report()
> > fi
> > local report=$tmp.report.xunit.$sect_name.xml
> >
> > - echo -e "\t<testcase classname=\"xfstests.$sect_name\" name=\"$test_name\" time=\"$test_time\">" >> $report
> > + [ -n "$test_md" ] && test_md=" status=\"$(echo "$test_md"|encode_xml)\""
>
> AFAICT the junit xml schema defines a @status attribute for <testcase>.
>
> https://raw.githubusercontent.com/windyroad/JUnit-Schema/master/JUnit.xsd
^ hmm, I don't see a @status attribute under your link. I was using a
different reference...
> And yes, it's confusing that fstests namespaced all this code with
> 'xunit', since there's a separate xunit test reporting schema too!
It is absolutely confusing. I've been using
https://llg.cubic.org/docs/junit/ but only came across that by searching
for specific attribute names.
> That said -- I'm not sure what's supposed to end up in this attribute?
> It looks like it's supposed to capture the aggregation report?
Correct, it's just the aggregation report for now.
> But then I wonder, why not stick to adding a separate <testcase> element
> for each test run, even the repeated ones? And let the xml consumer
> compute aggregation statistics from the elements?
This v2 patchset does add a <testcase> element for each rerun, with the
aggregation stats attached to the last rerun . The
aggregation stats are already calculated for non-xunit output, and
passing it through for xunit only costs a few lines of code. However, if
the @status attribute isn't an option then I suppose I could drop it -
Ted also mentioned that it's unnecessary.
Cheers, David
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH v2 6/6] check: stash full/dmesg/out.bad files on rerun
2022-06-28 15:16 ` Darrick J. Wong
@ 2022-06-28 22:36 ` David Disseldorp
0 siblings, 0 replies; 15+ messages in thread
From: David Disseldorp @ 2022-06-28 22:36 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: fstests, tytso
On Tue, 28 Jun 2022 08:16:08 -0700, Darrick J. Wong wrote:
> On Tue, Jun 28, 2022 at 12:22:56AM +0200, David Disseldorp wrote:
> > These files would otherwise be overwritten.
> >
> > Signed-off-by: David Disseldorp <ddiss@suse.de>
>
> Why not fold this into the previous patch?
Gladly. Will do so in a subsequent round.
Cheers, David
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-06-28 22:36 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27 22:22 [RFC PATCH v2 0/6] check: add option to rerun failed tests David Disseldorp
2022-06-27 22:22 ` [RFC PATCH v2 1/6] report: use array for REPORT_ENV_LIST David Disseldorp
2022-06-28 14:54 ` Darrick J. Wong
2022-06-27 22:22 ` [RFC PATCH v2 2/6] report: pass through most details as function parameters David Disseldorp
2022-06-28 14:55 ` Darrick J. Wong
2022-06-27 22:22 ` [RFC PATCH v2 3/6] check: make a few variables local David Disseldorp
2022-06-28 14:56 ` Darrick J. Wong
2022-06-27 22:22 ` [RFC PATCH v2 4/6] check: append bad / notrun arrays in helper function David Disseldorp
2022-06-28 15:00 ` Darrick J. Wong
2022-06-27 22:22 ` [RFC PATCH v2 5/6] check: add -L <n> parameter to rerun failed tests David Disseldorp
2022-06-28 15:15 ` Darrick J. Wong
2022-06-28 22:34 ` David Disseldorp
2022-06-27 22:22 ` [RFC PATCH v2 6/6] check: stash full/dmesg/out.bad files on rerun David Disseldorp
2022-06-28 15:16 ` Darrick J. Wong
2022-06-28 22:36 ` David Disseldorp
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.