The need for tests, expectations, frustrations and encouragement

[Andrew Bartlett]

Kai,

Is this reasonable to send to kukks and the list?

(Just want to bounce it off you before I send it, in case I'm totally
off-base). 

Thanks,

Andrew Bartlett

On Sun, 2013-06-02 at 00:57 +0200, Günter Kukkukk wrote:
> Am Freitag, 31. Mai 2013, 16:40:03 schrieb Günter Kukkukk:
> > Am Freitag, 31. Mai 2013, 11:03:46 schrieb Kai Blin:
> > > On 2013-05-31 05:04, Günter Kukkukk wrote:
> > > 
> > > Hi Günter,
> > > 
> > > > I've have prepared a very first patch (see attachment), which
> > > > addresses this issue.
> > > > Please comment whether this is the right approach.
> > > > Sure, the DEBUG() statements - beside one - should be removed.
> > > 

> Hi Andrew,
> 
> after other work i just came back to my PC and noticed the following 
> in the IRC backlog from yesterday:
> snip----
> 06/01/13 10:11:54 <kblin> and without Günter's patch, that obviously fails
> 06/01/13 10:12:30 <abartlet> such is the frustration of patches that come without the tests to prove them...
> snip----
> 
> Please explain to me "what you mean with that IRC comment", so i can better understand
> your personal view of samba development.
> 
> I DID NOT SEND a final PATCH.
> 
> In that email i just explained my detailed findings so far about a
> _serious_ samba bug - and asked FOR COMMENTS regarding my first patch.
> 
> That email contained detailed tests - ISC nsupdate and samba-tool.
> Unfortunately NOT samba torture tests - but those can also later be added
> when the patch has been discussed - as Kai noticed on reply....
> 
> You say that you are frustrated - me too.

Günter,

As a member of the Samba Team, and an experienced developer, I do expect
more from you than to supply a 'patch for comment'.  Yes, pinning an
issue down to the faulty lines of code is a massive part of the task,
but not following though with a final patch, specifically including
tests creates frustration.  The frustration comes from a job only
half-done:  Writing automated tests for the code you write is a key part
of the job, but it isn't the fun part.  Additionally, my experience is
that writing tests so often finds issues in the patch itself, even one
as small as this. 

The reason for the comment comes not from this patch alone, but also
from the long-running saga of the GSS-TSIG patch.  It just happens that
here again was a patch for comment, and no indication that you planned
to come back with tests, or in this case to clean it up for submission.

That indication doesn't need to be much, just saying 'assuming it's OK
I'll submit a final patch with tests tomorrow' (and following though)
would make a world of difference.

Perhaps you feel that that so many others do not write tests for their
code, but even if this was a special rule for DNS (it isn't), in the DNS
area it is even more important, because as it obvious at this point, we
do not have significant resources for fixing up regressions in this
area. 

As it was, Kai cleaned up your patch, and wrote tests.  Because the
patch wasn't ready for master he made an error doing so.  Fortunately
that was caught by other tests (hence why they are so important).  

This all takes time, time that would be avoided if you were able to
follow though with this much yourself, at the time you were writing the
patch.  I don't speak for Kai, but for myself I made my comment because
I would be frustrated in that situation.

But I also want to encourage you:  You are an experienced developer who
has spent a very long time looking at this area.  You should feel
confident to propose final patches, to be reviewed and merged into
master.  I'm confident you do understand what is required, and by this
point you have much the same level of experience as anyone else working
on DNS.  As you use that experience, you can do so knowing you have the
backing of our review system to get those into master. 

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org