[RFC PATCH 0/4] check: add --loop* family options for repeating tests

[RFC PATCH 0/4] check: add --loop* family options for repeating tests

[Zorro Lang]

This series originates from Ted Ts'o's RFC:

  [PATCH RFC] check: add new option "--loop <n>" which runs each test multiple times
  https://lore.kernel.org/fstests/20260415213248.1795275-1-tytso@mit.edu/

At the time, we didn't reach a consensus, and because the argument parsing in
./check was quite messy, I decided to refactor it before introducing more
options. That refactoring is now complete:

  https://lore.kernel.org/fstests/20260826102857.3224318-1-zlang@kernel.org/

so I think it's time to revisit this RFC and the follow-up changes.

 --loop re-runs each test unconditionally, fixing two problems of "-i
 <n>": per-run artifacts (e.g. NNN.out.bad) were not preserved, and
result.xml was rewritten for every test set, losing cumulative junit
statistics.  That RFC predates the check argument-parsing rewrite to
getopt, so patch 2 was rewritten on top of it, generalizing the -L
rerun state machine to support multiple loop modes.

Per-test looping options (mutually exclusive; repeating the same mode
overrides its count):

  --loop <n>              run each test <n> times.  Artifacts of every
                          run are stashed with .rerunN suffixes, the
                          junit report gets one testcase per run, an
                          aggregate pass/fail line is printed, and the
                          test fails if any of its runs fails.
  -L, --loop-on-fail <n>  rerun a test <n> times after its first
                          failure (<n>+1 runs in total).  Existing -L
                          behaviour, now with a long name.
  --loop-while-successful <n>
                          run each test up to <n> times, stopping at
                          the first failure; handy for bisecting
                          flaky tests.

Whole-list iteration gets long names as well:

  -i, --iterate <n>       iterate the entire test list <n> times
  -I, --iterate-while-successful <n>
                          same, but stop after the first round that
                          contains a failure

The short options -i, -I and -L keep their existing behaviour and
remain available. Maybe we can remove them one day, or keep them.

One open question about the junit report: with --loop each run adds a
testcase to result.xml, but the testsuite tests/failures attributes
still count unique tests, consistent with the "Ran: ... / Failed ..."
summary.  Before this series the two were always equal, so report
consumers that assume tests matches the number of testcase elements
would see them diverge for the first time.  We kept the current
semantics for now; comments on whether the attributes should count
per-run testcases instead (a follow-up common/report change) are
welcome.

Thanks,
Zorro

Theodore Ts'o (1):
  check: add --loop option to run each test multiple times

Zorro Lang (3):
  check: add long aliases --iterate and --iterate-while-successful
  check: add --loop-on-fail long name for -L
  check: add --loop-while-successful option

 check | 153 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 120 insertions(+), 33 deletions(-)

-- 
2.56.0

[RFC PATCH 1/4] check: add long aliases --iterate and --iterate-while-successful

[Zorro Lang]

The -i and -I options iterate the whole test list, but their
single-letter names don't describe what they do.  Add the long aliases
--iterate and --iterate-while-successful so that they can be named
consistently with the --loop family of per-test looping options that
will be added next.

Signed-off-by: Zorro Lang <zlang@kernel.org>
---
 check | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/check b/check
index afe855bc..538f22cf 100755
--- a/check
+++ b/check
@@ -62,8 +62,9 @@ General options:
     -b                      brief test summary
     -d                      dump test output to stdout
     -h, --help              show this help message
-    -i <n>                  iterate the test list <n> times
-    -I <n>                  iterate the test list <n> times, but stops
+    -i, --iterate <n>       iterate the test list <n> times
+    -I, --iterate-while-successful <n>
+                              iterate the test list <n> times, but stops
                               iterating further in case of any test failure
     -l                      line mode diff
     -L <n>                  loop tests <n> times following a failure, measuring
