[parted-devel] [PATCH] mkpartfs ext2 2 10 would erroneously report "file system too small"

[Otavio Salvador]

Jim Meyering <jim at meyering.net> writes:

> Otavio Salvador <otavio at debian.org> wrote:
>> Jim Meyering <jim at meyering.net> writes:
>>> Otavio Salvador <otavio at debian.org> wrote:
>>>> test also on the library unittesting so we can know if the library
>>>> isn't broken again too...
>>>>
>>>> Can you try?
>>>
>>> That would not be a good way to spend my limited time.
>>>
>>> Besides, why bother?
>>> The above test exercises the affected code already, and since no one
>>> will be changing the public command-line interface any time soon,
>>> it should be more than enough to keep the bug from resurfacing.
>>
>> I personally don't like to have integration tests only from basic
>> things. This specific case is handled by the library and then I think
>> we ought to have tests for it too.
>
> Then let's turn it around.
> It's something you want, so would you like to give it a try?

I'd prefer to defer the commit of it while we don't produce the tests
but you already commited it. No problem now but would be better to get
a consensus before commiting.

That's how we have being doing it here.

About I wanting it, well... let's examine it a bit further:

Integration tests aren't suppose to deal with individual units of
system (this is responsability of the unittest) and then your test is
wrongly designed if kept alone.

Your included test do test the UI system and detect if it fails but it
can't ensure we don't break the library.

compute_block_counts ought to be tested since it's being share by two
methods now. Would be nice to check the other two methods too since
you're changing it.

Those specific tests do not look too difficult that could justify its
ignoring.

Besides, there's a bad indentation on the commited patch, see:

diff --git a/libparted/fs/ext2/ext2_mkfs.c b/libparted/fs/ext2/ext2_mkfs.c
index 86175a8..8f74a45 100644
--- a/libparted/fs/ext2/ext2_mkfs.c
+++ b/libparted/fs/ext2/ext2_mkfs.c
@@ -518,6 +542,7 @@ struct ext2_fs *ext2_mkfs(struct ext2_dev_handle *handle,
 
 	if (numblocks == 0)
 		numblocks = handle->ops->get_size(handle->cookie);
+        PED_ASSERT(numblocks != 0, return NULL);

Wrongly indented...

-- 
        O T A V I O    S A L V A D O R
---------------------------------------------
 E-mail: otavio at debian.org      UIN: 5906116
 GNU/Linux User: 239058     GPG ID: 49A5F855
 Home Page: http://otavio.ossystems.com.br
---------------------------------------------
"Microsoft sells you Windows ... Linux gives
 you the whole house."