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 <[email protected]> > --- > 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/[email protected]/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. ---
signature.asc
Description: Digital signature
