[PATCH] Tests can use any name now, not 3 digits only.

[PATCH] Tests can use any name now, not 3 digits only.

[Jan Ťulák]

Tests can use any name now, not 3 digits only.
(e.g. a test can be named "tests/generic/some-name")

Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---
 README | 2 +-
 check  | 6 +++---
 new    | 7 +++++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/README b/README
index 0c9449a..2376674 100644
--- a/README
+++ b/README
@@ -205,7 +205,7 @@ Test script environment:
 
 Verified output:
 
-    Each test script has a numerical name, e.g. 007, and an associated
+    Each test script has a name, e.g. 007, and an associated
     verified output, e.g. 007.out.
 
     It is important that the verified output is deterministic, and
diff --git a/check b/check
index 0830e0c..d7814a8 100755
--- a/check
+++ b/check
@@ -58,7 +58,7 @@ then
     exit 1
 fi
 
-SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
+SUPPORTED_TESTS="\S\+"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
 
@@ -96,7 +96,7 @@ get_group_list()
 		l=$(sed -n < $SRC_DIR/$d/group \
 			-e 's/#.*//' \
 			-e 's/$/ /' \
-			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
+			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
 		grpl="$grpl $l"
 	done
 	echo $grpl
@@ -111,6 +111,7 @@ get_all_tests()
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
+			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
 			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
@@ -178,7 +179,6 @@ _prepare_test_list()
 		# no test numbers, do everything
 		get_all_tests
 	fi
-
 	# Specified groups to exclude
 	for xgroup in $XGROUP_LIST; do
 		list=$(get_group_list $xgroup)
diff --git a/new b/new
index 86f9075..f755da3 100755
--- a/new
+++ b/new
@@ -84,8 +84,11 @@ eof=1
 for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
 do
     line=$((line+1))
-    if [ -z "$found" ] || [ "$found" == "#" ];then
-	continue
+    if [ -z "$found" ] || [ "$found" == "#" ] ;then
+        continue
+    elif ! echo "$found"|grep "[0-9][0-9][0-9]";then
+        # this one is for tests not named by a number
+        continue
     fi
     i=$((i+1))
     id=`printf "%03d" $i`
-- 
2.1.0

Re: [PATCH] Tests can use any name now, not 3 digits only.

[Jan Tulak]

I got no reply on this patch, so bounce in case no one noticed it. :-)

Cheers,
Jan

----- Original Message -----
> From: "Jan Ťulák" <jtulak@redhat.com>
> To: fstests@vger.kernel.org
> Cc: "Jan Ťulák" <jtulak@redhat.com>
> Sent: Wednesday, 4 March, 2015 4:55:17 PM
> Subject: [PATCH] Tests can use any name now, not 3 digits only.
> 
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/some-name")
> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> ---
>  README | 2 +-
>  check  | 6 +++---
>  new    | 7 +++++--
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/README b/README
> index 0c9449a..2376674 100644
> --- a/README
> +++ b/README
> @@ -205,7 +205,7 @@ Test script environment:
>  
>  Verified output:
>  
> -    Each test script has a numerical name, e.g. 007, and an associated
> +    Each test script has a name, e.g. 007, and an associated
>      verified output, e.g. 007.out.
>  
>      It is important that the verified output is deterministic, and
> diff --git a/check b/check
> index 0830e0c..d7814a8 100755
> --- a/check
> +++ b/check
> @@ -58,7 +58,7 @@ then
>      exit 1
>  fi
>  
> -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> +SUPPORTED_TESTS="\S\+"
>  SRC_GROUPS="generic shared"
>  export SRC_DIR="tests"
>  
> @@ -96,7 +96,7 @@ get_group_list()
>  		l=$(sed -n < $SRC_DIR/$d/group \
>  			-e 's/#.*//' \
>  			-e 's/$/ /' \
> -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
>  		grpl="$grpl $l"
>  	done
>  	echo $grpl
> @@ -111,6 +111,7 @@ get_all_tests()
>  	for d in $SRC_GROUPS $FSTYP; do
>  		ls $SRC_DIR/$d/* | \
>  			grep -v "\..*" | \
> +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
>  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
>  	done
>  }
> @@ -178,7 +179,6 @@ _prepare_test_list()
>  		# no test numbers, do everything
>  		get_all_tests
>  	fi
> -
>  	# Specified groups to exclude
>  	for xgroup in $XGROUP_LIST; do
>  		list=$(get_group_list $xgroup)
> diff --git a/new b/new
> index 86f9075..f755da3 100755
> --- a/new
> +++ b/new
> @@ -84,8 +84,11 @@ eof=1
>  for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
>  do
>      line=$((line+1))
> -    if [ -z "$found" ] || [ "$found" == "#" ];then
> -	continue
> +    if [ -z "$found" ] || [ "$found" == "#" ] ;then
> +        continue
> +    elif ! echo "$found"|grep "[0-9][0-9][0-9]";then
> +        # this one is for tests not named by a number
> +        continue
>      fi
>      i=$((i+1))
>      id=`printf "%03d" $i`
> --
> 2.1.0
> 
> 

Re: [PATCH] Tests can use any name now, not 3 digits only.

[Eryu Guan]

On Wed, Mar 04, 2015 at 04:55:17PM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/some-name")

I think you can add "fstests: " prefix to summary

fstests: tests can use any name not 3 digits only

And it looks good to me overall. I tested with the name "some-test"
"some-test-001" "some_test" "some-test-001.test", all worked fine. Also
tested with ./check -g testgroup (added testgroup to some random tests),
it worked too.

Some comments inline.

> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> ---
>  README | 2 +-
>  check  | 6 +++---
>  new    | 7 +++++--
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/README b/README
> index 0c9449a..2376674 100644
> --- a/README
> +++ b/README
> @@ -205,7 +205,7 @@ Test script environment:
>  
>  Verified output:
>  
> -    Each test script has a numerical name, e.g. 007, and an associated
> +    Each test script has a name, e.g. 007, and an associated
>      verified output, e.g. 007.out.
>  
>      It is important that the verified output is deterministic, and
> diff --git a/check b/check
> index 0830e0c..d7814a8 100755
> --- a/check
> +++ b/check
> @@ -58,7 +58,7 @@ then
>      exit 1
>  fi
>  
> -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> +SUPPORTED_TESTS="\S\+"
>  SRC_GROUPS="generic shared"
>  export SRC_DIR="tests"
>  
> @@ -96,7 +96,7 @@ get_group_list()
>  		l=$(sed -n < $SRC_DIR/$d/group \
>  			-e 's/#.*//' \
>  			-e 's/$/ /' \
> -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
>  		grpl="$grpl $l"
>  	done
>  	echo $grpl
> @@ -111,6 +111,7 @@ get_all_tests()

The comments before get_all_tests() should be updated too, which says

"
# This assumes that tests are defined purely by alphanumeric filenames with no
# ".xyz" extensions in the name.
"

>  	for d in $SRC_GROUPS $FSTYP; do
>  		ls $SRC_DIR/$d/* | \
>  			grep -v "\..*" | \
> +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
>  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
>  	done
>  }
> @@ -178,7 +179,6 @@ _prepare_test_list()
>  		# no test numbers, do everything
>  		get_all_tests
>  	fi
> -

Leave this line not removed.

>  	# Specified groups to exclude
>  	for xgroup in $XGROUP_LIST; do
>  		list=$(get_group_list $xgroup)
> diff --git a/new b/new
> index 86f9075..f755da3 100755
> --- a/new
> +++ b/new
> @@ -84,8 +84,11 @@ eof=1
>  for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
>  do
>      line=$((line+1))
> -    if [ -z "$found" ] || [ "$found" == "#" ];then
> -	continue
> +    if [ -z "$found" ] || [ "$found" == "#" ] ;then
> +        continue
> +    elif ! echo "$found"|grep "[0-9][0-9][0-9]";then

I'm being picky here :)

put space around pipe, echo "$found" | grep

and add space between ";" and "then", no space before ";"

if [ condition ]; then
fi

> +        # this one is for tests not named by a number
> +        continue

If I have a test name like "some-test-001" which has three digits too,
"new" script is not working well

[root@hp-dl388eg8-01 xfstests]# ./new generic
Building include
Building lib
Building ltp
Building src
Building aio-dio-regress
Building m4
Building common
Building tests
001
002
003
some-test-001
Next test is 004
Error: test 004 already exists!

And do we need a prompt in "new" to give user a change to create user
defined test name? It's still defaulting to three digits.

Thanks,
Eryu
>      fi
>      i=$((i+1))
>      id=`printf "%03d" $i`
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fstests: tests can use any name now, not 3 digits only.

[Jan Ťulák]

Tests can use any name now, not 3 digits only.
(e.g. a test can be named "tests/generic/some-name")

Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---
 README |  2 +-
 check  | 11 ++++++-----
 new    | 27 +++++++++++++++++++++++++--
 3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/README b/README
index 0c9449a..2376674 100644
--- a/README
+++ b/README
@@ -205,7 +205,7 @@ Test script environment:
 
 Verified output:
 
-    Each test script has a numerical name, e.g. 007, and an associated
+    Each test script has a name, e.g. 007, and an associated
     verified output, e.g. 007.out.
 
     It is important that the verified output is deterministic, and
diff --git a/check b/check
index 0830e0c..394fae4 100755
--- a/check
+++ b/check
@@ -58,7 +58,7 @@ then
     exit 1
 fi
 
-SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
+SUPPORTED_TESTS="\S\+"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
 
@@ -96,21 +96,22 @@ get_group_list()
 		l=$(sed -n < $SRC_DIR/$d/group \
 			-e 's/#.*//' \
 			-e 's/$/ /' \
-			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
+			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
 		grpl="$grpl $l"
 	done
 	echo $grpl
 }
 
-# find all tests, excluding files that are test metadata such as group files.
-# This assumes that tests are defined purely by alphanumeric filenames with no
-# ".xyz" extensions in the name.
+# Find all tests, excluding files that are test metadata such as group files.
+# It matches test names against $SUPPORTED_TESTS defined at the top of this 
+# file.
 get_all_tests()
 {
 	touch $tmp.list
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
+			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
 			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
diff --git a/new b/new
index d1f8939..60b898a 100755
--- a/new
+++ b/new
@@ -84,8 +84,11 @@ eof=1
 for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
 do
     line=$((line+1))
-    if [ -z "$found" ] || [ "$found" == "#" ];then
-	continue
+    if [ -z "$found" ] || [ "$found" == "#" ]; then
+        continue
+    elif ! echo "$found" | grep "^[0-9][0-9][0-9]$" > /dev/null; then
+        # this one is for tests not named by a number
+        continue
     fi
     i=$((i+1))
     id=`printf "%03d" $i`
@@ -102,6 +105,26 @@ fi
 
 echo "Next test is $id"
 
+read -p "Do you want to use ANOTHER name? y,[n]: " -r
+if [[ "$REPLY" =~ ^[Yy]$ ]]; then
+    id=""
+    while [ "$id" = "" ]; do
+        read -p "Enter the new name: "
+        if [ "$REPLY" = "" ]; then
+            echo "Can't use empty name. For canceling, use ctrl+c."
+        elif [ -e "$tdir/$REPLY" ]; then
+            echo "File '$REPLY' already exists, use another one."
+            echo #
+        elif echo "$REPLY" | grep "^\\S\+$" > /dev/null; then
+            id="$REPLY"
+        else
+            echo "Filename must not contain whitespaces!"
+            echo
+        fi
+    done
+fi
+echo "Using '$id'."
+
 if [ -f $tdir/$id ]
 then
     echo "Error: test $id already exists!"
-- 
2.1.0

Re: [PATCH] Tests can use any name now, not 3 digits only.

[Jan Tulak]

I send a fixed patch, hopefully this time it looks good. :-)
The prompt for a new test name is a good idea, I added it into the patch too.

Jan

----- Original Message -----
> From: "Eryu Guan" <eguan@redhat.com>
> To: "Jan Ťulák" <jtulak@redhat.com>
> Cc: fstests@vger.kernel.org
> Sent: Friday, 20 March, 2015 12:13:20 PM
> Subject: Re: [PATCH] Tests can use any name now, not 3 digits only.
> 
> On Wed, Mar 04, 2015 at 04:55:17PM +0100, Jan Ťulák wrote:
> > Tests can use any name now, not 3 digits only.
> > (e.g. a test can be named "tests/generic/some-name")
> 
> I think you can add "fstests: " prefix to summary
> 
> fstests: tests can use any name not 3 digits only
> 
> And it looks good to me overall. I tested with the name "some-test"
> "some-test-001" "some_test" "some-test-001.test", all worked fine. Also
> tested with ./check -g testgroup (added testgroup to some random tests),
> it worked too.
> 
> Some comments inline.
> 
> > 
> > Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> > ---
> >  README | 2 +-
> >  check  | 6 +++---
> >  new    | 7 +++++--
> >  3 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/README b/README
> > index 0c9449a..2376674 100644
> > --- a/README
> > +++ b/README
> > @@ -205,7 +205,7 @@ Test script environment:
> >  
> >  Verified output:
> >  
> > -    Each test script has a numerical name, e.g. 007, and an associated
> > +    Each test script has a name, e.g. 007, and an associated
> >      verified output, e.g. 007.out.
> >  
> >      It is important that the verified output is deterministic, and
> > diff --git a/check b/check
> > index 0830e0c..d7814a8 100755
> > --- a/check
> > +++ b/check
> > @@ -58,7 +58,7 @@ then
> >      exit 1
> >  fi
> >  
> > -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> > +SUPPORTED_TESTS="\S\+"
> >  SRC_GROUPS="generic shared"
> >  export SRC_DIR="tests"
> >  
> > @@ -96,7 +96,7 @@ get_group_list()
> >  		l=$(sed -n < $SRC_DIR/$d/group \
> >  			-e 's/#.*//' \
> >  			-e 's/$/ /' \
> > -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> > +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
> >  		grpl="$grpl $l"
> >  	done
> >  	echo $grpl
> > @@ -111,6 +111,7 @@ get_all_tests()
> 
> The comments before get_all_tests() should be updated too, which says
> 
> "
> # This assumes that tests are defined purely by alphanumeric filenames with
> no
> # ".xyz" extensions in the name.
> "
> 
> >  	for d in $SRC_GROUPS $FSTYP; do
> >  		ls $SRC_DIR/$d/* | \
> >  			grep -v "\..*" | \
> > +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
> >  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
> >  	done
> >  }
> > @@ -178,7 +179,6 @@ _prepare_test_list()
> >  		# no test numbers, do everything
> >  		get_all_tests
> >  	fi
> > -
> 
> Leave this line not removed.
> 
> >  	# Specified groups to exclude
> >  	for xgroup in $XGROUP_LIST; do
> >  		list=$(get_group_list $xgroup)
> > diff --git a/new b/new
> > index 86f9075..f755da3 100755
> > --- a/new
> > +++ b/new
> > @@ -84,8 +84,11 @@ eof=1
> >  for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> >  do
> >      line=$((line+1))
> > -    if [ -z "$found" ] || [ "$found" == "#" ];then
> > -	continue
> > +    if [ -z "$found" ] || [ "$found" == "#" ] ;then
> > +        continue
> > +    elif ! echo "$found"|grep "[0-9][0-9][0-9]";then
> 
> I'm being picky here :)
> 
> put space around pipe, echo "$found" | grep
> 
> and add space between ";" and "then", no space before ";"
> 
> if [ condition ]; then
> fi
> 
> > +        # this one is for tests not named by a number
> > +        continue
> 
> If I have a test name like "some-test-001" which has three digits too,
> "new" script is not working well
> 
> [root@hp-dl388eg8-01 xfstests]# ./new generic
> Building include
> Building lib
> Building ltp
> Building src
> Building aio-dio-regress
> Building m4
> Building common
> Building tests
> 001
> 002
> 003
> some-test-001
> Next test is 004
> Error: test 004 already exists!
> 
> And do we need a prompt in "new" to give user a change to create user
> defined test name? It's still defaulting to three digits.
> 
> Thanks,
> Eryu
> >      fi
> >      i=$((i+1))
> >      id=`printf "%03d" $i`
> > --
> > 2.1.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] fstests: tests can use any name now, not 3 digits only.

[Eryu Guan]

On Fri, Mar 20, 2015 at 04:03:25PM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/some-name")
> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> ---
>  README |  2 +-
>  check  | 11 ++++++-----
>  new    | 27 +++++++++++++++++++++++++--
>  3 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/README b/README
> index 0c9449a..2376674 100644
> --- a/README
> +++ b/README
> @@ -205,7 +205,7 @@ Test script environment:
>  
>  Verified output:
>  
> -    Each test script has a numerical name, e.g. 007, and an associated
> +    Each test script has a name, e.g. 007, and an associated
>      verified output, e.g. 007.out.
>  
>      It is important that the verified output is deterministic, and
> diff --git a/check b/check
> index 0830e0c..394fae4 100755
> --- a/check
> +++ b/check
> @@ -58,7 +58,7 @@ then
>      exit 1
>  fi
>  
> -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> +SUPPORTED_TESTS="\S\+"
>  SRC_GROUPS="generic shared"
>  export SRC_DIR="tests"
>  
> @@ -96,21 +96,22 @@ get_group_list()
>  		l=$(sed -n < $SRC_DIR/$d/group \
>  			-e 's/#.*//' \
>  			-e 's/$/ /' \
> -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
>  		grpl="$grpl $l"
>  	done
>  	echo $grpl
>  }
>  
> -# find all tests, excluding files that are test metadata such as group files.
> -# This assumes that tests are defined purely by alphanumeric filenames with no
> -# ".xyz" extensions in the name.
> +# Find all tests, excluding files that are test metadata such as group files.
> +# It matches test names against $SUPPORTED_TESTS defined at the top of this 

Trailing whitespace in above line

