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³
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
