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

[Rowland Penny]

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.

>   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 chocolate 
fireguard.

To properly test fsmo.py, the test would have to do what I did, create 
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 populated, 
delete a role and then test again as before.

Would you like to advise me just how I could that without ending up with 
a test that wasn't several times the size of fsmo.py and wouldn't take 
an excessive time to run.

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 in 
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 fsmo.py 
throws an error.

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 faulty 
code in fsmo.py.

Rowland

> Andrew Bartlett

-------------- next part --------------
fsmo tests				expected	gotdc1		gotdc2		NOTES

(with no empty roles)
fsmo show				show all	show all	show all
transfer std role			transfer	transfer	transfer
transfer dns role			transfer	transfer	transfer	with password
seize std role without force		transfer	transfer	transfer
seize std role with force		seize		seize		seize
seize dns role without force		transfer	seize		seize		without password
seize dns role without force		transfer	transfer	transfer	with password
seize dns role with force		seize		seize		seize

(with empty roles)
fsmo show				show empty	show empty	show empty
transfer std role			transfer	transfer	transfer
transfer empty std role			told empty	told empty	told empty
seize std role without force		transfer	transfer	transfer
seize empty std role without force	seize		seize		seize
seize std role with force		seize		seize		seize
seize empty std role with force		seize		seize		seize
transfer dns role			fail		fail		fail		without password
transfer dns role			transfer	transfer	transfer	with password
seize dns role without force		seize		seize		seize		without password
seize dns role without force		transfer	transfer	transfer	with password
seize dns role with force		seize		seize		seize
transfer empty dns role			told empty	told empty	told empty
seize empty dns role without force	seize		seize		seize
seize empty dns role with force		seize		seize		seize