* [PATCH] Tests can use any name now, not 3 digits only. @ 2015-03-04 15:55 Jan Ťulák 2015-03-18 18:01 ` Jan Tulak 2015-03-20 11:13 ` Eryu Guan 0 siblings, 2 replies; 38+ messages in thread From: Jan Ťulák @ 2015-03-04 15:55 UTC (permalink / raw) To: fstests; +Cc: 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] Tests can use any name now, not 3 digits only. 2015-03-04 15:55 [PATCH] Tests can use any name now, not 3 digits only Jan Ťulák @ 2015-03-18 18:01 ` Jan Tulak 2015-03-20 11:13 ` Eryu Guan 1 sibling, 0 replies; 38+ messages in thread From: Jan Tulak @ 2015-03-18 18:01 UTC (permalink / raw) To: fstests 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 > > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] Tests can use any name now, not 3 digits only. 2015-03-04 15:55 [PATCH] Tests can use any name now, not 3 digits only Jan Ťulák 2015-03-18 18:01 ` Jan Tulak @ 2015-03-20 11:13 ` Eryu Guan 2015-03-20 15:03 ` [PATCH] fstests: tests " Jan Ťulák 2015-03-20 15:04 ` [PATCH] " Jan Tulak 1 sibling, 2 replies; 38+ messages in thread From: Eryu Guan @ 2015-03-20 11:13 UTC (permalink / raw) To: Jan Ťulák; +Cc: fstests 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH] fstests: tests can use any name now, not 3 digits only. 2015-03-20 11:13 ` Eryu Guan @ 2015-03-20 15:03 ` Jan Ťulák 2015-03-21 4:49 ` Eryu Guan 2015-03-20 15:04 ` [PATCH] " Jan Tulak 1 sibling, 1 reply; 38+ messages in thread From: Jan Ťulák @ 2015-03-20 15:03 UTC (permalink / raw) To: eguan; +Cc: fstests 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: tests can use any name now, not 3 digits only. 2015-03-20 15:03 ` [PATCH] fstests: tests " Jan Ťulák @ 2015-03-21 4:49 ` Eryu Guan 2015-03-21 12:02 ` Jan Tulak 0 siblings, 1 reply; 38+ messages in thread From: Eryu Guan @ 2015-03-21 4:49 UTC (permalink / raw) To: Jan Ťulák; +Cc: fstests 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: tests can use any name now, not 3 digits only. 2015-03-21 4:49 ` Eryu Guan @ 2015-03-21 12:02 ` Jan Tulak 2015-03-21 13:11 ` Eryu Guan 0 siblings, 1 reply; 38+ messages in thread From: Jan Tulak @ 2015-03-21 12:02 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: tests can use any name now, not 3 digits only. 2015-03-21 12:02 ` Jan Tulak @ 2015-03-21 13:11 ` Eryu Guan 2015-03-25 13:27 ` [PATCH] fstests: Tests " Jan Ťulák ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Eryu Guan @ 2015-03-21 13:11 UTC (permalink / raw) To: Jan Tulak; +Cc: fstests 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 > > ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-21 13:11 ` Eryu Guan @ 2015-03-25 13:27 ` Jan Ťulák 2015-03-25 13:32 ` Jan Tulak ` (2 more replies) 2015-03-26 13:35 ` Jan Ťulák 2015-03-26 15:33 ` [PATCH v6] " Jan Ťulák 2 siblings, 3 replies; 38+ messages in thread From: Jan Ťulák @ 2015-03-25 13:27 UTC (permalink / raw) To: eguan; +Cc: fstests 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-25 13:27 ` [PATCH] fstests: Tests " Jan Ťulák @ 2015-03-25 13:32 ` Jan Tulak 2015-03-25 14:44 ` David Sterba 2015-03-25 17:09 ` Eryu Guan 2 siblings, 0 replies; 38+ messages in thread From: Jan Tulak @ 2015-03-25 13:32 UTC (permalink / raw) To: eguan; +Cc: fstests 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-25 13:27 ` [PATCH] fstests: Tests " Jan Ťulák 2015-03-25 13:32 ` Jan Tulak @ 2015-03-25 14:44 ` David Sterba 2015-03-25 15:20 ` Lukáš Czerner 2015-03-25 17:09 ` Eryu Guan 2 siblings, 1 reply; 38+ messages in thread From: David Sterba @ 2015-03-25 14:44 UTC (permalink / raw) To: Jan Ťulák; +Cc: eguan, fstests 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-25 14:44 ` David Sterba @ 2015-03-25 15:20 ` Lukáš Czerner 2015-03-25 15:27 ` Jan Tulak 0 siblings, 1 reply; 38+ messages in thread From: Lukáš Czerner @ 2015-03-25 15:20 UTC (permalink / raw) To: David Sterba; +Cc: Jan Ťulák, eguan, fstests [-- 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-25 15:20 ` Lukáš Czerner @ 2015-03-25 15:27 ` Jan Tulak 2015-03-25 15:43 ` Lukáš Czerner 0 siblings, 1 reply; 38+ messages in thread From: Jan Tulak @ 2015-03-25 15:27 UTC (permalink / raw) To: Lukáš Czerner; +Cc: David Sterba, eguan, fstests ----- 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-25 15:27 ` Jan Tulak @ 2015-03-25 15:43 ` Lukáš Czerner 2015-03-26 13:32 ` Jan Tulak 0 siblings, 1 reply; 38+ messages in thread From: Lukáš Czerner @ 2015-03-25 15:43 UTC (permalink / raw) To: Jan Tulak; +Cc: David Sterba, eguan, fstests [-- 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-25 15:43 ` Lukáš Czerner @ 2015-03-26 13:32 ` Jan Tulak 0 siblings, 0 replies; 38+ messages in thread From: Jan Tulak @ 2015-03-26 13:32 UTC (permalink / raw) To: Lukáš Czerner; +Cc: David Sterba, eguan, fstests ----- 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-25 13:27 ` [PATCH] fstests: Tests " Jan Ťulák 2015-03-25 13:32 ` Jan Tulak 2015-03-25 14:44 ` David Sterba @ 2015-03-25 17:09 ` Eryu Guan 2015-03-25 17:39 ` Jan Tulak 2 siblings, 1 reply; 38+ messages in thread From: Eryu Guan @ 2015-03-25 17:09 UTC (permalink / raw) To: Jan Ťulák; +Cc: fstests 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-25 17:09 ` Eryu Guan @ 2015-03-25 17:39 ` Jan Tulak 0 siblings, 0 replies; 38+ messages in thread From: Jan Tulak @ 2015-03-25 17:39 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests ----- 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-21 13:11 ` Eryu Guan 2015-03-25 13:27 ` [PATCH] fstests: Tests " Jan Ťulák @ 2015-03-26 13:35 ` Jan Ťulák 2015-03-26 14:41 ` David Sterba 2015-03-26 15:33 ` [PATCH v6] " Jan Ťulák 2 siblings, 1 reply; 38+ messages in thread From: Jan Ťulák @ 2015-03-26 13:35 UTC (permalink / raw) To: eguan; +Cc: fstests, lczerner, dsterba 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-26 13:35 ` Jan Ťulák @ 2015-03-26 14:41 ` David Sterba 2015-03-26 15:16 ` Jan Tulak 0 siblings, 1 reply; 38+ messages in thread From: David Sterba @ 2015-03-26 14:41 UTC (permalink / raw) To: Jan Ťulák; +Cc: eguan, fstests, lczerner 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-26 14:41 ` David Sterba @ 2015-03-26 15:16 ` Jan Tulak 2015-03-26 15:44 ` David Sterba 0 siblings, 1 reply; 38+ messages in thread From: Jan Tulak @ 2015-03-26 15:16 UTC (permalink / raw) To: dsterba; +Cc: eguan, fstests, lczerner ----- 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] fstests: Tests can use any name now, not 3 digits only. 2015-03-26 15:16 ` Jan Tulak @ 2015-03-26 15:44 ` David Sterba 0 siblings, 0 replies; 38+ messages in thread From: David Sterba @ 2015-03-26 15:44 UTC (permalink / raw) To: Jan Tulak; +Cc: eguan, fstests, lczerner 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6] fstests: Tests can use any name now, not 3 digits only. 2015-03-21 13:11 ` Eryu Guan 2015-03-25 13:27 ` [PATCH] fstests: Tests " Jan Ťulák 2015-03-26 13:35 ` Jan Ťulák @ 2015-03-26 15:33 ` Jan Ťulák 2015-03-27 7:25 ` Eryu Guan 2 siblings, 1 reply; 38+ messages in thread From: Jan Ťulák @ 2015-03-26 15:33 UTC (permalink / raw) To: eguan; +Cc: fstests, lczerner, dsterba 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v6] fstests: Tests can use any name now, not 3 digits only. 2015-03-26 15:33 ` [PATCH v6] " Jan Ťulák @ 2015-03-27 7:25 ` Eryu Guan 2015-03-27 9:15 ` Jan Tulak ` (3 more replies) 0 siblings, 4 replies; 38+ messages in thread From: Eryu Guan @ 2015-03-27 7:25 UTC (permalink / raw) To: Jan Ťulák; +Cc: fstests, lczerner, dsterba 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v6] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 7:25 ` Eryu Guan @ 2015-03-27 9:15 ` Jan Tulak 2015-03-27 9:19 ` Eryu Guan 2015-03-27 9:15 ` [PATCH v7] " Jan Ťulák ` (2 subsequent siblings) 3 siblings, 1 reply; 38+ messages in thread From: Jan Tulak @ 2015-03-27 9:15 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests, lczerner, dsterba ----- 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v6] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 9:15 ` Jan Tulak @ 2015-03-27 9:19 ` Eryu Guan 0 siblings, 0 replies; 38+ messages in thread From: Eryu Guan @ 2015-03-27 9:19 UTC (permalink / raw) To: Jan Tulak; +Cc: fstests, lczerner, dsterba 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v7] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 7:25 ` Eryu Guan 2015-03-27 9:15 ` Jan Tulak @ 2015-03-27 9:15 ` Jan Ťulák 2015-03-27 9:39 ` Eryu Guan 2015-03-27 11:29 ` [PATCH v8] " Jan Ťulák 2015-03-27 11:49 ` [PATCH v9] " Jan Ťulák 3 siblings, 1 reply; 38+ messages in thread From: Jan Ťulák @ 2015-03-27 9:15 UTC (permalink / raw) To: eguan; +Cc: fstests, lczerner, dsterba 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 9:15 ` [PATCH v7] " Jan Ťulák @ 2015-03-27 9:39 ` Eryu Guan 2015-03-27 9:48 ` Jan Tulak 0 siblings, 1 reply; 38+ messages in thread From: Eryu Guan @ 2015-03-27 9:39 UTC (permalink / raw) To: Jan Ťulák; +Cc: fstests, lczerner, dsterba 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 9:39 ` Eryu Guan @ 2015-03-27 9:48 ` Jan Tulak 2015-03-27 11:15 ` Eryu Guan 0 siblings, 1 reply; 38+ messages in thread From: Jan Tulak @ 2015-03-27 9:48 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests, lczerner, dsterba ----- 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 9:48 ` Jan Tulak @ 2015-03-27 11:15 ` Eryu Guan 2015-03-27 11:30 ` Jan Tulak 0 siblings, 1 reply; 38+ messages in thread From: Eryu Guan @ 2015-03-27 11:15 UTC (permalink / raw) To: Jan Tulak; +Cc: fstests, lczerner, dsterba 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 > > ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v7] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 11:15 ` Eryu Guan @ 2015-03-27 11:30 ` Jan Tulak 0 siblings, 0 replies; 38+ messages in thread From: Jan Tulak @ 2015-03-27 11:30 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests, lczerner, dsterba 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v8] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 7:25 ` Eryu Guan 2015-03-27 9:15 ` Jan Tulak 2015-03-27 9:15 ` [PATCH v7] " Jan Ťulák @ 2015-03-27 11:29 ` Jan Ťulák 2015-03-27 11:49 ` [PATCH v9] " Jan Ťulák 3 siblings, 0 replies; 38+ messages in thread From: Jan Ťulák @ 2015-03-27 11:29 UTC (permalink / raw) To: eguan; +Cc: fstests, lczerner, dsterba 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v9] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 7:25 ` Eryu Guan ` (2 preceding siblings ...) 2015-03-27 11:29 ` [PATCH v8] " Jan Ťulák @ 2015-03-27 11:49 ` Jan Ťulák 2015-03-27 14:33 ` Eryu Guan ` (2 more replies) 3 siblings, 3 replies; 38+ messages in thread From: Jan Ťulák @ 2015-03-27 11:49 UTC (permalink / raw) To: fstests; +Cc: eguan 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 11:49 ` [PATCH v9] " Jan Ťulák @ 2015-03-27 14:33 ` Eryu Guan 2015-03-30 13:44 ` David Sterba 2015-04-01 4:35 ` Dave Chinner 2 siblings, 0 replies; 38+ messages in thread From: Eryu Guan @ 2015-03-27 14:33 UTC (permalink / raw) To: Jan Ťulák; +Cc: fstests 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 11:49 ` [PATCH v9] " Jan Ťulák 2015-03-27 14:33 ` Eryu Guan @ 2015-03-30 13:44 ` David Sterba 2015-04-01 4:35 ` Dave Chinner 2 siblings, 0 replies; 38+ messages in thread From: David Sterba @ 2015-03-30 13:44 UTC (permalink / raw) To: Jan Ťulák; +Cc: fstests, eguan 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> ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only. 2015-03-27 11:49 ` [PATCH v9] " Jan Ťulák 2015-03-27 14:33 ` Eryu Guan 2015-03-30 13:44 ` David Sterba @ 2015-04-01 4:35 ` Dave Chinner 2015-04-01 12:09 ` Jan Tulak 2015-04-01 13:17 ` [PATCH v10] " Jan Ťulák 2 siblings, 2 replies; 38+ messages in thread From: Dave Chinner @ 2015-04-01 4:35 UTC (permalink / raw) To: Jan Ťulák; +Cc: fstests, eguan 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only. 2015-04-01 4:35 ` Dave Chinner @ 2015-04-01 12:09 ` Jan Tulak 2015-04-01 12:15 ` Lukáš Czerner 2015-04-01 13:17 ` [PATCH v10] " Jan Ťulák 1 sibling, 1 reply; 38+ messages in thread From: Jan Tulak @ 2015-04-01 12:09 UTC (permalink / raw) To: Dave Chinner; +Cc: fstests, eguan ----- 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v9] fstests: Tests can use any name now, not 3 digits only. 2015-04-01 12:09 ` Jan Tulak @ 2015-04-01 12:15 ` Lukáš Czerner 0 siblings, 0 replies; 38+ messages in thread From: Lukáš Czerner @ 2015-04-01 12:15 UTC (permalink / raw) To: Jan Tulak; +Cc: Dave Chinner, fstests, eguan 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v10] fstests: Tests can use any name now, not 3 digits only. 2015-04-01 4:35 ` Dave Chinner 2015-04-01 12:09 ` Jan Tulak @ 2015-04-01 13:17 ` Jan Ťulák 1 sibling, 0 replies; 38+ messages in thread From: Jan Ťulák @ 2015-04-01 13:17 UTC (permalink / raw) To: fstests; +Cc: david, eguan 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 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] Tests can use any name now, not 3 digits only. 2015-03-20 11:13 ` Eryu Guan 2015-03-20 15:03 ` [PATCH] fstests: tests " Jan Ťulák @ 2015-03-20 15:04 ` Jan Tulak 1 sibling, 0 replies; 38+ messages in thread From: Jan Tulak @ 2015-03-20 15:04 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests 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 > ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2015-04-01 13:17 UTC | newest] Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-04 15:55 [PATCH] Tests can use any name now, not 3 digits only Jan Ťulák 2015-03-18 18:01 ` Jan Tulak 2015-03-20 11:13 ` Eryu Guan 2015-03-20 15:03 ` [PATCH] fstests: tests " Jan Ťulák 2015-03-21 4:49 ` Eryu Guan 2015-03-21 12:02 ` Jan Tulak 2015-03-21 13:11 ` Eryu Guan 2015-03-25 13:27 ` [PATCH] fstests: Tests " Jan Ťulák 2015-03-25 13:32 ` Jan Tulak 2015-03-25 14:44 ` David Sterba 2015-03-25 15:20 ` Lukáš Czerner 2015-03-25 15:27 ` Jan Tulak 2015-03-25 15:43 ` Lukáš Czerner 2015-03-26 13:32 ` Jan Tulak 2015-03-25 17:09 ` Eryu Guan 2015-03-25 17:39 ` Jan Tulak 2015-03-26 13:35 ` Jan Ťulák 2015-03-26 14:41 ` David Sterba 2015-03-26 15:16 ` Jan Tulak 2015-03-26 15:44 ` David Sterba 2015-03-26 15:33 ` [PATCH v6] " Jan Ťulák 2015-03-27 7:25 ` Eryu Guan 2015-03-27 9:15 ` Jan Tulak 2015-03-27 9:19 ` Eryu Guan 2015-03-27 9:15 ` [PATCH v7] " Jan Ťulák 2015-03-27 9:39 ` Eryu Guan 2015-03-27 9:48 ` Jan Tulak 2015-03-27 11:15 ` Eryu Guan 2015-03-27 11:30 ` Jan Tulak 2015-03-27 11:29 ` [PATCH v8] " Jan Ťulák 2015-03-27 11:49 ` [PATCH v9] " Jan Ťulák 2015-03-27 14:33 ` Eryu Guan 2015-03-30 13:44 ` David Sterba 2015-04-01 4:35 ` Dave Chinner 2015-04-01 12:09 ` Jan Tulak 2015-04-01 12:15 ` Lukáš Czerner 2015-04-01 13:17 ` [PATCH v10] " Jan Ťulák 2015-03-20 15:04 ` [PATCH] " Jan Tulak
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.