> +# file.
>  get_all_tests()
>  {
>  	touch $tmp.list
>  	for d in $SRC_GROUPS $FSTYP; do
>  		ls $SRC_DIR/$d/* | \
>  			grep -v "\..*" | \
> +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \

I just found that this will skip all tests with dot in the name, if
there's a test has dot in the name(e.g. some-name.test) it won't be run
when ./check with no other arguments. "./check -n" could show all tests
to be run, and "some-name.test" is not in the list.

Or "." should not be allowed in test name?

>  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
>  	done
>  }
> diff --git a/new b/new
> index d1f8939..60b898a 100755
> --- a/new
> +++ b/new
> @@ -84,8 +84,11 @@ eof=1
>  for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
>  do
>      line=$((line+1))
> -    if [ -z "$found" ] || [ "$found" == "#" ];then
> -	continue
> +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> +        continue
> +    elif ! echo "$found" | grep "^[0-9][0-9][0-9]$" > /dev/null; then

grep -q to be quiet

> +        # this one is for tests not named by a number
> +        continue
>      fi
>      i=$((i+1))
>      id=`printf "%03d" $i`
> @@ -102,6 +105,26 @@ fi
>  
>  echo "Next test is $id"
>  
> +read -p "Do you want to use ANOTHER name? y,[n]: " -r
> +if [[ "$REPLY" =~ ^[Yy]$ ]]; then
> +    id=""
> +    while [ "$id" = "" ]; do
> +        read -p "Enter the new name: "
> +        if [ "$REPLY" = "" ]; then
> +            echo "Can't use empty name. For canceling, use ctrl+c."
> +        elif [ -e "$tdir/$REPLY" ]; then
> +            echo "File '$REPLY' already exists, use another one."
> +            echo #
> +        elif echo "$REPLY" | grep "^\\S\+$" > /dev/null; then

here too, grep -q

> +            id="$REPLY"
> +        else
> +            echo "Filename must not contain whitespaces!"
> +            echo
> +        fi
> +    done
> +fi
> +echo "Using '$id'."
> +

Now new case with customized name is inserted in numbered tests, as

eguan@dhcp-13-216:~/workspace/src/xfstests$ git diff
diff --git a/tests/generic/group b/tests/generic/group
index 8154401..e409035 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -69,6 +69,10 @@
 064 auto quick prealloc
 065 metadata auto quick
 066 metadata auto quick
+some-test.001 other
+some-test-002 other
+some-test-003 other
+hello! other
 068 other auto freeze dangerous stress
 069 rw udf auto quick
 070 attr udf auto quick stress

I'd like to see all customized test names are at the end(or beginning)
of group file, sorted if possible.

Maybe I'm asking too much, but this is what I'd like to see :)

And the test case template in "new" can be updated too, I think, we
don't need "No." anymore

	# FS QA Test No. $id

Thanks,
Eryu
>  if [ -f $tdir/$id ]
>  then
>      echo "Error: test $id already exists!"
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fstests: tests can use any name now, not 3 digits only.

[Jan Tulak]

Allowing the dot in test name has one important consequence - how to know what is a test and what is a test output?
Now, I see it is a bit inconsistent, because it is possible to create such test but not run it, but that's a bug.

If we don't want to touch existing tests, then I see two possibilities regarding this:
- dot "." must not be part of a test name at all
- string ".out" must not be part of test name (including .out.something, because there are files like generic/088.out.linux)

I would say forbidding dot is cleaner approach. What do you think?


And I will look on sorting the non-numbered tests.

Cheers,
Jan


----- Original Message -----
> From: "Eryu Guan" <eguan@redhat.com>
> To: "Jan Ťulák" <jtulak@redhat.com>
> Cc: fstests@vger.kernel.org
> Sent: Saturday, 21 March, 2015 5:49:32 AM
> Subject: Re: [PATCH] fstests: tests can use any name now, not 3 digits only.
> 
> On Fri, Mar 20, 2015 at 04:03:25PM +0100, Jan Ťulák wrote:
> > Tests can use any name now, not 3 digits only.
> > (e.g. a test can be named "tests/generic/some-name")
> > 
> > Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> > ---
> >  README |  2 +-
> >  check  | 11 ++++++-----
> >  new    | 27 +++++++++++++++++++++++++--
> >  3 files changed, 32 insertions(+), 8 deletions(-)
> > 
> > diff --git a/README b/README
> > index 0c9449a..2376674 100644
> > --- a/README
> > +++ b/README
> > @@ -205,7 +205,7 @@ Test script environment:
> >  
> >  Verified output:
> >  
> > -    Each test script has a numerical name, e.g. 007, and an associated
> > +    Each test script has a name, e.g. 007, and an associated
> >      verified output, e.g. 007.out.
> >  
> >      It is important that the verified output is deterministic, and
> > diff --git a/check b/check
> > index 0830e0c..394fae4 100755
> > --- a/check
> > +++ b/check
> > @@ -58,7 +58,7 @@ then
> >      exit 1
> >  fi
> >  
> > -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> > +SUPPORTED_TESTS="\S\+"
> >  SRC_GROUPS="generic shared"
> >  export SRC_DIR="tests"
> >  
> > @@ -96,21 +96,22 @@ get_group_list()
> >  		l=$(sed -n < $SRC_DIR/$d/group \
> >  			-e 's/#.*//' \
> >  			-e 's/$/ /' \
> > -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> > +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
> >  		grpl="$grpl $l"
> >  	done
> >  	echo $grpl
> >  }
> >  
> > -# find all tests, excluding files that are test metadata such as group
> > files.
> > -# This assumes that tests are defined purely by alphanumeric filenames
> > with no
> > -# ".xyz" extensions in the name.
> > +# Find all tests, excluding files that are test metadata such as group
> > files.
> > +# It matches test names against $SUPPORTED_TESTS defined at the top of
> > this
> 
> Trailing whitespace in above line
> 
> > +# file.
> >  get_all_tests()
> >  {
> >  	touch $tmp.list
> >  	for d in $SRC_GROUPS $FSTYP; do
> >  		ls $SRC_DIR/$d/* | \
> >  			grep -v "\..*" | \
> > +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
> 
> I just found that this will skip all tests with dot in the name, if
> there's a test has dot in the name(e.g. some-name.test) it won't be run
> when ./check with no other arguments. "./check -n" could show all tests
> to be run, and "some-name.test" is not in the list.
> 
> Or "." should not be allowed in test name?
> 
> >  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
> >  	done
> >  }
> > diff --git a/new b/new
> > index d1f8939..60b898a 100755
> > --- a/new
> > +++ b/new
> > @@ -84,8 +84,11 @@ eof=1
> >  for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> >  do
> >      line=$((line+1))
> > -    if [ -z "$found" ] || [ "$found" == "#" ];then
> > -	continue
> > +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> > +        continue
> > +    elif ! echo "$found" | grep "^[0-9][0-9][0-9]$" > /dev/null; then
> 
> grep -q to be quiet
> 
> > +        # this one is for tests not named by a number
> > +        continue
> >      fi
> >      i=$((i+1))
> >      id=`printf "%03d" $i`
> > @@ -102,6 +105,26 @@ fi
> >  
> >  echo "Next test is $id"
> >  
> > +read -p "Do you want to use ANOTHER name? y,[n]: " -r
> > +if [[ "$REPLY" =~ ^[Yy]$ ]]; then
> > +    id=""
> > +    while [ "$id" = "" ]; do
> > +        read -p "Enter the new name: "
> > +        if [ "$REPLY" = "" ]; then
> > +            echo "Can't use empty name. For canceling, use ctrl+c."
> > +        elif [ -e "$tdir/$REPLY" ]; then
> > +            echo "File '$REPLY' already exists, use another one."
> > +            echo #
> > +        elif echo "$REPLY" | grep "^\\S\+$" > /dev/null; then
> 
> here too, grep -q
> 
> > +            id="$REPLY"
> > +        else
> > +            echo "Filename must not contain whitespaces!"
> > +            echo
> > +        fi
> > +    done
> > +fi
> > +echo "Using '$id'."
> > +
> 
> Now new case with customized name is inserted in numbered tests, as
> 
> eguan@dhcp-13-216:~/workspace/src/xfstests$ git diff
> diff --git a/tests/generic/group b/tests/generic/group
> index 8154401..e409035 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -69,6 +69,10 @@
>  064 auto quick prealloc
>  065 metadata auto quick
>  066 metadata auto quick
> +some-test.001 other
> +some-test-002 other
> +some-test-003 other
> +hello! other
>  068 other auto freeze dangerous stress
>  069 rw udf auto quick
>  070 attr udf auto quick stress
> 
> I'd like to see all customized test names are at the end(or beginning)
> of group file, sorted if possible.
> 
> Maybe I'm asking too much, but this is what I'd like to see :)
> 
> And the test case template in "new" can be updated too, I think, we
> don't need "No." anymore
> 
> 	# FS QA Test No. $id
> 
> Thanks,
> Eryu
> >  if [ -f $tdir/$id ]
> >  then
> >      echo "Error: test $id already exists!"
> > --
> > 2.1.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] fstests: tests can use any name now, not 3 digits only.

[Eryu Guan]

On Sat, Mar 21, 2015 at 08:02:54AM -0400, Jan Tulak wrote:
> Allowing the dot in test name has one important consequence - how to know what is a test and what is a test output?
> Now, I see it is a bit inconsistent, because it is possible to create such test but not run it, but that's a bug.
> 
> If we don't want to touch existing tests, then I see two possibilities regarding this:
> - dot "." must not be part of a test name at all
> - string ".out" must not be part of test name (including .out.something, because there are files like generic/088.out.linux)
> 
> I would say forbidding dot is cleaner approach. What do you think?

I agree, no dot in test name is easier and cleaner.

But you may also want to hear what do others think :)

> 
> 
> And I will look on sorting the non-numbered tests.

Thanks!

Eryu
> 
> Cheers,
> Jan
> 
> 
> ----- Original Message -----
> > From: "Eryu Guan" <eguan@redhat.com>
> > To: "Jan Ťulák" <jtulak@redhat.com>
> > Cc: fstests@vger.kernel.org
> > Sent: Saturday, 21 March, 2015 5:49:32 AM
> > Subject: Re: [PATCH] fstests: tests can use any name now, not 3 digits only.
> > 
> > On Fri, Mar 20, 2015 at 04:03:25PM +0100, Jan Ťulák wrote:
> > > Tests can use any name now, not 3 digits only.
> > > (e.g. a test can be named "tests/generic/some-name")
> > > 
> > > Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> > > ---
> > >  README |  2 +-
> > >  check  | 11 ++++++-----
> > >  new    | 27 +++++++++++++++++++++++++--
> > >  3 files changed, 32 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/README b/README
> > > index 0c9449a..2376674 100644
> > > --- a/README
> > > +++ b/README
> > > @@ -205,7 +205,7 @@ Test script environment:
> > >  
> > >  Verified output:
> > >  
> > > -    Each test script has a numerical name, e.g. 007, and an associated
> > > +    Each test script has a name, e.g. 007, and an associated
> > >      verified output, e.g. 007.out.
> > >  
> > >      It is important that the verified output is deterministic, and
> > > diff --git a/check b/check
> > > index 0830e0c..394fae4 100755
> > > --- a/check
> > > +++ b/check
> > > @@ -58,7 +58,7 @@ then
> > >      exit 1
> > >  fi
> > >  
> > > -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> > > +SUPPORTED_TESTS="\S\+"
> > >  SRC_GROUPS="generic shared"
> > >  export SRC_DIR="tests"
> > >  
> > > @@ -96,21 +96,22 @@ get_group_list()
> > >  		l=$(sed -n < $SRC_DIR/$d/group \
> > >  			-e 's/#.*//' \
> > >  			-e 's/$/ /' \
> > > -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> > > +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
> > >  		grpl="$grpl $l"
> > >  	done
> > >  	echo $grpl
> > >  }
> > >  
> > > -# find all tests, excluding files that are test metadata such as group
> > > files.
> > > -# This assumes that tests are defined purely by alphanumeric filenames
> > > with no
> > > -# ".xyz" extensions in the name.
> > > +# Find all tests, excluding files that are test metadata such as group
> > > files.
> > > +# It matches test names against $SUPPORTED_TESTS defined at the top of
> > > this
> > 
> > Trailing whitespace in above line
> > 
> > > +# file.
> > >  get_all_tests()
> > >  {
> > >  	touch $tmp.list
> > >  	for d in $SRC_GROUPS $FSTYP; do
> > >  		ls $SRC_DIR/$d/* | \
> > >  			grep -v "\..*" | \
> > > +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
> > 
> > I just found that this will skip all tests with dot in the name, if
> > there's a test has dot in the name(e.g. some-name.test) it won't be run
> > when ./check with no other arguments. "./check -n" could show all tests
> > to be run, and "some-name.test" is not in the list.
> > 
> > Or "." should not be allowed in test name?
> > 
> > >  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
> > >  	done
> > >  }
> > > diff --git a/new b/new
> > > index d1f8939..60b898a 100755
> > > --- a/new
> > > +++ b/new
> > > @@ -84,8 +84,11 @@ eof=1
> > >  for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> > >  do
> > >      line=$((line+1))
> > > -    if [ -z "$found" ] || [ "$found" == "#" ];then
> > > -	continue
> > > +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> > > +        continue
> > > +    elif ! echo "$found" | grep "^[0-9][0-9][0-9]$" > /dev/null; then
> > 
> > grep -q to be quiet
> > 
> > > +        # this one is for tests not named by a number
> > > +        continue
> > >      fi
> > >      i=$((i+1))
> > >      id=`printf "%03d" $i`
> > > @@ -102,6 +105,26 @@ fi
> > >  
> > >  echo "Next test is $id"
> > >  
> > > +read -p "Do you want to use ANOTHER name? y,[n]: " -r
> > > +if [[ "$REPLY" =~ ^[Yy]$ ]]; then
> > > +    id=""
> > > +    while [ "$id" = "" ]; do
> > > +        read -p "Enter the new name: "
> > > +        if [ "$REPLY" = "" ]; then
> > > +            echo "Can't use empty name. For canceling, use ctrl+c."
> > > +        elif [ -e "$tdir/$REPLY" ]; then
> > > +            echo "File '$REPLY' already exists, use another one."
> > > +            echo #
> > > +        elif echo "$REPLY" | grep "^\\S\+$" > /dev/null; then
> > 
> > here too, grep -q
> > 
> > > +            id="$REPLY"
> > > +        else
> > > +            echo "Filename must not contain whitespaces!"
> > > +            echo
> > > +        fi
> > > +    done
> > > +fi
> > > +echo "Using '$id'."
> > > +
> > 
> > Now new case with customized name is inserted in numbered tests, as
> > 
> > eguan@dhcp-13-216:~/workspace/src/xfstests$ git diff
> > diff --git a/tests/generic/group b/tests/generic/group
> > index 8154401..e409035 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -69,6 +69,10 @@
> >  064 auto quick prealloc
> >  065 metadata auto quick
> >  066 metadata auto quick
> > +some-test.001 other
> > +some-test-002 other
> > +some-test-003 other
> > +hello! other
> >  068 other auto freeze dangerous stress
> >  069 rw udf auto quick
> >  070 attr udf auto quick stress
> > 
> > I'd like to see all customized test names are at the end(or beginning)
> > of group file, sorted if possible.
> > 
> > Maybe I'm asking too much, but this is what I'd like to see :)
> > 
> > And the test case template in "new" can be updated too, I think, we
> > don't need "No." anymore
> > 
> > 	# FS QA Test No. $id
> > 
> > Thanks,
> > Eryu
> > >  if [ -f $tdir/$id ]
> > >  then
> > >      echo "Error: test $id already exists!"
> > > --
> > > 2.1.0
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 

[PATCH] fstests: Tests can use any name now, not 3 digits only.

[Jan Ťulák]

Tests can use any name now, not 3 digits only.
(e.g. a test can be named "tests/generic/some-name")

The only limitation on a test name is no whitespace and no dot.

Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---
 README |  2 +-
 check  | 11 ++++++-----
 new    | 50 +++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/README b/README
index 0c9449a..2376674 100644
--- a/README
+++ b/README
@@ -205,7 +205,7 @@ Test script environment:
 
 Verified output:
 
-    Each test script has a numerical name, e.g. 007, and an associated
+    Each test script has a name, e.g. 007, and an associated
     verified output, e.g. 007.out.
 
     It is important that the verified output is deterministic, and
diff --git a/check b/check
index 0830e0c..da0bc31 100755
--- a/check
+++ b/check
@@ -58,7 +58,7 @@ then
     exit 1
 fi
 
-SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
+SUPPORTED_TESTS="[^\s.]\+"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
 
@@ -96,21 +96,22 @@ get_group_list()
 		l=$(sed -n < $SRC_DIR/$d/group \
 			-e 's/#.*//' \
 			-e 's/$/ /' \
-			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
+			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
 		grpl="$grpl $l"
 	done
 	echo $grpl
 }
 
-# find all tests, excluding files that are test metadata such as group files.
-# This assumes that tests are defined purely by alphanumeric filenames with no
-# ".xyz" extensions in the name.
+# Find all tests, excluding files that are test metadata such as group files.
+# It matches test names against $SUPPORTED_TESTS defined at the top of this
+# file.
 get_all_tests()
 {
 	touch $tmp.list
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
+			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
 			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
diff --git a/new b/new
index d1f8939..6cf67a7 100755
--- a/new
+++ b/new
@@ -84,8 +84,11 @@ eof=1
 for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
 do
     line=$((line+1))
-    if [ -z "$found" ] || [ "$found" == "#" ];then
-	continue
+    if [ -z "$found" ] || [ "$found" == "#" ]; then
+        continue
+    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
+        # this one is for tests not named by a number
+        continue
     fi
     i=$((i+1))
     id=`printf "%03d" $i`
@@ -99,9 +102,50 @@ if [ $eof -eq 1 ]; then
    i=$((i+1))
    id=`printf "%03d" $i`
 fi
+auto_id=$id
 
 echo "Next test is $id"
 
+read -p "Do you want to use ANOTHER name? y,[n]: " -r
+if [[ "$REPLY" =~ ^[Yy]$ ]]; then
+    # get the new name from user
+    id=""
+    while [ "$id" = "" ]; do
+        read -p "Enter the new name: "
+        if [ "$REPLY" = "" ]; then
+            echo "Can't use empty name. For canceling, use ctrl+c."
+        elif [ -e "$tdir/$REPLY" ]; then
+            echo "File '$REPLY' already exists, use another one."
+            echo #
+        elif echo "$REPLY" | grep -q "^[^\\s.]\+$"; then
+            id="$REPLY"
+        else
+            echo "Filename must not contain whitespaces and dots!"
+            echo 
+        fi
+    done
+
+    # now find where to insert this name
+    eof=1
+    line=0
+    for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
+    do
+        line=$((line+1))
+        if [ -z "$found" ] || [ "$found" == "#" ]; then
+            continue
+        elif [[ "$found" > "$id" ]]; then
+            eof=0
+            break
+        fi
+    done
+    if [ $eof -eq 1 ]; then
+        # If place wasn't found, let $line be the end of the file
+        line=$((line+1))
+    fi
+
+fi
+echo "Using '$id'."
+
 if [ -f $tdir/$id ]
 then
     echo "Error: test $id already exists!"
@@ -115,7 +159,7 @@ year=`date +%Y`
 
 cat <<End-of-File >$tdir/$id
 #! /bin/bash
-# FS QA Test No. $id
+# FS QA Test $id
 #
 # what am I here for?
 #
-- 
2.1.0

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[Jan Tulak]

Eryu,
the sorting now puts the named tests at the end of group file and dots can't be in test names any more.

I hope this version looks finally ok.
Anyway, thank you for the feedback. :-)

Cheers,
Jan

