[PATCH] samba-tool throws error if there is an empty FSMO role
abartlet at samba.org
Fri Apr 8 20:30:01 UTC 2016
On Fri, 2016-04-08 at 20:48 +0100, Rowland Penny wrote:
> On 08/04/16 20:08, Andrew Bartlett wrote:
> > On Fri, 2016-04-08 at 10:58 +0100, Rowland Penny wrote:
> > > I would also like to point out that I cannot seem to find any
> > > tests
> > > for
> > > 'samba-tool fsmo'
> > Indeed. All the more reason why we need that added.
> > We have all been caught out with untested code.
> I did test the code, see the attached file.
Thankyou for testing it manually.
> > I got embarrassed
> > recently when the 'samba-tool domain demote' code was shown to be
> > untested (in that case, it had a test, but that test got marked as
> > flapping, so was ignored).
> > Have a look at the tests in:
> > python/samba/tests/samba_tool/
> > for some good examples.
> Well, yes they are good examples of testing what they are designed to
> test for, but, for testing fsmo.py, they are as much use as a
> To properly test fsmo.py, the test would have to do what I did,
> two DCs in two separate VMs, test transferring roles between the two
> with all roles populated, test seizing roles without force with all
> roles populated, test seizing roles with force with all roles
> delete a role and then test again as before.
All of the above is entirely possible in 'make test'. Indeed, I missed
it in my first search, much of that part is tested here:
However, all I'm asking for in this case is that you show that 'samba
-tool fsmo show' produces non-faulting output in all of the
environments we already have, and the output is reasonable. Bonus
points if it checks the results are correct.
> Would you like to advise me just how I could that without ending up
> a test that wasn't several times the size of fsmo.py and wouldn't
> an excessive time to run.
Tests are often the same size as the code they test, and I don't expect
it to take more than trivial time to run.
Have a look at timecmd.py and rodc.py. See how they assert that the
command ran successfully, and see how they check for strings in the out
(output) and err (stderr) variables?
That is what you need to do. Then run that in the different
environments by changing source4/selftest/tests.py to run that test in
each of fl2000dc, fl2003dc, fl2008r2dc and vampire_dc
> I personally think that testing fsmo.py in the way you suggest is a
> waste of time, if everything is created correctly (by code that isn't
> fsmo.py) then fsmo.py will work without my changes, but it seems that
> there are times when everything isn't created correctly and then
> throws an error.
Indeed. This happens in the real world. It is great that you patched
it - I already needed to point a client at the patch when they hit this
> Tests are all well and good, but only for things that are created
> automatically. The code in fsmo.py shows, transfers or seizes FSMO
> roles, it doesn't (in the first instance) create the owners of these
> roles, bearing this in mind, perhaps the tests you ask for should be
> aimed at other code.
> Please bear in mind, whilst we are arguing about this, there is
> code in fsmo.py.
I agree, and I would like it fixed. I'm also trying to teach you how
to write tests for your code, so that it stays that way.
I know I'm asking you to spend non-trivial additional time on this. I
realise that I'm stretching you and I know that is frustrating.
Most of us who work on Samba find ourselves spending as much time on
the tests as the original code, and that has thankfully become part of
our culture, and what makes Samba as great as it is.
My hope is that you can learn the art and habit of automated testing,
because you have shown great ability to learn the Samba craft already,
and it makes your patches much, much, easier to accept.
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
More information about the samba-technical