[parted-devel] [PATCH 2/2] tests: add one: try to remove a busy partition

[Jim Meyering]

Otavio Salvador wrote:
> This test try to reproduce the issue reported in Debian bug #582818[1].
>
>  1. http://bugs.debian.org/582818
>
> Basically it does:
>
>  * create two primary partitions
>  * mount the second one
>  * remove the first (must works)
>  * try to remove the second (must fail)
>
> * tests/t1101-busy-partition.sh: new file.
> * tests/Makefile.am: add the new test in check target.

Thank you for adding a test.
The idea is welcome, but it is using a framework that
I am trying to avoid.  This style of test works only
when you have selected a DEVICE_TO_ERASE, while the
newer scsi_debug_setup_-using tests work by creating
a memory-mapped device, and hence are easier to run,
especially on systems where you cannot plug in a USB key
or find a spare spindle.

Also, I find the test-lib.sh framework relatively hard to debug.
I have been migrating tests to a newer framework, that uses
t-lib.sh instead:

    $ g grep test-lib.sh|wc -l
    16
    $ g grep t-lib.sh|wc -l
    47

Would you mind adjusting your new test to use the newer tools?

> +: ${srcdir=.}
> +. $srcdir/test-lib.sh
> +require_512_byte_sector_size_
> +dev=$DEVICE_TO_ERASE
> +
> +test_expect_success \
> +    "setup: create a fat32 file system on $dev" \
> +    'dd if=/dev/zero "of=$dev" bs=1k count=1 2> /dev/null &&
> +     parted -s "$dev" mklabel msdos                > out 2>&1 &&
> +     parted -s "$dev" mkpartfs primary fat32 1 40 >> out 2>&1 &&
> +     parted -s "$dev" mkpartfs primary fat32 40 80 >> out 2>&1'
> +test_expect_success 'expect no output' 'compare out /dev/null'
> +
> +mount_point="`pwd`/mnt"
> +
> +# Be sure to unmount upon interrupt, failure, etc.
> +cleanup_() { umount "${dev}2" > /dev/null 2>&1; }
> +
> +# There's a race condition here: on udev-based systems, the partition#1
> +# device, ${dev}1 (i.e., /dev/sdd1) is not created immediately, and

This comment should say ${dev}2, not ${dev}1.
Rather than using that open-coded loop, you might want to use the
wait_for_dev_to_appear_ function defined in t-local.sh.

> +# without some delay, this mount command would fail.  Using a flash card
> +# as $dev, the loop below typically iterates 7-20 times.
> +test_expect_success \
> +    'create mount point dir. and mount the just-created partition on it' \
> +    'mkdir $mount_point &&
> +     i=0; while :; do test -e "${dev}2" && break; test $i = 90 && break;
> +	              i=$(expr $i + 1); done;
> +     mount "${dev}2" $mount_point'
> +
> +test_expect_success \
> +    'now that a partition 2 is mounted, rm 1 must work' \
> +    'parted -s "$dev" rm 1 > out 2>&1'
> +
> +
> +test_expect_failure \
> +    'now that a partition 1 is removed, rm 2 attempt must fail' \
> +    'parted -s "$dev" rm 2 > out 2>&1'

Another reason not to like this framework.
I think expect_failure will succeed even if parted segfaults.
If you're stuck using this framework,
it's better to expect_success and to ensure an exit code of 1,
say, via test $? = 1

At worst, go ahead and commit this (taking into account
the smaller suggestions) and eventually I'll rewrite it.