[Freeipa-devel] [PATCH 0121] ipatests: Add support for hosts referenced by a keyword

[Petr Viktorin]

On 10/22/2013 10:09 AM, Tomas Babej wrote:
> On 10/22/2013 09:54 AM, Petr Viktorin wrote:
>> On 10/22/2013 09:20 AM, Tomas Babej wrote:
>>> Hi,
>>>
>>> Adds support for host definition by a environment variables of the
>>> following form:
>>>
>>> KEYWORDHOST__envX, where X is the number of the environment
>>> for which host referenced by a keyword should be defined.
>>>
>>> You can also optionally use KEYWORDHOST__IP_envX to define
>>> the IP address directly, otherwise the framework will try to resolve
>>> the hostname.
>>>
>>> Adds a required_keyword_hosts attribute to the IntegrationTest class,
>>> which can test developer use to specify the keyword hosts that this
>>> particular test requires. If not all required keyword hosts are
>>> available, the test will be skipped.
>>>
>>> All keyword hosts are accessible to the IntegrationTests via the
>>> keyword_hosts attribute, which contains a dictionary keyed by the
>>> keywords.
>>>
>>
>> Why is this necessary?
>> What's wrong with just extending the current scheme with more roles?
>>
>>
>
> The need for keyword hosts arised with the implementation of the legacy
> client test suite.
>
> As each of these tests needs a precise type (pre-defined and
> pre-configured) of VM, and not a generic client, you need to restrict
> the test case to specific host type.
>
> One test might be restricted to RHEL 5.10 with nss-pam-ldapd, another to
> FreeBSD, next one to CentOS with nss-pam-ldapd, next to CentOS with old
> version of SSSD...
>
> Each testcase that needs a new type of preconfigured host, we would need
> to introduce a new role, which would need to be integrated in the
> framework. In such implementation, we would lose loose coupling between
> the test framework and the test themselves, and make them less pluggable.

The framework itself (excluding the configuration part) should be able 
to handle this nicely using the existing role mechanism. It's true that 
in some places it's currently hard-wired to just a few roles, but 
supporting completely custom roles shouldn't be a problem.
I'd prefer if this system was used, rather than basically adding a 
second role system, which we'd have to support even if we get rid of the 
current config part.

The envvar-based configuration is not as flexible, but I'd rather make 
this part somewhat unpleasant than making the framework complex. We plan 
to move to a simpler configuration method anyway.
That said, you can basically keep the variable name scheme you use in 
this patch; just maybe use TESTHOST rather than KEYWORDHOST.

-- 
Petr³