[Freeipa-devel] [Test][patch-0052] Test for incorrect client domain

Oleg Fayans ofayans at redhat.com
Fri Jul 1 14:55:21 UTC 2016


Hi Martin,

Thanks for the review. The updated patch is attached

On 07/01/2016 04:09 PM, Martin Basti wrote:
> 
> 
> On 01.07.2016 14:38, Oleg Fayans wrote:
>> Hi Martin. Now I have this client installation thing sorted out. The
>> test works as expected
>>
>> On 06/30/2016 02:57 PM, Martin Basti wrote:
>>>
>>> On 30.06.2016 14:40, Oleg Fayans wrote:
>>>> Hi Martin,
>>>>
>>>> Attached is a new version of the patch with two test cases separated.
>>>>
>>>> On 06/29/2016 12:23 PM, Martin Basti wrote:
>>>>> On 29.06.2016 10:56, Oleg Fayans wrote:
>>>>> Hello,
>>>>>
>>>>> +        assert_error(result,
>>>>> +                     "Failed to verify that %s is an IPA Server" %
>>>>> +                     self.master.hostname)
>>>>>
>>>>>
>>>>> I would expect this error there:
>>>>>
>>>>> "Cannot promote this client to a replica. Local domain '{local}' does
>>>>> not match IPA domain '{ipadomain}'. "
>>>> Right, that's what this ticket is about:
>>>> https://fedorahosted.org/freeipa/ticket/6006
>>>>
>>>> Once these changes are implemented, we can update this test
>>> Wat?
>>>
>>> You get exactly the right message from ipa-replica-install, tested,
>>> reviewed by several people.
>>>>> You should not use random REALM, in this case you don't test
>>>>> domains but
>>>>> realms. You can leave the test with incorrect realm there, but as
>>>>> separated testcase
>>>> Oh, ok. But it does not seem possible to setup client providing only
>>>> --realm without --domain: installer would not do it.
>>>>
>>> Try to read again: "should not use *random* REALM". Nothing prevents you
>>> to use, --realm=TEST.REALM --domain=random-blah-domain
>>>>> Martin^2
>>>>>
>>>>>
>>>>
>>> NACK
>>>
>>> +    domain_name = 'exxample.test'
>>> +    realm_name = domain_name.upper()
>>>
>>> you still use random realm name, and you still don't test
>>> ipa-replica-install, that ticket has nothing related to domain in
>>> ipa-client-install, it is related to replica promotion
>>>
>>> Martin^2
> 
> I have a few comments:
> 
> 1)
> This is unused and should not be there
> +    realm_name = domain_name.upper()
Done
> 
> 2)
> teardown_method
> shouldn't be more robust, what happens if client uninstall raises an error?

Agree. Done

> 
> 3) in both tests
> +                            '-w', self.master.config.dirman_password,
> 
> -w means admin password (ipa-client-install --help), so you should use
> admin not directory manager password

Fixed

> 
> 4)
> +        result = client.run_command(['ipa-client-install', '-U',
> '--domain',
> +                                     self.master.domain.realm, '-w',
> 
> did you mean:  '--domain', self.master.domain.name.upper()
Yes. Fixed.

> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ofayans-0052.3-Test-for-incorrect-client-domain.patch
Type: text/x-patch
Size: 3547 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160701/73ab2582/attachment.bin>


More information about the Freeipa-devel mailing list