diff mbox series

[17/28] check-parallel: rebuild test list when FSTYP changes

Message ID 20250417031208.1852171-18-david@fromorbit.com (mailing list archive)
State New
Headers show
Series check-parallel: Running tests without check | expand

Commit Message

Dave Chinner April 17, 2025, 3 a.m. UTC 
From: Dave Chinner <dchinner@redhat.com>

When a config section changes FSTYP, we do not rebuild the test list
that is to be run. This means that if we go from xfs to ext4, the
ext4 section will still try to run all the XFS tests, and won't try
to run the ext4 tests.

To fix this for check-parallel, check if the FSTYP has changed and
if it has strip the old tests from the test list and add the new
tests back to it.

This is effectively a zero-day bug in the check config section
support code.

While there, remain the internal _tl_file variable and the filename
used to hold command line tests. The check-parallel change uncovered
that it was using the same filename ($tmp.test_list) as the test
list code was using and they were stepping on each other.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 check-parallel   | 81 +++++++++++++++++++++++++++++-------------------
 common/test_list |  8 ++---
 2 files changed, 53 insertions(+), 36 deletions(-)
diff mbox series

Patch

diff --git a/check-parallel b/check-parallel
index 23d29c7a8..374ac8e96 100755
--- a/check-parallel
+++ b/check-parallel
@@ -175,7 +175,6 @@  time_order_test_list()
 	local rx=0
 	local ix
 	local jx
-	#set -x
 	for ((ix = 0; ix < ${#_list[*]}; ix++)); do
 		echo $_tl_tests | grep -q ${_list[$ix]}
 		if [ $? == 0 ]; then
@@ -198,6 +197,26 @@  if ! $_tl_randomise -a ! $_tl_exact_order; then
 	fi
 fi
 
+# Build the test list from the test sepcification and FSTYP settings. This can
+# be called at any time to regenerate the list as the include/exclude lists
+# generated from the command line are retained separately to the test list
+# itself.
+setup_test_list()
+{
+	_tl_prepare_test_list
+	_tl_strip_test_list
+
+	if ! $_tl_randomise -a ! $_tl_exact_order; then
+		if [ -f $basedir/runner-0/$prev_results/check.time ]; then
+			time_order_test_list
+		fi
+	fi
+
+	# reverse the order of tests so that the get_next_test() can pull from the file
+	# tail rather than the head.
+	echo $_tl_tests |sed -e 's/ /\n/g' | tac > $test_list
+}
+
 # Grab the next test to be run from the tail of the file.
 # Returns an empty string if there is no tests remaining to run.
 # File operations are run under flock so concurrent gets are serialised against
@@ -242,6 +261,7 @@  _destroy_loop_device()
 run_tests()
 {
 	local section="$1"
+	local logfile="$2"
 
 	exec 99<>$tmp.test_list_lock
 
@@ -257,7 +277,7 @@  run_tests()
 		echo -n " $test_to_run "
 		unset FSTESTS_ISOL
 		if ! _tl_expunge_test $test_to_run; then
-			tools/run_privatens ./check -s $section $test_to_run >> $me/log 2>&1
+			tools/run_privatens ./check -s $section $test_to_run >> $logfile 2>&1
 		fi
 
 		test_to_run=$(get_next_test $section)
@@ -308,12 +328,12 @@  runner_go()
 	rm -f $RESULT_BASE/check.*
 
 	# Only supports default mkfs parameters right now
-	wipefs -a $TEST_DEV > $me/log 2>&1
-	yes | mkfs -t $FSTYP $TEST_MKFS_OPTS $TEST_DEV >> $me/log 2>&1
+	wipefs -a $TEST_DEV >> $_results/log 2>&1
+	yes | mkfs -t $FSTYP $TEST_MKFS_OPTS $TEST_DEV >> $_results/log 2>&1
 
 #	export DUMP_CORRUPT_FS=1
 
-	run_tests $section
+	run_tests $section $_results/log
 
 	wait
 	sleep 1
@@ -327,10 +347,10 @@  runner_go()
 	_destroy_loop_device $SCRATCH_LOGDEV
 	_destroy_loop_device $LOGWRITES_DEV
 
-	grep -q Failures: $me/log
+	grep -q Failures: $_results/log
 	if [ $? -eq 0 ]; then
 		echo -n "Runner $id Failures: "
-		grep Failures: $me/log | uniq | sed -e "s/^.*Failures://"
+		grep Failures: $_results/log | uniq | sed -e "s/^.*Failures://"
 	fi
 
 }
@@ -339,19 +359,33 @@  run_section()
 {
 	local section="$1"
 	local now="$2"
+	local results="$basedir/*/results-$now"
 	local i
 
 	echo $run_section |grep -qw $section || return
 	echo $exclude_section |grep -qw $section && return
 
 	echo
-	echo Running section: $section
+	echo Running section: $section $now
 	echo
 
 	parse_config_section $section
 
-	# set up consumable test list first
+	# update the test list if necessary, then set up
+	# the consumable test list for this section to use.
+	if [ "$last_fstyp" != "$FSTYP" ]; then
+		setup_test_list
+		last_fstyp=$FSTYP
+	fi
+	echo FS Type: $FSTYP
+
+	if [ -n "$show_test_list" ]; then
+		echo Test list to run:
+		cat $test_list
+		return
+	fi
 	cp $test_list $test_list.$section
+
 	for ((i = 0; i < $runners; i++)); do
 		runner_go $i $now $section &
 	done
@@ -360,17 +394,18 @@  run_section()
 	echo
 	echo Section: $section
 	echo -n "Tests run: "
-	grep Ran $basedir/*/log | sed -e 's,^.*:,,' -e 's, ,\n,g' | sort | uniq | wc -l
+	grep Ran $results/log | sed -e 's,^.*:,,' -e 's, ,\n,g' | sort | uniq | wc -l
 
 	echo -n "Tests _notrun: "
-	grep "^Not run" $basedir/*/log | uniq | sed -e 's,^.*:,,' -e 's, ,\n,g' -e 's,^\n,,' | wc -l
+	grep "^Not run" $results/log | uniq | sed -e 's,^.*:,,' -e 's, ,\n,g' -e 's,^\n,,' | wc -l
 
 	echo -n "Failure count: "
-	grep Failures: $basedir/*/log | uniq | sed -e "s/^.*Failures://" -e "s,\([0-9]\) \([gx]\),\1\n \2,g" |wc -l
+	grep Failures: $results/log | uniq | sed -e "s/^.*Failures://" -e "s,\([0-9]\) \([gx]\),\1\n \2,g" |wc -l
 	echo
 
 	echo Ten slowest tests - runtime in seconds:
-	cat $basedir/*/results-$now/check.time | sort -k 2 -nr | head -10
+	cat $results/check.time | sort -k 2 -nr | head -10
+
 }
 
 cleanup()
@@ -386,27 +421,8 @@  cleanup()
 trap "cleanup; exit" HUP INT QUIT TERM
 
 _config_setup_parallel
-
 run_section=${run_section:="$HOST_OPTIONS_SECTIONS"}
-
 _tl_setup_exclude_group "unreliable_in_parallel"
-_tl_prepare_test_list
-_tl_strip_test_list
-
-if ! $_tl_randomise -a ! $_tl_exact_order; then
-	if [ -f $basedir/runner-0/$prev_results/check.time ]; then
-		time_order_test_list
-	fi
-fi
-
-# reverse the order of tests so that the get_next_test() can pull from the file
-# tail rather than the head.
-echo $_tl_tests |sed -e 's/ /\n/g' | tac > $test_list
-if [ -n "$show_test_list" ]; then
-	echo Time ordered test list:
-	cat $test_list
-	exit 0
-fi
 
 # Each parallel test runner needs to only see it's own mount points. If we
 # leave the basedir as shared, then all tests see all mounts and then we get
@@ -422,6 +438,7 @@  fi
 mount --make-private $basedir
 
 now=`date +%Y-%m-%d-%H:%M:%S`
+last_fstyp=
 for section in $HOST_OPTIONS_SECTIONS; do
 	run_section $section $now
 	if [ "$sum_bad" != 0 ] && [ "$istop" = true ]; then
diff --git a/common/test_list b/common/test_list
index 2b3ae9fbf..092b3ed17 100644
--- a/common/test_list
+++ b/common/test_list
@@ -20,7 +20,7 @@  _XGROUP_LIST=
 _tl_exact_order=false
 _tl_randomise=false
 _tl_have_test_args=false
-_tl_file="$tmp.test_list"
+_tl_cli_tests="$tmp._tl_cli_tests"
 _tl_exclude_tests=()
 _tl_tests=
 
@@ -122,8 +122,8 @@  _tl_prepare_test_list()
 {
 	unset _tl_tests
 	# Tests specified on the command line
-	if [ -s $_tl_file ]; then
-		cat $_tl_file > $tmp.list
+	if [ -s $_tl_cli_tests ]; then
+		cat $_tl_cli_tests > $tmp.list
 	else
 		touch $tmp.list
 	fi
@@ -281,7 +281,7 @@  _tl_setup_cli()
 				if grep -Eq "^$test_name" $group_file; then
 					# in group file ... OK
 					echo $_tl_src_dir/$test_dir/$test_name \
-						>> $_tl_file
+						>> $_tl_cli_tests
 					_tl_have_test_args=true
 				else
 					# oops