----- Original Message -----
> From: "Jan Ťulák" <jtulak@redhat.com>
> To: eguan@redhat.com
> Cc: fstests@vger.kernel.org
> Sent: Wednesday, 25 March, 2015 2:27:35 PM
> Subject: [PATCH] fstests: Tests can use any name now, not 3 digits only.
> 
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/some-name")
> 
> The only limitation on a test name is no whitespace and no dot.
> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> ---
>  README |  2 +-
>  check  | 11 ++++++-----
>  new    | 50 +++++++++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 54 insertions(+), 9 deletions(-)
> 
> diff --git a/README b/README
> index 0c9449a..2376674 100644
> --- a/README
> +++ b/README
> @@ -205,7 +205,7 @@ Test script environment:
>  
>  Verified output:
>  
> -    Each test script has a numerical name, e.g. 007, and an associated
> +    Each test script has a name, e.g. 007, and an associated
>      verified output, e.g. 007.out.
>  
>      It is important that the verified output is deterministic, and
> diff --git a/check b/check
> index 0830e0c..da0bc31 100755
> --- a/check
> +++ b/check
> @@ -58,7 +58,7 @@ then
>      exit 1
>  fi
>  
> -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> +SUPPORTED_TESTS="[^\s.]\+"
>  SRC_GROUPS="generic shared"
>  export SRC_DIR="tests"
>  
> @@ -96,21 +96,22 @@ get_group_list()
>  		l=$(sed -n < $SRC_DIR/$d/group \
>  			-e 's/#.*//' \
>  			-e 's/$/ /' \
> -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
>  		grpl="$grpl $l"
>  	done
>  	echo $grpl
>  }
>  
> -# find all tests, excluding files that are test metadata such as group
> files.
> -# This assumes that tests are defined purely by alphanumeric filenames with
> no
> -# ".xyz" extensions in the name.
> +# Find all tests, excluding files that are test metadata such as group
> files.
> +# It matches test names against $SUPPORTED_TESTS defined at the top of this
> +# file.
>  get_all_tests()
>  {
>  	touch $tmp.list
>  	for d in $SRC_GROUPS $FSTYP; do
>  		ls $SRC_DIR/$d/* | \
>  			grep -v "\..*" | \
> +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
>  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
>  	done
>  }
> diff --git a/new b/new
> index d1f8939..6cf67a7 100755
> --- a/new
> +++ b/new
> @@ -84,8 +84,11 @@ eof=1
>  for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
>  do
>      line=$((line+1))
> -    if [ -z "$found" ] || [ "$found" == "#" ];then
> -	continue
> +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> +        continue
> +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> +        # this one is for tests not named by a number
> +        continue
>      fi
>      i=$((i+1))
>      id=`printf "%03d" $i`
> @@ -99,9 +102,50 @@ if [ $eof -eq 1 ]; then
>     i=$((i+1))
>     id=`printf "%03d" $i`
>  fi
> +auto_id=$id
>  
>  echo "Next test is $id"
>  
> +read -p "Do you want to use ANOTHER name? y,[n]: " -r
> +if [[ "$REPLY" =~ ^[Yy]$ ]]; then
> +    # get the new name from user
> +    id=""
> +    while [ "$id" = "" ]; do
> +        read -p "Enter the new name: "
> +        if [ "$REPLY" = "" ]; then
> +            echo "Can't use empty name. For canceling, use ctrl+c."
> +        elif [ -e "$tdir/$REPLY" ]; then
> +            echo "File '$REPLY' already exists, use another one."
> +            echo #
> +        elif echo "$REPLY" | grep -q "^[^\\s.]\+$"; then
> +            id="$REPLY"
> +        else
> +            echo "Filename must not contain whitespaces and dots!"
> +            echo
> +        fi
> +    done
> +
> +    # now find where to insert this name
> +    eof=1
> +    line=0
> +    for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> +    do
> +        line=$((line+1))
> +        if [ -z "$found" ] || [ "$found" == "#" ]; then
> +            continue
> +        elif [[ "$found" > "$id" ]]; then
> +            eof=0
> +            break
> +        fi
> +    done
> +    if [ $eof -eq 1 ]; then
> +        # If place wasn't found, let $line be the end of the file
> +        line=$((line+1))
> +    fi
> +
> +fi
> +echo "Using '$id'."
> +
>  if [ -f $tdir/$id ]
>  then
>      echo "Error: test $id already exists!"
> @@ -115,7 +159,7 @@ year=`date +%Y`
>  
>  cat <<End-of-File >$tdir/$id
>  #! /bin/bash
> -# FS QA Test No. $id
> +# FS QA Test $id
>  #
>  # what am I here for?
>  #
> --
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[David Sterba]

On Wed, Mar 25, 2015 at 02:27:35PM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/some-name")

Good idea.

> The only limitation on a test name is no whitespace and no dot.

IMHO we don't need to be too creative, the limitations make sense.

I have a proposal for slight modification to the naming scheme:

  NNN-free-text

where NNN is a unique number among all tests in the same directory.

Why? Convenience, a shortcut for the long test descriptions. We usually
say that test 123 fails and some other does not, I personally find it
very handy and would like to keep that.

I've enforced this naming scheme for btrfs-progs userspace tests:
https://github.com/kdave/btrfs-progs/tree/master/tests/fsck-tests

The preference might be different for others though, but we can still
try to follow the scheme inside the tests/btrfs/ directory.

Otherwise the patch looks ok to me.

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1941 bytes --]

On Wed, 25 Mar 2015, David Sterba wrote:

> Date: Wed, 25 Mar 2015 15:44:52 +0100
> From: David Sterba <dsterba@suse.cz>
> To: Jan Ťulák <jtulak@redhat.com>
> Cc: eguan@redhat.com, fstests@vger.kernel.org
> Subject: Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.
> 
> On Wed, Mar 25, 2015 at 02:27:35PM +0100, Jan Ťulák wrote:
> > Tests can use any name now, not 3 digits only.
> > (e.g. a test can be named "tests/generic/some-name")
> 
> Good idea.
> 
> > The only limitation on a test name is no whitespace and no dot.
> 
> IMHO we don't need to be too creative, the limitations make sense.
> 
> I have a proposal for slight modification to the naming scheme:
> 
>   NNN-free-text
> 
> where NNN is a unique number among all tests in the same directory.
> 
> Why? Convenience, a shortcut for the long test descriptions. We usually
> say that test 123 fails and some other does not, I personally find it
> very handy and would like to keep that.

Yes, I like that, but then we want to make sure that we do not have
tests with the same numbers, but different name. Also having more more
constrains on the names is a good thing especially when people feel like
being creative with test names.

So we can make it

NNN-test-name

where we only allow numbers in the first three characters, and only
alphabetic ASCII characters and a dash afterwards (or underscore,
whichever you prefer).

Thanks!
-Lukas

> 
> I've enforced this naming scheme for btrfs-progs userspace tests:
> https://github.com/kdave/btrfs-progs/tree/master/tests/fsck-tests
> 
> The preference might be different for others though, but we can still
> try to follow the scheme inside the tests/btrfs/ directory.
> 
> Otherwise the patch looks ok to me.
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[Jan Tulak]

----- Original Message -----
> From: "David Sterba" <dsterba@suse.cz>
> 
> I have a proposal for slight modification to the naming scheme:
> 
>   NNN-free-text
> 
> where NNN is a unique number among all tests in the same directory.
> 
> Why? Convenience, a shortcut for the long test descriptions. We usually
> say that test 123 fails and some other does not, I personally find it
> very handy and would like to keep that.
> 
> I've enforced this naming scheme for btrfs-progs userspace tests:
> https://github.com/kdave/btrfs-progs/tree/master/tests/fsck-tests
> 
> The preference might be different for others though, but we can still
> try to follow the scheme inside the tests/btrfs/ directory.
> 

I see the reason, but I have a note. This format breaks alphabetic ordering, so if we use names for grouping tests together, they are not listed that way. There is an example of what I mean by the grouping:

performance/group:
fsmark-small-files-001                fsmark small_files rw sequential
fsmark-small-files-002                fsmark small_files rw random
fsmark-small-files-003                fsmark small_files traverse
fsmark-small-files-004                fsmark small_files unlink
fsmark-large-files-001                fsmark large_files rw
fsmark-large-files-002                fsmark large_files unlink
fsmark-1m-empty-files-001        fsmark metadata scale create
fsmark-10m-empty-files-001        fsmark metadata scale create
fsmark-100m-empty-files-001        fsmark metadata scale create
fsmark-100m-empty-files-002        fsmark metadata scale traverse
fsmark-100m-empty-files-003        fsmark metadata scale unlink
.....

If we put the unique number at the end (some-name-NNN), then this issue is eliminated. Of course, with this you can't do NNN<tab> for completion, but it keeps the number reference. But this way it makes harder to find the test by number...


----- Original Message -----
> From: "Lukáš Czerner" <lczerner@redhat.com>
> Sent: Wednesday, 25 March, 2015 4:20:24 PM
> 
> 
> Yes, I like that, but then we want to make sure that we do not have
> tests with the same numbers, but different name. Also having more more
> constrains on the names is a good thing especially when people feel like
> being creative with test names.
> 
> So we can make it
> 
> NNN-test-name
> 
> where we only allow numbers in the first three characters, and only
> alphabetic ASCII characters and a dash afterwards (or underscore,
> whichever you prefer).
> 
> Thanks!
> -Lukas

The stricter rules are all right, I agree with that too.

Jan

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3512 bytes --]

On Wed, 25 Mar 2015, Jan Tulak wrote:

> Date: Wed, 25 Mar 2015 11:27:13 -0400 (EDT)
> From: Jan Tulak <jtulak@redhat.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: David Sterba <dsterba@suse.cz>, eguan@redhat.com, fstests@vger.kernel.org
> Subject: Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.
> 
> ----- Original Message -----
> > From: "David Sterba" <dsterba@suse.cz>
> > 
> > I have a proposal for slight modification to the naming scheme:
> > 
> >   NNN-free-text
> > 
> > where NNN is a unique number among all tests in the same directory.
> > 
> > Why? Convenience, a shortcut for the long test descriptions. We usually
> > say that test 123 fails and some other does not, I personally find it
> > very handy and would like to keep that.
> > 
> > I've enforced this naming scheme for btrfs-progs userspace tests:
> > https://github.com/kdave/btrfs-progs/tree/master/tests/fsck-tests
> > 
> > The preference might be different for others though, but we can still
> > try to follow the scheme inside the tests/btrfs/ directory.
> > 
> 
> I see the reason, but I have a note. This format breaks alphabetic ordering, so if we use names for grouping tests together, they are not listed that way. There is an example of what I mean by the grouping:
> 
> performance/group:
> fsmark-small-files-001                fsmark small_files rw sequential
> fsmark-small-files-002                fsmark small_files rw random
> fsmark-small-files-003                fsmark small_files traverse
> fsmark-small-files-004                fsmark small_files unlink
> fsmark-large-files-001                fsmark large_files rw
> fsmark-large-files-002                fsmark large_files unlink
> fsmark-1m-empty-files-001        fsmark metadata scale create
> fsmark-10m-empty-files-001        fsmark metadata scale create
> fsmark-100m-empty-files-001        fsmark metadata scale create
> fsmark-100m-empty-files-002        fsmark metadata scale traverse
> fsmark-100m-empty-files-003        fsmark metadata scale unlink
> .....
> 
> If we put the unique number at the end (some-name-NNN), then this issue is eliminated. Of course, with this you can't do NNN<tab> for completion, but it keeps the number reference. But this way it makes harder to find the test by number...
> 

First of all, what's the point of the names if they are the same ?
And secondly what's the point of numbers if they repeat so often ?

This is probably only relevant to your performance patches, but can
you elaborate a bit more on how you plan to name the tests ? Because I am
not sure the example you've just shown is the best approach.

Also is there a reason for you to see it grouped by the name when
you do ls ? It's not like it'll help you run a group of the tests at
once.

-Lukas


> 
> ----- Original Message -----
> > From: "Lukáš Czerner" <lczerner@redhat.com>
> > Sent: Wednesday, 25 March, 2015 4:20:24 PM
> > 
> > 
> > Yes, I like that, but then we want to make sure that we do not have
> > tests with the same numbers, but different name. Also having more more
> > constrains on the names is a good thing especially when people feel like
> > being creative with test names.
> > 
> > So we can make it
> > 
> > NNN-test-name
> > 
> > where we only allow numbers in the first three characters, and only
> > alphabetic ASCII characters and a dash afterwards (or underscore,
> > whichever you prefer).
> > 
> > Thanks!
> > -Lukas
> 
> The stricter rules are all right, I agree with that too.
> 
> Jan
> 

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[Eryu Guan]

On Wed, Mar 25, 2015 at 02:27:35PM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/some-name")
> 
> The only limitation on a test name is no whitespace and no dot.
> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> ---
>  README |  2 +-
>  check  | 11 ++++++-----
>  new    | 50 +++++++++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 54 insertions(+), 9 deletions(-)
> 
> diff --git a/README b/README
> index 0c9449a..2376674 100644
> --- a/README
> +++ b/README
> @@ -205,7 +205,7 @@ Test script environment:
>  
>  Verified output:
>  
> -    Each test script has a numerical name, e.g. 007, and an associated
> +    Each test script has a name, e.g. 007, and an associated
>      verified output, e.g. 007.out.
>  
>      It is important that the verified output is deterministic, and
> diff --git a/check b/check
> index 0830e0c..da0bc31 100755
> --- a/check
> +++ b/check
> @@ -58,7 +58,7 @@ then
>      exit 1
>  fi
>  
> -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> +SUPPORTED_TESTS="[^\s.]\+"
>  SRC_GROUPS="generic shared"
>  export SRC_DIR="tests"
>  
> @@ -96,21 +96,22 @@ get_group_list()
>  		l=$(sed -n < $SRC_DIR/$d/group \
>  			-e 's/#.*//' \
>  			-e 's/$/ /' \
> -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
>  		grpl="$grpl $l"
>  	done
>  	echo $grpl
>  }
>  
> -# find all tests, excluding files that are test metadata such as group files.
> -# This assumes that tests are defined purely by alphanumeric filenames with no
> -# ".xyz" extensions in the name.
> +# Find all tests, excluding files that are test metadata such as group files.
> +# It matches test names against $SUPPORTED_TESTS defined at the top of this
> +# file.
>  get_all_tests()
>  {
>  	touch $tmp.list
>  	for d in $SRC_GROUPS $FSTYP; do
>  		ls $SRC_DIR/$d/* | \
>  			grep -v "\..*" | \
> +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
>  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
>  	done
>  }
> diff --git a/new b/new
> index d1f8939..6cf67a7 100755
> --- a/new
> +++ b/new
> @@ -84,8 +84,11 @@ eof=1
>  for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
>  do
>      line=$((line+1))
> -    if [ -z "$found" ] || [ "$found" == "#" ];then
> -	continue
> +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> +        continue
> +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> +        # this one is for tests not named by a number
> +        continue
>      fi
>      i=$((i+1))
>      id=`printf "%03d" $i`
> @@ -99,9 +102,50 @@ if [ $eof -eq 1 ]; then
>     i=$((i+1))
>     id=`printf "%03d" $i`
>  fi
> +auto_id=$id
>  
>  echo "Next test is $id"
>  
> +read -p "Do you want to use ANOTHER name? y,[n]: " -r
> +if [[ "$REPLY" =~ ^[Yy]$ ]]; then
> +    # get the new name from user
> +    id=""
> +    while [ "$id" = "" ]; do
> +        read -p "Enter the new name: "
> +        if [ "$REPLY" = "" ]; then
> +            echo "Can't use empty name. For canceling, use ctrl+c."
> +        elif [ -e "$tdir/$REPLY" ]; then
> +            echo "File '$REPLY' already exists, use another one."
> +            echo #
> +        elif echo "$REPLY" | grep -q "^[^\\s.]\+$"; then
> +            id="$REPLY"

seems basic regular expression of grep doesn't support \s, I entered
"some-test-001" as test name and it always tells me it's containing
whitespace/dot.

I changed the regex to [:space:] (found in grep manpage) and it works.

	elif echo "$REPLY" | grep -q "^[^[:space:].]\+$"; then

If so, you may want to double check SUPPORTED_TESTS, as it's used by
grep in function get_all_tests()

I found tests are not properly found by group. e.g. I added three new
tests called "001-hello-test" "a-first-test" "some-test-001", and added
them to "testgroup", but ./check -n -g testgroup listed wrong test names
(the names are truncated and didn't find some-test-001)

[root@hp-dl388eg8-01 xfstests]# ./check -n -g testgroup
FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/x86_64 hp-dl388eg8-01 4.0.0-rc4+
MKFS_OPTIONS  -- -f -bsize=4096 /dev/mapper/rhel_hp--dl388eg8--01-testlv2
MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/mapper/rhel_hp--dl388eg8--01-testlv2 /mnt/testarea/scratch

generic/001-hello-te
generic/a-fir

This is the diff of my generic/group file

diff --git a/tests/generic/group b/tests/generic/group
index b2f0680..49a8eed 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -4,6 +4,7 @@
 # - comment line before each group is "new" description
 #
 001 rw dir udf auto quick
+001-hello-test other testgroup
 002 metadata udf auto quick
 003 atime auto quick
 004 auto quick
@@ -74,6 +75,8 @@
 069 rw udf auto quick
 070 attr udf auto quick stress
 071 auto quick prealloc
+072 other
+073 other
 074 rw udf auto
 075 rw udf auto quick
 076 metadata rw udf auto quick stress
@@ -184,3 +187,6 @@
 323 auto aio stress
 324 auto fsr quick
 325 auto quick data log
+a-first-test other testgroup
+hello-world-002 other
+some-test-001 other testgroup

> +        else
> +            echo "Filename must not contain whitespaces and dots!"
> +            echo 

trailing whitespace in above line

> +        fi
> +    done
> +
> +    # now find where to insert this name
> +    eof=1
> +    line=0
> +    for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> +    do

this for loop has different code style, use

	for xxx; do
	done

too, as other places you write

> +        line=$((line+1))
> +        if [ -z "$found" ] || [ "$found" == "#" ]; then
> +            continue
> +        elif [[ "$found" > "$id" ]]; then
> +            eof=0
> +            break
> +        fi
> +    done
> +    if [ $eof -eq 1 ]; then
> +        # If place wasn't found, let $line be the end of the file
> +        line=$((line+1))
> +    fi
> +
> +fi

As above is a new code block(about reading in test name and finding the
right place to insert the new test), you can use tab to indent, not 4
spaces.

Thanks for your work!

Eryu
> +echo "Using '$id'."
> +
>  if [ -f $tdir/$id ]
>  then
>      echo "Error: test $id already exists!"
> @@ -115,7 +159,7 @@ year=`date +%Y`
>  
>  cat <<End-of-File >$tdir/$id
>  #! /bin/bash
> -# FS QA Test No. $id
> +# FS QA Test $id
>  #
>  # what am I here for?
>  #
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[Jan Tulak]

----- Original Message -----
> From: "Eryu Guan" <eguan@redhat.com>
> Sent: Wednesday, 25 March, 2015 6:09:38 PM
> 
> 
> seems basic regular expression of grep doesn't support \s, I entered
> "some-test-001" as test name and it always tells me it's containing
> whitespace/dot.
>  [...]
> 

In another thread under the patch, Lukáš proposed a stricter naming. I changed the regex to "^[a-zA-Z0-9-]\+$", so this is already fixed. There is a small debate of mandatory appending a unique number to the test, so all tests could be referred by it.
This also caused your second issue, with not finding/truncating the tests. You can notice that it truncated them on the place of "s" in the name.

Thanks. :-)

Jan

> 
> I found tests are not properly found by group. e.g. I added three new
> tests called "001-hello-test" "a-first-test" "some-test-001", and added
> them to "testgroup", but ./check -n -g testgroup listed wrong test names
> (the names are truncated and didn't find some-test-001)
> 
> [root@hp-dl388eg8-01 xfstests]# ./check -n -g testgroup
> FSTYP         -- xfs (non-debug)
> PLATFORM      -- Linux/x86_64 hp-dl388eg8-01 4.0.0-rc4+
> MKFS_OPTIONS  -- -f -bsize=4096 /dev/mapper/rhel_hp--dl388eg8--01-testlv2
> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0
> /dev/mapper/rhel_hp--dl388eg8--01-testlv2 /mnt/testarea/scratch
> 
> generic/001-hello-te
> generic/a-fir
> 
> This is the diff of my generic/group file
> 
> diff --git a/tests/generic/group b/tests/generic/group
> index b2f0680..49a8eed 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -4,6 +4,7 @@
>  # - comment line before each group is "new" description
>  #
>  001 rw dir udf auto quick
> +001-hello-test other testgroup
>  002 metadata udf auto quick
>  003 atime auto quick
>  004 auto quick
> @@ -74,6 +75,8 @@
>  069 rw udf auto quick
>  070 attr udf auto quick stress
>  071 auto quick prealloc
> +072 other
> +073 other
>  074 rw udf auto
>  075 rw udf auto quick
>  076 metadata rw udf auto quick stress
> @@ -184,3 +187,6 @@
>  323 auto aio stress
>  324 auto fsr quick
>  325 auto quick data log
> +a-first-test other testgroup
> +hello-world-002 other
> +some-test-001 other testgroup
> 
> > +        else
> > +            echo "Filename must not contain whitespaces and dots!"
> > +            echo
> 
> trailing whitespace in above line
> 
> > +        fi
> > +    done
> > +
> > +    # now find where to insert this name
> > +    eof=1
> > +    line=0
> > +    for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> > +    do
> 
> this for loop has different code style, use
> 
> 	for xxx; do
> 	done
> 
> too, as other places you write
> 
> > +        line=$((line+1))
> > +        if [ -z "$found" ] || [ "$found" == "#" ]; then
> > +            continue
> > +        elif [[ "$found" > "$id" ]]; then
> > +            eof=0
> > +            break
> > +        fi
> > +    done
> > +    if [ $eof -eq 1 ]; then
> > +        # If place wasn't found, let $line be the end of the file
> > +        line=$((line+1))
> > +    fi
> > +
> > +fi
> 
> As above is a new code block(about reading in test name and finding the
> right place to insert the new test), you can use tab to indent, not 4
> spaces.
> 
> Thanks for your work!
> 
> Eryu
> > +echo "Using '$id'."
> > +
> >  if [ -f $tdir/$id ]
> >  then
> >      echo "Error: test $id already exists!"
> > @@ -115,7 +159,7 @@ year=`date +%Y`
> >  
> >  cat <<End-of-File >$tdir/$id
> >  #! /bin/bash
> > -# FS QA Test No. $id
> > +# FS QA Test $id
> >  #
> >  # what am I here for?
> >  #
> > --
> > 2.1.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[Jan Tulak]

----- Original Message -----
> From: "Lukáš Czerner" <lczerner@redhat.com>
> To: "Jan Tulak" <jtulak@redhat.com>
> Cc: "David Sterba" <dsterba@suse.cz>, eguan@redhat.com, fstests@vger.kernel.org
> Sent: Wednesday, 25 March, 2015 4:43:22 PM
> Subject: Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.
> 
> First of all, what's the point of the names if they are the same ?
> And secondly what's the point of numbers if they repeat so often ?
> 
> This is probably only relevant to your performance patches, but can
> you elaborate a bit more on how you plan to name the tests ? Because I am
> not sure the example you've just shown is the best approach.
> 
> Also is there a reason for you to see it grouped by the name when
> you do ls ? It's not like it'll help you run a group of the tests at
> once.
> 
> -Lukas
> 

Rather than in "ls", I thought about seeing them grouped in the group file. But when I thought about it more, I found it pretty much useless for most of cases (as long as the test has all groups it should have). So I'm taking back my objections about it. 
You can call it a small scope creep with groups. :-)

I'm sending a patch with the unique IDs in front of the name.

Jan

[PATCH] fstests: Tests can use any name now, not 3 digits only.

[Jan Ťulák]

Tests can use any name now, not 3 digits only.
(e.g. a test can be named "tests/generic/001-some-name")

Names are limited to alphanumeric characters and dash and are always prefixed
with an unique id for easier identification of a specific patch.

Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---
 README |  2 +-
 check  | 11 ++++++-----
 new    | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/README b/README
index 0c9449a..2376674 100644
--- a/README
+++ b/README
@@ -205,7 +205,7 @@ Test script environment:
 
 Verified output:
 
-    Each test script has a numerical name, e.g. 007, and an associated
+    Each test script has a name, e.g. 007, and an associated
     verified output, e.g. 007.out.
 
     It is important that the verified output is deterministic, and
diff --git a/check b/check
index 0830e0c..2730284 100755
--- a/check
+++ b/check
@@ -58,7 +58,7 @@ then
     exit 1
 fi
 
-SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
+SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
 
@@ -96,21 +96,22 @@ get_group_list()
 		l=$(sed -n < $SRC_DIR/$d/group \
 			-e 's/#.*//' \
 			-e 's/$/ /' \
-			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
+			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
 		grpl="$grpl $l"
 	done
 	echo $grpl
 }
 
