[PATCH 0/5] xfstests: A few updates to non-test code

[PATCH 0/5] xfstests: A few updates to non-test code

[David Sterba]

Minor bugfixes or enhancements.

David Sterba (5):
  xfstests: fix minor build warning
  xfstests: use value of FSTYP if defined externally
  xfstests: don't suggest to run full diff when DIFF_LENGTH is 0
  xfstests: accept tests/ prefix for test name on commandline
  xfstests: update lsqa.pl for new tests layout

 Makefile |  3 +--
 README   |  3 +++
 check    | 16 +++++++++-------
 lsqa.pl  | 32 ++++++++++++++++++++++++--------
 4 files changed, 37 insertions(+), 17 deletions(-)

-- 
1.8.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/5] xfstests: fix minor build warning

[David Sterba]

Make in the toplevel directory tries to process the 'group' file that
existed in the previous file layout

$ make
 sed: can't read group: No such file or directory
 sed: can't read group: No such file or directory
 ...

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index a7f6f9d374ca..30d8747d1f79 100644
--- a/Makefile
+++ b/Makefile
@@ -41,7 +41,6 @@ endif
 
 SRCTAR = $(PKG_NAME)-$(PKG_VERSION).tar.gz
 
-TESTS = $(shell sed -n -e '/^[0-9][0-9][0-9]*/s/ .*//p' group)
 CONFIGURE = configure include/builddefs include/config.h
 LSRCFILES = configure configure.ac aclocal.m4 README VERSION
 LDIRT = config.log .ltdep .dep config.status config.cache confdefs.h \
@@ -60,7 +59,7 @@ endif
 export TESTS_DIR = tests
 SUBDIRS = $(LIB_SUBDIRS) $(TOOL_SUBDIRS) $(TESTS_DIR)
 
-default: include/builddefs $(DMAPI_MAKEFILE) $(TESTS)
+default: include/builddefs $(DMAPI_MAKEFILE)
 ifeq ($(HAVE_BUILDDEFS), no)
 	$(Q)$(MAKE) $(MAKEOPTS) $@
 else
-- 
1.8.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/5] xfstests: use value of FSTYP if defined externally

[David Sterba]

The initial value of FSTYP is unconditionally set to 'xfs' and the
filesystem type is taken from the TEST_DEV. This could lead to confusion
if the device hasn't been formatted yet, eg. an empty image in VM, or
a different test setup took place before.

Now one can specify the desired FSTYP in advance and be safe. If unset,
the fallback to TEST_DEV type continues to work.

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 README | 3 +++
 check  | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/README b/README
index a49ca7c3ff15..62974ef87b89 100644
--- a/README
+++ b/README
@@ -65,6 +65,9 @@ Preparing system for tests (IRIX and Linux):
                environment variable set to "yes" will enable their use.
              - setenv DIFF_LENGTH "number of diff lines to print from a failed test",
                by default 10, set to 0 to print the full diff
+             - setenv FSTYP "the filesystem you want to test", the filesystem
+               type is devised from the TEST_DEV device, but you may want to
+               override it
         - or add a case to the switch in common/config assigning
           these variables based on the hostname of your test
           machine
diff --git a/check b/check
index 320ad26b7c2c..9e4275b1fa37 100755
--- a/check
+++ b/check
@@ -33,7 +33,7 @@ showme=false
 have_test_arg=false
 randomize=false
 here=`pwd`
-FSTYP=xfs
+FSTYP=${FSTYP:-xfs}
 xfile=""
 
 # start the initialisation work now
@@ -57,8 +57,9 @@ then
     exit 1
 fi
 
-# Autodetect fs type based on what's on $TEST_DEV
-if [ "$HOSTOS" == "Linux" ]; then
+# Autodetect fs type based on what's on $TEST_DEV unless it's been set
+# externally
+if [ -z "$FSTYP" -a "$HOSTOS" == "Linux" ]; then
     FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
 fi
 export FSTYP
-- 
1.8.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/5] xfstests: don't suggest to run full diff when DIFF_LENGTH is 0

[David Sterba]

If DIFF_LENGTH is set to 0, the full output is available and the message
does not make sense.

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 check | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/check b/check
index 9e4275b1fa37..d578e49b78d4 100755
--- a/check
+++ b/check
@@ -510,11 +510,11 @@ do
 				cat
 			else
 				head -n "$DIFF_LENGTH"
