[Freeipa-devel] [PATCH] 0058 Add integration tests for forced client re-enrollment

[Petr Viktorin]

On 08/30/2013 05:25 PM, Ana Krivokapic wrote:
> On 08/30/2013 03:28 PM, Petr Viktorin wrote:
>> On 08/28/2013 03:04 PM, Ana Krivokapic wrote:
>>> Hello,
>>>
>>> This patch adds integration tests for the forced client re-enrollment
>>> feature, according to the test plan at:
>>> http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan
>>>
>>>
>>> https://fedorahosted.org/freeipa/ticket/3832
>>
>> Thank you! The tests are good. As expected I have some nitpicks below.
>>
>>
>> I recommend putting the test case names from the wiki in the method docstrings.
>>
>> [...]
>>> +    def restore_client(self):
>>> +        client = self.clients[0]
>>
>> I'll ask you to allow SSH here, because the controller can theoretically also
>> act as one of the test hosts:
>>
>> iptables -A INPUT -p tcp --dport 22 -j ACCEPT
>>
>>> +        client.run_command([
>>> +            'iptables',
>>> +            '-A', 'INPUT',
>>> +            '-j', 'REJECT',
>>> +            '-p', 'all',
>>> +            '--source', self.master.ip
>>> +        ])
>>> +        self.uninstall_client()
>>> +        client.run_command(['iptables', '-F'])
>>
>> [...]
>>> +    def backup_keytab(self):
>>> +        self.clients[0].get_file(CLIENT_KEYTAB, TEMP_KEYTAB)
>>> +        self.master.put_file(TEMP_KEYTAB, BACKUP_KEYTAB)
>>
>> Please use get_file_contents & put_file_contents. There might be more tests
>> running on the controller machine, we don't want to use a shared file.
>> For BACKUP_KEYTAB and EMPTY_KEYTAB please use a file inside
>> master.config.test_dir; we don't want to leave files on the testing machines.
>> The test_dir gets cleaned up.
>>
>
> Thanks for the review!
>
> All issues are fixed in the updated patch.
>

Thanks, ACK, pushed to:
master: f40cb4c031b21940309ff1fbbf6b4f64aa5a6c39
ipa-3-3: adac5549a0fe57611e825fe7a1c4b538b498297b

-- 
Petr³