cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chip Childers <chip.child...@sungard.com>
Subject Re: [DISCUSS] All checkins must include unit testing
Date Mon, 12 Nov 2012 21:39:10 GMT
On Mon, Nov 12, 2012 at 3:56 AM, Hugo Trippaers
<HTrippaers@schubergphilis.com> wrote:
> While I really like the idea of including a unittest with every commit, I don't feel
very comfortable with doing this right now. The current code coverage is pretty low and just
adding new unittests for commits might even give a false sense of security. Next to that,
enforcing this through technical means might prevent people from participating as we make
the bar higher for anybody willing to join. I'd rather have somebody submit a patch without
a unittest and having to do it myself, than not receiving the patch at all.
>

The community engagement concern is a great point Hugo.  Lower the
barrier to entry, and more people will help.  On the other hand, we do
need to ensure that those that accept the code take the time to deal
with test cases and documentation.  With a low barrier to entry comes
an increase in committer responsibilities.

> I think the way forward would be to devote serious effort to get the current code base
covered with unittests first. That way we will gain the much needed confidence in the current
code, which means that we can more easily integrate changes coming in from other branches
and from reviews and still have a level of certainty that stuff still works. Next to that
it will provide an enormous amount of example code for would be committers and people sending
patches. For a lot of people it is much easier to adapt an exisiting unittest then creating
a new one from scratch, especially with a complex code base like ours. Custom injection code
(like ComponentLocator) requires some knowledge to work into unittests and might be a tad
difficult for people not intimately familiar with it.
>

That custom injection code is absolutely something that challenges
test writers today.  I'm certainly looking forward to the Javelin
branch merging into master!

> We as the committer team should be able to govern ourselves and take the responsibility
to start including unittests for exisiting code. If we can't agree to get this done as a community,
implementing a technical solution will not help, but probably have the opposite effect of
alienating the people. And that is something that we should not want.

+1 - absolutely

> Cheers,
>
> Hugo
>
>> -----Original Message-----
>> From: prasanna [mailto:srivatsav.prasanna@gmail.com]
>> Sent: Saturday, November 10, 2012 5:30 PM
>> To: cloudstack-dev@incubator.apache.org
>> Subject: Re: [DISCUSS] All checkins must include unit testing
>>
>> I like the idea of unittests with every commit.
>>
>> A few related topics I have that might be worth considering in this
>> discussion:
>>
>> 1) What about unittests for the existing code? Or do we fill that gap slowly?
>> 2) Minor but we should probably update the unittesting wiki on how to
>> mock our managers, daos etc?
>>
>> And a few things I'm confused about:
>>
>> 1) Gerrit is a code review tool just like reviewboard? So - would it really
>> serve the purpose if committers can still check-in without unittests?
>> 2) Or do we enforce everyone to send in for gerrit review? That seems
>> counter-intuitive to the idea of committer-ship?
>>
>> --
>> Prasanna.,

Mime
View raw message