+				echo "..."
+				echo "(Run '$diff $seq.out $seqres.out.bad'"
+					" to see the entire diff)"
 			fi; } | \
 			sed -e 's/^\(.\)/    \1/'
-		    echo "     ..."
-		    echo "     (Run '$diff $seq.out $seqres.out.bad' to see the" \
-			 "entire diff)"
 		    err=true
 		fi
 	    fi
-- 
1.8.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/5] xfstests: accept tests/ prefix for test name on commandline

[David Sterba]

Just for convenience to let tab completion or shell globs work (files
that are not in the group file are ignored).

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 check | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/check b/check
index d578e49b78d4..a787c4a9f69f 100755
--- a/check
+++ b/check
@@ -263,12 +263,13 @@ if $have_test_arg; then
 			exit $status
 			;;
 		*)	test_dir=`dirname $1`
+			test_dir=${test_dir#$SRC_DIR/*}
 			test_name=`basename $1`
 			group_file=$SRC_DIR/$test_dir/group
 
 			if egrep "^$test_name" $group_file >/dev/null ; then
 				# in group file ... OK
-				echo $SRC_DIR/$1 >>$tmp.arglist
+				echo $SRC_DIR/$test_dir/$test_name >>$tmp.arglist
 			else
 				# oops
 				echo "$1 - unknown test, ignored"
-- 
1.8.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 5/5] xfstests: update lsqa.pl for new tests layout

[David Sterba]