@@ -388,7 +389,7 @@ process_remain_tests()
 parse_check_args()
 {
 	local short_opts="g:x:X:e:E:s:S:lnri:I:TdbR:L:h"
-	local long_opts="fs:,exact-order,large-fs,extra-space:,udiff,help"
+	local long_opts="fs:,exact-order,large-fs,extra-space:,udiff,help,iterate:,iterate-while-successful:"
 
 	compat_old_option "$@"
 
@@ -466,11 +467,11 @@ parse_check_args()
 			fi
 			exact_order=true
 			;;
-		-i)
+		-i|--iterate)
 			iterations=$2
 			shift
 			;;
-		-I)
+		-I|--iterate-while-successful)
 			iterations=$2
 			istop=true
 			shift
-- 
2.56.0

[RFC PATCH 2/4] check: add --loop option to run each test multiple times

[Zorro Lang]

From: Theodore Ts'o <tytso@mit.edu>

Teach the check script a new option --loop, which re-runs each test
multiple times.  This works very similarly to -L, which retries a test
after it first fails, except that the test is rerun unconditionally.

This differs from the "-i <n>" option, which iterates each set of
tests <n> times instead of each test.  The -i option is problematic in
two ways.  First, it doesn't save the test artifacts from each test
run.  This is unfortunate because when the developer is trying to debug
a flaky test failure, running "check -i 100" will run a test 100
times, but if only the 42nd test fails, the NNN.out.bad file for that
failing test run is not preserved.  The second difference between
--loop and -i is that the result.xml file is rewritten after each test
set, so we do not have the cumulative statistics of the 100 test runs
in the junit XML file.

Refactor the rerun-on-failure state machine so that it can support
multiple loop modes: the loop count is normalized to the total number
of runs at option parsing time, and the aggregate-results logic is
shared between the initial run and the reruns.  This also fixes an
off-by-one where --loop=1 would run the test twice.  A test run with
--loop fails if any of its runs fails, matching the behaviour of -i.
--loop and -L select different loop modes and cannot be combined.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Zorro Lang <zlang@kernel.org>
---
 check | 122 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 95 insertions(+), 27 deletions(-)

diff --git a/check b/check
index 538f22cf..b30532cf 100755
--- a/check
+++ b/check
@@ -26,7 +26,10 @@ do_report=false
 DUMP_OUTPUT=false
 iterations=1
 istop=false
-loop_on_fail=0
+# Selected by -L / --loop and its siblings: "", "always" or "on-fail"
+loop_mode=""
+# Total number of runs for the current loop mode
+loop_total=0
 exclude_tests=()
 
 # This is a global variable used to pass test failure text to reporting gunk
@@ -69,6 +72,8 @@ General options:
     -l                      line mode diff
     -L <n>                  loop tests <n> times following a failure, measuring
                               aggregate pass/fail metrics
+    --loop <n>              run each test <n> times, measuring aggregate
+                              pass/fail metrics (test fails if any run fails)
     -n                      show me, do not run tests
     -r                      randomize test order
     -s section              run only specified section from config file
@@ -386,10 +391,34 @@ process_remain_tests()
 	done
 }
 
+# Record the loop mode selected by -L / --loop / --loop-on-fail /
+# --loop-while-successful and normalize its count into the total number of
+# runs (including the first one).  The loop modes are mutually exclusive,
+# while repeating the same mode just overrides its count.
+_set_loop_mode()
+{
+	local mode="$1"
+	local count="$2"
+
+	[[ $count =~ ^[0-9]+$ ]] || usage
+	if [ -n "$loop_mode" ] && [ "$loop_mode" != "$mode" ]; then
+		echo "check: conflicting loop modes: -L and --loop are mutually exclusive" >&2
+		usage
+	fi
+	loop_mode="$mode"
+	if [ "$mode" = "on-fail" ]; then
+		# <n> reruns after the first failure, <n>+1 runs in total
+		loop_total=$((count + 1))
+	else
+		# <n> runs in total
+		loop_total=$count
+	fi
+}
+
 parse_check_args()
 {
 	local short_opts="g:x:X:e:E:s:S:lnri:I:TdbR:L:h"
-	local long_opts="fs:,exact-order,large-fs,extra-space:,udiff,help,iterate:,iterate-while-successful:"
+	local long_opts="fs:,exact-order,large-fs,extra-space:,udiff,help,iterate:,iterate-while-successful:,loop:"
 
 	compat_old_option "$@"
 
@@ -498,8 +527,11 @@ parse_check_args()
 			shift
 			;;
 		-L)
