[parted-devel] [PATCH 2/2] test for the bootcode-in-extended-partition fix

[Jim Meyering]

Petr Uzel wrote:
> * tests/t2300-dos-label-extended-bootcode.sh: New file.
> * tests/Makefile.am (TESTS): Add t2300-dos-label-extended-bootcode.sh.

Hi Petr,

This looks good.
A few suggestions below.

> diff --git a/tests/t2300-dos-label-extended-bootcode.sh b/tests/t2300-dos-label-extended-bootcode.sh
...
> +test_description='Ensure parted preserves bootcode in extended partition.'
> +
> +: ${srcdir=.}
> +. $srcdir/test-lib.sh
> +
> +require_512_byte_sector_size_
> +
> +# Note: the bootcode size is 440B

Rather than that comment, please just define a well-named variable,

bootcode_size=440

And then use it in place of the two hard-coded constants below.

> +
> +dev=loop-file
> +
> +test_expect_success \
> +  'Create the test file' \
> +  'dd if=/dev/zero of=$dev bs=1024c count=100 >/dev/null 2>&1'
> +
> +test_expect_success \
> +  'Create msdos label' \
> +  'parted -s $dev mklabel msdos > out 2>&1'
> +test_expect_success 'Expect no output' 'compare out /dev/null'
> +
> +test_expect_success \
> +  'Create extended partition' \
> +  'parted -s $dev mkpart extended 32s 127s > out 2>&1'
> +test_expect_success 'Expect no output' 'compare out /dev/null'
> +
> +test_expect_success \
> +  'Create logical partition' \
> +  'parted -s $dev mkpart logical 64s 127s > out 2>&1'
> +test_expect_success 'Expect no output' 'compare out /dev/null'
> +
> +test_expect_success \
> +  'Install fake bootcode' \
> +  'dd if=/dev/urandom of=$dev bs=1c seek=16384 count=400 \

Why do you use /dev/urandom?
That might not exist.  How about just using e.g., if=Makefile ?

Shouldn't the above count=400 be count=440 (or now, count=$bootcode_size)?