-# find all tests, excluding files that are test metadata such as group files.
-# This assumes that tests are defined purely by alphanumeric filenames with no
-# ".xyz" extensions in the name.
+# Find all tests, excluding files that are test metadata such as group files.
+# It matches test names against $SUPPORTED_TESTS defined at the top of this
+# file.
 get_all_tests()
 {
 	touch $tmp.list
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
+			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
 			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
diff --git a/new b/new
index d1f8939..ccfb424 100755
--- a/new
+++ b/new
@@ -25,6 +25,9 @@
 iam=new
 . ./common/rc
 
+SUPPORTED_TESTS="^[a-zA-Z0-9-]\+$"
+
+
 trap "rm -f /tmp/$$.; exit" 0 1 2 3 15
 
 _cleanup()
@@ -81,11 +84,14 @@ line=0
 eof=1
 [ -f "$tdir/group" ] || usage
 
-for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
+for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
 do
     line=$((line+1))
-    if [ -z "$found" ] || [ "$found" == "#" ];then
-	continue
+    if [ -z "$found" ] || [ "$found" == "#" ]; then
+        continue
+    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
+        # this one is for tests not named by a number
+        continue
     fi
     i=$((i+1))
     id=`printf "%03d" $i`
@@ -99,8 +105,49 @@ if [ $eof -eq 1 ]; then
    i=$((i+1))
    id=`printf "%03d" $i`
 fi
-
-echo "Next test is $id"
+auto_id=$id
+
+echo "Next test id is $id"
+
+read -p "Do you want to give a name to the test? y,[n]: " -r
+if [[ "$REPLY" =~ ^[Yy]$ ]]; then
+	# get the new name from user
+	name=""
+	while [ "$name" = "" ]; do
+		read -p "Enter the new name: "
+		if [ "$REPLY" = "" ]; then
+			echo "For canceling, use ctrl+c."
+		elif [ -e "$tdir/$REPLY" ]; then
+			echo "File '$REPLY' already exists, use another one."
+			echo #
+		elif echo "$REPLY" | grep -q "$SUPPORTED_TESTS"; then
+			name="$REPLY"
+		else
+			echo "Filename must contain only alphanumeric symbols and dash!"
+			echo "(Used regex: $SUPPORTED_TESTS)"
+			echo
+		fi
+	done
+
+	# now find where to insert this name
+	eof=1
+	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
+		foundId=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
+		line=$((line+1))
+		if [ -z "$found" ] || [ "$found" == "#" ]; then
+			continue
+		elif [[ "$found" > "$name" ]] || [ "$foundId" -gt "$id" ]; then
+			eof=0
+			break
+		fi
+	done
+	if [ $eof -eq 0 ]; then
+		# If place wasn't found, let $line be the end of the file
+		line=$((line-1))
+	fi
+	id="$id-$name"
+fi
+echo "Creating test file '$id'"
 
 if [ -f $tdir/$id ]
 then
@@ -115,7 +162,7 @@ year=`date +%Y`
 
 cat <<End-of-File >$tdir/$id
 #! /bin/bash
-# FS QA Test No. $id
+# FS QA Test $id
 #
 # what am I here for?
 #
-- 
2.1.0

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[David Sterba]

Please put a revision number in the subject so we know what's the latest
one, eg:

[PATCH v5] fstests: Tests can use any name now, not 3 digits only

On Thu, Mar 26, 2015 at 02:35:33PM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/001-some-name")
> 
> Names are limited to alphanumeric characters and dash and are always prefixed
> with an unique id for easier identification of a specific patch.

patch or test?

> --- a/README
> +++ b/README
> @@ -205,7 +205,7 @@ Test script environment:
>  
>  Verified output:
>  
> -    Each test script has a numerical name, e.g. 007, and an associated
> +    Each test script has a name, e.g. 007, and an associated
>      verified output, e.g. 007.out.

I think the naming scheme could be described in the README, probably in
the "Creating new tests scripts:" section.

> +if [[ "$REPLY" =~ ^[Yy]$ ]]; then

You don't need to quote variables in the [[ ]] block as it's a builtin,
unlike [ (in general). Also, [[ $variable = glob ]] does really match
globs, so it's simple [[ $REPLY = [yY] ]] if you insist on using [[ .

> +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> +		foundId=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
> +		line=$((line+1))
> +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> +			continue
> +		elif [[ "$found" > "$name" ]] || [ "$foundId" -gt "$id" ]; then

Bash guide advices not to use [[ ]] for arithmetic expressions, in favor
of (( )). Besides, I find mixing [[ ]] and [ ] inconsistent, choose one.

> +			eof=0
> +			break
> +		fi
> +	done

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[Jan Tulak]

----- Original Message -----
> From: "David Sterba" <dsterba@suse.cz>
> To: "Jan Ťulák" <jtulak@redhat.com>
> Cc: eguan@redhat.com, fstests@vger.kernel.org, lczerner@redhat.com
> Sent: Thursday, 26 March, 2015 3:41:09 PM
> Subject: Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.
> 
> Please put a revision number in the subject so we know what's the latest
> one, eg:
> 
> [PATCH v5] fstests: Tests can use any name now, not 3 digits only
> 
> On Thu, Mar 26, 2015 at 02:35:33PM +0100, Jan Ťulák wrote:
> > Tests can use any name now, not 3 digits only.
> > (e.g. a test can be named "tests/generic/001-some-name")
> > 
> > Names are limited to alphanumeric characters and dash and are always
> > prefixed
> > with an unique id for easier identification of a specific patch.
> 
> patch or test?

Fixed.


> 
> > --- a/README
> > +++ b/README
> > @@ -205,7 +205,7 @@ Test script environment:
> >  
> >  Verified output:
> >  
> > -    Each test script has a numerical name, e.g. 007, and an associated
> > +    Each test script has a name, e.g. 007, and an associated
> >      verified output, e.g. 007.out.
> 
> I think the naming scheme could be described in the README, probably in
> the "Creating new tests scripts:" section.

True, added into the readme.


> 
> > +if [[ "$REPLY" =~ ^[Yy]$ ]]; then
> 
> You don't need to quote variables in the [[ ]] block as it's a builtin,
> unlike [ (in general). Also, [[ $variable = glob ]] does really match
> globs, so it's simple [[ $REPLY = [yY] ]] if you insist on using [[ .
> 
Thank you for this info, I didn't knew this. :-)

> > +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> > +		foundId=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
> > +		line=$((line+1))
> > +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> > +			continue
> > +		elif [[ "$found" > "$name" ]] || [ "$foundId" -gt "$id" ]; then
> 
> Bash guide advices not to use [[ ]] for arithmetic expressions, in favor
> of (( )). Besides, I find mixing [[ ]] and [ ] inconsistent, choose one.

The [[ "$found" > "$name" ]] is a string expression, for lexicographic ordering. :-)
The second [ ] is for arithmetic - so I used the mix of [[]] and [] deliberately to distinguish it. Though if it is a bad practise, I will stick to not mixing it in one condition.

Jan

[PATCH v6] fstests: Tests can use any name now, not 3 digits only.

[Jan Ťulák]

Tests can use any name now, not 3 digits only.
(e.g. a test can be named "tests/generic/001-some-name")

Names are limited to alphanumeric characters and dash and are always prefixed
with an unique id for easier identification of a specific test.

Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---
 README |  7 ++++++-
 check  | 11 ++++++-----
 new    | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/README b/README
index 0c9449a..1841052 100644
--- a/README
+++ b/README
@@ -202,10 +202,15 @@ Test script environment:
 	- When calling getfacl in a test, pass the "-n" argument so
 	  that numeric rather than symbolic identifiers are used in
 	  the output.
+	- When creating a new test, it is possible to enter a custom name
+	  for the file. Filenames are in form NNN-custom-name, where NNN
+	  is automatically added by the ./new script as an unique ID,
+	  and "custom-name" is the optional string entered into a prompt
+	  in the ./new script. Note the "NNN-" part is added automatically.
 
 Verified output:
 
-    Each test script has a numerical name, e.g. 007, and an associated
+    Each test script has a name, e.g. 007, and an associated
     verified output, e.g. 007.out.
 
     It is important that the verified output is deterministic, and
diff --git a/check b/check
index 0830e0c..2730284 100755
--- a/check
+++ b/check
@@ -58,7 +58,7 @@ then
     exit 1
 fi
 
-SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
+SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
 
@@ -96,21 +96,22 @@ get_group_list()
 		l=$(sed -n < $SRC_DIR/$d/group \
 			-e 's/#.*//' \
 			-e 's/$/ /' \
-			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
+			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
 		grpl="$grpl $l"
 	done
 	echo $grpl
 }
 
-# find all tests, excluding files that are test metadata such as group files.
-# This assumes that tests are defined purely by alphanumeric filenames with no
-# ".xyz" extensions in the name.
+# Find all tests, excluding files that are test metadata such as group files.
+# It matches test names against $SUPPORTED_TESTS defined at the top of this
+# file.
 get_all_tests()
 {
 	touch $tmp.list
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
+			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
 			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
diff --git a/new b/new
index d1f8939..4ff71d3 100755
--- a/new
+++ b/new
@@ -25,6 +25,9 @@
 iam=new
 . ./common/rc
 
+SUPPORTED_TESTS="^[a-zA-Z0-9-]\+$"
+
+
 trap "rm -f /tmp/$$.; exit" 0 1 2 3 15
 
 _cleanup()
@@ -81,11 +84,14 @@ line=0
 eof=1
 [ -f "$tdir/group" ] || usage
 
-for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
+for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
 do
     line=$((line+1))
-    if [ -z "$found" ] || [ "$found" == "#" ];then
-	continue
+    if [ -z "$found" ] || [ "$found" == "#" ]; then
+        continue
+    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
+        # this one is for tests not named by a number
+        continue
     fi
     i=$((i+1))
     id=`printf "%03d" $i`
@@ -99,8 +105,49 @@ if [ $eof -eq 1 ]; then
    i=$((i+1))
    id=`printf "%03d" $i`
 fi
-
-echo "Next test is $id"
+auto_id=$id
+
+echo "Next test id is $id"
+
+read -p "Do you want to give a name to the test? y,[n]: " -r
+if [[ $REPLY = [Yy] ]]; then
+	# get the new name from user
+	name=""
+	while [ "$name" = "" ]; do
+		read -p "Enter the new name: "
+		if [ "$REPLY" = "" ]; then
+			echo "For canceling, use ctrl+c."
+		elif [ -e "$tdir/$REPLY" ]; then
+			echo "File '$REPLY' already exists, use another one."
+			echo #
+		elif echo "$REPLY" | grep -q "$SUPPORTED_TESTS"; then
+			name="$REPLY"
+		else
+			echo "Filename must contain only alphanumeric symbols and dash!"
+			echo "(Used regex: $SUPPORTED_TESTS)"
+			echo
+		fi
+	done
+
+	# now find where to insert this name
+	eof=1
+	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
+		foundId=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
+		line=$((line+1))
+		if [ -z "$found" ] || [ "$found" == "#" ]; then
+			continue
+		elif [[ $found > $name ]] || (( 10#$foundId > 10#$id )); then
+			eof=0
+			break
+		fi
+	done
+	if [ $eof -eq 0 ]; then
+		# If place wasn't found, let $line be the end of the file
+		line=$((line-1))
+	fi
+	id="$id-$name"
+fi
+echo "Creating test file '$id'"
 
 if [ -f $tdir/$id ]
 then
@@ -115,7 +162,7 @@ year=`date +%Y`
 
 cat <<End-of-File >$tdir/$id
 #! /bin/bash
-# FS QA Test No. $id
+# FS QA Test $id
 #
 # what am I here for?
 #
-- 
2.1.0

Re: [PATCH] fstests: Tests can use any name now, not 3 digits only.

[David Sterba]

On Thu, Mar 26, 2015 at 11:16:02AM -0400, Jan Tulak wrote:
> > > +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> > > +		foundId=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
> > > +		line=$((line+1))
> > > +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> > > +			continue
> > > +		elif [[ "$found" > "$name" ]] || [ "$foundId" -gt "$id" ]; then
> > 
> > Bash guide advices not to use [[ ]] for arithmetic expressions, in favor
> > of (( )). Besides, I find mixing [[ ]] and [ ] inconsistent, choose one.
> 
> The [[ "$found" > "$name" ]] is a string expression, for lexicographic ordering. :-)

Ah, I see, then it's correct.

> The second [ ] is for arithmetic - so I used the mix of [[]] and []
> deliberately to distinguish it. Though if it is a bad practise, I will
> stick to not mixing it in one condition.

And [ -gt ] is ok.

Re: [PATCH v6] fstests: Tests can use any name now, not 3 digits only.

[Eryu Guan]

On Thu, Mar 26, 2015 at 04:33:11PM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/001-some-name")
> 
> Names are limited to alphanumeric characters and dash and are always prefixed
> with an unique id for easier identification of a specific test.
> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> ---

Besides the version number in summary, it's better to put changelog
under the "---", so it's clear what has been changed from last version

>  README |  7 ++++++-
>  check  | 11 ++++++-----
>  new    | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  3 files changed, 65 insertions(+), 12 deletions(-)
> 
> diff --git a/README b/README
> index 0c9449a..1841052 100644
> --- a/README
> +++ b/README
> @@ -202,10 +202,15 @@ Test script environment:
>  	- When calling getfacl in a test, pass the "-n" argument so
>  	  that numeric rather than symbolic identifiers are used in
>  	  the output.
> +	- When creating a new test, it is possible to enter a custom name
> +	  for the file. Filenames are in form NNN-custom-name, where NNN
> +	  is automatically added by the ./new script as an unique ID,
> +	  and "custom-name" is the optional string entered into a prompt
> +	  in the ./new script. Note the "NNN-" part is added automatically.
>  
>  Verified output:
>  
> -    Each test script has a numerical name, e.g. 007, and an associated
> +    Each test script has a name, e.g. 007, and an associated
>      verified output, e.g. 007.out.
>  
>      It is important that the verified output is deterministic, and
> diff --git a/check b/check
> index 0830e0c..2730284 100755
> --- a/check
> +++ b/check
> @@ -58,7 +58,7 @@ then
>      exit 1
>  fi
>  
> -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> +SUPPORTED_TESTS="[a-zA-Z0-9-]\+"

I see this is used in both check and new, is it possible to put it in
common/rc, then define it once there?

>  SRC_GROUPS="generic shared"
>  export SRC_DIR="tests"
>  
> @@ -96,21 +96,22 @@ get_group_list()
>  		l=$(sed -n < $SRC_DIR/$d/group \
>  			-e 's/#.*//' \
>  			-e 's/$/ /' \
> -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
>  		grpl="$grpl $l"
>  	done
>  	echo $grpl
>  }
>  
> -# find all tests, excluding files that are test metadata such as group files.
> -# This assumes that tests are defined purely by alphanumeric filenames with no
> -# ".xyz" extensions in the name.
> +# Find all tests, excluding files that are test metadata such as group files.
> +# It matches test names against $SUPPORTED_TESTS defined at the top of this
> +# file.
>  get_all_tests()
>  {
>  	touch $tmp.list
>  	for d in $SRC_GROUPS $FSTYP; do
>  		ls $SRC_DIR/$d/* | \
>  			grep -v "\..*" | \
> +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
>  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
>  	done
>  }
> diff --git a/new b/new
> index d1f8939..4ff71d3 100755
> --- a/new
> +++ b/new
> @@ -25,6 +25,9 @@
>  iam=new
>  . ./common/rc
>  
> +SUPPORTED_TESTS="^[a-zA-Z0-9-]\+$"
> +
> +
>  trap "rm -f /tmp/$$.; exit" 0 1 2 3 15
>  
>  _cleanup()
> @@ -81,11 +84,14 @@ line=0
>  eof=1
>  [ -f "$tdir/group" ] || usage
>  
> -for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> +for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
>  do
>      line=$((line+1))
> -    if [ -z "$found" ] || [ "$found" == "#" ];then
> -	continue
> +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> +        continue
> +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> +        # this one is for tests not named by a number
> +        continue
>      fi
>      i=$((i+1))
>      id=`printf "%03d" $i`
> @@ -99,8 +105,49 @@ if [ $eof -eq 1 ]; then
>     i=$((i+1))
>     id=`printf "%03d" $i`
>  fi
> -
> -echo "Next test is $id"
> +auto_id=$id

Seems auto_id is not used anywhere.

> +
> +echo "Next test id is $id"
> +
> +read -p "Do you want to give a name to the test? y,[n]: " -r
> +if [[ $REPLY = [Yy] ]]; then
> +	# get the new name from user
> +	name=""
> +	while [ "$name" = "" ]; do
> +		read -p "Enter the new name: "

This prompt is kind of misleading now, because it's appending test name
to the current id.

> +		if [ "$REPLY" = "" ]; then
> +			echo "For canceling, use ctrl+c."
> +		elif [ -e "$tdir/$REPLY" ]; then

test "$tdir/$id-$REPLY"?

> +			echo "File '$REPLY' already exists, use another one."
> +			echo #
> +		elif echo "$REPLY" | grep -q "$SUPPORTED_TESTS"; then
> +			name="$REPLY"
> +		else
> +			echo "Filename must contain only alphanumeric symbols and dash!"
> +			echo "(Used regex: $SUPPORTED_TESTS)"
> +			echo
> +		fi
> +	done
> +
> +	# now find where to insert this name
> +	eof=1
> +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> +		foundId=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')

foundid? there's no other var name in camel-case style

> +		line=$((line+1))
> +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> +			continue
> +		elif [[ $found > $name ]] || (( 10#$foundId > 10#$id )); then

(( 10#$foundId > 10#$id )) part is tricky, I have to lookup the bash
manpage to understand it :)

10 is the default base, the "10#" part can be omitted.

But I think simple [ $foundId -gt $id ] is better, and David Sterba is
OK with it too now (in another reply) :-)


Thank you so much for continuous improvements!

Eryu
> +			eof=0
> +			break
> +		fi
> +	done
> +	if [ $eof -eq 0 ]; then
> +		# If place wasn't found, let $line be the end of the file
> +		line=$((line-1))
> +	fi
> +	id="$id-$name"
> +fi
> +echo "Creating test file '$id'"
>  
>  if [ -f $tdir/$id ]
>  then
> @@ -115,7 +162,7 @@ year=`date +%Y`
>  
>  cat <<End-of-File >$tdir/$id
>  #! /bin/bash
> -# FS QA Test No. $id
> +# FS QA Test $id
>  #
>  # what am I here for?
>  #
> -- 
> 2.1.0
> 

Re: [PATCH v6] fstests: Tests can use any name now, not 3 digits only.

[Jan Tulak]

----- Original Message -----
> From: "Eryu Guan" <eguan@redhat.com>
> To: "Jan Ťulák" <jtulak@redhat.com>
> Cc: fstests@vger.kernel.org, lczerner@redhat.com, dsterba@suse.cz
> Sent: Friday, 27 March, 2015 8:25:39 AM
> Subject: Re: [PATCH v6] fstests: Tests can use any name now, not 3 digits only.
> 
> [...]
> 
> (( 10#$foundId > 10#$id )) part is tricky, I have to lookup the bash
> manpage to understand it :)
> 
> 10 is the default base, the "10#" part can be omitted.
> 
> But I think simple [ $foundId -gt $id ] is better, and David Sterba is
> OK with it too now (in another reply) :-)
> 
> 
> Thank you so much for continuous improvements!
> 
> Eryu

The 10 base was necessary here, because as I found, (( )) takes numbers beginning with zeroes as octal. So it complained about tests like 008, 009, 019, 029... The [ ] does not complain, so I will put it back here. :-)

And thanks for patience. :-)
Jan

[PATCH v7] fstests: Tests can use any name now, not 3 digits only.

[Jan Ťulák]

Tests can use any name now, not 3 digits only.
(e.g. a test can be named "tests/generic/001-some-name")

Names are limited to alphanumeric characters and dash and are always prefixed
with an unique id for easier identification of a specific test.

Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---

 Changes in this patch version:

 SUPPORTED_TESTS regex moved into common/rc
 removed unused variable auto_id
 prompt for name changed to be less confusing
 fixed test for existing file
 foundId -> found_id
 (( expression )) -> [ expression ]


 README    |  7 ++++++-
 check     | 10 +++++-----
 common/rc |  1 +
 new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/README b/README
index 0c9449a..1841052 100644
--- a/README
+++ b/README
@@ -202,10 +202,15 @@ Test script environment:
 	- When calling getfacl in a test, pass the "-n" argument so
 	  that numeric rather than symbolic identifiers are used in
 	  the output.
+	- When creating a new test, it is possible to enter a custom name
+	  for the file. Filenames are in form NNN-custom-name, where NNN
+	  is automatically added by the ./new script as an unique ID,
+	  and "custom-name" is the optional string entered into a prompt
+	  in the ./new script. Note the "NNN-" part is added automatically.
 
 Verified output:
 
-    Each test script has a numerical name, e.g. 007, and an associated
+    Each test script has a name, e.g. 007, and an associated
     verified output, e.g. 007.out.
 
     It is important that the verified output is deterministic, and
diff --git a/check b/check
index 0830e0c..b905259 100755
--- a/check
+++ b/check
@@ -58,7 +58,6 @@ then
     exit 1
 fi
 
-SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
 
@@ -96,21 +95,22 @@ get_group_list()
 		l=$(sed -n < $SRC_DIR/$d/group \
 			-e 's/#.*//' \
 			-e 's/$/ /' \
-			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
+			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
 		grpl="$grpl $l"
 	done
 	echo $grpl
 }
 
-# find all tests, excluding files that are test metadata such as group files.
-# This assumes that tests are defined purely by alphanumeric filenames with no
-# ".xyz" extensions in the name.
+# Find all tests, excluding files that are test metadata such as group files.
+# It matches test names against $SUPPORTED_TESTS defined at the top of this
+# file.
 get_all_tests()
 {
 	touch $tmp.list
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
+			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
 			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
diff --git a/common/rc b/common/rc
index 857308a..cc9a894 100644
--- a/common/rc
+++ b/common/rc
@@ -21,6 +21,7 @@
 #-----------------------------------------------------------------------
 
 BC=$(which bc 2> /dev/null) || BC=
+SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
 
 _require_math()
 {
diff --git a/new b/new
index d1f8939..4257e00 100755
--- a/new
+++ b/new
@@ -81,11 +81,14 @@ line=0
 eof=1
 [ -f "$tdir/group" ] || usage
 
-for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
+for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
 do
     line=$((line+1))
-    if [ -z "$found" ] || [ "$found" == "#" ];then
-	continue
+    if [ -z "$found" ] || [ "$found" == "#" ]; then
+        continue
+    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
+        # this one is for tests not named by a number
+        continue
     fi
     i=$((i+1))
     id=`printf "%03d" $i`
@@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
    id=`printf "%03d" $i`
 fi
 
-echo "Next test is $id"
+echo "Next test id is $id"
+
+read -p "Do you want to append a name to the ID? y,[n]: " -r
+if [[ $REPLY = [Yy] ]]; then
+	# get the new name from user
+	name=""
+	while [ "$name" = "" ]; do
+		read -p "Enter the new name: "
+		if [ "$REPLY" = "" ]; then
+			echo "For canceling, use ctrl+c."
+		elif [ -e "$tdir/$id-$REPLY" ]; then
+			echo "File '$id-$REPLY' already exists, use another one."
+			echo #
+		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
+			name="$REPLY"
+		else
+			echo "Filename must contain only alphanumeric symbols and dash!"
+			echo "(Used regex: ^$SUPPORTED_TESTS$)"
+			echo
+		fi
+	done
+
+	# now find where to insert this name
+	eof=1
+	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
+		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
+		line=$((line+1))
+		if [ -z "$found" ] || [ "$found" == "#" ]; then
+			continue
+		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then
+			eof=0
+			break
+		fi
+	done
+	if [ $eof -eq 0 ]; then
+		# If place wasn't found, let $line be the end of the file
+		line=$((line-1))
+	fi
+	id="$id-$name"
+fi
+echo "Creating test file '$id'"
 
 if [ -f $tdir/$id ]
 then
@@ -115,7 +158,7 @@ year=`date +%Y`
 
 cat <<End-of-File >$tdir/$id
 #! /bin/bash
-# FS QA Test No. $id
+# FS QA Test $id
 #
 # what am I here for?
 #
-- 
2.1.0

Re: [PATCH v6] fstests: Tests can use any name now, not 3 digits only.

[Eryu Guan]

On Fri, Mar 27, 2015 at 05:15:33AM -0400, Jan Tulak wrote:
> 
> 
> ----- Original Message -----
> > From: "Eryu Guan" <eguan@redhat.com>
> > To: "Jan Ťulák" <jtulak@redhat.com>
> > Cc: fstests@vger.kernel.org, lczerner@redhat.com, dsterba@suse.cz
> > Sent: Friday, 27 March, 2015 8:25:39 AM
> > Subject: Re: [PATCH v6] fstests: Tests can use any name now, not 3 digits only.
> > 
> > [...]
> > 
> > (( 10#$foundId > 10#$id )) part is tricky, I have to lookup the bash
> > manpage to understand it :)
> > 
> > 10 is the default base, the "10#" part can be omitted.
> > 
> > But I think simple [ $foundId -gt $id ] is better, and David Sterba is
> > OK with it too now (in another reply) :-)
> > 
> > 
> > Thank you so much for continuous improvements!
> > 
> > Eryu
> 
> The 10 base was necessary here, because as I found, (( )) takes numbers beginning with zeroes as octal. So it complained about tests like 008, 009, 019, 029... The [ ] does not complain, so I will put it back here. :-)

Ah, I missed that, lesson learned.

Thanks,
Eryu
> 
> And thanks for patience. :-)
> Jan
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only.

[Eryu Guan]

On Fri, Mar 27, 2015 at 10:15:48AM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/001-some-name")
> 
> Names are limited to alphanumeric characters and dash and are always prefixed
> with an unique id for easier identification of a specific test.
> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> ---
> 
>  Changes in this patch version:
> 
>  SUPPORTED_TESTS regex moved into common/rc
>  removed unused variable auto_id
>  prompt for name changed to be less confusing
>  fixed test for existing file
>  foundId -> found_id
>  (( expression )) -> [ expression ]
> 
> 
>  README    |  7 ++++++-
>  check     | 10 +++++-----
>  common/rc |  1 +
>  new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  4 files changed, 60 insertions(+), 11 deletions(-)
> 
> diff --git a/README b/README
> index 0c9449a..1841052 100644
> --- a/README
> +++ b/README
> @@ -202,10 +202,15 @@ Test script environment:
>  	- When calling getfacl in a test, pass the "-n" argument so
>  	  that numeric rather than symbolic identifiers are used in
>  	  the output.
> +	- When creating a new test, it is possible to enter a custom name
> +	  for the file. Filenames are in form NNN-custom-name, where NNN
> +	  is automatically added by the ./new script as an unique ID,
> +	  and "custom-name" is the optional string entered into a prompt
> +	  in the ./new script. Note the "NNN-" part is added automatically.
>  
>  Verified output:
>  
> -    Each test script has a numerical name, e.g. 007, and an associated
> +    Each test script has a name, e.g. 007, and an associated
>      verified output, e.g. 007.out.
>  
>      It is important that the verified output is deterministic, and
> diff --git a/check b/check
> index 0830e0c..b905259 100755
> --- a/check
> +++ b/check
> @@ -58,7 +58,6 @@ then
>      exit 1
>  fi
>  
> -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
>  SRC_GROUPS="generic shared"
>  export SRC_DIR="tests"
>  
> @@ -96,21 +95,22 @@ get_group_list()
>  		l=$(sed -n < $SRC_DIR/$d/group \
>  			-e 's/#.*//' \
>  			-e 's/$/ /' \
> -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
>  		grpl="$grpl $l"
>  	done
>  	echo $grpl
>  }
>  
> -# find all tests, excluding files that are test metadata such as group files.
> -# This assumes that tests are defined purely by alphanumeric filenames with no
> -# ".xyz" extensions in the name.
> +# Find all tests, excluding files that are test metadata such as group files.
> +# It matches test names against $SUPPORTED_TESTS defined at the top of this
> +# file.
>  get_all_tests()
>  {
>  	touch $tmp.list
>  	for d in $SRC_GROUPS $FSTYP; do
>  		ls $SRC_DIR/$d/* | \
>  			grep -v "\..*" | \
> +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
>  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
>  	done
>  }
> diff --git a/common/rc b/common/rc
> index 857308a..cc9a894 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -21,6 +21,7 @@
>  #-----------------------------------------------------------------------
>  
>  BC=$(which bc 2> /dev/null) || BC=
> +SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
>  
>  _require_math()
>  {
> diff --git a/new b/new
> index d1f8939..4257e00 100755
> --- a/new
> +++ b/new
> @@ -81,11 +81,14 @@ line=0
>  eof=1
>  [ -f "$tdir/group" ] || usage
>  
> -for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> +for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
>  do
>      line=$((line+1))
> -    if [ -z "$found" ] || [ "$found" == "#" ];then
> -	continue
> +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> +        continue
> +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> +        # this one is for tests not named by a number
> +        continue
>      fi
>      i=$((i+1))
>      id=`printf "%03d" $i`
> @@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
>     id=`printf "%03d" $i`
>  fi
>  
> -echo "Next test is $id"
> +echo "Next test id is $id"
> +
> +read -p "Do you want to append a name to the ID? y,[n]: " -r
> +if [[ $REPLY = [Yy] ]]; then
> +	# get the new name from user
> +	name=""
> +	while [ "$name" = "" ]; do
> +		read -p "Enter the new name: "

"new name" here still seems confusing to me. Otherwise look good to me.

Reviewed-by: Eryu Guan <eguan@redhat.com>

Thank you!

Eryu
> +		if [ "$REPLY" = "" ]; then
> +			echo "For canceling, use ctrl+c."
> +		elif [ -e "$tdir/$id-$REPLY" ]; then
> +			echo "File '$id-$REPLY' already exists, use another one."
> +			echo #
> +		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
> +			name="$REPLY"
> +		else
> +			echo "Filename must contain only alphanumeric symbols and dash!"
> +			echo "(Used regex: ^$SUPPORTED_TESTS$)"
> +			echo
> +		fi
> +	done
> +
> +	# now find where to insert this name
> +	eof=1
> +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> +		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
> +		line=$((line+1))
> +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> +			continue
> +		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then
> +			eof=0
> +			break
> +		fi
> +	done
> +	if [ $eof -eq 0 ]; then
> +		# If place wasn't found, let $line be the end of the file
> +		line=$((line-1))
> +	fi
> +	id="$id-$name"
> +fi
> +echo "Creating test file '$id'"
>  
>  if [ -f $tdir/$id ]
>  then
> @@ -115,7 +158,7 @@ year=`date +%Y`
>  
>  cat <<End-of-File >$tdir/$id
>  #! /bin/bash
> -# FS QA Test No. $id
> +# FS QA Test $id
>  #
>  # what am I here for?
>  #
> -- 
> 2.1.0
> 

Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only.

[Jan Tulak]

----- Original Message -----
> From: "Eryu Guan" <eguan@redhat.com>
> To: "Jan Ťulák" <jtulak@redhat.com>
> Cc: fstests@vger.kernel.org, lczerner@redhat.com, dsterba@suse.cz
> Sent: Friday, 27 March, 2015 10:39:40 AM
> Subject: Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only.
> 
> On Fri, Mar 27, 2015 at 10:15:48AM +0100, Jan Ťulák wrote:
> > Tests can use any name now, not 3 digits only.
> > (e.g. a test can be named "tests/generic/001-some-name")
> > 
> > Names are limited to alphanumeric characters and dash and are always
> > prefixed
> > with an unique id for easier identification of a specific test.
> > 
> > Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> > ---
> > 
> >  Changes in this patch version:
> > 
> >  SUPPORTED_TESTS regex moved into common/rc
> >  removed unused variable auto_id
> >  prompt for name changed to be less confusing
> >  fixed test for existing file
> >  foundId -> found_id
> >  (( expression )) -> [ expression ]
> > 
> > 
> >  README    |  7 ++++++-
> >  check     | 10 +++++-----
> >  common/rc |  1 +
> >  new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  4 files changed, 60 insertions(+), 11 deletions(-)
> > 
> > diff --git a/README b/README
> > index 0c9449a..1841052 100644
> > --- a/README
> > +++ b/README
> > @@ -202,10 +202,15 @@ Test script environment:
> >  	- When calling getfacl in a test, pass the "-n" argument so
> >  	  that numeric rather than symbolic identifiers are used in
> >  	  the output.
> > +	- When creating a new test, it is possible to enter a custom name
> > +	  for the file. Filenames are in form NNN-custom-name, where NNN
> > +	  is automatically added by the ./new script as an unique ID,
> > +	  and "custom-name" is the optional string entered into a prompt
> > +	  in the ./new script. Note the "NNN-" part is added automatically.
> >  
> >  Verified output:
> >  
> > -    Each test script has a numerical name, e.g. 007, and an associated
> > +    Each test script has a name, e.g. 007, and an associated
> >      verified output, e.g. 007.out.
> >  
> >      It is important that the verified output is deterministic, and
> > diff --git a/check b/check
> > index 0830e0c..b905259 100755
> > --- a/check
> > +++ b/check
> > @@ -58,7 +58,6 @@ then
> >      exit 1
> >  fi
> >  
> > -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> >  SRC_GROUPS="generic shared"
> >  export SRC_DIR="tests"
> >  
> > @@ -96,21 +95,22 @@ get_group_list()
> >  		l=$(sed -n < $SRC_DIR/$d/group \
> >  			-e 's/#.*//' \
> >  			-e 's/$/ /' \
> > -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> > +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
> >  		grpl="$grpl $l"
> >  	done
> >  	echo $grpl
> >  }
> >  
> > -# find all tests, excluding files that are test metadata such as group
> > files.
> > -# This assumes that tests are defined purely by alphanumeric filenames
> > with no
> > -# ".xyz" extensions in the name.
> > +# Find all tests, excluding files that are test metadata such as group
> > files.
> > +# It matches test names against $SUPPORTED_TESTS defined at the top of
> > this
> > +# file.
> >  get_all_tests()
> >  {
> >  	touch $tmp.list
> >  	for d in $SRC_GROUPS $FSTYP; do
> >  		ls $SRC_DIR/$d/* | \
> >  			grep -v "\..*" | \
> > +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
> >  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
> >  	done
> >  }
> > diff --git a/common/rc b/common/rc
> > index 857308a..cc9a894 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -21,6 +21,7 @@
> >  #-----------------------------------------------------------------------
> >  
> >  BC=$(which bc 2> /dev/null) || BC=
> > +SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
> >  
> >  _require_math()
> >  {
> > diff --git a/new b/new
> > index d1f8939..4257e00 100755
> > --- a/new
> > +++ b/new
> > @@ -81,11 +81,14 @@ line=0
> >  eof=1
> >  [ -f "$tdir/group" ] || usage
> >  
> > -for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> > +for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
> >  do
> >      line=$((line+1))
> > -    if [ -z "$found" ] || [ "$found" == "#" ];then
> > -	continue
> > +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> > +        continue
> > +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> > +        # this one is for tests not named by a number
> > +        continue
> >      fi
> >      i=$((i+1))
> >      id=`printf "%03d" $i`
> > @@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
> >     id=`printf "%03d" $i`
> >  fi
> >  
> > -echo "Next test is $id"
> > +echo "Next test id is $id"
> > +
> > +read -p "Do you want to append a name to the ID? y,[n]: " -r
> > +if [[ $REPLY = [Yy] ]]; then
> > +	# get the new name from user
> > +	name=""
> > +	while [ "$name" = "" ]; do
> > +		read -p "Enter the new name: "
> 
> "new name" here still seems confusing to me. Otherwise look good to me.
> 
> Reviewed-by: Eryu Guan <eguan@redhat.com>
> 
> Thank you!
> 
> Eryu

So perhaps just remove the "new" and leave only the "name"?
Should I send one more patch for it?

Jan

> > +		if [ "$REPLY" = "" ]; then
> > +			echo "For canceling, use ctrl+c."
> > +		elif [ -e "$tdir/$id-$REPLY" ]; then
> > +			echo "File '$id-$REPLY' already exists, use another one."
> > +			echo #
> > +		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
> > +			name="$REPLY"
> > +		else
> > +			echo "Filename must contain only alphanumeric symbols and dash!"
> > +			echo "(Used regex: ^$SUPPORTED_TESTS$)"
> > +			echo
> > +		fi
> > +	done
> > +
> > +	# now find where to insert this name
> > +	eof=1
> > +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> > +		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
> > +		line=$((line+1))
> > +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> > +			continue
> > +		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then
> > +			eof=0
> > +			break
> > +		fi
> > +	done
> > +	if [ $eof -eq 0 ]; then
> > +		# If place wasn't found, let $line be the end of the file
> > +		line=$((line-1))
> > +	fi
> > +	id="$id-$name"
> > +fi
> > +echo "Creating test file '$id'"
> >  
> >  if [ -f $tdir/$id ]
> >  then
> > @@ -115,7 +158,7 @@ year=`date +%Y`
> >  
> >  cat <<End-of-File >$tdir/$id
> >  #! /bin/bash
> > -# FS QA Test No. $id
> > +# FS QA Test $id
> >  #
> >  # what am I here for?
> >  #
> > --
> > 2.1.0
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only.

[Eryu Guan]

On Fri, Mar 27, 2015 at 05:48:23AM -0400, Jan Tulak wrote:
> 
> 
> ----- Original Message -----
> > From: "Eryu Guan" <eguan@redhat.com>
> > To: "Jan Ťulák" <jtulak@redhat.com>
> > Cc: fstests@vger.kernel.org, lczerner@redhat.com, dsterba@suse.cz
> > Sent: Friday, 27 March, 2015 10:39:40 AM
> > Subject: Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only.
> > 
> > On Fri, Mar 27, 2015 at 10:15:48AM +0100, Jan Ťulák wrote:
> > > Tests can use any name now, not 3 digits only.
> > > (e.g. a test can be named "tests/generic/001-some-name")
> > > 
> > > Names are limited to alphanumeric characters and dash and are always
> > > prefixed
> > > with an unique id for easier identification of a specific test.
> > > 
> > > Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> > > ---
> > > 
> > >  Changes in this patch version:
> > > 
> > >  SUPPORTED_TESTS regex moved into common/rc
> > >  removed unused variable auto_id
> > >  prompt for name changed to be less confusing
> > >  fixed test for existing file
> > >  foundId -> found_id
> > >  (( expression )) -> [ expression ]
> > > 
> > > 
> > >  README    |  7 ++++++-
> > >  check     | 10 +++++-----
> > >  common/rc |  1 +
> > >  new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> > >  4 files changed, 60 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/README b/README
> > > index 0c9449a..1841052 100644
> > > --- a/README
> > > +++ b/README
> > > @@ -202,10 +202,15 @@ Test script environment:
> > >  	- When calling getfacl in a test, pass the "-n" argument so
> > >  	  that numeric rather than symbolic identifiers are used in
> > >  	  the output.
> > > +	- When creating a new test, it is possible to enter a custom name
> > > +	  for the file. Filenames are in form NNN-custom-name, where NNN
> > > +	  is automatically added by the ./new script as an unique ID,
> > > +	  and "custom-name" is the optional string entered into a prompt
> > > +	  in the ./new script. Note the "NNN-" part is added automatically.
> > >  
> > >  Verified output:
> > >  
> > > -    Each test script has a numerical name, e.g. 007, and an associated
> > > +    Each test script has a name, e.g. 007, and an associated
> > >      verified output, e.g. 007.out.
> > >  
> > >      It is important that the verified output is deterministic, and
> > > diff --git a/check b/check
> > > index 0830e0c..b905259 100755
> > > --- a/check
> > > +++ b/check
> > > @@ -58,7 +58,6 @@ then
> > >      exit 1
> > >  fi
> > >  
> > > -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> > >  SRC_GROUPS="generic shared"
> > >  export SRC_DIR="tests"
> > >  
> > > @@ -96,21 +95,22 @@ get_group_list()
> > >  		l=$(sed -n < $SRC_DIR/$d/group \
> > >  			-e 's/#.*//' \
> > >  			-e 's/$/ /' \
> > > -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> > > +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
> > >  		grpl="$grpl $l"
> > >  	done
> > >  	echo $grpl
> > >  }
> > >  
> > > -# find all tests, excluding files that are test metadata such as group
> > > files.
> > > -# This assumes that tests are defined purely by alphanumeric filenames
> > > with no
> > > -# ".xyz" extensions in the name.
> > > +# Find all tests, excluding files that are test metadata such as group
> > > files.
> > > +# It matches test names against $SUPPORTED_TESTS defined at the top of
> > > this
> > > +# file.
> > >  get_all_tests()
> > >  {
> > >  	touch $tmp.list
> > >  	for d in $SRC_GROUPS $FSTYP; do
> > >  		ls $SRC_DIR/$d/* | \
> > >  			grep -v "\..*" | \
> > > +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
> > >  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
> > >  	done
> > >  }
> > > diff --git a/common/rc b/common/rc
> > > index 857308a..cc9a894 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -21,6 +21,7 @@
> > >  #-----------------------------------------------------------------------
> > >  
> > >  BC=$(which bc 2> /dev/null) || BC=
> > > +SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
> > >  
> > >  _require_math()
> > >  {
> > > diff --git a/new b/new
> > > index d1f8939..4257e00 100755
> > > --- a/new
> > > +++ b/new
> > > @@ -81,11 +81,14 @@ line=0
> > >  eof=1
> > >  [ -f "$tdir/group" ] || usage
> > >  
> > > -for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> > > +for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
> > >  do
> > >      line=$((line+1))
> > > -    if [ -z "$found" ] || [ "$found" == "#" ];then
> > > -	continue
> > > +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> > > +        continue
> > > +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> > > +        # this one is for tests not named by a number
> > > +        continue
> > >      fi
> > >      i=$((i+1))
> > >      id=`printf "%03d" $i`
> > > @@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
> > >     id=`printf "%03d" $i`
> > >  fi
> > >  
> > > -echo "Next test is $id"
> > > +echo "Next test id is $id"
> > > +
> > > +read -p "Do you want to append a name to the ID? y,[n]: " -r
> > > +if [[ $REPLY = [Yy] ]]; then
> > > +	# get the new name from user
> > > +	name=""
> > > +	while [ "$name" = "" ]; do
> > > +		read -p "Enter the new name: "
> > 
> > "new name" here still seems confusing to me. Otherwise look good to me.
> > 
> > Reviewed-by: Eryu Guan <eguan@redhat.com>
> > 
> > Thank you!
> > 
> > Eryu
> 
> So perhaps just remove the "new" and leave only the "name"?
> Should I send one more patch for it?

I'm not good at this.. but I'm thinking about something like:

[root@hp-dl388eg8-01 xfstests]# git diff
diff --git a/new b/new
index 4257e00..7e9ce2c 100755
--- a/new
+++ b/new
@@ -105,12 +105,12 @@ fi
 
 echo "Next test id is $id"
 
-read -p "Do you want to append a name to the ID? y,[n]: " -r
+read -p "Append a name to the ID? Test name will be $id-\$name. y,[n]: " -r
 if [[ $REPLY = [Yy] ]]; then
        # get the new name from user
        name=""
        while [ "$name" = "" ]; do
-               read -p "Enter the new name: "
+               read -p "Enter the name: "
                if [ "$REPLY" = "" ]; then
                        echo "For canceling, use ctrl+c."
                elif [ -e "$tdir/$id-$REPLY" ]; then

A new patch is good.

Thanks,
Eryu
> 
> Jan
> 
> > > +		if [ "$REPLY" = "" ]; then
> > > +			echo "For canceling, use ctrl+c."
> > > +		elif [ -e "$tdir/$id-$REPLY" ]; then
> > > +			echo "File '$id-$REPLY' already exists, use another one."
> > > +			echo #
> > > +		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
> > > +			name="$REPLY"
> > > +		else
> > > +			echo "Filename must contain only alphanumeric symbols and dash!"
> > > +			echo "(Used regex: ^$SUPPORTED_TESTS$)"
> > > +			echo
> > > +		fi
> > > +	done
> > > +
> > > +	# now find where to insert this name
> > > +	eof=1
> > > +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> > > +		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
> > > +		line=$((line+1))
> > > +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> > > +			continue
> > > +		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then
> > > +			eof=0
> > > +			break
> > > +		fi
> > > +	done
> > > +	if [ $eof -eq 0 ]; then
> > > +		# If place wasn't found, let $line be the end of the file
> > > +		line=$((line-1))
> > > +	fi
> > > +	id="$id-$name"
> > > +fi
> > > +echo "Creating test file '$id'"
> > >  
> > >  if [ -f $tdir/$id ]
> > >  then
> > > @@ -115,7 +158,7 @@ year=`date +%Y`
> > >  
> > >  cat <<End-of-File >$tdir/$id
> > >  #! /bin/bash
> > > -# FS QA Test No. $id
> > > +# FS QA Test $id
> > >  #
> > >  # what am I here for?
> > >  #
> > > --
> > > 2.1.0
> > > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 

[PATCH v8] fstests: Tests can use any name now, not 3 digits only.

[Jan Ťulák]

Tests can use any name now, not 3 digits only.
(e.g. a test can be named "tests/generic/001-some-name")

Names are limited to alphanumeric characters and dash and are always prefixed
with an unique id for easier identification of a specific test.

Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---
 Change in prompt text for a test name.

 README    |  7 ++++++-
 check     | 10 +++++-----
 common/rc |  1 +
 new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/README b/README
index 0c9449a..1841052 100644
--- a/README
+++ b/README
@@ -202,10 +202,15 @@ Test script environment:
 	- When calling getfacl in a test, pass the "-n" argument so
 	  that numeric rather than symbolic identifiers are used in
 	  the output.
+	- When creating a new test, it is possible to enter a custom name
+	  for the file. Filenames are in form NNN-custom-name, where NNN
+	  is automatically added by the ./new script as an unique ID,
+	  and "custom-name" is the optional string entered into a prompt
+	  in the ./new script. Note the "NNN-" part is added automatically.
 
 Verified output:
 
-    Each test script has a numerical name, e.g. 007, and an associated
+    Each test script has a name, e.g. 007, and an associated
     verified output, e.g. 007.out.
 
     It is important that the verified output is deterministic, and
diff --git a/check b/check
index 0830e0c..b905259 100755
--- a/check
+++ b/check
@@ -58,7 +58,6 @@ then
     exit 1
 fi
 
-SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
 
@@ -96,21 +95,22 @@ get_group_list()
 		l=$(sed -n < $SRC_DIR/$d/group \
 			-e 's/#.*//' \
 			-e 's/$/ /' \
-			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
+			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
 		grpl="$grpl $l"
 	done
 	echo $grpl
 }
 
-# find all tests, excluding files that are test metadata such as group files.
-# This assumes that tests are defined purely by alphanumeric filenames with no
-# ".xyz" extensions in the name.
+# Find all tests, excluding files that are test metadata such as group files.
+# It matches test names against $SUPPORTED_TESTS defined at the top of this
+# file.
 get_all_tests()
 {
 	touch $tmp.list
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
+			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
 			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
diff --git a/common/rc b/common/rc
index 857308a..cc9a894 100644
--- a/common/rc
+++ b/common/rc
@@ -21,6 +21,7 @@
 #-----------------------------------------------------------------------
 
 BC=$(which bc 2> /dev/null) || BC=
+SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
 
 _require_math()
 {
diff --git a/new b/new
index d1f8939..7e9ce2c 100755
--- a/new
+++ b/new
@@ -81,11 +81,14 @@ line=0
 eof=1
 [ -f "$tdir/group" ] || usage
 
-for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
+for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
 do
     line=$((line+1))
-    if [ -z "$found" ] || [ "$found" == "#" ];then
-	continue
+    if [ -z "$found" ] || [ "$found" == "#" ]; then
+        continue
+    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
+        # this one is for tests not named by a number
+        continue
     fi
     i=$((i+1))
     id=`printf "%03d" $i`
@@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
    id=`printf "%03d" $i`
 fi
 
-echo "Next test is $id"
+echo "Next test id is $id"
+
+read -p "Append a name to the ID? Test name will be $id-\$name. y,[n]: " -r
+if [[ $REPLY = [Yy] ]]; then
+	# get the new name from user
+	name=""
+	while [ "$name" = "" ]; do
+		read -p "Enter the name: "
+		if [ "$REPLY" = "" ]; then
+			echo "For canceling, use ctrl+c."
+		elif [ -e "$tdir/$id-$REPLY" ]; then
+			echo "File '$id-$REPLY' already exists, use another one."
+			echo #
+		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
+			name="$REPLY"
+		else
+			echo "Filename must contain only alphanumeric symbols and dash!"
+			echo "(Used regex: ^$SUPPORTED_TESTS$)"
+			echo
+		fi
+	done
+
+	# now find where to insert this name
+	eof=1
+	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
+		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
+		line=$((line+1))
+		if [ -z "$found" ] || [ "$found" == "#" ]; then
+			continue
+		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then
+			eof=0
+			break
+		fi
+	done
+	if [ $eof -eq 0 ]; then
+		# If place wasn't found, let $line be the end of the file
+		line=$((line-1))
+	fi
+	id="$id-$name"
+fi
+echo "Creating test file '$id'"
 
 if [ -f $tdir/$id ]
 then
@@ -115,7 +158,7 @@ year=`date +%Y`
 
 cat <<End-of-File >$tdir/$id
 #! /bin/bash
-# FS QA Test No. $id
+# FS QA Test $id
 #
 # what am I here for?
 #
-- 
2.1.0

Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only.

[Jan Tulak]

That is a good idea, thank you. 
Changed. :-)

Jan

----- Original Message -----
> From: "Eryu Guan" <eguan@redhat.com>
> To: "Jan Tulak" <jtulak@redhat.com>
> Cc: fstests@vger.kernel.org, lczerner@redhat.com, dsterba@suse.cz
> Sent: Friday, 27 March, 2015 12:15:04 PM
> Subject: Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only.
> 
> On Fri, Mar 27, 2015 at 05:48:23AM -0400, Jan Tulak wrote:
> > 
> > 
> > ----- Original Message -----
> > > From: "Eryu Guan" <eguan@redhat.com>
> > > To: "Jan Ťulák" <jtulak@redhat.com>
> > > Cc: fstests@vger.kernel.org, lczerner@redhat.com, dsterba@suse.cz
> > > Sent: Friday, 27 March, 2015 10:39:40 AM
> > > Subject: Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits
> > > only.
> > > 
> > > On Fri, Mar 27, 2015 at 10:15:48AM +0100, Jan Ťulák wrote:
> > > > Tests can use any name now, not 3 digits only.
> > > > (e.g. a test can be named "tests/generic/001-some-name")
> > > > 
> > > > Names are limited to alphanumeric characters and dash and are always
> > > > prefixed
> > > > with an unique id for easier identification of a specific test.
> > > > 
> > > > Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> > > > ---
> > > > 
> > > >  Changes in this patch version:
> > > > 
> > > >  SUPPORTED_TESTS regex moved into common/rc
> > > >  removed unused variable auto_id
> > > >  prompt for name changed to be less confusing
> > > >  fixed test for existing file
> > > >  foundId -> found_id
> > > >  (( expression )) -> [ expression ]
> > > > 
> > > > 
> > > >  README    |  7 ++++++-
> > > >  check     | 10 +++++-----
> > > >  common/rc |  1 +
> > > >  new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> > > >  4 files changed, 60 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/README b/README
> > > > index 0c9449a..1841052 100644
> > > > --- a/README
> > > > +++ b/README
> > > > @@ -202,10 +202,15 @@ Test script environment:
> > > >  	- When calling getfacl in a test, pass the "-n" argument so
> > > >  	  that numeric rather than symbolic identifiers are used in
> > > >  	  the output.
> > > > +	- When creating a new test, it is possible to enter a custom name
> > > > +	  for the file. Filenames are in form NNN-custom-name, where NNN
> > > > +	  is automatically added by the ./new script as an unique ID,
> > > > +	  and "custom-name" is the optional string entered into a prompt
> > > > +	  in the ./new script. Note the "NNN-" part is added automatically.
> > > >  
> > > >  Verified output:
> > > >  
> > > > -    Each test script has a numerical name, e.g. 007, and an associated
> > > > +    Each test script has a name, e.g. 007, and an associated
> > > >      verified output, e.g. 007.out.
> > > >  
> > > >      It is important that the verified output is deterministic, and
> > > > diff --git a/check b/check
> > > > index 0830e0c..b905259 100755
> > > > --- a/check
> > > > +++ b/check
> > > > @@ -58,7 +58,6 @@ then
> > > >      exit 1
> > > >  fi
> > > >  
> > > > -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> > > >  SRC_GROUPS="generic shared"
> > > >  export SRC_DIR="tests"
> > > >  
> > > > @@ -96,21 +95,22 @@ get_group_list()
> > > >  		l=$(sed -n < $SRC_DIR/$d/group \
> > > >  			-e 's/#.*//' \
> > > >  			-e 's/$/ /' \
> > > > -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> > > > +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
> > > >  		grpl="$grpl $l"
> > > >  	done
> > > >  	echo $grpl
> > > >  }
> > > >  
> > > > -# find all tests, excluding files that are test metadata such as group
> > > > files.
> > > > -# This assumes that tests are defined purely by alphanumeric filenames
> > > > with no
> > > > -# ".xyz" extensions in the name.
> > > > +# Find all tests, excluding files that are test metadata such as group
> > > > files.
> > > > +# It matches test names against $SUPPORTED_TESTS defined at the top of
> > > > this
> > > > +# file.
> > > >  get_all_tests()
> > > >  {
> > > >  	touch $tmp.list
> > > >  	for d in $SRC_GROUPS $FSTYP; do
> > > >  		ls $SRC_DIR/$d/* | \
> > > >  			grep -v "\..*" | \
> > > > +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
> > > >  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
> > > >  	done
> > > >  }
> > > > diff --git a/common/rc b/common/rc
> > > > index 857308a..cc9a894 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -21,6 +21,7 @@
> > > >  #-----------------------------------------------------------------------
> > > >  
> > > >  BC=$(which bc 2> /dev/null) || BC=
> > > > +SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
> > > >  
> > > >  _require_math()
> > > >  {
> > > > diff --git a/new b/new
> > > > index d1f8939..4257e00 100755
> > > > --- a/new
> > > > +++ b/new
> > > > @@ -81,11 +81,14 @@ line=0
> > > >  eof=1
> > > >  [ -f "$tdir/group" ] || usage
> > > >  
> > > > -for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> > > > +for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
> > > >  do
> > > >      line=$((line+1))
> > > > -    if [ -z "$found" ] || [ "$found" == "#" ];then
> > > > -	continue
> > > > +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> > > > +        continue
> > > > +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> > > > +        # this one is for tests not named by a number
> > > > +        continue
> > > >      fi
> > > >      i=$((i+1))
> > > >      id=`printf "%03d" $i`
> > > > @@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
> > > >     id=`printf "%03d" $i`
> > > >  fi
> > > >  
> > > > -echo "Next test is $id"
> > > > +echo "Next test id is $id"
> > > > +
> > > > +read -p "Do you want to append a name to the ID? y,[n]: " -r
> > > > +if [[ $REPLY = [Yy] ]]; then
> > > > +	# get the new name from user
> > > > +	name=""
> > > > +	while [ "$name" = "" ]; do
> > > > +		read -p "Enter the new name: "
> > > 
> > > "new name" here still seems confusing to me. Otherwise look good to me.
> > > 
> > > Reviewed-by: Eryu Guan <eguan@redhat.com>
> > > 
> > > Thank you!
> > > 
> > > Eryu
> > 
> > So perhaps just remove the "new" and leave only the "name"?
> > Should I send one more patch for it?
> 
> I'm not good at this.. but I'm thinking about something like:
> 
> [root@hp-dl388eg8-01 xfstests]# git diff
> diff --git a/new b/new
> index 4257e00..7e9ce2c 100755
> --- a/new
> +++ b/new
> @@ -105,12 +105,12 @@ fi
>  
>  echo "Next test id is $id"
>  
> -read -p "Do you want to append a name to the ID? y,[n]: " -r
> +read -p "Append a name to the ID? Test name will be $id-\$name. y,[n]: " -r
>  if [[ $REPLY = [Yy] ]]; then
>         # get the new name from user
>         name=""
>         while [ "$name" = "" ]; do
> -               read -p "Enter the new name: "
> +               read -p "Enter the name: "
>                 if [ "$REPLY" = "" ]; then
>                         echo "For canceling, use ctrl+c."
>                 elif [ -e "$tdir/$id-$REPLY" ]; then
> 
> A new patch is good.
> 
> Thanks,
> Eryu
> > 
> > Jan
> > 
> > > > +		if [ "$REPLY" = "" ]; then
> > > > +			echo "For canceling, use ctrl+c."
> > > > +		elif [ -e "$tdir/$id-$REPLY" ]; then
> > > > +			echo "File '$id-$REPLY' already exists, use another one."
> > > > +			echo #
> > > > +		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
> > > > +			name="$REPLY"
> > > > +		else
> > > > +			echo "Filename must contain only alphanumeric symbols and dash!"
> > > > +			echo "(Used regex: ^$SUPPORTED_TESTS$)"
> > > > +			echo
> > > > +		fi
> > > > +	done
> > > > +
> > > > +	# now find where to insert this name
> > > > +	eof=1
> > > > +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`;
> > > > do
> > > > +		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
> > > > +		line=$((line+1))
> > > > +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> > > > +			continue
> > > > +		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then
> > > > +			eof=0
> > > > +			break
> > > > +		fi
> > > > +	done
> > > > +	if [ $eof -eq 0 ]; then
> > > > +		# If place wasn't found, let $line be the end of the file
> > > > +		line=$((line-1))
> > > > +	fi
> > > > +	id="$id-$name"
> > > > +fi
> > > > +echo "Creating test file '$id'"
> > > >  
> > > >  if [ -f $tdir/$id ]
> > > >  then
> > > > @@ -115,7 +158,7 @@ year=`date +%Y`
> > > >  
> > > >  cat <<End-of-File >$tdir/$id
> > > >  #! /bin/bash
> > > > -# FS QA Test No. $id
> > > > +# FS QA Test $id
> > > >  #
> > > >  # what am I here for?
> > > >  #
> > > > --
> > > > 2.1.0
> > > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v9] fstests: Tests can use any name now, not 3 digits only.

[Jan Ťulák]

Tests can use any name now, not 3 digits only.
(e.g. a test can be named "tests/generic/001-some-name")

Names are limited to alphanumeric characters and dash and are always prefixed
with an unique id for easier identification of a specific test.

Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---
 Just a small fix in comment to reflect $SUPPORTED_TESTS is in common/rc.

 README    |  7 ++++++-
 check     |  9 ++++-----
 common/rc |  1 +
 new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/README b/README
index 0c9449a..1841052 100644
--- a/README
+++ b/README
@@ -202,10 +202,15 @@ Test script environment:
 	- When calling getfacl in a test, pass the "-n" argument so
 	  that numeric rather than symbolic identifiers are used in
 	  the output.
+	- When creating a new test, it is possible to enter a custom name
+	  for the file. Filenames are in form NNN-custom-name, where NNN
+	  is automatically added by the ./new script as an unique ID,
+	  and "custom-name" is the optional string entered into a prompt
+	  in the ./new script. Note the "NNN-" part is added automatically.
 
 Verified output:
 
-    Each test script has a numerical name, e.g. 007, and an associated
+    Each test script has a name, e.g. 007, and an associated
     verified output, e.g. 007.out.
 
     It is important that the verified output is deterministic, and
diff --git a/check b/check
index 0830e0c..df9a5be 100755
--- a/check
+++ b/check
@@ -58,7 +58,6 @@ then
     exit 1
 fi
 
-SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
 
@@ -96,21 +95,21 @@ get_group_list()
 		l=$(sed -n < $SRC_DIR/$d/group \
 			-e 's/#.*//' \
 			-e 's/$/ /' \
-			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
+			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
 		grpl="$grpl $l"
 	done
 	echo $grpl
 }
 
-# find all tests, excluding files that are test metadata such as group files.
-# This assumes that tests are defined purely by alphanumeric filenames with no
-# ".xyz" extensions in the name.
+# Find all tests, excluding files that are test metadata such as group files.
+# It matches test names against $SUPPORTED_TESTS defined in common/rc
 get_all_tests()
 {
 	touch $tmp.list
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
+			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
 			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
diff --git a/common/rc b/common/rc
index 857308a..cc9a894 100644
--- a/common/rc
+++ b/common/rc
@@ -21,6 +21,7 @@
 #-----------------------------------------------------------------------
 
 BC=$(which bc 2> /dev/null) || BC=
+SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
 
 _require_math()
 {
diff --git a/new b/new
index d1f8939..7e9ce2c 100755
--- a/new
+++ b/new
@@ -81,11 +81,14 @@ line=0
 eof=1
 [ -f "$tdir/group" ] || usage
 
-for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
+for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
 do
     line=$((line+1))
-    if [ -z "$found" ] || [ "$found" == "#" ];then
-	continue
+    if [ -z "$found" ] || [ "$found" == "#" ]; then
+        continue
+    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
+        # this one is for tests not named by a number
+        continue
     fi
     i=$((i+1))
     id=`printf "%03d" $i`
@@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
    id=`printf "%03d" $i`
 fi
 
-echo "Next test is $id"
+echo "Next test id is $id"
+
+read -p "Append a name to the ID? Test name will be $id-\$name. y,[n]: " -r
+if [[ $REPLY = [Yy] ]]; then
+	# get the new name from user
+	name=""
+	while [ "$name" = "" ]; do
+		read -p "Enter the name: "
+		if [ "$REPLY" = "" ]; then
+			echo "For canceling, use ctrl+c."
+		elif [ -e "$tdir/$id-$REPLY" ]; then
+			echo "File '$id-$REPLY' already exists, use another one."
+			echo #
+		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
+			name="$REPLY"
+		else
+			echo "Filename must contain only alphanumeric symbols and dash!"
+			echo "(Used regex: ^$SUPPORTED_TESTS$)"
+			echo
+		fi
+	done
+
+	# now find where to insert this name
+	eof=1
+	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
+		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
+		line=$((line+1))
+		if [ -z "$found" ] || [ "$found" == "#" ]; then
+			continue
+		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then
+			eof=0
+			break
+		fi
+	done
+	if [ $eof -eq 0 ]; then
+		# If place wasn't found, let $line be the end of the file
+		line=$((line-1))
+	fi
+	id="$id-$name"
+fi
+echo "Creating test file '$id'"
 
 if [ -f $tdir/$id ]
 then
@@ -115,7 +158,7 @@ year=`date +%Y`
 
 cat <<End-of-File >$tdir/$id
 #! /bin/bash
-# FS QA Test No. $id
+# FS QA Test $id
 #
 # what am I here for?
 #
-- 
2.1.0

Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only.

[Eryu Guan]

On Fri, Mar 27, 2015 at 12:49:26PM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/001-some-name")
> 
> Names are limited to alphanumeric characters and dash and are always prefixed
> with an unique id for easier identification of a specific test.
> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>

Reviewed-by: Eryu Guan <eguan@redhat.com>

Thanks!

> ---
>  Just a small fix in comment to reflect $SUPPORTED_TESTS is in common/rc.
> 
>  README    |  7 ++++++-
>  check     |  9 ++++-----
>  common/rc |  1 +
>  new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  4 files changed, 59 insertions(+), 11 deletions(-)
> 
> diff --git a/README b/README
> index 0c9449a..1841052 100644
> --- a/README
> +++ b/README
> @@ -202,10 +202,15 @@ Test script environment:
>  	- When calling getfacl in a test, pass the "-n" argument so
>  	  that numeric rather than symbolic identifiers are used in
>  	  the output.
> +	- When creating a new test, it is possible to enter a custom name
> +	  for the file. Filenames are in form NNN-custom-name, where NNN
> +	  is automatically added by the ./new script as an unique ID,
> +	  and "custom-name" is the optional string entered into a prompt
> +	  in the ./new script. Note the "NNN-" part is added automatically.
>  
>  Verified output:
>  
> -    Each test script has a numerical name, e.g. 007, and an associated
> +    Each test script has a name, e.g. 007, and an associated
>      verified output, e.g. 007.out.
>  
>      It is important that the verified output is deterministic, and
> diff --git a/check b/check
> index 0830e0c..df9a5be 100755
> --- a/check
> +++ b/check
> @@ -58,7 +58,6 @@ then
>      exit 1
>  fi
>  
> -SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
>  SRC_GROUPS="generic shared"
>  export SRC_DIR="tests"
>  
> @@ -96,21 +95,21 @@ get_group_list()
>  		l=$(sed -n < $SRC_DIR/$d/group \
>  			-e 's/#.*//' \
>  			-e 's/$/ /' \
> -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
>  		grpl="$grpl $l"
>  	done
>  	echo $grpl
>  }
>  
> -# find all tests, excluding files that are test metadata such as group files.
> -# This assumes that tests are defined purely by alphanumeric filenames with no
> -# ".xyz" extensions in the name.
> +# Find all tests, excluding files that are test metadata such as group files.
> +# It matches test names against $SUPPORTED_TESTS defined in common/rc
>  get_all_tests()
>  {
>  	touch $tmp.list
>  	for d in $SRC_GROUPS $FSTYP; do
>  		ls $SRC_DIR/$d/* | \
>  			grep -v "\..*" | \
> +			grep "^$SRC_DIR/$d/$SUPPORTED_TESTS"| \
>  			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
>  	done
>  }
> diff --git a/common/rc b/common/rc
> index 857308a..cc9a894 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -21,6 +21,7 @@
>  #-----------------------------------------------------------------------
>  
>  BC=$(which bc 2> /dev/null) || BC=
> +SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
>  
>  _require_math()
>  {
> diff --git a/new b/new
> index d1f8939..7e9ce2c 100755
> --- a/new
> +++ b/new
> @@ -81,11 +81,14 @@ line=0
>  eof=1
>  [ -f "$tdir/group" ] || usage
>  
> -for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> +for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
>  do
>      line=$((line+1))
> -    if [ -z "$found" ] || [ "$found" == "#" ];then
> -	continue
> +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> +        continue
> +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> +        # this one is for tests not named by a number
> +        continue
>      fi
>      i=$((i+1))
>      id=`printf "%03d" $i`
> @@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
>     id=`printf "%03d" $i`
>  fi
>  
> -echo "Next test is $id"
> +echo "Next test id is $id"
> +
> +read -p "Append a name to the ID? Test name will be $id-\$name. y,[n]: " -r
> +if [[ $REPLY = [Yy] ]]; then
> +	# get the new name from user
> +	name=""
> +	while [ "$name" = "" ]; do
> +		read -p "Enter the name: "
> +		if [ "$REPLY" = "" ]; then
> +			echo "For canceling, use ctrl+c."
> +		elif [ -e "$tdir/$id-$REPLY" ]; then
> +			echo "File '$id-$REPLY' already exists, use another one."
> +			echo #
> +		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
> +			name="$REPLY"
> +		else
> +			echo "Filename must contain only alphanumeric symbols and dash!"
> +			echo "(Used regex: ^$SUPPORTED_TESTS$)"
> +			echo
> +		fi
> +	done
> +
> +	# now find where to insert this name
> +	eof=1
> +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> +		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
> +		line=$((line+1))
> +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> +			continue
> +		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then
> +			eof=0
> +			break
> +		fi
> +	done
> +	if [ $eof -eq 0 ]; then
> +		# If place wasn't found, let $line be the end of the file
> +		line=$((line-1))
> +	fi
> +	id="$id-$name"
> +fi
> +echo "Creating test file '$id'"
>  
>  if [ -f $tdir/$id ]
>  then
> @@ -115,7 +158,7 @@ year=`date +%Y`
>  
>  cat <<End-of-File >$tdir/$id
>  #! /bin/bash
> -# FS QA Test No. $id
> +# FS QA Test $id
>  #
>  # what am I here for?
>  #
> -- 
> 2.1.0
> 

Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only.

[David Sterba]

On Fri, Mar 27, 2015 at 12:49:26PM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/001-some-name")
> 
> Names are limited to alphanumeric characters and dash and are always prefixed
> with an unique id for easier identification of a specific test.
> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>

Reviewed-by: David Sterba <dsterba@suse.cz>

Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only.

[Dave Chinner]

On Fri, Mar 27, 2015 at 12:49:26PM +0100, Jan Ťulák wrote:
> Tests can use any name now, not 3 digits only.
> (e.g. a test can be named "tests/generic/001-some-name")
> 
> Names are limited to alphanumeric characters and dash and are always prefixed
> with an unique id for easier identification of a specific test.
> 
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> ---
>  Just a small fix in comment to reflect $SUPPORTED_TESTS is in common/rc.

Which I have a couple of nits about, seeing we've settled on an
acceptible convention. :)

> 
>  README    |  7 ++++++-
>  check     |  9 ++++-----
>  common/rc |  1 +
>  new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  4 files changed, 59 insertions(+), 11 deletions(-)
> 
> diff --git a/README b/README
> index 0c9449a..1841052 100644
> --- a/README
> +++ b/README
> @@ -202,10 +202,15 @@ Test script environment:
>  	- When calling getfacl in a test, pass the "-n" argument so
>  	  that numeric rather than symbolic identifiers are used in
>  	  the output.
> +	- When creating a new test, it is possible to enter a custom name
> +	  for the file. Filenames are in form NNN-custom-name, where NNN
> +	  is automatically added by the ./new script as an unique ID,
> +	  and "custom-name" is the optional string entered into a prompt
> +	  in the ./new script. Note the "NNN-" part is added automatically.

We have a defined format for the test names, yet it does not have
the clear explanation of what constitutes a valid name i.e. from an
error message below:

> +	echo "Filename must contain only alphanumeric symbols and dash!"

If we have a defined format for the test names, we should enforce
it strictly and code directly to it. The description here needs to
spell this out directly.

> @@ -96,21 +95,21 @@ get_group_list()
>  		l=$(sed -n < $SRC_DIR/$d/group \
>  			-e 's/#.*//' \
>  			-e 's/$/ /' \
> -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")

Here we filter by $SUPPORTED_TESTS for valid tests, but....

> --- a/common/rc
> +++ b/common/rc
> @@ -21,6 +21,7 @@
>  #-----------------------------------------------------------------------
>  
>  BC=$(which bc 2> /dev/null) || BC=
> +SUPPORTED_TESTS="[a-zA-Z0-9-]\+"

but $SUPPORTED_TESTS doesn't define the correct format for the test
names. It defines valid characters, but does not enforce the fact
that the first 3 characters must match [0-9] (i.e. the NNN bit), and
then the rest is optional but must be alphanumeric symbols and dash.
i.e it should be matching against:

# Valid test names start with 3 digits "NNN":
#  "[0-9]\{3\}"
# followed by an optional "-":
#  "-\?"
# followed by an optional combination of alphanumeric and "-" chars:
#  "[[:alnum:]-]*"
# e.g. 999-the-mark-of-fstests
#
VALID_TEST_NAME="[0-9]\{3\}-\?[[:alnum:]-]*"

>  _require_math()
>  {
> diff --git a/new b/new
> index d1f8939..7e9ce2c 100755
> --- a/new
> +++ b/new
> @@ -81,11 +81,14 @@ line=0
>  eof=1
>  [ -f "$tdir/group" ] || usage
>  
> -for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> +for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
>  do
>      line=$((line+1))
> -    if [ -z "$found" ] || [ "$found" == "#" ];then
> -	continue
> +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> +        continue
> +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> +        # this one is for tests not named by a number

This should check for "$VALID_TEST_NAME$", too, as it will match
[0-9]{3}$ test names just fine.

> +        continue
>      fi
>      i=$((i+1))
>      id=`printf "%03d" $i`
> @@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
>     id=`printf "%03d" $i`
>  fi
>  
> -echo "Next test is $id"
> +echo "Next test id is $id"
> +
> +read -p "Append a name to the ID? Test name will be $id-\$name. y,[n]: " -r
> +if [[ $REPLY = [Yy] ]]; then
> +	# get the new name from user
> +	name=""
> +	while [ "$name" = "" ]; do
> +		read -p "Enter the name: "
> +		if [ "$REPLY" = "" ]; then
> +			echo "For canceling, use ctrl+c."
> +		elif [ -e "$tdir/$id-$REPLY" ]; then
> +			echo "File '$id-$REPLY' already exists, use another one."
> +			echo #

This check should happen *after* input validation, though it should
never be true because we have selected a unique, unused $id....

> +		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
> +			name="$REPLY"

i.e. this should be first, and given that we have a well defined
input validation, we should make that as clear as possible. e.g.

		elif echo $REPLY | grep -q "^[[:alnum:]-]$"; then

> +		else
> +			echo "Filename must contain only alphanumeric symbols and dash!"
> +			echo "(Used regex: ^$SUPPORTED_TESTS$)"

Don't confuse the poor users with a regex - make it clear what the
format is in the documentation.

> +			echo
> +		fi
> +	done
> +
> +	# now find where to insert this name
> +	eof=1
> +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> +		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')

That's an interesting way of cutting up a string.

		found_id=$(echo "$found" | cut -d "-" -f 1 -)

> +		line=$((line+1))
> +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> +			continue
> +		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then

This doesn't work. $found will be "298-I-want-a-spoon" or just
"298", but $name has no number prefix i.e. "but-I-like-sporks". We
will always have number prefixes, and tests should always have unique
numbers, so there is no reason to check anything but $id against
$found_id.  Which means:

	for found_id in `tail -n +$line $tdir/group | cut -d "-" -f 1" -`; do
		line=$((line+1))
		if [ -z "$found" ] || [ "$found" == "#" ]; then
			continue
		elif [ $found_id -gt $id ]; then
			eof=0
			break
		fi
	done

And once we've done all this and decided on a test name, we should
finally validate it again against $VALID_TEST_NAME....

Cheers,

Dave.



-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only.

[Jan Tulak]

----- Original Message -----
> From: "Dave Chinner" <david@fromorbit.com>
> To: "Jan Ťulák" <jtulak@redhat.com>
> Cc: fstests@vger.kernel.org, eguan@redhat.com
> Sent: Wednesday, 1 April, 2015 6:35:42 AM
> Subject: Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only.
> 
> On Fri, Mar 27, 2015 at 12:49:26PM +0100, Jan Ťulák wrote:
> > Tests can use any name now, not 3 digits only.
> > (e.g. a test can be named "tests/generic/001-some-name")
> > 
> > Names are limited to alphanumeric characters and dash and are always
> > prefixed
> > with an unique id for easier identification of a specific test.
> > 
> > Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> > ---
> >  Just a small fix in comment to reflect $SUPPORTED_TESTS is in common/rc.
> 
> Which I have a couple of nits about, seeing we've settled on an
> acceptible convention. :)
> 
> > 
> >  README    |  7 ++++++-
> >  check     |  9 ++++-----
> >  common/rc |  1 +
> >  new       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  4 files changed, 59 insertions(+), 11 deletions(-)
> > 
> > diff --git a/README b/README
> > index 0c9449a..1841052 100644
> > --- a/README
> > +++ b/README
> > @@ -202,10 +202,15 @@ Test script environment:
> >  	- When calling getfacl in a test, pass the "-n" argument so
> >  	  that numeric rather than symbolic identifiers are used in
> >  	  the output.
> > +	- When creating a new test, it is possible to enter a custom name
> > +	  for the file. Filenames are in form NNN-custom-name, where NNN
> > +	  is automatically added by the ./new script as an unique ID,
> > +	  and "custom-name" is the optional string entered into a prompt
> > +	  in the ./new script. Note the "NNN-" part is added automatically.
> 
> We have a defined format for the test names, yet it does not have
> the clear explanation of what constitutes a valid name i.e. from an
> error message below:
> 
> > +	echo "Filename must contain only alphanumeric symbols and dash!"

Thanks, I'm adding a note into the readme.

> 
> If we have a defined format for the test names, we should enforce
> it strictly and code directly to it. The description here needs to
> spell this out directly.
> 
> > @@ -96,21 +95,21 @@ get_group_list()
> >  		l=$(sed -n < $SRC_DIR/$d/group \
> >  			-e 's/#.*//' \
> >  			-e 's/$/ /' \
> > -			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
> > +			-e "s;^\($SUPPORTED_TESTS\).* $grp .*;$SRC_DIR/$d/\1;p")
> 
> Here we filter by $SUPPORTED_TESTS for valid tests, but....
> 
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -21,6 +21,7 @@
> >  #-----------------------------------------------------------------------
> >  
> >  BC=$(which bc 2> /dev/null) || BC=
> > +SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
> 
> but $SUPPORTED_TESTS doesn't define the correct format for the test
> names. It defines valid characters, but does not enforce the fact
> that the first 3 characters must match [0-9] (i.e. the NNN bit), and
> then the rest is optional but must be alphanumeric symbols and dash.
> i.e it should be matching against:
> 

I did it to be "generous on input, strict on output" - the ./new script always put 3 numbers at the beginning, but if someone for some reason needs to avoid it, it will still work. Enforcing it is easy, but is it needed?


> # Valid test names start with 3 digits "NNN":
> #  "[0-9]\{3\}"
> # followed by an optional "-":
> #  "-\?"
> # followed by an optional combination of alphanumeric and "-" chars:
> #  "[[:alnum:]-]*"
> # e.g. 999-the-mark-of-fstests
> #
> VALID_TEST_NAME="[0-9]\{3\}-\?[[:alnum:]-]*"
> 
> >  _require_math()
> >  {
> > diff --git a/new b/new
> > index d1f8939..7e9ce2c 100755
> > --- a/new
> > +++ b/new
> > @@ -81,11 +81,14 @@ line=0
> >  eof=1
> >  [ -f "$tdir/group" ] || usage
> >  
> > -for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
> > +for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
> >  do
> >      line=$((line+1))
> > -    if [ -z "$found" ] || [ "$found" == "#" ];then
> > -	continue
> > +    if [ -z "$found" ] || [ "$found" == "#" ]; then
> > +        continue
> > +    elif ! echo "$found" | grep -q "^[0-9][0-9][0-9]$"; then
> > +        # this one is for tests not named by a number
> 
> This should check for "$VALID_TEST_NAME$", too, as it will match
> [0-9]{3}$ test names just fine.
> 
> > +        continue
> >      fi
> >      i=$((i+1))
> >      id=`printf "%03d" $i`
> > @@ -100,7 +103,47 @@ if [ $eof -eq 1 ]; then
> >     id=`printf "%03d" $i`
> >  fi
> >  
> > -echo "Next test is $id"
> > +echo "Next test id is $id"
> > +
> > +read -p "Append a name to the ID? Test name will be $id-\$name. y,[n]: "
> > -r
> > +if [[ $REPLY = [Yy] ]]; then
> > +	# get the new name from user
> > +	name=""
> > +	while [ "$name" = "" ]; do
> > +		read -p "Enter the name: "
> > +		if [ "$REPLY" = "" ]; then
> > +			echo "For canceling, use ctrl+c."
> > +		elif [ -e "$tdir/$id-$REPLY" ]; then
> > +			echo "File '$id-$REPLY' already exists, use another one."
> > +			echo #
> 
> This check should happen *after* input validation, though it should
> never be true because we have selected a unique, unused $id....

This check can be true - ID check is done in a group file, it does not reflects existence of files with such ID and name. So if you for example interrupt the ./new script after creating the file, but before editing groups, the file can exist without an entry in groups and the ID is used again.

I agree with validating the input first.

> 
> > +		elif echo "$REPLY" | grep -q "^$SUPPORTED_TESTS$"; then
> > +			name="$REPLY"
> 
> i.e. this should be first, and given that we have a well defined
> input validation, we should make that as clear as possible. e.g.
> 
> 		elif echo $REPLY | grep -q "^[[:alnum:]-]$"; then
> 

You mean to put the regex directly into the condition? I prefer to keep in the variable defined in common/rc. 

> > +		else
> > +			echo "Filename must contain only alphanumeric symbols and dash!"
> > +			echo "(Used regex: ^$SUPPORTED_TESTS$)"
> 
> Don't confuse the poor users with a regex - make it clear what the
> format is in the documentation.
>

Well, I think any user who will write tests for a filesystem knows what regex is. ;-)
But I take the point, removed.


> 
> > +			echo
> > +		fi
> > +	done
> > +
> > +	# now find where to insert this name
> > +	eof=1
> > +	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
> > +		found_id=$(echo "$found" | tr - ' ' | $AWK_PROG '{ print $1 }')
> 
> That's an interesting way of cutting up a string.
> 
> 		found_id=$(echo "$found" | cut -d "-" -f 1 -)

:-)

> 
> > +		line=$((line+1))
> > +		if [ -z "$found" ] || [ "$found" == "#" ]; then
> > +			continue
> > +		elif [[ $found > $name ]] || [ $found_id -gt $id ]; then
> 
> This doesn't work. $found will be "298-I-want-a-spoon" or just
> "298", but $name has no number prefix i.e. "but-I-like-sporks". We
> will always have number prefixes, and tests should always have unique
> numbers, so there is no reason to check anything but $id against
> $found_id.  Which means:
> 
> 	for found_id in `tail -n +$line $tdir/group | cut -d "-" -f 1" -`; do
> 		line=$((line+1))
> 		if [ -z "$found" ] || [ "$found" == "#" ]; then
> 			continue
> 		elif [ $found_id -gt $id ]; then
> 			eof=0
> 			break
> 		fi
> 	done
> 

I see. Most probably, it is a relict from before unique IDs. 
Fixed, thank you.

Jan


> And once we've done all this and decided on a test name, we should
> finally validate it again against $VALID_TEST_NAME....
> 
> Cheers,
> 
> Dave.
> 
> 
> 
> --
> Dave Chinner
> david@fromorbit.com
> 

Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only.

[Lukáš Czerner]

On Wed, 1 Apr 2015, Jan Tulak wrote:

> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -21,6 +21,7 @@
> > >  #-----------------------------------------------------------------------
> > >  
> > >  BC=$(which bc 2> /dev/null) || BC=
> > > +SUPPORTED_TESTS="[a-zA-Z0-9-]\+"
> > 
> > but $SUPPORTED_TESTS doesn't define the correct format for the test
> > names. It defines valid characters, but does not enforce the fact
> > that the first 3 characters must match [0-9] (i.e. the NNN bit), and
> > then the rest is optional but must be alphanumeric symbols and dash.
> > i.e it should be matching against:
> > 
> 
> I did it to be "generous on input, strict on output" - the ./new script always put 3 numbers at the beginning, but if someone for some reason needs to avoid it, it will still work. Enforcing it is easy, but is it needed?

Hi Jan,

I thought that we've already established that indeed yes, it is
needed. Having ./new script to create the right format is nice, but
it's not enough.

You can easily create new tests without ./new script and while I do
not know how many people are using it, I've never used it myself.

Please enforce the naming format.

Thanks!
-Lukas

[PATCH v10] fstests: Tests can use any name now, not 3 digits only.

[Jan Ťulák]

Tests can use any name now, not 3 digits only.
(e.g. a test can be named "tests/generic/001-some-name")

Names are limited to alphanumeric characters and dash and are always prefixed
with an unique id for easier identification of a specific test.

Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---

 ./new:
 - input validation is moved before file check
 - no longer printing regex to user
 - removed name comparison, as we have unique IDs.
 - better string cutting

 edited README, changed regex for valid test names


 README    |  8 +++++++-
 check     |  9 ++++-----
 common/rc | 10 ++++++++++
 new       | 54 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/README b/README
index 0c9449a..f8a878c 100644
--- a/README
+++ b/README
@@ -202,10 +202,16 @@ Test script environment:
 	- When calling getfacl in a test, pass the "-n" argument so
 	  that numeric rather than symbolic identifiers are used in
 	  the output.
+	- When creating a new test, it is possible to enter a custom name
+	  for the file. Filenames are in form NNN-custom-name, where NNN
+	  is automatically added by the ./new script as an unique ID,
+	  and "custom-name" is the optional string entered into a prompt
+	  in the ./new script. It can contain only alphanumeric characters
+	  and dash. Note the "NNN-" part is added automatically.
 
 Verified output:
 
-    Each test script has a numerical name, e.g. 007, and an associated
+    Each test script has a name, e.g. 007, and an associated
     verified output, e.g. 007.out.
 
     It is important that the verified output is deterministic, and
diff --git a/check b/check
index 0830e0c..4fa96ed 100755
--- a/check
+++ b/check
@@ -58,7 +58,6 @@ then
     exit 1
 fi
 
-SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
 
@@ -96,21 +95,21 @@ get_group_list()
 		l=$(sed -n < $SRC_DIR/$d/group \
 			-e 's/#.*//' \
 			-e 's/$/ /' \
-			-e "s;\(^[0-9][0-9][0-9]\).* $grp .*;$SRC_DIR/$d/\1;p")
+			-e "s;^\($VALID_TEST_NAME\).* $grp .*;$SRC_DIR/$d/\1;p")
 		grpl="$grpl $l"
 	done
 	echo $grpl
 }
 
-# find all tests, excluding files that are test metadata such as group files.
-# This assumes that tests are defined purely by alphanumeric filenames with no
-# ".xyz" extensions in the name.
+# Find all tests, excluding files that are test metadata such as group files.
+# It matches test names against $VALID_TEST_NAME defined in common/rc
 get_all_tests()
 {
 	touch $tmp.list
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
+			grep "^$SRC_DIR/$d/$VALID_TEST_NAME"| \
 			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
diff --git a/common/rc b/common/rc
index 857308a..869b56b 100644
--- a/common/rc
+++ b/common/rc
@@ -22,6 +22,16 @@
 
 BC=$(which bc 2> /dev/null) || BC=
 
+# Valid test names start with 3 digits "NNN":
+#  "[0-9]\{3\}"
+# followed by an optional "-":
+#  "-\?"
+# followed by an optional combination of alphanumeric and "-" chars:
+#  "[[:alnum:]-]*"
+# e.g. 999-the-mark-of-fstests
+#
+VALID_TEST_NAME="[0-9]\{3\}-\?[[:alnum:]-]*"
+
 _require_math()
 {
 	if [ -z "$BC" ]; then
diff --git a/new b/new
index d1f8939..c734bdc 100755
--- a/new
+++ b/new
@@ -81,11 +81,14 @@ line=0
 eof=1
 [ -f "$tdir/group" ] || usage
 
-for found in `cat $tdir/group | $AWK_PROG '{ print $1 }'`
+for found in `cat $tdir/group | tr - ' ' | $AWK_PROG '{ print $1 }'`
 do
     line=$((line+1))
-    if [ -z "$found" ] || [ "$found" == "#" ];then
-	continue
+    if [ -z "$found" ] || [ "$found" == "#" ]; then
+        continue
+    elif ! echo "$found" | grep -q "^$VALID_TEST_NAME$"; then
+        # this one is for tests not named by a number
+        continue
     fi
     i=$((i+1))
     id=`printf "%03d" $i`
@@ -100,7 +103,48 @@ if [ $eof -eq 1 ]; then
    id=`printf "%03d" $i`
 fi
 
-echo "Next test is $id"
+echo "Next test id is $id"
+
+read -p "Append a name to the ID? Test name will be $id-\$name. y,[n]: " -r
+if [[ $REPLY = [Yy] ]]; then
+	# get the new name from user
+	name=""
+	while [ "$name" = "" ]; do
+		read -p "Enter the name: "
+		if [ "$REPLY" = "" ]; then
+			echo "For canceling, use ctrl+c."
+		elif echo "$id-$REPLY" | grep -q "^$VALID_TEST_NAME$"; then
+			if [ -e "$tdir/$id-$REPLY" ]; then
+				echo "File '$id-$REPLY' already exists, use another one."
+				echo
+			else
+				name="$REPLY"
+			fi
+		else
+			echo "A name can contain only alphanumeric symbols and dash!"
+			echo
+		fi
+	done
+
+	# now find where to insert this name
+	eof=1
+	for found in `tail -n +$line $tdir/group | $AWK_PROG '{ print $1 }'`; do
+		found_id=$(echo "$found" | cut -d "-" -f 1 - )
+		line=$((line+1))
+		if [ -z "$found" ] || [ "$found" == "#" ]; then
+			continue
+		elif [ $found_id -gt $id ]; then
+			eof=0
+			break
+		fi
+	done
+	if [ $eof -eq 0 ]; then
+		# If place wasn't found, let $line be the end of the file
+		line=$((line-1))
+	fi
+	id="$id-$name"
+fi
+echo "Creating test file '$id'"
 
 if [ -f $tdir/$id ]
 then
@@ -115,7 +159,7 @@ year=`date +%Y`
 
 cat <<End-of-File >$tdir/$id
 #! /bin/bash
-# FS QA Test No. $id
+# FS QA Test $id
 #
 # what am I here for?
 #
-- 
2.1.0