Petr Viktorin wrote:
On 04/30/2014 07:25 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
Hello,
The first patch adds "==" to ACI object to simplify comparisons.
The second patch moves existing "tests" to the test suite.
The third patch adds support for an alternate "aci" keyword that DS
supports (but I couldn't get any documentaion on it). Dogtag adds ACIs
with this keyword to cn=config, so we'll need this fix when parsing ACIs
there.
Rob, you wrote the parser; does this look OK to you?
ACK.
Only minor quibble is you left a couple of print statements in the tests.
Those are intentional. When a failed test prints something, Nose shows
the output. It's pretty nice for debugging.
Ok.
As you note, I had some "tests" that I ran when I was implementing the
aci module. Moving these to formal testing is definitely the right thing
to do.
I do wonder one thing though. In the equality test I had reversed some
ordering of things to ensure that things were normalized in the same
way. For the check_aci_parsing() tests is it worth considering doing
something similar?
I'm not sure I follow; what part are you referring to?
The ordering is problematic for sure; everything relies on dict order.
That's the main reason why in my work I only use this class to parse
ACIs, and I made a stable routine for generating them.
Until we start running the tests with PYTHONHASHSEED=random, I figured
we can just use them as they are.
Ok. I had some vague memory I had stuck a sort() in there to not rely on
dict ordering, maybe that was just an intention.
I noticed that we are apparently not normalizing target filters because
there is a space in the DN. Something for later.
Or not; I'd like to get rid of the export_to_string part completely.
There is no ticket. Probably fine since this is mostly just shuffling
deck chairs.
Yeah, same thinking here. It's part of the general ACI work.
Alright, push away then.
rob
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
