[Freeipa-devel] [PATCH] ipatests: port of p11helper test from github

[Martin Basti]

On 17/03/15 10:38, Milan Kubik wrote:
> Hi,
>
> On 03/16/2015 05:23 PM, Martin Basti wrote:
>> On 16/03/15 15:32, Milan Kubik wrote:
>>> On 03/16/2015 12:03 PM, Milan Kubik wrote:
>>>> On 03/13/2015 02:59 PM, Milan Kubik wrote:
>>>>> Hi,
>>>>>
>>>>> this is a patch with port of [1] to pytest.
>>>>>
>>>>> [1]: 
>>>>> https://github.com/spacekpe/freeipa-pkcs11/blob/master/python/run.py
>>>>>
>>>>> Cheers,
>>>>> Milan
>>>>>
>>>>>
>>>>>
>>>> Added few more asserts in methods where the test could fail and 
>>>> cause other errors.
>>>>
>>>>
>>> New version of the patch after brief discussion with Martin Basti. 
>>> Removed unnecessary variable assignments and separated a new test case.
>>>
>>>
>> Hello,
>>
>> thank you for the patch.
>> I have a few nitpicks:
>> 1)
>> You can remove this and use just hexlify(s)
>> +def str_to_hex(s):
>> +    return ''.join("{:02x}".format(ord(c)) for c in s)
> done
>>
>> 2)
>> + def test_find_secret_key(self, p11):
>> +     assert p11.find_keys(_ipap11helper.KEY_CLASS_SECRET_KEY, 
>> label=u"žžž-aest")
>>
>> In tests before you tested the exact number of expected IDs returned 
>> by find_keys method, why not here?
> Lack of attention.
> Fixed the assert in `test_search_for_master_key` which does the same 
> thing. Merged `test_find_secret_key` with `test_search_for_master_key` 
> where it belongs.
>>
>> Martin^2
>
> Milan
>
>
Thank you for patches, just two nitpicks:

1)
Can you use the ipaplatform.paths constant? This is platform specific.
LIBSOFTHSM2_SO = "/usr/lib/pkcs11/libsofthsm2.so"
LIBSOFTHSM2_SO_64 = "/usr/lib64/pkcs11/libsofthsm2.so"

Respectively use just LIBSOFTHSM2_SO, on 64bit systems it is 
automatically mapped into LIBSOFTHSM2_SO_64

instead of:
+
+libsofthsm = "/usr/lib64/pkcs11/libsofthsm2.so"
+

2)
Can you please check if keys were really deleted?
+    def test_delete_key(self, p11):

-- 
Martin Basti

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150324/8742e818/attachment.htm>