Accepted parameter types:
- nothing - list all tests from all subdirectories in tests/*
- tests/DIR - list all tests from DIR
- tests/DIR/123 - show header from single test

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 lsqa.pl | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/lsqa.pl b/lsqa.pl
index 040196f70120..b4fc83f43f99 100755
--- a/lsqa.pl
+++ b/lsqa.pl
@@ -15,13 +15,20 @@
 # along with this program; if not, write the Free Software Foundation,
 # Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 #
+
+# Print headers of given tests
+# Accepted parameter types:
+# - nothing - list all tests from all subdirectories in tests/*
+# - tests/DIR - list all tests from DIR
+# - tests/DIR/123 - show header from single test
+
 use strict;
 
 use Getopt::Long;
 
 sub help();
 sub get_qa_header($);
-sub get_qa_tests();
+sub get_qa_tests;
 
 my %opt;
 
@@ -43,8 +50,19 @@ if ($opt{'help'}) {
     die help();
 }
 
-my @qatests = map {sprintf("%03d", $_)} @ARGV;
-@qatests = get_qa_tests() unless (@qatests);
+my @qatests;
+
+if (!@ARGV) {
+    my $d="tests";
+    opendir(DIR, $d);
+    map { push @qatests,get_qa_tests("$d/$_") if -d "$d/$_" } readdir(DIR);
+    closedir(DIR);
+}
+
+foreach (@ARGV) {
+    push @qatests,$_ if -f && /\d{3}$/;
+    push @qatests,get_qa_tests($_) if -d;
+}
 
 foreach (@qatests) {
     my @h = get_qa_header($_);
@@ -103,13 +121,11 @@ sub get_qa_header($) {
     return @l;
 }
 
-sub get_qa_tests() {
+sub get_qa_tests {
     my $d = shift || $ENV{'PWD'};
 
     opendir(my $DIR, $d) || die "can't opendir $d: $!";
-    my @qa = grep {m/^\d\d\d$/ && -f "$d/$_" } readdir($DIR);
+    my @qa = sort grep { m/^\d\d\d$/ && -f "$d/$_" } readdir($DIR);
     closedir($DIR);
-
-    return @qa;
+    return map { $_ = "$d/$_" } @qa;
 }
-
-- 
1.8.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/5] xfstests: fix minor build warning

[Ben Myers]

On Thu, Jan 16, 2014 at 06:07:11PM +0100, David Sterba wrote:
> Make in the toplevel directory tries to process the 'group' file that
> existed in the previous file layout
> 
> $ make
>  sed: can't read group: No such file or directory
>  sed: can't read group: No such file or directory
>  ...
> 
> Signed-off-by: David Sterba <dsterba@suse.cz>

Looks good.

Reviewed-by: Ben Myers <bpm@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/5] xfstests: use value of FSTYP if defined externally

[Dave Chinner]

On Thu, Jan 16, 2014 at 06:07:12PM +0100, David Sterba wrote:
> The initial value of FSTYP is unconditionally set to 'xfs' and the
> filesystem type is taken from the TEST_DEV. This could lead to confusion
> if the device hasn't been formatted yet, eg. an empty image in VM, or
> a different test setup took place before.
> 
> Now one can specify the desired FSTYP in advance and be safe. If unset,
> the fallback to TEST_DEV type continues to work.
> 
> Signed-off-by: David Sterba <dsterba@suse.cz>
> ---
>  README | 3 +++
>  check  | 7 ++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/README b/README
> index a49ca7c3ff15..62974ef87b89 100644
> --- a/README
> +++ b/README
> @@ -65,6 +65,9 @@ Preparing system for tests (IRIX and Linux):
>                 environment variable set to "yes" will enable their use.
>               - setenv DIFF_LENGTH "number of diff lines to print from a failed test",
>                 by default 10, set to 0 to print the full diff
> +             - setenv FSTYP "the filesystem you want to test", the filesystem
> +               type is devised from the TEST_DEV device, but you may want to
> +               override it
>          - or add a case to the switch in common/config assigning
>            these variables based on the hostname of your test
>            machine
> diff --git a/check b/check
> index 320ad26b7c2c..9e4275b1fa37 100755
> --- a/check
> +++ b/check
> @@ -33,7 +33,7 @@ showme=false
>  have_test_arg=false
>  randomize=false
>  here=`pwd`
> -FSTYP=xfs
> +FSTYP=${FSTYP:-xfs}

":-xfs" means assign the value of $xfs if $FTYPE is null. xfs is not
a variable....

>  xfile=""
>  
>  # start the initialisation work now
> @@ -57,8 +57,9 @@ then
>      exit 1
>  fi
>  
> -# Autodetect fs type based on what's on $TEST_DEV
> -if [ "$HOSTOS" == "Linux" ]; then
> +# Autodetect fs type based on what's on $TEST_DEV unless it's been set
> +# externally
> +if [ -z "$FSTYP" -a "$HOSTOS" == "Linux" ]; then

If the default value expansion is fixed, FSTYP will always have a
value here Hence it will never, ever probe.

>      FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
>  fi
>  export FSTYP

I suspect what you want is:

-FSTYP=xfs

.....

if [ -z "$FSTYP" -a "$HOSTOS" == "Linux" ]; then
     FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
fi
FSTYP=${FSTYP:=xfs}
export FSTYP

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/5] xfstests: don't suggest to run full diff when DIFF_LENGTH is 0

[Dave Chinner]

On Thu, Jan 16, 2014 at 06:07:13PM +0100, David Sterba wrote:
> If DIFF_LENGTH is set to 0, the full output is available and the message
> does not make sense.
> 
> Signed-off-by: David Sterba <dsterba@suse.cz>
> ---
>  check | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/check b/check
> index 9e4275b1fa37..d578e49b78d4 100755
> --- a/check
> +++ b/check
> @@ -510,11 +510,11 @@ do
>  				cat
>  			else
>  				head -n "$DIFF_LENGTH"
> +				echo "..."
> +				echo "(Run '$diff $seq.out $seqres.out.bad'"
> +					" to see the entire diff)"

You're missing a trailing "\" there. Even simple patches need
testing. ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/5] xfstests: A few updates to non-test code

[Dave Chinner]

On Thu, Jan 16, 2014 at 06:07:10PM +0100, David Sterba wrote:
> Minor bugfixes or enhancements.
> 
> David Sterba (5):
>   xfstests: fix minor build warning
>   xfstests: use value of FSTYP if defined externally
>   xfstests: don't suggest to run full diff when DIFF_LENGTH is 0
>   xfstests: accept tests/ prefix for test name on commandline
>   xfstests: update lsqa.pl for new tests layout

Hi David, I committed the three patches that were OK, so if you can
fix up the other two and resubmit them, that'd be great.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/5] xfstests: use value of FSTYP if defined externally

[David Sterba]

On Mon, Jan 20, 2014 at 01:13:45PM +1100, Dave Chinner wrote:
> On Thu, Jan 16, 2014 at 06:07:12PM +0100, David Sterba wrote:
> > --- a/check
> > +++ b/check
> > @@ -33,7 +33,7 @@ showme=false
> >  have_test_arg=false
> >  randomize=false
> >  here=`pwd`
> > -FSTYP=xfs
> > +FSTYP=${FSTYP:-xfs}
> 
> ":-xfs" means assign the value of $xfs if $FTYPE is null. xfs is not
> a variable....

Docs say that

  ${parameter:-word}
  If parameter is unset or null, the expansion of word is substituted.
  Otherwise, the value of parameter is substituted.

so 'xfs' will expand to itself.  And I did test this one with:

$ cat default.sh << EOF
FSTYP=${FSTYP:-xfs}
echo $FSTYP
EOF

$ cat external.sh << EOF
FSTYP=btrfs
source default.sh
EOF

$ sh default.sh
xfs

$ sh external.sh
btrfs

> > -# Autodetect fs type based on what's on $TEST_DEV
> > -if [ "$HOSTOS" == "Linux" ]; then
> > +# Autodetect fs type based on what's on $TEST_DEV unless it's been set
> > +# externally
> > +if [ -z "$FSTYP" -a "$HOSTOS" == "Linux" ]; then
> 
> If the default value expansion is fixed, FSTYP will always have a
> value here Hence it will never, ever probe.

> >      FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
> >  fi
> >  export FSTYP
> 
> I suspect what you want is:
> 
> -FSTYP=xfs
> 
> .....
> 
> if [ -z "$FSTYP" -a "$HOSTOS" == "Linux" ]; then
>      FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
> fi
> FSTYP=${FSTYP:=xfs}
> export FSTYP

Right, the default assignment has to be last. I'll use the := form
to be consistent with what's used in the file, though :- works here as
well, just does not assign the variable within the ${...} expression.
The result is the same.


thanks,
david

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/5] xfstests: use value of FSTYP if defined externally

[Dave Chinner]

On Mon, Jan 20, 2014 at 07:19:01PM +0100, David Sterba wrote:
> On Mon, Jan 20, 2014 at 01:13:45PM +1100, Dave Chinner wrote:
> > On Thu, Jan 16, 2014 at 06:07:12PM +0100, David Sterba wrote:
> > > --- a/check
> > > +++ b/check
> > > @@ -33,7 +33,7 @@ showme=false
> > >  have_test_arg=false
> > >  randomize=false
> > >  here=`pwd`
> > > -FSTYP=xfs
> > > +FSTYP=${FSTYP:-xfs}
> > 
> > ":-xfs" means assign the value of $xfs if $FTYPE is null. xfs is not
> > a variable....
> 
> Docs say that
> 
>   ${parameter:-word}
>   If parameter is unset or null, the expansion of word is substituted.
>   Otherwise, the value of parameter is substituted.
> 
> so 'xfs' will expand to itself.  And I did test this one with:
> 
> $ cat default.sh << EOF
> FSTYP=${FSTYP:-xfs}
> echo $FSTYP
> EOF
> 
> $ cat external.sh << EOF
> FSTYP=btrfs
> source default.sh
> EOF
> 
> $ sh default.sh
> xfs
> 
> $ sh external.sh
> btrfs

Funny, I did the same tests to check and kept getting no output
instead of the "xfs" result. Changing it to := made it work just
fine. Oh well... :/

> > > -# Autodetect fs type based on what's on $TEST_DEV
> > > -if [ "$HOSTOS" == "Linux" ]; then
> > > +# Autodetect fs type based on what's on $TEST_DEV unless it's been set
> > > +# externally
> > > +if [ -z "$FSTYP" -a "$HOSTOS" == "Linux" ]; then
> > 
> > If the default value expansion is fixed, FSTYP will always have a
> > value here Hence it will never, ever probe.
> 
> > >      FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
> > >  fi
> > >  export FSTYP
> > 
> > I suspect what you want is:
> > 
> > -FSTYP=xfs
> > 
> > .....
> > 
> > if [ -z "$FSTYP" -a "$HOSTOS" == "Linux" ]; then
> >      FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
> > fi
> > FSTYP=${FSTYP:=xfs}
> > export FSTYP
> 
> Right, the default assignment has to be last. I'll use the := form
> to be consistent with what's used in the file, though :- works here as
> well, just does not assign the variable within the ${...} expression.
> The result is the same.

Thanks.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs