[Freeipa-devel] [PATCH 36/36] ticket 1600 - convert unittests to use DN objects

Alexander Bokovoy abokovoy at redhat.com
Wed Aug 10 13:41:22 UTC 2011


On 10.08.2011 05:16, John Dennis wrote:
> We have a larger goal of replacing all DN creation via string
> formatting/concatenation with DN object operations because string
> operations are not a safe way to form a DN nor to compare a DN. This
> work needs to be broken into smaller chunks for easier review and
> testing.
> 
> Addressing the unit tests first makes sense because we don't want to
> be modifying both the core code and the tests used to verify the core
> code simultaneously. If we modify the unittests first with existing
> core code and no regressions are found then we can move on to
> modifying parts of the core code with the belief the unittests can
> validate the changes in the core code. Also by doing the unittests
> first we also help to validate the DN objects are working correctly
> (although they do have an extensive unittest).
> 
> The fundamental changes are:
> 
> * replace string substitution & concatenation with DN object
>   constructor
> 
> * when comparing dn's the comparision is done after promotion
>   to a DN object, then two DN objects are compared
> 
> * when a list of string dn's are to be compared a new list is
>   formed where each string dn is replaced by a DN object
> 
> * because the unittest framework accepts a complex data structure of
>   expected values where dn's are represeted as strings the unittest
>   needs to express the expected value of a dn as a callable object
>   (e.g. a lambda expression) which promotes the dn string to a DN
>   object in order to do the comparision.
*Huge* work, very appreciated! It is much cleaner to see now what we are
actually expecting in a structured way.

ACK.

Unrelated comment:
There are few place where we have CN vs cn like
-        assert str(subject) == 'CN=ipa.example.com,O=IPA'
+        assert DN(str(subject)) == DN(('CN','ipa.example.com'),('O','IPA'))

Does it make sense to normalize to lowcase for those attributes that are
case-insensitive like cn, sn, uid, etc? It makes no functional
difference but looks a bit out of style to have a mix and also may trick
into wrongly using those attributes which are case-sensitive due to
schema definition.
-- 
/ Alexander Bokovoy




More information about the Freeipa-devel mailing list