[Freeipa-devel] [PATCHES 100-106] Initial implementation of AD integration tests

[Tomas Babej]

On 10/21/2013 04:50 PM, Tomas Babej wrote:
> On 10/16/2013 03:44 PM, Petr Viktorin wrote:
>> On 10/14/2013 05:47 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> Sending the updated version of the patchset. It includes many fixes,
>>> plus incorporations of the comments by the QE. Tests are now broken 
>>> down
>>> into multiple test cases.
>>>
>>> This requires my patch 119.
>>>
>>> Tomas
>>
>> Patches 0100 & 0101:
>>
>> I still think it would be simpler if IPA and AD domains shared the 
>> numbering namespace (users would need to define $AD_env2; if they had 
>> $MASTER_env1 and $AD_env1 they would be in the same Domain).
>>
>> But if we use _env1 for both the AD and the IPA domain, they need to 
>> be separated in Domain.from_env. With patch 0101 both MASTER_env1 and 
>> AD_env1 will be in both domains[0] and ad_domains[0].
>
> I would rather not join IPA and AD domains as they even cannot be in 
> the same domain, as the service records would clash. So these will
> always be separate / sub / super domain relationship.
>
> As we already pass ad_domain flag to Domain.from_env, I did 
> incorporate code that joins the machines to the domain depending on 
> the their role. Is that a viable solution for you?
>
>> Also ipa-test-config should have an --ad-domain option to get info 
>> about the AD domain.
>>
>>
> Fixed, also modified the man page.
>> Patch 0102: Looks good
>>
>>
>> Patch 0103:
>> Looks like run_repeatedly should always be called with an assert in 
>> front. We may want it to raise an exception directly if it times out, 
>> so forgetting the assert won't hide errors.
>>
> Fixed.
>>
>> Patch 0104: Looks good
>>
>>
>> Patch 0105:
>> In /ipatests/man/ipa-test-task.1:
>> - Typo in  - s/generatee/generate/
>> - Subcommand names have unescaped hyphens (e.g. 
>> configure\-dns-for-trust should be configure\-dns\-for\-trust)
>> - The last subcommand should be sync-time
>>
> Fixed.
>> Wrappers for the tasks need to be added to ipatests/ipa-test-task.
>>
> Phew, fixed!
>> Nitpicks:
>> - In configure_dns_for_trust:is_subdomain, lockstep iteration over 
>> two lists at once is better done with zip() than `for i in 
>> range(len(...))`.
>> - In estabilish_trust_with_ad, don't use mutable values for argument 
>> defaults; do `extra_args=()` and `+ list(extra_args)`
>> - I can't say I'm a fan of configure_auth_to_local_rule, but I guess 
>> it's OK for tests.
>>
> Fixed.
>>
>> Patch 0106: Looks OK; unfortunately I don't have an AD machine to 
>> test functionality
>>
>>
>>
>
> I also incorporated Alexander's comments.
>
> Thanks for reviews, Alexander and Petr!
>
> Update patches attached.
>

In the patch 100, there was missing import for the itertools package, in 
ended up in another patch in the pile, which is not yet on the list. 
Sorry for the confusion, fixed patchset attached.

-- 
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0106-8-ipatests-Add-AD-integration-test-case.patch
Type: text/x-patch
Size: 7765 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/0e23d869/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0105-8-ipatests-Add-AD-integration-related-tasks.patch
Type: text/x-patch
Size: 17232 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/0e23d869/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0104-8-ipatests-Add-WinHost-class.patch
Type: text/x-patch
Size: 1733 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/0e23d869/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0103-8-ipatests-Create-util-module-for-ipatests.patch
Type: text/x-patch
Size: 2896 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/0e23d869/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0102-8-ipatests-Extend-IntegrationTest-with-multiple-AD-dom.patch
Type: text/x-patch
Size: 2840 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/0e23d869/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0101-8-ipatests-Extend-domain-object-with-ad-role-support-a.patch
Type: text/x-patch
Size: 6829 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/0e23d869/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0100-8-ipatests-Add-Active-Directory-support-to-configurati.patch
Type: text/x-patch
Size: 5803 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/0e23d869/attachment-0006.bin>