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

[Petr Viktorin]

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].
Also ipa-test-config should have an --ad-domain option to get info about 
the AD domain.


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.


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

Wrappers for the tasks need to be added to ipatests/ipa-test-task.

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.


Patch 0106: Looks OK; unfortunately I don't have an AD machine to test 
functionality



-- 
Petr³