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

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

On Mon, Jul 03, 2023 at 10:51:20PM +0800, Zorro Lang wrote:
> 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[@]})

readarray -t -O "${#exclude_tests[@]}" exclude_tests < <(echo "$xfile" | tr ',' ' '))

> >  		;;
> >  
> >  	-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.

Four digit ids work fine; I've had xfs/12xx and xfs/15xx in djwong-dev
for years now with no problems.

> If so "grep generic/100" will match generic/1000 and
> generic/100 both.

Yeah, that grep string ought to be grep -q "^$TEST_ID$" or something.

Or

is_excluded() {
	for f in "${exclude_tests[@]}"; do
		[[ "$f" == "$1" ]] && return 0
	done
	return 1
}

(Though saving the overhead of one fork/exec is probably minimal.)

	if is_excluded "$TEST_ID"; then ...

--D

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