* Re: [PATCH] check: fix excluded tests are only expunged in the first iteration
2023-06-30 12:28 [PATCH] check: fix excluded tests are only expunged in the first iteration yuezhang.mo
@ 2023-06-30 13:33 ` Bill O'Donnell
2023-06-30 15:41 ` Darrick J. Wong
2023-07-03 14:51 ` Zorro Lang
2 siblings, 0 replies; 6+ messages in thread
From: Bill O'Donnell @ 2023-06-30 13:33 UTC (permalink / raw)
To: yuezhang.mo; +Cc: fstests
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.
>
> 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>
Makes sense.
Reviewed-by: Bill O'Donnell <bodonnel@redhat.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'))
> ;;
>
> -E) xfile=$2; shift ;
> if [ -f $xfile ]; then
> - sed "s/#.*$//" "$xfile" >> $tmp.xlist
> - fi
> + exclude_tests+=($(sed "s/#.*$//" "$xfile"))
> + fi
> ;;
> -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
> + echo " [expunged]"
> + return 1
> fi
> return 0
> }
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] check: fix excluded tests are only expunged in the first iteration
2023-06-30 12:28 [PATCH] check: fix excluded tests are only expunged in the first iteration yuezhang.mo
2023-06-30 13:33 ` Bill O'Donnell
@ 2023-06-30 15:41 ` Darrick J. Wong
2023-07-03 14:51 ` Zorro Lang
2 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2023-06-30 15:41 UTC (permalink / raw)
To: yuezhang.mo; +Cc: fstests
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.
I don't get it. ./check only deletes $tmp.xlist at startup time, not
between iterations. Is there somewhere that ./check exec's itself when
you run ./check -i 5 ?
> 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'))
Please use readarray/mapfile for storing to a bash array, please.
--D
> ;;
>
> -E) xfile=$2; shift ;
> if [ -f $xfile ]; then
> - sed "s/#.*$//" "$xfile" >> $tmp.xlist
> - fi
> + exclude_tests+=($(sed "s/#.*$//" "$xfile"))
> + fi
> ;;
> -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
> + echo " [expunged]"
> + return 1
> fi
> return 0
> }
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] check: fix excluded tests are only expunged in the first iteration
2023-06-30 12:28 [PATCH] check: fix excluded tests are only expunged in the first iteration yuezhang.mo
2023-06-30 13:33 ` Bill O'Donnell
2023-06-30 15:41 ` Darrick J. Wong
@ 2023-07-03 14:51 ` Zorro Lang
2023-07-03 15:44 ` Darrick J. Wong
2023-07-04 0:24 ` Yuezhang Mo
2 siblings, 2 replies; 6+ messages in thread
From: Zorro Lang @ 2023-07-03 14:51 UTC (permalink / raw)
To: yuezhang.mo; +Cc: fstests
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] check: fix excluded tests are only expunged in the first iteration
2023-07-03 14:51 ` Zorro Lang
@ 2023-07-03 15:44 ` Darrick J. Wong
2023-07-04 0:24 ` Yuezhang Mo
1 sibling, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2023-07-03 15:44 UTC (permalink / raw)
To: Zorro Lang; +Cc: yuezhang.mo, fstests
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
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] check: fix excluded tests are only expunged in the first iteration
2023-07-03 14:51 ` Zorro Lang
2023-07-03 15:44 ` Darrick J. Wong
@ 2023-07-04 0:24 ` Yuezhang Mo
1 sibling, 0 replies; 6+ messages in thread
From: Yuezhang Mo @ 2023-07-04 0:24 UTC (permalink / raw)
To: Zorro Lang; +Cc: fstests
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.
If run `./check -i 5 -e generic/013`.
- generic/013 is expunged in the iteration 1.
generic/013 [expunged]
- But generic/013 is not expunged in the iteration 2/3/4/5.
generic/013 76s ... 78s
>> -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[@]})
>> + 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.
>> + 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.
OK, I will update the patch.
^ permalink raw reply [flat|nested] 6+ messages in thread