Message ID | 1427290055-32647-1-git-send-email-jtulak@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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 > -- 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
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. -- 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
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 >
----- 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 -- 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
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 >
----- 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 -- 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
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? #
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(-)