On Fri, Mar 27, 2015 at 05:48:23AM -0400, Jan Tulak wrote:
>
>
> ----- Original Message -----
> > From: "Eryu Guan" <[email protected]>
> > To: "Jan Ťulák" <[email protected]>
> > Cc: [email protected], [email protected], [email protected]
> > 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 <[email protected]>
> > > ---
> > >
> > > 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 <[email protected]>
> >
> > 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 [email protected]
> > 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
