Re: quality assurance (was: Re: GNUstep Base 1.11.0)

[Richard Frith-Macdonald]

On 2005-07-23 16:11:30 +0100 Lars Sonchocky-Helldorf 
<lars.sonchocky-helldorf@hamburg.de> wrote:

> Am Samstag, 23.07.05 um 13:47 Uhr schrieb Benoit Astruc:
>
> > Hello,
> >
> >
> > I just tried to compil GNUstep Base 1.11.0, freshly download from  GNUstep 
> > http) and I found that there is an error GSXML.m at line 5 142  :
> >
> > request = [tequest initWithURL: [NSURL URLWithString: connectionURL]];
> >
> > should be
> >
> > request = [request initWithURL: [NSURL URLWithString: connectionURL]];
> >
> > I suppose.
> >
> > Maybe it's corrected in CVS but it seems quite bad to have such error  in 
> > the last stable version.
>
> IIRC the odd minor release number (11 in 1.11.0) points out that this  is 
> an 
> unstable release of GNUstep.
>
>
> But nevertheless I think it is a bad sign that this kind of a bug  (doesn't 
> compile) could creep into a *release* of GNUstep. And even  worse, it would 
> not have been detected if Benoit would not have built  this special 
> configuration. The bug has gone unnoticed for a long time,  it has been 
> introduced with:
>
> http://savannah.gnu.org/cgi-bin/viewcvs/gnustep/gnustep/core/base/ 
> Source/Additions/GSXML.m.diff?r1=1.78&r2=1.79
>
> on Sat Mar 12 17:38:18 2005 annotated with "Patch to port GSXMLRPC to 
> MacOSX" which tells me the submitter did *not* test his patch before 
> committing. Otherwise he would have experienced what Benoit experienced.
>
>
> Please don't get me wrong here. I don't want to do finger pointing  because 
> of a small bug that only occurs in border cases. My issue is  something 
> else:
>
> While we keep fixing things here while inadvertently destroying things  
> there.
>
> This shows that we could need some sort of quality assurance in our 
> development process. Otherwise GNUstep will never become really mature.
>
>
> There are several things we could do in this regard (all stolen from  the 
> GCC 
> team)
>
> CVS check-ins:
>
> - require at least a compile test before things get checked in (this  would 
> catch this sort of bugs)
> - peer review of patches to be checked in
>
> - do a regression test before checking in things (needs test cases, see 
> below)
>
> bug fixes:
>
> - write test cases that prove a bug is fixed
>
> releases:
>
> - create a CVS branch (not just a tag) for each release (this helps to 
> insure that no new bugs are introduced in bugfix releases (caused by  new 
> features added meanwhile))
>
> - maintain release branches for a while, at least until the next stable 
> release (and port bugfixes back to mainline of course)
>
> - create a series of release candidates before actually doing the  release
>
>
> some of those are very complex (peer review comes to my mind here)  others 
> not. I think we should at least require some of them
>
>
> What do you think?

Well ... all those things are nice to have, but they are obviously of 
varying cost/benefit ratios.

None of them caught this bug (and most of them are in routine use in varying 
degrees) ... because the sad fact is that nobody had this particular setup 
... realistically there is no way to catch bugs in unusual configurations 
because people won't have the machines/operating systems  or resources to 
change machine/OS/configuration etc, so you have to adopt one of two 
approaches ...

1. reduce all the options/configurations to a minimal set such that they can 
be properly tested/supported by the core developers..
2. accept that the odd options/configurations won't get tested except when 
people who need to use them try them out.

I think we generally prefer the second approach, but there is always a 
balance struck between the two extremes.

Personally, I think that branching is more overhead than it's worth, but 
that regression testing is good, which is why that I wrote the regression 
testing framework we've been using up till now, and support the move to the 
latest one (because it's easier for ObjC programmers to write tests for).

I don't think that a test on all platforms/configurations is reasonable 
before a checkin ... that's just not ever going to be practical.  I do agree 
that a test of the standard gnu/linux build is needed ... but I think 
everyone does that.

You say 'which tells me the submitter did *not* test his patch before 
committing'

If I remember correctly, I committed that patch on behalf of the submitter, 
and I'm sure he tested the code before he sent it to us ... as he was using 
it himself.  The fact is that accidental alterations to files/patches do 
occur between testing and being submitted/committed ... this particular 
single character change probably was a result of viewing in an editor for a 
visual check. I don't think it suggests that the submitter was bad/sloppy  
in the processes he used.

So ... my vote for priorities would be ..

1. ask people to volunteer to test the odd configurations they have the 
machines for.  If we had a list of people who actuyally used the less 
popular hardware, operating systems, and configuration/build options, we 
could do an email reminder asking them to test the latest cvs code just 
before a release (even better if they could do automated builds/tests on a 
daily basis, I think someone was working on that)
2. keep asking people to contribute regression testcases if/when they have 
the time.  The fact that people had to install guile to run the old 
regression tests was an excuse for them not to help ... with the new 
testsuite that excuse no longer exists.
3. probably everything else is either way lower priority or routine anyway.