[PATCH] samba-tool throws error if there is an empty FSMO role

[Andrew Bartlett]

On Thu, 2016-05-05 at 09:56 +0100, Rowland Penny wrote:
> On 04/05/16 22:36, Andrew Bartlett wrote:
> > On Wed, 2016-05-04 at 20:50 +0100, Rowland Penny wrote:
> > > On 04/05/16 20:27, Andrew Bartlett wrote:
> > > > On Mon, 2016-05-02 at 20:41 +0100, Rowland Penny wrote:
> > > > 
> > > > > So Andrew, how do I write a test for something that tests
> > > > > itself
> > > > > every
> > > > > time it runs ??? and if there are errors, these errors will
> > > > > probably
> > > > > have nothing to do with my code
> > > > Exactly!  The vast majority of errors introduced into Samba,
> > > > particularly into the python code, are not intentional nor
> > > > present
> > > > at
> > > > the time that particular code was developed.
> > > > 
> > > > I'm quite happy to trust, for the purpose of argument, that at
> > > > this
> > > > point in time, your patches are OK.
> > > > 
> > > > > Or to put it another way, I cannot think how to write a test
> > > > > for
> > > > > fsmo.py
> > > > > that doesn't replicate how I tested it before I sent my patch
> > > > > and
> > > > > there
> > > > > is no point in doing the same test over and over again, on
> > > > > the
> > > > > off
> > > > > chance it will develop an error.
> > > > The purpose of automated testing is to assert that this remains
> > > > the
> > > > case!
> > > > Additionally, in python, there is no pre-compilation.  That
> > > > means
> > > > that
> > > > the only way to guard against errors, such as we have seen in
> > > > demote,
> > > > where additional arguments are added BY OTHERS to functions
> > > > called,
> > > > is
> > > > to run the code, and assert at the very least successful
> > > > completion.
> > > > Ideally we also assert some meaningful output, against what we
> > > > expect,
> > > > given the environment it runs in.
> > > > I know it feels really odd writing an assertion in much the
> > > > same
> > > > way as
> > > > the code it is testing does internally - but it works, partly
> > > > because
> > > > the unintentional or helpful fix typically won't also
> > > > unintentionally
> > > > update the test to mask a breakage.
> > > > Without continuous integration, with such a small team Samba
> > > > would
> > > > not
> > > > be what it is today - we would be chasing our tail, endlessly
> > > > fixing
> > > > our own regressions.
> > > > I hope this clarifies things,
> > > > 
> > > > Andrew Bartlett
> > > > 
> > > OK, I understand the why
> > Good.  This first step is very, very important.
> > 
> > > , but I will say this very loud, I HAVE NO IDEA
> > > HOW TO DO THIS! :-)
> > > 
> > > How do I create two DCs ? how do I provision the first and then
> > > join
> > > the
> > > second to the first ? Having done that, how do I then empty FSMO
> > > roles ?
> > > how do I then call fsmo.py ?
> > I think you have perhaps imagined that your task is more complex
> > than
> > it actually is.
> > 
> > There are already a number of test environments defined in
> > selftest/targets/Samba4.pm
> 
> OK, first problem:
> line 184 in Samba4.pm is:
> 
>      my $nmblookup =  Samba::bindir_path($self, "nmblookup4");
> 
> root at dc1:~# ls /usr/local/samba/sbin/nmblookup4
> ls: cannot access /usr/local/samba/sbin/nmblookup4: No such file or 
> directory
> 
> root at dc1:~# samba -V
> Version 4.4.2
> 
> Shall I prepare a patch ???
> 
Rowland,

There is much magic in our build and test systems.  It might be best if
you focussed on just implementing your test, rather than trying to find
reasons not to.

While not installed, nmblookup4 is still built from the sources in
source4/.  It also quite clearly succeeds in every autobuild we do.

If you could focus on the issues that actually block you making a copy
of timecmd.py and patching it to instead test 'samba-tool fsmo show',
this will be a little more productive.

Thanks,

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