All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] add option to rerun failed tests
@ 2022-06-21 16:01 David Disseldorp
  2022-06-21 16:01 ` [RFC PATCH 1/2] check: append bad / notrun arrays in helper function David Disseldorp
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: David Disseldorp @ 2022-06-21 16:01 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.

There are a couple of things which I'd like to resolve before dropping
the RFC flag, but would appreciate early feedback on the approach here.
The caveats are:
- rerun tests will be tracked as a single failure in @try and @bad
  + xunit reports do not include any rerun details
- .bad files generated on failure will be overwritten by test reruns

For xunit reports, I think it'll make sense to stash the aggregates in a
separate <test>.agg-results file or something. Similarly for .bad file
overwrites, I could add a .<rerun #> suffix for capturing all failure
data.

Cheers, David


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC PATCH 1/2] check: append bad / notrun arrays in helper function
  2022-06-21 16:01 [RFC PATCH 0/2] add option to rerun failed tests David Disseldorp
@ 2022-06-21 16:01 ` David Disseldorp
  2022-06-21 16:01 ` [RFC PATCH 2/2] check: add -L <n> parameter to rerun failed tests David Disseldorp
  2022-06-24  4:42 ` [RFC PATCH 0/2] add option " Theodore Ts'o
  2 siblings, 0 replies; 8+ messages in thread
From: David Disseldorp @ 2022-06-21 16:01 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 2ea2920f..3e8a232c 100755
--- a/check
+++ b/check
@@ -546,6 +546,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
@@ -729,9 +750,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 "$prev_seq" "$tc_status"
 		fi
@@ -783,7 +802,6 @@ function run_section()
 			start=0
 			stop=0
 			tc_status="list"
-			notrun+=("$seqnum")
 			continue
 		fi
 
@@ -849,7 +867,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
@@ -933,9 +950,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 "$prev_seq" "$tc_status"
 	fi
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH 2/2] check: add -L <n> parameter to rerun failed tests
  2022-06-21 16:01 [RFC PATCH 0/2] add option to rerun failed tests David Disseldorp
  2022-06-21 16:01 ` [RFC PATCH 1/2] check: append bad / notrun arrays in helper function David Disseldorp
@ 2022-06-21 16:01 ` David Disseldorp
  2022-06-24  4:42 ` [RFC PATCH 0/2] add option " Theodore Ts'o
  2 siblings, 0 replies; 8+ messages in thread
From: David Disseldorp @ 2022-06-21 16:01 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.

Caveats:
- rerun tests will be tracked as a single failure in @try and @bad
  + xunit reports do not include any rerun details
- .bad files generated on failure will be overwritten by test reruns

Suggested-by: Theodore Ts'o <tytso@mit.edu>
Link: https://lwn.net/Articles/897061/
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 check | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 75 insertions(+), 11 deletions(-)

diff --git a/check b/check
index 3e8a232c..97fa50ad 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,7 @@ while [ $# -gt 0 ]; do
 		;;
 	--large-fs) export LARGE_SCRATCH_DEV=yes ;;
 	--extra-space=*) export SCRATCH_DEV_EMPTY_SPACE=${r#*=} ;;
+	-L)	loop_on_fail=$2; shift ;;
 
 	-*)	usage ;;
 	*)	# not an argument, we've got tests now.
@@ -606,6 +609,40 @@ _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() {
+	local t="$1" && shift
+	awk "BEGIN {
+		n=split(\"$*\", arr);"'
+		for (i = 1; i <= n; i++)
+			stats[arr[i]]++;
+		printf("'"$t"' aggregate results across %d runs: ", n);
+		for (x in stats)
+			printf("%s=%d (%.1f%%)", (i-- > n ? x : ", " x),
+			       stats[x], 100 * stats[x] / n);
+		print;
+	     }'
+}
+
 _detect_kmemleak
 _prepare_test_list
 
@@ -746,13 +783,33 @@ function run_section()
 	seqres="$check"
 	_check_test_fs
 
-	local tc_status="init"
+	local tc_status="init" ix
 	prev_seq=""
-	for seq in $list ; do
-		# Run report for previous test!
-		_stash_test_status "$seqnum" "$tc_status"
-		if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
-			_make_testcase_report "$prev_seq" "$tc_status"
+	local -a _list=( $list ) loop_status=()
+	for ((ix = 0; ix < ${#_list[*]};
+	      ix += $(_ix_inc "$tc_status" "${#loop_status[*]}"))); do
+		seq="${_list[$ix]}"
+
+		if ((!loop_on_fail)); then
+			# Run report for previous test!
+			_stash_test_status "$seqnum" "$tc_status"
+			if $do_report && [[ ! $tc_status =~ ^(init|expunge)$ ]]; then
+				_make_testcase_report "$prev_seq" "$tc_status"
+			fi
+		else
+			if [ "$seq" == "$prev_seq" ]; then
+				if ((!${#loop_status[*]})); then
+					_stash_test_status "$seqnum" "$tc_status"
+					if $do_report; then
+						_make_testcase_report "$prev_seq" "$tc_status"
+					fi
+				fi
+				loop_status+=("$tc_status")
+			elif ((${#loop_status[*]})); then
+				loop_status+=("$tc_status")
+				_failure_loop_dump_stats "$seqnum" "${loop_status[@]}"
+				loop_status=()
+			fi
 		fi
 
 		prev_seq="$seq"
@@ -822,7 +879,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} \
@@ -949,10 +1008,15 @@ 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 "$prev_seq" "$tc_status"
+	if ((${#loop_status[*]})); then
+		loop_status+=("$tc_status")
+		_failure_loop_dump_stats "$seqnum" "${loop_status[@]}"
+	else
+		# 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 "$prev_seq" "$tc_status"
+		fi
 	fi
 
 	sect_stop=`_wallclock`
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 0/2] add option to rerun failed tests
  2022-06-21 16:01 [RFC PATCH 0/2] add option to rerun failed tests David Disseldorp
  2022-06-21 16:01 ` [RFC PATCH 1/2] check: append bad / notrun arrays in helper function David Disseldorp
  2022-06-21 16:01 ` [RFC PATCH 2/2] check: add -L <n> parameter to rerun failed tests David Disseldorp
@ 2022-06-24  4:42 ` Theodore Ts'o
  2022-06-24  6:15   ` Zorro Lang
  2022-06-24  8:32   ` David Disseldorp
  2 siblings, 2 replies; 8+ messages in thread
From: Theodore Ts'o @ 2022-06-24  4:42 UTC (permalink / raw)
  To: David Disseldorp; +Cc: fstests

On Tue, Jun 21, 2022 at 06:01:51PM +0200, David Disseldorp wrote:
> 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.
> 
> There are a couple of things which I'd like to resolve before dropping
> the RFC flag, but would appreciate early feedback on the approach here.

This is really exciting!  I was hoping to try it out, but the first
patch doesn't apply to origin/master on xfstests-dev.

For example this patch hunk:

@@ -729,9 +750,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 "$prev_seq" "$tc_status"
 		fi

The relevant section of check looks like this:

		# Run report for previous test!
		if $err ; then
			bad="$bad $seqnum"
			n_bad=`expr $n_bad + 1`
			tc_status="fail"
		fi

And "git blame" shows that this part of check hasn't changed since
2018, and I'm on the latest version upstream version of xfstests:

commit 568ac9fffeb6afec03e5d6c9936617232fd7fc6d (HEAD, tag: v2022.06.05, origin/master, origin/HEAD, kernel/master)
Author: Dave Chinner <dchinner@redhat.com>
Date:   Fri Jun 3 11:54:13 2022 +1000

    xfs/189: systemd monitoring of /etc/fstab sucks


Was your patch based xfstests with some out-of-tree patches?

> The caveats are:
> - rerun tests will be tracked as a single failure in @try and @bad
>   + xunit reports do not include any rerun details
> - .bad files generated on failure will be overwritten by test reruns
> 
> For xunit reports, I think it'll make sense to stash the aggregates in a
> separate <test>.agg-results file or something. Similarly for .bad file
> overwrites, I could add a .<rerun #> suffix for capturing all failure
> data.

For xunit results fie, was assuming that simply we would just have
multiple repeated testcase entries stored in the single results.xml
file.  For example:

<testcase classname="xfstests.global" name="generic/476" time="354">
		<failure message="Test  failed, reason unknown" type="TestFail" />
		<system-out>
		...
	</testcase>
<testcase classname="xfstests.global" name="generic/476" time="343">
	</testcase>
<testcase classname="xfstests.global" name="generic/476" time="353">
	</testcase>
	...

I don't know that we need a separate file for the rerun tests, since
it's not that hard to create a python script which parses the results
and calculates the pass/fail percentages for any test which is run
mutiple times.


As far as haivng the .bad and .full files, I agree that some kind of
.rerun-NN suffix would make a lot of sense.

Cheers,

						- Ted

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 0/2] add option to rerun failed tests
  2022-06-24  4:42 ` [RFC PATCH 0/2] add option " Theodore Ts'o
@ 2022-06-24  6:15   ` Zorro Lang
  2022-06-24  8:32   ` David Disseldorp
  1 sibling, 0 replies; 8+ messages in thread
From: Zorro Lang @ 2022-06-24  6:15 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: David Disseldorp, fstests

On Fri, Jun 24, 2022 at 12:42:03AM -0400, Theodore Ts'o wrote:
> On Tue, Jun 21, 2022 at 06:01:51PM +0200, David Disseldorp wrote:
> > 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.
> > 
> > There are a couple of things which I'd like to resolve before dropping
> > the RFC flag, but would appreciate early feedback on the approach here.
> 
> This is really exciting!  I was hoping to try it out, but the first
> patch doesn't apply to origin/master on xfstests-dev.

Hi Ted,

The origin/for-next has latest (testing) update. But for this issue you hit
below, you might need this patchset:

  https://lore.kernel.org/fstests/20220620192934.21694-1-ddiss@suse.de/

David sent that at first, to fix/change something before having this feature.
I've acked that, and will merge it this week. For this patchset, I'd like to
give it more time to get more reivew points.

Thanks,
Zorro

> 
> For example this patch hunk:
> 
> @@ -729,9 +750,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 "$prev_seq" "$tc_status"
>  		fi
> 
> The relevant section of check looks like this:
> 
> 		# Run report for previous test!
> 		if $err ; then
> 			bad="$bad $seqnum"
> 			n_bad=`expr $n_bad + 1`
> 			tc_status="fail"
> 		fi
> 
> And "git blame" shows that this part of check hasn't changed since
> 2018, and I'm on the latest version upstream version of xfstests:
> 
> commit 568ac9fffeb6afec03e5d6c9936617232fd7fc6d (HEAD, tag: v2022.06.05, origin/master, origin/HEAD, kernel/master)
> Author: Dave Chinner <dchinner@redhat.com>
> Date:   Fri Jun 3 11:54:13 2022 +1000
> 
>     xfs/189: systemd monitoring of /etc/fstab sucks
> 
> 
> Was your patch based xfstests with some out-of-tree patches?
> 
> > The caveats are:
> > - rerun tests will be tracked as a single failure in @try and @bad
> >   + xunit reports do not include any rerun details
> > - .bad files generated on failure will be overwritten by test reruns
> > 
> > For xunit reports, I think it'll make sense to stash the aggregates in a
> > separate <test>.agg-results file or something. Similarly for .bad file
> > overwrites, I could add a .<rerun #> suffix for capturing all failure
> > data.
> 
> For xunit results fie, was assuming that simply we would just have
> multiple repeated testcase entries stored in the single results.xml
> file.  For example:
> 
> <testcase classname="xfstests.global" name="generic/476" time="354">
> 		<failure message="Test  failed, reason unknown" type="TestFail" />
> 		<system-out>
> 		...
> 	</testcase>
> <testcase classname="xfstests.global" name="generic/476" time="343">
> 	</testcase>
> <testcase classname="xfstests.global" name="generic/476" time="353">
> 	</testcase>
> 	...
> 
> I don't know that we need a separate file for the rerun tests, since
> it's not that hard to create a python script which parses the results
> and calculates the pass/fail percentages for any test which is run
> mutiple times.
> 
> 
> As far as haivng the .bad and .full files, I agree that some kind of
> .rerun-NN suffix would make a lot of sense.
> 
> Cheers,
> 
> 						- Ted
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 0/2] add option to rerun failed tests
  2022-06-24  4:42 ` [RFC PATCH 0/2] add option " Theodore Ts'o
  2022-06-24  6:15   ` Zorro Lang
@ 2022-06-24  8:32   ` David Disseldorp
  2022-06-24 15:18     ` Theodore Ts'o
  1 sibling, 1 reply; 8+ messages in thread
From: David Disseldorp @ 2022-06-24  8:32 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: fstests

Thanks for the feedback, Ted...

On Fri, 24 Jun 2022 00:42:03 -0400, Theodore Ts'o wrote:

> On Tue, Jun 21, 2022 at 06:01:51PM +0200, David Disseldorp wrote:
> > 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.
> > 
> > There are a couple of things which I'd like to resolve before dropping
> > the RFC flag, but would appreciate early feedback on the approach here.  
> 
> This is really exciting!  I was hoping to try it out, but the first
> patch doesn't apply to origin/master on xfstests-dev.
> 
> For example this patch hunk:
> 
> @@ -729,9 +750,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 "$prev_seq" "$tc_status"
>  		fi
> 
> The relevant section of check looks like this:
> 
> 		# Run report for previous test!
> 		if $err ; then
> 			bad="$bad $seqnum"
> 			n_bad=`expr $n_bad + 1`
> 			tc_status="fail"
> 		fi
> 
> And "git blame" shows that this part of check hasn't changed since
> 2018, and I'm on the latest version upstream version of xfstests:
> 
> commit 568ac9fffeb6afec03e5d6c9936617232fd7fc6d (HEAD, tag: v2022.06.05, origin/master, origin/HEAD, kernel/master)
> Author: Dave Chinner <dchinner@redhat.com>
> Date:   Fri Jun 3 11:54:13 2022 +1000
> 
>     xfs/189: systemd monitoring of /etc/fstab sucks
> 
> 
> Was your patch based xfstests with some out-of-tree patches?

Yes, I forgot to mention that, sorry. As Zorro indicated, these were
done atop the v2022.06.12 tag with the following series applied:
https://lore.kernel.org/fstests/20220620192934.21694-1-ddiss@suse.de/

> > The caveats are:
> > - rerun tests will be tracked as a single failure in @try and @bad
> >   + xunit reports do not include any rerun details
> > - .bad files generated on failure will be overwritten by test reruns
> > 
> > For xunit reports, I think it'll make sense to stash the aggregates in a
> > separate <test>.agg-results file or something. Similarly for .bad file
> > overwrites, I could add a .<rerun #> suffix for capturing all failure
> > data.  
> 
> For xunit results fie, was assuming that simply we would just have
> multiple repeated testcase entries stored in the single results.xml
> file.  For example:
> 
> <testcase classname="xfstests.global" name="generic/476" time="354">
> 		<failure message="Test  failed, reason unknown" type="TestFail" />
> 		<system-out>
> 		...
> 	</testcase>
> <testcase classname="xfstests.global" name="generic/476" time="343">
> 	</testcase>
> <testcase classname="xfstests.global" name="generic/476" time="353">
> 	</testcase>
> 	...

That seems sensible, I'll add this functionality.

> I don't know that we need a separate file for the rerun tests, since
> it's not that hard to create a python script which parses the results
> and calculates the pass/fail percentages for any test which is run
> mutiple times.

It's just a string, so doesn't need to be in a file. I'll add an extra
parameter to _make_testcase_report so that it can be stashed somewhere
like <testcase ... status="$aggregate_stats"> on the last rerun.

> As far as haivng the .bad and .full files, I agree that some kind of
> .rerun-NN suffix would make a lot of sense.

I had a bit of a play with this and it does get a bit ugly if we want to
prefix things like .dmesg as well. The xunit rerun entries will already
capture everything, but I suppose it's still needed for those not using
xunit reports.

Cheers, David

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 0/2] add option to rerun failed tests
  2022-06-24  8:32   ` David Disseldorp
@ 2022-06-24 15:18     ` Theodore Ts'o
  2022-06-27 22:02       ` David Disseldorp
  0 siblings, 1 reply; 8+ messages in thread
From: Theodore Ts'o @ 2022-06-24 15:18 UTC (permalink / raw)
  To: David Disseldorp; +Cc: fstests

On Fri, Jun 24, 2022 at 10:32:43AM +0200, David Disseldorp wrote:
> Yes, I forgot to mention that, sorry. As Zorro indicated, these were
> done atop the v2022.06.12 tag with the following series applied:
> https://lore.kernel.org/fstests/20220620192934.21694-1-ddiss@suse.de/

Got it, thanks.  Sorry, I had forgotten that we had the next branch now.

I'll try to do a full review once I'm able to give the patches a spin.

> > <testcase classname="xfstests.global" name="generic/476" time="354">
> > 		<failure message="Test  failed, reason unknown" type="TestFail" />
> > 		<system-out>
> > 		...
> > 	</testcase>
> > <testcase classname="xfstests.global" name="generic/476" time="343">
> > 	</testcase>
> > <testcase classname="xfstests.global" name="generic/476" time="353">
> > 	</testcase>
> > 	...
> 
> That seems sensible, I'll add this functionality.

I *think* that should happen automatically when _make_testcase_report
gets called after each iteration.  So that might be easier than having
to do any kind of special case handling.  (Which is why that was going
to be how I was planning on tackling it before you went ahead and
implemented --- thanks for that!!)

> > As far as haivng the .bad and .full files, I agree that some kind of
> > .rerun-NN suffix would make a lot of sense.
> 
> I had a bit of a play with this and it does get a bit ugly if we want to
> prefix things like .dmesg as well. The xunit rerun entries will already
> capture everything, but I suppose it's still needed for those not using
> xunit reports.

Well, actually, one of the things on my TODO list was to implement a
new report type which would removed the xunit <system-out> fields from
the xunit file.  The reason behind that is sometimes the the
NNN.out.bad files can get huge --- and the Python library for parsing
junit XML files has a safety mechanism which will error out if a field
is larger than 10MB, to prevent some denial of service attacks.  And
I've had some XFS NNN.out.bad files get to be 30MB or larger!

When that happens, it causes the Python script I use to parse the XML
file to fail.  In addition, since I already have a different mechanism
for saving the full set of test artifiacts ---- sometimes having the
NNN.full file is really useful for root causing the failure --- having
two copies of the out.bad files in both the Xunit file and in my test
artifacts tarball is a bit of a waste.

I had a POC which implemented this, but then Darrick had a feature
request, since for his workflow, it would be useful if saved only the
first N lines and last N lines in the xunit file, since that's
typically sufficient to figure out what's going on.  And I haven't had
a chance to get back to it.

						- Ted

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 0/2] add option to rerun failed tests
  2022-06-24 15:18     ` Theodore Ts'o
@ 2022-06-27 22:02       ` David Disseldorp
  0 siblings, 0 replies; 8+ messages in thread
From: David Disseldorp @ 2022-06-27 22:02 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: fstests

