# Addressing possible strategic problems with LDAP module and unit testing code

I'm a sysadmin writing a tool to perform administrative tasks on our upcoming new account management system that runs over LDAP. I want the tool to be highly flexible, configurable, and reliable, so I'm using automated test-driven development. I've started writing a module to perform LDAP connections and commands, but the problem is that writing my unit tests for this module takes 90% of my time. As it stands at the moment, with only a couple of class methods implemented so far with four more on the drawing board, here is the module that I am testing:

import ldap
import ldap.sasl

SCOPE=ldap.SCOPE_SUBTREE # will be moved to configuration at a later point

class auth():
kerb, simple, noauth = range(3)

class LDAPObjectManager():

def __init__(self, uri, authtype, user=None, password=None, **kwargs):
self._ldo = ldap.initialize(uri)
for key, value in kwargs.items():
self._ldo.set_option(getattr(ldap, key), value)
if authtype == auth.simple:
elif authtype == auth.kerb:
self._ldo.sasl_interactive_bind_s('', ldap.sasl.gssapi())

def _stripReferences(self, ldif):
return filter(lambda x: x[0] is not None, ldif)

def gets(self, sbase, sfilter):
ldif = self._ldo.search_ext_s(sbase, SCOPE, sfilter)
result = self._stripReferences(ldif)
if not result:
base: %s
filter: %s""" %(sbase, sfilter))
if len(result) > 1:
raise RuntimeError("""Too many results found for single-object \
query:
base: %s
filter: %s
results: %s""" %(sbase, sfilter, [r[0] for r in result]))
return result[0]

def getm(self, sbase, sfilter):
return self._stripReferences(self._ldo.search_ext_s(sbase, SCOPE,
sfilter))


Here are my unit tests that I've written so far:

import mock
import unittest
import src.ldapobjectmanager

@mock.patch('src.ldapobjectmanager.ldap', autospec=True)
class TestLOMInitializationAndOptions(unittest.TestCase):

def testAuth(self, mock_ldap):
uri = 'ldaps://foo.bar:636'
def getNewLDOandLOM(auth, **kwargs):
ldo = mock_ldap.ldapobject.LDAPObject(uri)
mock_ldap.initialize.return_value = ldo
lom = src.ldapobjectmanager.LDAPObjectManager(uri, auth, **kwargs)
return ldo, lom

# no auth
ldo, lom = getNewLDOandLOM(src.ldapobjectmanager.auth.noauth)
self.assertEqual(ldo.simple_bind_s.call_args_list, [])
self.assertEqual(ldo.sasl_interactive_bind_s.call_args_list, [])

# simple auth
user = 'foo'
ldo, lom = getNewLDOandLOM(src.ldapobjectmanager.auth.simple,
self.assertEqual(ldo.simple_bind_s.call_args_list,

# kerb auth
sasl = mock.MagicMock()
mock_ldap.sasl.gssapi.return_value = sasl
ldo, lom = getNewLDOandLOM(src.ldapobjectmanager.auth.kerb)
self.assertEqual(ldo.sasl_interactive_bind_s.call_args_list,
[(('', sasl),)])

def testOptions(self, mock_ldap):
uri = 'ldaps://foo.bar:636'
ldo = mock_ldap.ldapobject.LDAPObject(uri)
mock_ldap.initialize.return_value = ldo
for key, value in kwargs.items():
if not hasattr(mock_ldap, key):
with self.assertRaises(AttributeError):
lom = src.ldapobjectmanager.LDAPObjectManager(uri,
src.ldapobjectmanager.auth.noauth, **{key:value})
else:
lom = src.ldapobjectmanager.LDAPObjectManager(uri,
src.ldapobjectmanager.auth.noauth, **{key:value})
self.assertEqual(ldo.set_option.call_args,
((getattr(mock_ldap, key), value),))

@mock.patch('src.ldapobjectmanager.ldap', autospec=True)
class TestLOMGetMethods(unittest.TestCase):

def testGets(self, mock_ldap):
uri = 'ldaps://foo.bar:636'
ldo = mock_ldap.ldapobject.LDAPObject(uri)
mock_ldap.initialize.return_value = ldo
lom = src.ldapobjectmanager.LDAPObjectManager(uri,
src.ldapobjectmanager.auth.kerb)

# if gets() fails to find an object, it should throw an exception
ldo.search_ext_s.return_value = []
with self.assertRaises(RuntimeError) as err:
lom.gets("", "")

# sometimes references are included in the result
# these have no DN and should be discarded from the result
ldo.search_ext_s.return_value = [(None, ['ldaps://foo.bar/cn=ref'])]
with self.assertRaises(RuntimeError) as err:
lom.gets("", "")

# if gets() finds > 1 object, it should throw an exception
ldo.search_ext_s.return_value = [
('CN=fred,OU=People,DC=foo,DC=bar', {'name': ['fred']}),
('CN=george,OU=People,DC=foo,DC=bar', {'name': ['george']})
]
with self.assertRaises(RuntimeError) as err:
lom.gets("", "(|(name=fred)(name=george))")

# if gets() finds exactly 1 object, it should return that object
expectedresult = ('CN=alice,OU=People,DC=foo,DC=bar', {'name': ['alice']})
ldo.search_ext_s.return_value = [expectedresult]
actualresult = lom.gets("", "name=alice")
self.assertEqual(expectedresult, actualresult)

# repeat with reference in result
expectedresult = ('CN=alice,OU=People,DC=foo,DC=bar', {'name': ['alice']})
ldo.search_ext_s.return_value = [expectedresult,
(None, ['ldaps://foo.bar/cn=ref'])]
actualresult = lom.gets("", "name=alice")
self.assertEqual(expectedresult, actualresult)

def testGetm(self, mock_ldap):
uri = 'ldaps://foo.bar:636'
ldo = mock_ldap.ldapobject.LDAPObject(uri)
mock_ldap.initialize.return_value = ldo
lom = src.ldapobjectmanager.LDAPObjectManager(uri,
src.ldapobjectmanager.auth.kerb)

expectedresult = [
('CN=fred,OU=People,DC=foo,DC=bar', {'name': ['fred']}),
('CN=george,OU=People,DC=foo,DC=bar', {'name': ['george']})
]
ldo.search_ext_s.return_value = expectedresult
actualresult = lom.getm("", "(|(name=fred)(name=george))")
self.assertEqual(expectedresult, actualresult)

# repeat with reference in result
alice = ('CN=alice,OU=People,DC=foo,DC=bar', {'name': ['alice']})
reference = (None, ['ldaps://foo.bar/cn=ref'])
ldo.search_ext_s.return_value = [reference, alice, alice, reference,
reference, alice, alice, reference, alice, reference]
actualresult = lom.getm("", "name=alice")
self.assertEqual([alice, alice, alice, alice, alice], actualresult)


Here are the problems that I perceive with my testing code:

• My unit tests are huge and cumbersome to work with
• Because of the need to mock out the LDAP library, my unit tests are tightly coupled with the module implementation
• The mock makes it difficult to factor out common code in my unit tests
• Because of the above two bullet points, it takes much longer to write & debug my unit tests than it does to write my module, and I don't feel like I'm getting much useful coverage from my unit tests other than confirming the way I call the LDAP library.

How do I address these problems?

One potential cause for these problems is that I'm simply writing a wrapper around the LDAP library that does too little, but I haven't been able to sketch out my project in a way that avoids the need for such a wrapper or module.

• For your exceptions you might like textwrap.dedent. – ferada Dec 2 '14 at 22:12
• Thanks. I've used that module before, I just couldn't remember the name of it when I was writing the exception messages :) – jayhendren Dec 2 '14 at 22:15

Nice practical question! I've spend a bit of time refactoring the test code, the actual class is good as it is, so only two comments there:

• The auth enum is cool, although I'd say that simply using strings (or an actual enum in Python 3) is better just because you can inspect and understand them easier than numbers. Even then, checking the validity of the argument will help you prevent very easily spotted errors. So something line if not authtype in [kerb, ...]: raise ... is good enough.
• And the method names gets and getm don't conway much meaning. This is a minor complaint, because it looks like in the context LDAP this might be okay.

Now to the tests. I'm attaching my version below so you can refer to that if I explain it badly. In general this level of detail is good; if you wanted to add a layer between your actual code and the library than that is fine, but I think that is overkill from what I can see (even though I don't know how many other library options there would be). Even then, you might be able to go the other way and implement the ldap interface for another library instead and keep your actual code the same.

Anyway, the test cases are good; I've split some of them up into separate methods if it was useful to reuse some data. The names of the test cases could also be changed so that they refer to the case you want to test -- the comments could also be just docstrings, then you have a bit inspectable documentation as well.

And then just by aggressively reusing, looking up some helper methods, extracting and creating some others, using a common base class and a bit of meta magic you'll have reasonably short and expressive test cases.

Helpers include assert_no_calls (I don't see how that is not an already existing method, oh well), assert_called_once_with and assert_called_with. I've moved dummy values to person and reference, since they are used very often and the result looks cleaner. Oh yeah and I've added the longer imports so that the code is shorter in general.

The setUpClass in TestLOMGetMethods is probably overkill, maybe there's even an implementation of that already available somewhere, but you used the same approach for mock_ldap I wanted to see if that was possible for other very commonly used fields as well -- and it is. The benefit are a few less selfs, YMMV though.

testOptions I don't quite fully understand. If you just want to test separate options, then don't call it addOption, either use a better name if you want the keyword argument syntax, or use a regular dictionary instead. This way it looks like you're testing a single method call, or option, so it's a bit confusing to call the function with multiple options, but then iterate over them instead.

import inspect
import mock
import unittest
import src.ldapobjectmanager
from src.ldapobjectmanager import LDAPObjectManager, auth

uri = 'ldaps://foo.bar:636'

def person(name):
return ('CN={},OU=People,DC=foo,DC=bar'.format(name), {'name': [name]})

def reference():
return (None, ['ldaps://foo.bar/cn=ref'])

class MyUnitTestCase(unittest.TestCase):
def setUp(self):
patcher = mock.patch('src.ldapobjectmanager.ldap', autospec=True)
self.mock_ldap = patcher.start()

def getNewLDOandLOM(self, auth, **kwargs):
ldo = self.mock_ldap.ldapobject.LDAPObject(uri)
self.mock_ldap.initialize.return_value = ldo
lom = LDAPObjectManager(uri, auth, **kwargs)
return ldo, lom

def assert_no_calls(self, method):
self.assertEqual(method.call_args_list, [])

class TestLOMInitializationAndOptions(MyUnitTestCase):
def testAuth(self):
# no auth
ldo, lom = self.getNewLDOandLOM(auth.noauth)
self.assert_no_calls(ldo.simple_bind_s)
self.assert_no_calls(ldo.sasl_interactive_bind_s)

# simple auth
user = 'foo'

# kerb auth
sasl = mock.MagicMock()
self.mock_ldap.sasl.gssapi.return_value = sasl
ldo, lom = self.getNewLDOandLOM(auth.kerb)
ldo.sasl_interactive_bind_s.assert_called_once_with('', sasl)

def testOptions(self):
for key, value in kwargs.items():
def newNoAuth():
return self.getNewLDOandLOM(auth.noauth, **{key: value})

if not hasattr(self.mock_ldap, key):
with self.assertRaises(AttributeError):
newNoAuth()
else:
ldo, lom = newNoAuth()
ldo.set_option.assert_called_with(getattr(self.mock_ldap, key), value)

class TestLOMGetMethods(MyUnitTestCase):
@classmethod
def setUpClass(cls):
for (name, method) in filter(lambda x: x[0].startswith("test"),
inspect.getmembers(TestLOMGetMethods,
predicate=inspect.ismethod)):
setattr(cls, name, lambda instance, *args, **kwargs: method.__func__.__get__(instance.ldo, instance.lom, *args, **kwargs))

def setUp(self):
super(TestLOMGetMethods, self).setUp()

self.ldo, self.lom = self.getNewLDOandLOM(auth.kerb)

def testGets(self, ldo, lom):
# if gets() fails to find an object, it should throw an exception
ldo.search_ext_s.return_value = []
with self.assertRaises(RuntimeError):
lom.gets("", "")

# sometimes references are included in the result
# these have no DN and should be discarded from the result
ldo.search_ext_s.return_value = [(None, ['ldaps://foo.bar/cn=ref'])]
with self.assertRaises(RuntimeError):
lom.gets("", "")

def testExactlyOneObject(self, ldo, lom):
alice = person('alice')

# if gets() finds exactly 1 object, it should return that object
ldo.search_ext_s.return_value = [alice]
self.assertEqual(alice, lom.gets("", "name=alice"))

# repeat with reference in result
ldo.search_ext_s.return_value = [alice, reference()]
self.assertEqual(alice, lom.gets("", "name=alice"))

def testNameQuery(self, ldo, lom):
query = "(|(name=fred)(name=george))"
expectedresult = [person('fred'), person('george')]
ldo.search_ext_s.return_value = expectedresult

# if gets() finds > 1 object, it should throw an exception
with self.assertRaises(RuntimeError):
lom.gets("", query)

self.assertEqual(expectedresult, lom.getm("", query))

def testReferenceInResult(self, ldo, lom):
# repeat with reference in result
alice = person('alice')
ref = reference()
ldo.search_ext_s.return_value = [
ref, alice, alice, ref,
ref, alice, alice, ref, alice, ref
]
actualresult = lom.getm("", "name=alice")
self.assertEqual([alice] * 5, actualresult)

• Thank you so much! I haven't read through your whole answer yet, but I just want to comment on one thing so far: "The auth enum is cool, although I'd say that simply using strings (or an actual enum in Python 3) is better just because you can inspect and understand them easier than numbers." - Yes, I would love to use Python 3 enums, but this script will be run on RHEL6 and RHEL5 boxes that only have Python 2.x. I'll try using strings instead. – jayhendren Dec 3 '14 at 21:35
• Ah yes, I assumed as much, I have the same problem. – ferada Dec 3 '14 at 21:37
• I'm really thankful for your refactoring efforts. The result is much cleaner and easier to read than my attempts at refactoring. But I want to address one thing before diving into the code: When you say: "if you wanted to add a layer between your actual code and the library than that is fine, but I think that is overkill from what I can see", do you mean that you think this module is unecessary and that I should interface directly with the library in my code? (I wrote this module because I figure that I need a class to keep track of a connection object and to factor out common methods on it.) – jayhendren Dec 3 '14 at 21:51
• No I meant if you wanted to add another layer between this code and the ldap library (e.g. if you wanted to be able to switch the library at some point), then I meant that would not be necessary from what I can tell. – ferada Dec 3 '14 at 22:25
• Thank you for the clarification. I really appreciate your help and I spent some time cleaning up the unit tests. I am happier with them now. I wrote an answer detailing some of my changes (I believe the self-answer is the standard way of showing code updates on Code Review?). – jayhendren Dec 4 '14 at 1:09

Huge thanks to @ferada. With his/her help and suggestions, I spent a little over an hour combing over my code and making all kinds of changes. My unit tests are now much easier to read & understand and new unit tests are much easier to write. Here is a list of some of the things I did, with the new code following the list:

• Factored out common test case patterns into parent test case class methods - in particular, this greatly alleviated my troubles with mocking out the LDAP module. Other useful helper functions were factored out as well.
• Renamed test cases to be much more descriptive - the refactor made it much easier for me to write arbitrarily short or long test cases, so I refactored my test cases into many shorter, easier to comprehend test cases.
• Did not use setUpClass test class method - though I thought this was really clever, I didn't think that the outcome (passing objects as test class method arguments instead of attaching the objects to the test class) was worth it.
• Cleaned up and renamed addOption tests to assertOptionsAdded - this should be much easier to read and understand now.
• Changed auth "enum" to use strings instead of integers for easier debugging. I still wish for an enumeration implementation in Python 2, but this is an improvement for now.
• Renamed gets to getSingle and getm to getMultiple for hopefully obvious reasons. Also, the tests for these two methods have been separated into different test classes.
• Cleaned up exception messages using textwrap.dedent.
• Added new tests and new class method - with the refactoring, writing the new unit tests was much easier. The new method is called addAttr. The unit test for it could be cleaned up a bit, but for now it works.

Module:

import ldap
import ldap.sasl
import ldap.modlist
import textwrap
import copy

SCOPE=ldap.SCOPE_SUBTREE # hardcoded for now; to be moved to configuration

class auth():
kerb, simple, noauth = "kerb_auth", "simple_auth", "no_auth"

class LDAPObjectManager():

def __init__(self, uri, authtype, user=None, password=None, **kwargs):
# not sure that I like hardcoding the list of supported auth types...
if not authtype in [auth.kerb, auth.simple, auth.noauth]:
raise ValueError("'%s' is not a supported authentication method" \
% authtype)
self._ldo = ldap.initialize(uri)
for key, value in kwargs.items():
self._ldo.set_option(getattr(ldap, key), value)
if authtype == auth.simple:
elif authtype == auth.kerb:
self._ldo.sasl_interactive_bind_s('', ldap.sasl.gssapi())

def _stripReferences(self, ldif):
return filter(lambda x: x[0] is not None, ldif)

def getSingle(self, sbase, sfilter, scope=SCOPE):
ldif = self._ldo.search_ext_s(sbase, scope, sfilter)
result = self._stripReferences(ldif)
if not result:
raise RuntimeError(textwrap.dedent("""\
base: '%s'
filter: '%s'""" %(sbase, sfilter)))
if len(result) > 1:
raise RuntimeError(textwrap.dedent("""\
Too many results found for single-object query:
base: '%s'
filter: '%s'""" %(sbase, sfilter)))
return result[0]

def getMultiple(self, sbase, sfilter, scope=SCOPE):
return self._stripReferences(self._ldo.search_ext_s(sbase, scope,
sfilter))

def addAttr(self, sbase, dn, attr, value):
oldobj = self.getSingle(sbase, "dn=%s" %dn)
newobj = copy.deepcopy(oldobj)
newobj[1][attr].append(value)
ml = ldap.modlist.modifyModlist(oldobj, newobj)
self._ldo.modify_ext_s(dn, ml)


Unit Tests:

import mock
import unittest
import src.ldapobjectmanager
from src.ldapobjectmanager import LDAPObjectManager, auth

uri = 'ldaps://foo.bar:636'

def person(name):
return ('CN={},OU=People,DC=foo,DC=bar'.format(name), {'name': [name]})

def reference():
return (None, ['ldaps://foo.bar/cn=ref'])

class LOMTestCase(unittest.TestCase):

def setUp(self):
patcher = mock.patch('src.ldapobjectmanager.ldap', autospec=True)
self.mock_ldap = patcher.start()

def getNewLDOandLOM(self, auth, **kwargs):
ldo = self.mock_ldap.ldapobject.LDAPObject(uri)
self.mock_ldap.initialize.return_value = ldo
lom = LDAPObjectManager(uri, auth, **kwargs)
return ldo, lom

def assert_no_calls(self, method):
self.assertEqual(method.call_args_list, [])

class TestLOMInitializationAndOptions(LOMTestCase):

with self.assertRaises(ValueError):
ldo, lom = self.getNewLDOandLOM("Totally bogus auth value")

def testNoAuthShouldNotCauseBindCall(self):
ldo, lom = self.getNewLDOandLOM(auth.noauth)
self.assert_no_calls(ldo.simple_bind_s)
self.assert_no_calls(ldo.sasl_interactive_bind_s)

def testSimpleAuthCausesSimpleBindCall(self):
user = 'foo'

def testKerbAuthCausesSASLBindCall(self):
sasl = mock.MagicMock()
self.mock_ldap.sasl.gssapi.return_value = sasl
ldo, lom = self.getNewLDOandLOM(auth.kerb)
ldo.sasl_interactive_bind_s.assert_called_once_with('', sasl)

with self.assertRaises(AttributeError):
ldo, lom = self.getNewLDOandLOM(auth.kerb, OPT_BOGUS=1)

def testOptionKwargsAreSetOnConnectionObject(self):
ldo, lom = self.getNewLDOandLOM(auth.noauth, **kwargs)
for key, value in kwargs.items():
ldo.set_option.assert_any_call(getattr(self.mock_ldap, key),
value)

class LOMMethodTestCase(LOMTestCase):

def setUp(self):
super(LOMMethodTestCase, self).setUp()
self.ldo, self.lom = self.getNewLDOandLOM(auth.kerb)

class TestLOMGetSingle(LOMMethodTestCase):

def testGetSingleThrowsExceptionForNoResultsFound(self):
self.ldo.search_ext_s.return_value = []
with self.assertRaises(RuntimeError):
self.lom.getSingle("", "")

def testGetSingleThrowsExceptionForOnlyReferencesFound(self):
# sometimes references are included in the result
# these have no DN and should be discarded from the result
self.ldo.search_ext_s.return_value = [(None, ['ldaps://foo.bar/cn=ref'])]
with self.assertRaises(RuntimeError):
self.lom.getSingle("", "")

def testGetSingleSuccessfullyReturnsExactlyOneObject(self):
alice = person('alice')
self.ldo.search_ext_s.return_value = [alice]
self.assertEqual(alice, self.lom.getSingle("", "name=alice"))

def testGetSingleSuccessfullyReturnsExactlyOneObject(self):
bob = person('bob')
self.ldo.search_ext_s.return_value = [bob, reference()]
self.assertEqual(bob, self.lom.getSingle("", "name=bob"))

def testGetSingleThrowsExceptionWhenMultipleResultsFound(self):
expectedresult = [person('fred'), person('george')]
self.ldo.search_ext_s.return_value = expectedresult
with self.assertRaises(RuntimeError):
self.lom.getSingle("", "")

self.assertEqual(expectedresult, self.lom.getMultiple("", ""))

class TestLOMGetMultiple(LOMMethodTestCase):

def testGetMultipleSuccessfullyReturnsMultipleResults(self):
expectedresult = [person('fred'), person('george')]
self.ldo.search_ext_s.return_value = expectedresult
self.assertEqual(expectedresult, self.lom.getMultiple("", ""))

def testGetMultipleRemovesReferenceFromResult(self):
susie = person('susie')
ref = reference()
self.ldo.search_ext_s.return_value = [
ref, susie, susie, ref,
ref, susie, susie, ref, susie, ref
]
actualresult = self.lom.getMultiple("", "name=susie")
self.assertEqual([susie] * 5, actualresult)

dn = 'cn=foo,dc=bar,dc=baz'
attr = 'awesome list'
value1 = 'item 1'
value2 = 'item 2'
oldobj = (dn, {attr: [value1]})
newobj = (dn, {attr: [value1, value2]})
modlist = mock.MagicMock()
self.mock_ldap.modlist.modifyModlist.return_value = modlist
self.ldo.search_ext_s.return_value = [oldobj]