xfs
[Top] [All Lists]

Re: [PATCH] xfstests 273 274 275: do test in dedicated dir and leave tes

To: Eryu Guan <guaneryu@xxxxxxxxx>
Subject: Re: [PATCH] xfstests 273 274 275: do test in dedicated dir and leave test files in it
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri, 27 Jan 2012 14:42:51 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1326388815-21181-1-git-send-email-guaneryu@xxxxxxxxx>
References: <1326388815-21181-1-git-send-email-guaneryu@xxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20111222 Thunderbird/9.0.1
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@xxxxxxxxx>
> ---
...

> 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 ]

<Prev in Thread] Current Thread [Next in Thread>