[Freeipa-devel] [PATCH] 0058 Add integration tests for forced client re-enrollment
Ana Krivokapic
akrivoka at redhat.com
Fri Aug 30 15:25:03 UTC 2013
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.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0058-02-Add-integration-tests-for-forced-client-re-enrollmen.patch
Type: text/x-patch
Size: 10816 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130830/2fffdccd/attachment.bin>
More information about the Freeipa-devel
mailing list