Re: [PATCH] check: fix excluded tests are only expunged in the first iteration

[Zorro Lang <zlang@redhat.com>]

On Fri, Jun 30, 2023 at 08:28:29PM +0800, yuezhang.mo@foxmail.com wrote:
> From: Yuezhang Mo <yuezhang.mo@foxmail.com>
> 
> If iterating more than once and excluding some tests, the
> excluded tests are expunged in the first iteration, but run in
> subsequent iterations. This is not expected.

Can you provide an example about how to reproduce the issue you metioned above.

> 
> The problem was caused by the temporary file saving the excluded
> tests being deleted at the end of the first iteration.
> 
> This commit saves the excluded tests into a variable instead of a
> temp file.
> 
> Signed-off-by: Yuezhang Mo <yuezhang.mo@foxmail.com>
> ---
>  check | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/check b/check
> index e36978c1..a8071121 100755
> --- a/check
> +++ b/check
> @@ -27,6 +27,7 @@ DUMP_OUTPUT=false
>  iterations=1
>  istop=false
>  loop_on_fail=0
> +exclude_tests=()
>  
>  # This is a global variable used to pass test failure text to reporting gunk
>  _err_msg=""
> @@ -46,7 +47,7 @@ export DIFF_LENGTH=${DIFF_LENGTH:=10}
>  # by default don't output timestamps
>  timestamp=${TIMESTAMP:=false}
>  
> -rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.xlist $tmp.report.* $tmp.arglist
> +rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
>  
>  SRC_GROUPS="generic shared"
>  export SRC_DIR="tests"
> @@ -302,13 +303,13 @@ while [ $# -gt 0 ]; do
>  		;;
>  	-e)
>  		xfile=$2; shift ;
> -		echo "$xfile" | tr ', ' '\n\n' >> $tmp.xlist
> +		exclude_tests+=($(echo "$xfile" | tr ', ' '\n\n'))

If print to a file, I don't mind above way. But if to an array, how about:

  read -a tmpxlist < <(echo "$xfile" | tr ',' ' ')
  exclude_tests+=(${tmpxlist[@]})

>  		;;
>  
>  	-E)	xfile=$2; shift ;
>  		if [ -f $xfile ]; then
> -			sed "s/#.*$//" "$xfile" >> $tmp.xlist
> -	        fi
> +			exclude_tests+=($(sed "s/#.*$//" "$xfile"))
> +		fi

Similar as above, how about:

  readarray tmpxlist < <(sed "s/#.*$//" $xfile)
  exclude_tests+=(${tmpxlist[@]})

Feel free to provide better methods if anyone has.

>  		;;
>  	-s)	RUN_SECTION="$RUN_SECTION $2"; shift ;;
>  	-S)	EXCLUDE_SECTION="$EXCLUDE_SECTION $2"; shift ;;
> @@ -383,7 +384,7 @@ if [ -n "$subdir_xfile" ]; then
>  	for d in $SRC_GROUPS $FSTYP; do
>  		[ -f $SRC_DIR/$d/$subdir_xfile ] || continue
>  		for f in `sed "s/#.*$//" $SRC_DIR/$d/$subdir_xfile`; do
> -			echo $d/$f >> $tmp.xlist
> +			exclude_tests+=($d/$f)
>  		done
>  	done
>  fi
> @@ -570,11 +571,10 @@ _check_filesystems()
>  _expunge_test()
>  {
>  	local TEST_ID="$1"
> -	if [ -s $tmp.xlist ]; then
> -		if grep -q $TEST_ID $tmp.xlist; then
> -			echo "       [expunged]"
> -			return 1
> -		fi
> +
> +	if echo ${exclude_tests[*]} | grep -q $TEST_ID; then

How about using the "-w" option to grep. Due to the number of fstests' cases
will out of 999 (I'm thinking about how to deal with it), we might have
generic/1000 in one day. If so "grep generic/100" will match generic/1000 and
generic/100 both.

Thanks,
Zorro

> +		echo "       [expunged]"
> +		return 1
>  	fi
>  	return 0
>  }
> -- 
> 2.25.1
>