test/t1000-lvcreate-usage.sh: exercise lvcreate

test/t1000-lvcreate-usage.sh: exercise lvcreate

[Jim Meyering]

Dave and I were talking last week about the lack of
integration tests, and since he was working on lvcreate
and I'd just created this test framework, he sent me a bunch
of commands he'd been using to test lvcreate;  Today I
made them work with the new framework.

When I run the new test by itself, it does this:

    # cd test && ./t1000-lvcreate-usage.sh
    *   ok 1: set up temp files, loopback devices, PVs, and a VG
    *   ok 2: lvcreate w/negative stripesize must fail
    *   ok 3: lvcreate w/too-large stripesize must fail
    *   ok 4: lvcreate w/single stripe succeeds with diagnostics to stdout
    *   ok 5: lvcreate w/no stripe size succeeds with diagnostics to stdout
    *   ok 6: lvcreate w/invalid number of stripes must fail
    *   ok 7: lvcreate w/invalid stripe size must fail
    * passed all 7 test(s)

[Remember, I'm new to LVM, so if there are better/cleaner ways to test
 these things, please let me know, soon.  Otherwise, anything that's ugly
 or plain wrong in this test may end up being copy/pasted into new tests.
 Of course, if there's too much commonality, things will be factored out
 into shell functions that are shared, but still... ]

Here's the patch:

	Test how lvcreate handles its command-line options.

	* test/t1000-lvcreate-usage.sh: New tests.
	* test/Makefile.in (T): Add it.
	Derived from test cases by Dave Wysochanski.

Signed-off-by: Jim Meyering <jim@meyering.net>
---
 test/Makefile.in             |    1 +
 test/t1000-lvcreate-usage.sh |   70 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100755 test/t1000-lvcreate-usage.sh

diff --git a/test/Makefile.in b/test/Makefile.in
index a7f2876..b54560f 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -32,6 +32,7 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 # T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
 T = \
   t0000-basic.sh \
+  t1000-lvcreate-usage.sh \
   t3000-lvcreate-pvtags.sh \
   t4000-pv-range-overflow.sh

diff --git a/test/t1000-lvcreate-usage.sh b/test/t1000-lvcreate-usage.sh
new file mode 100755
index 0000000..5f04b3e
--- /dev/null
+++ b/test/t1000-lvcreate-usage.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+
+test_description='Exercise some lvcreate diagnostics'
+privileges_required_=1
+
+. ./test-lib.sh
+
+cleanup_()
+{
+  test -n "$vg" && {
+    vgchange -an "$vg"
+    lvremove -ff "$vg"
+    vgremove "$vg"
+  } > /dev/null
+  test -n "$d1" && losetup -d "$d1"
+  test -n "$d2" && losetup -d "$d2"
+  rm -f "$f1" "$f2"
+}
+
+test_expect_success \
+  'set up temp files, loopback devices, PVs, and a VG' \
+  'f1="$(pwd)/1" && d1=$(loop_setup_ "$f1") &&
+   f2="$(pwd)/2" && d2=$(loop_setup_ "$f2") &&
+   pvcreate $d1 $d2      &&
+   vg=$(this_test_)-test-vg-$$  &&
+   vgcreate $vg $d1 $d2'
+
+lv=lvcreate-usage-$$
+
+test_expect_success \
+  'lvcreate w/negative stripesize must fail' \
+  'lvcreate -L 64M -n $lv -i2 --stripesize -4 $vg 2>err; test $? = 3 &&
+   grep "^  Negative stripesize is invalid\$" err'
+
+test_expect_success \
+  'lvcreate w/too-large stripesize must fail' \
+  'lvcreate -L 64M -n $lv -i2 --stripesize 4294967291 $vg 2>err; test $? = 3 &&
+   grep "^  Stripe size cannot be larger than 512.00 GB\$" err'
+
+test_expect_success \
+  'lvcreate w/single stripe succeeds with diagnostics to stdout' \
+  'lvcreate -L 64M -n $lv -i1 --stripesize 4 $vg >out 2>err &&
+   grep "^  Redundant stripes argument: default is 1\$" out &&
+   grep "^  Ignoring stripesize argument with single stripe\$" out &&
+   lvdisplay $vg &&
+   lvremove -ff $vg'
+
+test_expect_success \
+  'lvcreate w/no stripe size succeeds with diagnostics to stdout' \
+  'lvcreate -L 64M -n $lv -i2 $vg > out &&
+   grep "^  Using default stripesize" out &&
+   lvdisplay $vg &&
+   lvremove -ff $vg'
+
+test_expect_success \
+  'lvcreate w/invalid number of stripes must fail' \
+  'lvcreate -L 64M -n $lv -i129 $vg 2>err; test $? = 3 &&
+   grep "^  Number of stripes (129) must be between 1 and 128\$" err'
+
+# The case on lvdisplay output is to verify that the LV was not created.
+test_expect_success \
+  'lvcreate w/invalid stripe size must fail' \
+  'lvcreate -L 64M -n $lv -i2 --stripesize 3 $vg 2>err; test $? = 3 &&
+   grep "^  Invalid stripe size 3\.00 KB\$" err &&
+   case "$(lvdisplay $vg)" in "") true ;; *) false ;; esac'
+
+test_done
+# Local Variables:
+# indent-tabs-mode: nil
+# End:
--
1.5.3.1.19.gb5ef6-dirty

test/t1000-lvcreate-usage.sh: exercise lvcreate

[Alasdair G Kergon]

On Mon, Sep 17, 2007 at 04:42:59PM +0200, Jim Meyering wrote:
> When I run the new test by itself, it does this:
>     # cd test && ./t1000-lvcreate-usage.sh
>     *   ok 1: set up temp files, loopback devices, PVs, and a VG
>     *   ok 2: lvcreate w/negative stripesize must fail
>     *   ok 3: lvcreate w/too-large stripesize must fail
>     *   ok 4: lvcreate w/single stripe succeeds with diagnostics to stdout
>     *   ok 5: lvcreate w/no stripe size succeeds with diagnostics to stdout
>     *   ok 6: lvcreate w/invalid number of stripes must fail
>     *   ok 7: lvcreate w/invalid stripe size must fail
>     * passed all 7 test(s)
 
Can you make that easier to parse?
Should every test have a reference number (perhaps derived from its
filename?) that is not going to change and appears on the output line?
Should each test belong to a defined category, e.g. 'lvcreate' above.
What role do the *s serve?  Would a wider range of characters make
the output more readable?
Can the version of the test also appear in the test logs along with
complete command output?  [I noticed a '>/dev/null' earlier that ought
to disappear:-) ]

'ok' with 'must fail' is misleading - try inverting the descriptions
e.g. 'rejects negative stripesizes'  (plural as I'd assume a range of
values are being tested each time)

Alasdair
-- 
agk at redhat.com

test/t1000-lvcreate-usage.sh: exercise lvcreate

[Jim Meyering]

Alasdair G Kergon <agk@redhat.com> wrote:

> On Mon, Sep 17, 2007 at 04:42:59PM +0200, Jim Meyering wrote:
>> When I run the new test by itself, it does this:
>>     # cd test && ./t1000-lvcreate-usage.sh
>>     *   ok 1: set up temp files, loopback devices, PVs, and a VG
>>     *   ok 2: lvcreate w/negative stripesize must fail
>>     *   ok 3: lvcreate w/too-large stripesize must fail
>>     *   ok 4: lvcreate w/single stripe succeeds with diagnostics to stdout
>>     *   ok 5: lvcreate w/no stripe size succeeds with diagnostics to stdout
>>     *   ok 6: lvcreate w/invalid number of stripes must fail
>>     *   ok 7: lvcreate w/invalid stripe size must fail
>>     * passed all 7 test(s)
>
> Can you make that easier to parse?

The idea is that you don't bother to parse it unless something fails :)
But see proposed rewordings below.

> Should every test have a reference number (perhaps derived from its

All of those tests come from the same file: t1000-lvcreate-usage.
Currently, I have these files:

  $ ls -1 t[0-9]*.sh
  t0000-basic.sh*
  t1000-lvcreate-usage.sh*
  t3000-lvcreate-pvtags.sh*
  t4000-pv-range-overflow.sh*

The numbers are pretty arbitrary, now.
Ideally, you'd put simplest tests early and ones that rely on
more and more functionality/tools later.

> filename?) that is not going to change and appears on the output line?

IMHO, the combination of the file name, t1000-lvcreate-usage,
and the textual "title" of each test within that file is sufficient
to identify it.

> Should each test belong to a defined category, e.g. 'lvcreate' above.
> What role do the *s serve?  Would a wider range of characters make
> the output more readable?

The output format is from that of git's tests.

> Can the version of the test also appear in the test logs along with
> complete command output?

If you want more output, you can run the test with --debug --verbose.
Or set those options in test/Makefile.in.  This is the first line of
that file:

    #TEST_OPTS=--verbose --debug

> [I noticed a '>/dev/null' earlier that ought
> to disappear:-) ]

If you mean the one in t1000*'s cleanup_ function, then
I'd rather not remove it.  Otherwise, currently you'd see
this at the end of the test:

    0 logical volume(s) in volume group "t1000-test-vg-11070" now active
    Volume group "t1000-test-vg-11070" successfully removed

> 'ok' with 'must fail' is misleading - try inverting the descriptions
> e.g. 'rejects negative stripesizes'  (plural as I'd assume a range of
> values are being tested each time)

How about these rewordings:

    ./t1000-lvcreate-usage.sh
    *   ok 1: set up temp files, loopback devices, PVs, and a VG
    *   ok 2: lvcreate rejects a negative stripesize
    *   ok 3: lvcreate rejects a too-large stripesize
    *   ok 4: lvcreate w/single stripe succeeds with diagnostics to stdout
    *   ok 5: lvcreate w/no stripe size succeeds with diagnostics to stdout
    *   ok 6: lvcreate rejects an invalid number of stripes
    *   ok 7: lvcreate rejects an invalid stripe size

test/t1000-lvcreate-usage.sh: exercise lvcreate

[Alasdair G Kergon]

> > [I noticed a '>/dev/null' earlier that ought
> > to disappear:-) ]
> If you mean the one in t1000*'s cleanup_ function, then
> I'd rather not remove it.  

What I mean is no output should be discarded anywhere.  It should all
be captured into log files in case someone wants to refer to it.

Alasdair
-- 
agk at redhat.com