On Fri, 24 Jun 2022 11:18:58 -0400, Theodore Ts'o wrote:

> On Fri, Jun 24, 2022 at 10:32:43AM +0200, David Disseldorp wrote:
> > Yes, I forgot to mention that, sorry. As Zorro indicated, these were
> > done atop the v2022.06.12 tag with the following series applied:
> > https://lore.kernel.org/fstests/20220620192934.21694-1-ddiss@suse.de/  
> 
> Got it, thanks.  Sorry, I had forgotten that we had the next branch now.
> 
> I'll try to do a full review once I'm able to give the patches a spin.
> 
> > > <testcase classname="xfstests.global" name="generic/476" time="354">
> > > 		<failure message="Test  failed, reason unknown" type="TestFail" />
> > > 		<system-out>
> > > 		...
> > > 	</testcase>
> > > <testcase classname="xfstests.global" name="generic/476" time="343">
> > > 	</testcase>
> > > <testcase classname="xfstests.global" name="generic/476" time="353">
> > > 	</testcase>
> > > 	...  
> > 
> > That seems sensible, I'll add this functionality.  
> 
> I *think* that should happen automatically when _make_testcase_report
> gets called after each iteration.  So that might be easier than having
> to do any kind of special case handling.  (Which is why that was going
> to be how I was planning on tackling it before you went ahead and
> implemented --- thanks for that!!)

It does, but I've messed around with a few things in that code path, so
just need to make sure that this works as expected :). It should be
working this way in the v2 patchset that I'm about to send...

> > > As far as haivng the .bad and .full files, I agree that some kind of
> > > .rerun-NN suffix would make a lot of sense.  
> > 
> > I had a bit of a play with this and it does get a bit ugly if we want to
> > prefix things like .dmesg as well. The xunit rerun entries will already
> > capture everything, but I suppose it's still needed for those not using
> > xunit reports.  
> 
> Well, actually, one of the things on my TODO list was to implement a
> new report type which would removed the xunit <system-out> fields from
> the xunit file.  The reason behind that is sometimes the the
> NNN.out.bad files can get huge --- and the Python library for parsing
> junit XML files has a safety mechanism which will error out if a field
> is larger than 10MB, to prevent some denial of service attacks.  And
> I've had some XFS NNN.out.bad files get to be 30MB or larger!

Ouch, that does sound hard to parse. One thing I also noticed is that a
stray "]]>" CDATA terminator in the any of the captured content will
likely also cause some parsing headaches, so should be filtered.

> When that happens, it causes the Python script I use to parse the XML
> file to fail.  In addition, since I already have a different mechanism
> for saving the full set of test artifiacts ---- sometimes having the
> NNN.full file is really useful for root causing the failure --- having
> two copies of the out.bad files in both the Xunit file and in my test
> artifacts tarball is a bit of a waste.
> 
> I had a POC which implemented this, but then Darrick had a feature
> request, since for his workflow, it would be useful if saved only the
> first N lines and last N lines in the xunit file, since that's
> typically sufficient to figure out what's going on.  And I haven't had
> a chance to get back to it.

Given that the extra debugging details are already there in the current
xunit output, I think we may be stuck with them. That said, it should be
pretty straightforward to add a new "xunit-brief" or similar report type
under the (currently single-purpose) report API.

Cheers, David

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-06-27 22:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-21 16:01 [RFC PATCH 0/2] add option to rerun failed tests David Disseldorp
2022-06-21 16:01 ` [RFC PATCH 1/2] check: append bad / notrun arrays in helper function David Disseldorp
2022-06-21 16:01 ` [RFC PATCH 2/2] check: add -L <n> parameter to rerun failed tests David Disseldorp
2022-06-24  4:42 ` [RFC PATCH 0/2] add option " Theodore Ts'o
2022-06-24  6:15   ` Zorro Lang
2022-06-24  8:32   ` David Disseldorp
2022-06-24 15:18     ` Theodore Ts'o
2022-06-27 22:02       ` 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.