All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: Eryu Guan <guaneryu@gmail.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] xfstests 273 274 275: do test in dedicated dir and leave test files in it
Date: Fri, 27 Jan 2012 14:42:51 -0600	[thread overview]
Message-ID: <4F230C4B.2010200@sandeen.net> (raw)
In-Reply-To: <1326388815-21181-1-git-send-email-guaneryu@gmail.com>

On 1/12/12 11:20 AM, Eryu Guan wrote:
> Do all testings in dedicated dir($SCRATCH_MNT/$seq) instead of
> $SCRATCH_MNT and don't remove test files in _cleanup() for debug
> purpose.
> 
> Signed-off-by: Eryu Guan <guaneryu@gmail.com>
> ---
...

> diff --git a/274 b/274
> index b658004..c351c40 100755
> --- a/274
> +++ b/274
> @@ -35,13 +35,15 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>  _cleanup()
>  {
>  	cd /
> -	rm -f $SCRATCH_MNT/* $tmp.*
> +	rm -f $tmp.*
>  	_scratch_unmount
>  }
>  
>  . ./common.rc
>  . ./common.filter
>  
> +WORK_DIR="$SCRATCH_MNT/$seq"
> +
>  # real QA test starts here
>  _supported_fs generic
>  _supported_os IRIX Linux
> @@ -57,8 +59,9 @@ umount $SCRATCH_DEV 2>/dev/null
>  _scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
>  _scratch_mount
>  
> -rm -rf $SCRATCH_MNT/*
> -cd $SCRATCH_MNT
> +rm -rf $WORK_DIR
> +mkdir -p $WORK_DIR
> +cd $WORK_DIR

I think this test could use more work; for one, I don't like cd-ing into
the dir, because then if you send output to $seq.full it doesn't go
where you expect it.  I'd rather not cd, and:

>  dd if=/dev/zero of=test bs=4K count=1 >/dev/null 2>&1

dd if=/dev/zero of=$WORK_DIR/test bs=4K count=1 >/dev/null 2>&1

then specify the full path to the files instead.

TBH I'm still trying to work out what exactly 274 is supposed to test;
there are no comments about the behavior other than:

# preallocation test

As a general plea to those writing tests, _please_ include a concise
but descriptive comment at the top so that the user knows just what
the test tests, and how it tests it.  Inline comments help too;
bash isn't always so easy to read.

I'm thinking that an overhaul for 273 and 274 like I did for
275 might be in order...

-Eric

>  if [ $? -ne 0 ]
>  then
> diff --git a/275 b/275
> index 214262e..7a4d414 100755
> --- a/275
> +++ b/275
> @@ -36,13 +36,15 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>  _cleanup()
>  {
>  	cd /
> -	rm -f $SCRATCH_MNT/* $tmp.*
> +	rm -f $tmp.*
>  	_scratch_unmount
>  }
>  
>  . ./common.rc
>  . ./common.filter
>  
> +WORK_DIR="$SCRATCH_MNT/$seq"
> +
>  # real QA test starts here
>  _supported_fs generic
>  _supported_os IRIX Linux
> @@ -58,8 +60,9 @@ umount $SCRATCH_DEV 2>/dev/null
>  _scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
>  _scratch_mount
>  
> -rm -rf $SCRATCH_MNT/*
> -cd $SCRATCH_MNT
> +rm -rf $WORK_DIR
> +mkdir -p $WORK_DIR
> +cd $WORK_DIR
>  
>  dd if=/dev/zero of=tmp1 bs=4K count=1 >/dev/null 2>&1
>  if [ $? -ne 0 ]

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

  parent reply	other threads:[~2012-01-27 20:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-12 17:20 [PATCH] xfstests 273 274 275: do test in dedicated dir and leave test files in it Eryu Guan
2012-01-27 11:01 ` Christoph Hellwig
2012-01-27 16:28   ` Eryu Guan
2012-01-27 20:42 ` Eric Sandeen [this message]
2012-01-27 21:16 ` Eric Sandeen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F230C4B.2010200@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=guaneryu@gmail.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.