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

[David Cantrell]

On Fri, 2007-05-18 at 12:39 -0300, Otavio Salvador wrote:
> 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.

Otavio, Jim.... First, you both have good points here, but I think we've
missed something here with our unit testing framework.  Ideally, the
developer shouldn't also be the one to write all the testing.  For
maximum testing effectiveness, unit tests should be written by another
person rather than the author.  We should think about this going
forward.

We also need to take in to account that the parted code base is not in
great shape.  It's had many hands in it and we all are trying to bring
it up to date and correct a lot of defects.  Jim's patch clears a make
distcheck failure which, in my opinion, is more important in the short
term than a unit test for the function.

Jim is fixing code in libparted based on his available time, so if he's
unable to do something we as a team want, we should pick up the slack
and finish it up for him.

As development progresses and we begin making more substantial changes
to parted, we can start enforcing more strict policies such as requiring
full unit testing for a new function at commit time.  For now, I think
we should all be a little more flexible with regards to incoming
patches.  If it fixes a bug, let's take it.

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

This is a different problem entirely.  Did we all ever agree on an
indentation style?  We discussed it, but I don't remember a standard
being agreed upon.

I want spaces rather than tabs, 8 spaces per indentation level.  Tabs
used in Makefiles for obvious reasons.  No more than 80 chars per line.

-- 
David Cantrell <dcantrell at redhat.com>
Red Hat / Westford, MA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.alioth.debian.org/pipermail/parted-devel/attachments/20070518/c07a7869/attachment.pgp