The responsibility for flapping tests (was: Re: [PATCHES] avoid searching for wellknownobjects when not needed, cache results for speed)

[Andrew Bartlett]

On Tue, 2015-05-26 at 05:46 -0700, Matthieu Patou wrote:
> On 05/25/2015 09:54 PM, Andrew Bartlett wrote:
> > On Mon, 2015-05-25 at 21:50 -0700, Matthieu Patou wrote:
> >> Hello All,
> >>
> >> Please find attached 2 patches that reduce the time of dbcheck by 10%,
> >> they avoid calling getwellknowobjects when not needed (ie. when dealing
> >> with the schema NC) and also cache the result of the function as well
> >> known object didn't change.
> >>
> >> Pass make test.
> > As we got into the unfortunate situation where dbcheck was marked as
> > flapping, sadly this means this code is untested in 'make test'.  :-(
> >
> > Andrew Bartlett
> >
> Ok but still it doesn't mean that those changes are wrong.
> One of those is to cache the wellknownobjects when found; as the name 
> imply the wellknownobjects doesn't change. Also I think I did several 
> rounds of make tests without getting an error so if let's say 3 rounds 
> of make test didn't show any regression we can treat this as a reliable 
> patch no ?

No, because tests in the flapping file are essentially skipped - we have
no idea if they succeeded or failed, or if they continue to.  

Before we get this in, we have to revert that unfortunate addition, and
to do that we probably need to revert whatever made it flap (presumably
the tombstone reanimation work). 

I realise this is totally unfair.  Untested code is really bad, but
changing untested code is even worse. 

The only way forward for this in the long term would be to declare that
we will remove, after a timeout of two months, each entry from flapping,
and that as part of this we will review those items to decide that they
are either pointless (and so remove the pretence that they have value),
or fix them not to flap.  That, and better pre-commit tests that make
flapping tests harder to get in, could be a better compromise to release
the pressure of failed autobuilds in the short-term without abandoning
tests for the long term.

That is, the current process where tests get added to flapping 'one
way', just leaves the costs of the underlying failure on our users and
never on us as developers.

Like portability, it will only change if work is invested. 

When I'm over jet-lag, I will propose to remove tombstone reanimation as
a supported feature (by removing the module from the default stack), and
knownfail or skip the associated tests.  

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba