[Freeipa-devel] [PATCH] 1085 cert-find command

[Petr Viktorin]

On 02/15/2013 10:43 PM, Rob Crittenden wrote:
> Petr Viktorin wrote:
>> On 02/06/2013 07:23 PM, Rob Crittenden wrote:
>>> Petr Viktorin wrote:
>>>> On 02/06/2013 12:44 AM, Rob Crittenden wrote:
>>>>> This adds a cert-find command for the dogtag backend.
>>>>>
>>>>> Searches can be done by serial number, by subject, revocation reason,
>>>>> issue date, notbefore, notafter and revocation dates.
>>>>>
>>>>> I added some basic tests for this. I made it a separate test file
>>>>> because the cert plugin tests do not use the declarative format and
>>>>> rely
>>>>> on the selfsign backend by default.
>>>>>
>>>>> rob
>>>>
>>>> Thanks! The code works well, but I found a few issues.
>>>>
>>>>
>>>> These tests don't work when the full test suite is run: test_cert adds
>>>> and revokes additional certs that throw the code off.
>>>> Perhaps have the tests only query valid certs? I don't see that option
>>>> but I think it would be helpful to support.
>>>
>>> I added some rather nasty hacks to the test to make things pass. I limit
>>> the search to 10 certificates, which is the number start with by
>>> default. There is an open dogtag ticket to return only VALID
>>> certificates so we should be able to drop this eventually.
>>>
>>> I had to go further on one of the revocation tests, limiting it to a
>>> sizelimit of 1. The count changes every time the suite runs so this is
>>> the safest for now. It also means that one test will fail if this is the
>>> only part of the suite executed.
>>
>> This gets rid of most of the failures, but it still fails the "certs for
>> this IPA server/short name" tests if the cert from ./make-cert is
>> present (creating it is part of `make test`).
>> If make-cert is run more times, it'll revoke the previous cert, so the
>> test for revocation reason 4 fails as well.
>>
>> It looks like when using sizelimit Dogtag will always discard *newer*
>> certs, ones with higher serials. Is it documented behavior or does
>> Dogtag just happen to do that?
>
> It isn't documented anywhere I could find, it is just what dogtag returns
>
>> I wonder how other people run their tests. This solution looks like it
>> could break easily if people do something differently :(
>>
>> I'm not sure how to solve this properly. Perhaps not using Declarative,
>> and checking "by hand" that the wanted certs are in the response and the
>> unwanted ones are not, would work better.
>
> I ended up switching the test class. It is not a very fine-grained set
> of tests, mostly searching with limits and verifying that we fall within
> a reasonable range, but it is better than nothing.
>
> rob

This works much better, thanks! Just two nitpicks now.

The patch doesn't apply well, there's a conflict in VERSION and some 
added trailing whitespace,

AFAIK this would be the only (first?) test that relies on Nose's 
ordering of test modules -- tests 0011 and 0030 rely on the other cert 
tests running first. Please at least mention that in a comment. Or 
better, move class test_cert_find to test_cert.py


-- 
Petr³