Hi, Hemanth,
Here's a question -- what are you testing? (Not just here, but in
general, with your test infrastructure)
There are (at least) three classes of tests you could be doing:
1) Unit tests, which test individual functions within the code and
ensure they do what they're meant to do.
2) Integration tests, which test the full end-to-end system.
3) Partial integration tests, which exercise the kernel
filesystem code.
4) Partial integration tests, which exercise the userspace tools code.
Now, clearly you're not doing (1) here. It's going to be hard to
separate (2) from (3) and (4), but it's possible to write your tests
to do more of one or of the other. (*)
xfstests clearly is much more geared to (3), and stresses the
kernel filesystem implementation rather than the userspace tools. If
you want to test the implementation of qgroups, it belongs in
xfstests. If you want to test the userspace code, you need to make
sure that (over all your tests) you cover every command-line option,
and every different way of using the tool, and ensure that it does the
right things.
What you've written in this patch seems to be more about testing
the kernel behaviour than the userspace tools, but it'd be good if you
can put your work into the context I've just talked about above.
More comments below...
On Fri, Feb 15, 2013 at 06:35:41PM +0530, Hemanth Kumar wrote:
>
> Signed-off-by: Hemanth Kumar <hemanthkumar51@xxxxxxxxx>
> ---
> hq.sh | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
> create mode 100644 hq.sh
>
> diff --git a/hq.sh b/hq.sh
> new file mode 100644
> index 0000000..6a0a820
> --- /dev/null
> +++ b/hq.sh
Rather cryptic filename here.
If this is to be applied to btrfs-progs, I'd recommend putting all
your test scripts in a test subdir, and a "test" target in the
Makefile that invokes the tests.
> @@ -0,0 +1,33 @@
> +#! /bin/bash
> +# Btrfs quotas test case
> +# Hi all,
> +# This is shell script to test the hierarchical quotas feature of Btrfs
> +# I created Btrfs filesystem on a new
> +# partition, then activated quota management ('btrfs quota enable'), and
> +# created a few subvolumes of multiple levels.
This text doesn't belong in the script -- it's your commit message,
and needs to go above the S-o-B line above.
I'd recommend using git for your development, because when you do a
commit, it'll give you a place to put the commit message. Then, with
git-format-patch and git-send-email (and the dry-run option), you can
get it to format the mail exactly right, instead of producing slightly
mangled output as here.
> +cleanup()
> +{
> + btrfs subvolume delete $TEST_DIR/vol1/vol2/vol3
> + btrfs subvolume delete $TEST_DIR/vol1/vol2
> + btrfs subvolume delete $TEST_DIR/vol1
> + btrfs subvolume disable $TEST_DIR
> +}
> +
> +#trap "_cleanup ; exit \$status" 0 1 2 3 15
> +
> +btrfs quota enable $TEST_DIR
> +echo "quota enabled on $TEST_DEV"
> +btrfs subvolume create $TEST_DIR/vol1
> +btrfs subvolume create $TEST_DIR/vol1/vol2
> +btrfs subvolume create $TEST_DIR/vol1/vol2/vol3
> +btrfs qgroup limit 5m $TEST_DIR/vol1
> +btrfs qgroup limit 3m $TEST_DIR/vol1/vol2
> +btrfs qgroup limit 2m $TEST_DIR/vol1/vol2/vol3
> +dd if=$TEST_DEV of=$TEST_DIR/vol1/vol2/vol3/file1 bs=3M count=1
> +dd if=$TEST_DEV of=$TEST_DIR/vol1/vol2/file1 bs=2M count=1
> +dd if=$TEST_DEV of=$TEST_DIR/vol1/file1 bs=5M count=1
What happens if one of these commands fails? You should be testing
the exit status of almost every command. Then you can fail in one of
two different ways: a failure of the test (where the thing you were
trying to test has not succeeded), or a failure of the test harness
(where the setup functions have gone wrong).
I'd do this with a setup function to set up the test environment, a
teardown function to undo it cleanly afterwards, and one or more test
functions which can be used to run tests.
You may want to take a look at my earlier work on this, at:
http://www.mail-archive.com/linux-btrfs@xxxxxxxxxxxxxxx/msg13153.html
That should at least give you a basic infrastructure to work in.
> +cleanup
> +exit
It's the end of the script. You don't need to use exit here.
Hugo.
(*) OK, you could actually test quite a bit of the userspace tools
even on a system with no btrfs module, by using an LD_PRELOAD library
that stubs out the btrfs ioctls and records which ioctls have been
called. But for now, I'd suggest that you leave that idea alone.
--
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
--- vi vi vi: the Editor of the Beast. ---
Attachment:
signature.asc
Description: Digital signature
