Re: [RFC PATCH v2 4/6] check: append bad / notrun arrays in helper function

["Darrick J. Wong" <djwong@kernel.org>]

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
>