[SSSD] [PATCH] Add integration tests

Michal Židek mzidek at redhat.com
Tue Apr 21 14:07:27 UTC 2015


On 04/21/2015 01:08 PM, Nikolai Kondrashov wrote:
> Hi Michal,
>
> On 03/18/2015 03:06 AM, Michal Židek wrote:
>> I also do not like how the entries are passed to the
>> ldap_fixture. IMO it would be better to create a class
>> (for example LDAPEntriesList) with methods like
>> addUser
>> addGroup
>> addGroupBis
>> getList
>>
>> The instance of that class would then be passed as an
>> argument to the ldap_fixture. So the code would look
>> something like:
>>
>> ldap_entries = LDAPEntriesListt()
>> ldap_entries.addUser("user1", 1001, 2001)
>> ldap_entries.addUser(...)
>> ldap_entries.addGroup(...)
>> ... etc etc
>> ldap_fixture(request, ldap_conn, ldap_entries)
>>
>> Then you would call ldap.add_s on every entry
>> in ldap_entries.getList.
>
> I can still do this the way you suggest, but I tried it this way and
> that and
> it feels a bit suboptimal. I'll give some arguments and maybe you'll like
> another way more.
>
> Consider these two approaches for creating a simple and small entry list:
>
>      ldap_ents = LDAPEntriesList()                |   ldap_ents = [
>                                                   |
> ldap_ent.user("user1", 1001, 2001),
>      ldap_ents.add_user("user1", 1001, 2001)      |
> ldap_ent.user("user2", 1002, 2002),
>      ldap_ents.add_user("user2", 1002, 2002)      |
> ldap_ent.user("user3", 1003, 2003),
>      ldap_ents.add_user("user3", 1003, 2003)      |
>                                                   |
> ldap_ent.group("group1", 2001),
>      ldap_ents.add_group("group1", 2001)          |
> ldap_ent.group("group2", 2002),
>      ldap_ents.add_group("group2", 2002)          |
> ldap_ent.group("group3", 2003),
>      ldap_ents.add_group("group3", 2003)          |
>                                                   |
> ldap_ent.group("empty_group", 2010),
>      ldap_ents.add_group("empty_group", 2010)     |
>                                                   |
> ldap_ent.group("two_user_group", 2012,
>      ldap_ents.add_group("two_user_group", 2012,  |
> ["user1", "user2"]),
>                          ["user1", "user2"])      |   ]
>
> On the left we use a LDAP entry list class "LDAPEntriesList", as you
> suggested. On the right we use a builtin list and a module named
> "ldap_ent",
> with functions generating tuples in the way python-ldap expects.
>
> Even with similar length of variable and module names, the left side
> requires
> more characters. If variable name length on the left is increased, code
> size
> will be increased proportionally to the list size. Not so on the right
> side.
>
> The left side doesn't allow the list-creation syntax of the right side
> approach, but the other way is possible. I.e. you can still write this:
>
>      ldap_ents += [ldap_ent.user("user4", 1004, 2004)]
>
> or preferably this:
>
>      ldap_ents.append(ldap_ent.user("user4", 1004, 2004))
>
> The above is more verbose than with the left-side approach, but such
> operations are usually only needed in list generation where these would be
> few.
>
> You can also easily concatenate lists with the right-side approach:
>
>      ldap_ents = [
>          ldap_ent.user("user1", 1001, 2001),
>          ldap_ent.user("user2", 1002, 2002),
>          ldap_ent.user("user3", 1003, 2003),
>      ]
>
>      ldap_ents += [
>          ldap_ent.group("group1", 2001),
>          ldap_ent.group("group2", 2002),
>          ldap_ent.group("group3", 2003),
>      ]
>
> which is not possible with the left-side approach.
>
> Admittedly, we can implement all of these into the LDAPEntriesList() class,
> but it will still be more verbose, and it seems, wouldn't provide much
> benefits.  Ultimately, that will be wrapping standard list operations in a
> class interface.
>
> Regardless, I can still do it the way you request.
>
> Thank you.
>
> Nick

The approach you described would indeed save us some keystrokes, but
I still think that the class approach is better. The code looks (to me)
more self-describing with the class. But I admit the difference is not
big. If it is not a big deal for you, I would prefer the class.

Michal



More information about the sssd-devel mailing list