Re: [PATCH v3 fstests] check: add support for --start-after

From: David Disseldorp <ddiss@suse.de>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: fstests@vger.kernel.org, anand.jain@oracle.com,
	aalbersh@redhat.com, linux-fsdevel@vger.kernel.org,
	kdevops@lists.linux.dev, patches@lists.linux.dev
Subject: Re: [PATCH v3 fstests] check: add support for --start-after
Date: Tue, 20 Feb 2024 00:20:15 +1100	[thread overview]
Message-ID: <20240220002015.52ba31bc@echidna> (raw)
In-Reply-To: <20240216180946.784869-1-mcgrof@kernel.org>

On Fri, 16 Feb 2024 10:09:46 -0800, Luis Chamberlain wrote:

...
>  check | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/check b/check
> index 71b9fbd07522..f081bf8ce685 100755
> --- a/check
> +++ b/check
> @@ -18,6 +18,7 @@ showme=false
>  have_test_arg=false
>  randomize=false
>  exact_order=false
> +start_after_test=""
>  export here=`pwd`
>  xfile=""
>  subdir_xfile=""
> @@ -80,6 +81,7 @@ check options
>      -b			brief test summary
>      -R fmt[,fmt]	generate report in formats specified. Supported formats: xunit, xunit-quiet
>      --large-fs		optimise scratch device for large filesystems
> +    --start-after	only start testing after the test specified

The usage text should include the <test> parameter.

>      -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
> @@ -120,6 +122,8 @@ examples:
>   check -x stress xfs/*
>   check -X .exclude -g auto
>   check -E ~/.xfstests.exclude
> + check --start-after btrfs/010
> + check -n -g soak --start-after generic/522
>  '
>  	    exit 1
>  }
> @@ -204,6 +208,23 @@ trim_test_list()
>  	rm -f $tmp.grep
>  }
>  
> +# takes the list of tests to run in $tmp.list and skips all tests until
> +# the specified test is found. This will ensure the tests start after the
> +# test specified, it skips the test specified.
> +trim_start_after()
> +{
> +	local skip_test="$1"
> +	local starts_regexp=$(echo $skip_test | sed -e 's|\/|\\/|')
> +
> +	if grep -q $skip_test $tmp.list ; then
> +		rm -f $tmp.grep
> +		awk 'f;/.*'$starts_regexp'/{f=1}' $tmp.list > $tmp.tmp
> +		mv $tmp.tmp $tmp.list
> +	else
> +		echo "Test $skip_test not found in test list, be sure to use a valid test if using --start-after"
> +		exit 1
> +	fi
> +}

nit: it should be possible to collapse the sed+grep+awk into something a
bit more straightforward, e.g.

trim_start_after()
{
        if awk -v skip_test="tests/$1" \
           'f; $0 == skip_test {f=1} END {exit f}' $tmp.list > $tmp.tmp; then
                echo "$1 not found in test list, be sure to use a valid test if using --start-after"
                rm $tmp.tmp
                exit 1
        fi
        mv $tmp.tmp $tmp.list
}

>  
>  _wallclock()
>  {
> @@ -233,6 +254,9 @@ _prepare_test_list()
>  		# no test numbers, do everything
>  		get_all_tests
>  	else
> +		local group_all
> +		local start_after_found=0
> +		list=""
>  		for group in $GROUP_LIST; do
>  			list=$(get_group_list $group)
>  			if [ -z "$list" ]; then
> @@ -240,11 +264,28 @@ _prepare_test_list()
>  				exit 1
>  			fi
>  
> +			if [[ "$start_after_test" != "" && $start_after_found -ne 1 ]]; then
> +				echo $list | grep -q $start_after_test
> +				if [[ $? -eq 0 ]]; then
> +					start_after_found=1
> +				fi
> +			fi
>  			for t in $list; do
>  				grep -s "^$t\$" $tmp.list >/dev/null || \
>  							echo "$t" >>$tmp.list
>  			done
> +			group_all="$group_all $list"
>  		done
> +		if [[ "$start_after_test" != "" && $start_after_found -ne 1 ]]; then
> +			group_all=$(echo $group_all | sed -e 's|tests/||g')
> +			echo "Start after test $start_after_test not found in any group specified."
> +			echo "Be sure you specify a test present in one of your test run groups if using --start-after."
> +			echo
> +			echo "Your set of groups have these tests:"
> +			echo
> +			echo $group_all
> +			exit 1
> +		fi
>  	fi

Can't all of the _prepare_test_list() changes above be dropped?
trim_start_after() already performs the check that the test exists, and
with the above dropped we could also do things like, e.g.
  ./check --start-after generic/002 generic/00[1-9]

>  	# Specified groups to exclude
> @@ -258,6 +299,10 @@ _prepare_test_list()
>  		trim_test_list $list
>  	done
>  
> +	if [[ "$start_after_test" != "" ]]; then
> +		trim_start_after $start_after_test
> +	fi
> +
>  	# sort the list of tests into numeric order unless we're running tests
>  	# in the exact order specified
>  	if ! $exact_order; then
> @@ -313,6 +358,14 @@ while [ $# -gt 0 ]; do
>  				<(sed "s/#.*$//" $xfile)
>  		fi
>  		;;
> +	--start-after)
> +		if $randomize; then
> +			echo "Cannot specify -r and --start-after."
> +			exit 1
> +		fi

IIUC, this will only trigger if -r is parsed before --start-after, so it
should be moved after the loop.

> +		start_after_test="$2"
> +		shift
> +		;;
>  	-s)	RUN_SECTION="$RUN_SECTION $2"; shift ;;
>  	-S)	EXCLUDE_SECTION="$EXCLUDE_SECTION $2"; shift ;;
>  	-l)	diff="diff" ;;