-			[[ $2 =~ ^[0-9]+$ ]] || usage
-			loop_on_fail=$2
+			_set_loop_mode on-fail "$2"
+			shift
+			;;
+		--loop)
+			_set_loop_mode always "$2"
 			shift
 			;;
 		-h|--help)
@@ -747,7 +779,7 @@ _expunge_test()
 }
 
 # retain files which would be overwritten in subsequent reruns of the same test
-_stash_fail_loop_files() {
+_stash_loop_files() {
 	local seq_prefix="${REPORT_DIR}/${1}"
 	local cp_suffix="$2"
 
@@ -759,6 +791,41 @@ _stash_fail_loop_files() {
 	done
 }
 
+# Record a failed test in @bad, avoiding duplicates for tests that already
+# failed in an earlier run of their loop.
+_add_bad() {
+	local test_seq="$1"
+	local b
+
+	for b in "${bad[@]}"; do
+		[ "$b" = "$test_seq" ] && return
+	done
+	bad+=("$test_seq")
+}
+
+# Print the aggregate results of the current loop once all loop_total runs
+# have been recorded, then reset the loop state so that the next test can
+# start.
+_try_loop_aggregate() {
+	local test_seq="$1"
+
+	if ((${#loop_status[*]} >= loop_total)); then
+		printf "%s aggregate results across %d runs: " \
+			"$test_seq" "${#loop_status[*]}"
+		awk "BEGIN {
+			n=split(\"${loop_status[*]}\", arr);"'
+			for (i = 1; i <= n; i++)
+				stats[arr[i]]++;
+			for (x in stats)
+				printf("%s=%d (%.1f%%)",
+				       (i-- > n ? x : ", " x),
+				       stats[x], 100 * stats[x] / n);
+			}'
+		echo
+		loop_status=()
+	fi
+}
+
 # Retain in @bad / @notrun the result of the just-run @test_seq. @try array
 # entries are added prior to execution.
 _stash_test_status() {
@@ -771,40 +838,41 @@ _stash_test_status() {
 	fi
 
 	if ((${#loop_status[*]} > 0)); then
-		# continuing or completing rerun-on-failure loop
-		_stash_fail_loop_files "$test_seq" ".rerun${#loop_status[*]}"
+		# continuing or completing a rerun loop
+		_stash_loop_files "$test_seq" ".rerun${#loop_status[*]}"
 		loop_status+=("$test_status")
-		if ((${#loop_status[*]} > loop_on_fail)); then
-			printf "%s aggregate results across %d runs: " \
-				"$test_seq" "${#loop_status[*]}"
-			awk "BEGIN {
-				n=split(\"${loop_status[*]}\", arr);"'
-				for (i = 1; i <= n; i++)
-					stats[arr[i]]++;
-				for (x in stats)
-					printf("%s=%d (%.1f%%)",
-					       (i-- > n ? x : ", " x),
-					       stats[x], 100 * stats[x] / n);
-				}'
-			echo
-			loop_status=()
+		if [ "$test_status" = "fail" ]; then
+			# any failed run fails the test
+			_add_bad "$test_seq"
 		fi
-		return	# only stash @bad result for initial failure in loop
+		_try_loop_aggregate "$test_seq"
+		# the rerun loop handles @bad/@notrun by itself
+		return
 	fi
 
 	case "$test_status" in
 	fail)
-		if ((loop_on_fail > 0)); then
-			# initial failure, start rerun-on-failure loop
-			_stash_fail_loop_files "$test_seq" ".rerun0"
+		bad+=("$test_seq")
+		if { [ "$loop_mode" = "always" ] || [ "$loop_mode" = "on-fail" ]; } \
+		   && ((loop_total > 1)); then
+			# initial failure, start a rerun loop
+			_stash_loop_files "$test_seq" ".rerun0"
 			loop_status+=("$test_status")
+			_try_loop_aggregate "$test_seq"
 		fi
-		bad+=("$test_seq")
 		;;
 	list|notrun)
 		notrun+=("$test_seq")
 		;;
-	pass|expunge)
+	pass)
+		if [ "$loop_mode" = "always" ] && ((loop_total > 1)); then
+			# start a rerun loop
+			_stash_loop_files "$test_seq" ".rerun0"
+			loop_status+=("$test_status")
+			_try_loop_aggregate "$test_seq"
+		fi
+		;;
+	expunge)
 		;;
 	*)
 		echo "Unexpected test $test_seq status: $test_status"
-- 
2.56.0

[RFC PATCH 3/4] check: add --loop-on-fail long name for -L

[Zorro Lang]

The -L option reruns a test <n> times after its first failure.  Give
it the long name --loop-on-fail so that all the per-test looping
options can be described consistently as a --loop family.

Signed-off-by: Zorro Lang <zlang@kernel.org>
---
 check | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/check b/check
index b30532cf..96e4f2fd 100755
--- a/check
+++ b/check
@@ -70,7 +70,7 @@ General options:
                               iterate the test list <n> times, but stops
                               iterating further in case of any test failure
     -l                      line mode diff
-    -L <n>                  loop tests <n> times following a failure, measuring
+    -L, --loop-on-fail <n>  loop tests <n> times following a failure, measuring
                               aggregate pass/fail metrics
     --loop <n>              run each test <n> times, measuring aggregate
                               pass/fail metrics (test fails if any run fails)
@@ -402,7 +402,7 @@ _set_loop_mode()
 
 	[[ $count =~ ^[0-9]+$ ]] || usage
 	if [ -n "$loop_mode" ] && [ "$loop_mode" != "$mode" ]; then
-		echo "check: conflicting loop modes: -L and --loop are mutually exclusive" >&2
+		echo "check: conflicting loop modes: -L, --loop-on-fail and --loop are mutually exclusive" >&2
 		usage
 	fi
 	loop_mode="$mode"
@@ -418,7 +418,7 @@ _set_loop_mode()
 parse_check_args()
 {
 	local short_opts="g:x:X:e:E:s:S:lnri:I:TdbR:L:h"
-	local long_opts="fs:,exact-order,large-fs,extra-space:,udiff,help,iterate:,iterate-while-successful:,loop:"
+	local long_opts="fs:,exact-order,large-fs,extra-space:,udiff,help,iterate:,iterate-while-successful:,loop:,loop-on-fail:"
 
 	compat_old_option "$@"
 
@@ -526,7 +526,7 @@ parse_check_args()
 			export SCRATCH_DEV_EMPTY_SPACE="$2"
 			shift
 			;;
-		-L)
+		-L|--loop-on-fail)
 			_set_loop_mode on-fail "$2"
 			shift
 			;;
-- 
2.56.0

[RFC PATCH 4/4] check: add --loop-while-successful option

[Zorro Lang]

Add one more member of the --loop family: --loop-while-successful runs
each test up to <n> times but stops rerunning it at the first failure.
This is faster than --loop for bisecting flaky tests, since the loop
ends as soon as the failure is reproduced.  The failing run is recorded
in @bad, so a reproduced failure is still reported as a failed test.

Signed-off-by: Zorro Lang <zlang@kernel.org>
---
 check | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/check b/check
index 96e4f2fd..6b7bd23e 100755
--- a/check
+++ b/check
@@ -26,7 +26,8 @@ do_report=false
 DUMP_OUTPUT=false
 iterations=1
 istop=false
-# Selected by -L / --loop and its siblings: "", "always" or "on-fail"
+# Selected by -L / --loop and its siblings: "", "always", "on-fail" or
+# "while-successful"
 loop_mode=""
 # Total number of runs for the current loop mode
 loop_total=0
@@ -74,6 +75,9 @@ General options:
                               aggregate pass/fail metrics
     --loop <n>              run each test <n> times, measuring aggregate
                               pass/fail metrics (test fails if any run fails)
+    --loop-while-successful <n>
+                              run each test up to <n> times, stopping at the
+                              first failure
     -n                      show me, do not run tests
     -r                      randomize test order
     -s section              run only specified section from config file
@@ -402,7 +406,7 @@ _set_loop_mode()
 
 	[[ $count =~ ^[0-9]+$ ]] || usage
 	if [ -n "$loop_mode" ] && [ "$loop_mode" != "$mode" ]; then
-		echo "check: conflicting loop modes: -L, --loop-on-fail and --loop are mutually exclusive" >&2
+		echo "check: conflicting loop modes: -L, --loop, --loop-on-fail and --loop-while-successful are mutually exclusive" >&2
 		usage
 	fi
 	loop_mode="$mode"
@@ -418,7 +422,9 @@ _set_loop_mode()
 parse_check_args()
 {
 	local short_opts="g:x:X:e:E:s:S:lnri:I:TdbR:L:h"
-	local long_opts="fs:,exact-order,large-fs,extra-space:,udiff,help,iterate:,iterate-while-successful:,loop:,loop-on-fail:"
+	local long_opts="fs:,exact-order,large-fs,extra-space:,udiff,help,"
+	long_opts+="iterate:,iterate-while-successful:,"
+	long_opts+="loop:,loop-on-fail:,loop-while-successful:"
 
 	compat_old_option "$@"
 
@@ -534,6 +540,10 @@ parse_check_args()
 			_set_loop_mode always "$2"
 			shift
 			;;
+		--loop-while-successful)
+			_set_loop_mode while-successful "$2"
+			shift
+			;;
 		-h|--help)
 			usage
 			;;
@@ -805,11 +815,13 @@ _add_bad() {
 
 # Print the aggregate results of the current loop once all loop_total runs
 # have been recorded, then reset the loop state so that the next test can
-# start.
+# start.  A non-empty @force argument ends the loop before loop_total runs,
+# which is used to stop a while-successful loop at the first failure.
 _try_loop_aggregate() {
 	local test_seq="$1"
+	local force="$2"
 
-	if ((${#loop_status[*]} >= loop_total)); then
+	if [ -n "$force" ] || ((${#loop_status[*]} >= loop_total)); then
 		printf "%s aggregate results across %d runs: " \
 			"$test_seq" "${#loop_status[*]}"
 		awk "BEGIN {
@@ -845,7 +857,12 @@ _stash_test_status() {
 			# any failed run fails the test
 			_add_bad "$test_seq"
 		fi
-		_try_loop_aggregate "$test_seq"
+		# a while-successful loop stops at the first failed run
+		if [ "$test_status" = "fail" ] && [ "$loop_mode" = "while-successful" ]; then
+			_try_loop_aggregate "$test_seq" force
+		else
+			_try_loop_aggregate "$test_seq"
+		fi
 		# the rerun loop handles @bad/@notrun by itself
 		return
 	fi
@@ -865,7 +882,8 @@ _stash_test_status() {
 		notrun+=("$test_seq")
 		;;
 	pass)
-		if [ "$loop_mode" = "always" ] && ((loop_total > 1)); then
+		if { [ "$loop_mode" = "always" ] || [ "$loop_mode" = "while-successful" ]; } \
+		   && ((loop_total > 1)); then
 			# start a rerun loop
 			_stash_loop_files "$test_seq" ".rerun0"
 			loop_status+=("$test_status")
-- 
2.56.0