HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Addressing possible strategic problems with LDAP module and unit testing code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
strategicaddressingwithmoduleldapcodepossibletestingandproblems

Problem

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:
            self._ldo.simple_bind_s(user, password)
        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:
            raise RuntimeError("""No results found for single-object query:
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:

```
imp

Solution

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()
self.addCleanup(patcher.stop)

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'
password = 'bar'
ldo, lom = self.getNewLDOandLOM(auth.simple, user=user, password=password)
ldo.simple_bind_s.assert_called_once_with(user, password)

# 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):
def addOption(**kwargs):
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)

addOption(OPT_X_TLS=1, OPT_BOGUS=1, OPT_URI="ldaps://baz.bar")

class TestLOMGetMethods(MyUnitTestCase):
@classmet

Code Snippets

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()
        self.addCleanup(patcher.stop)

    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'
        password = 'bar'
        ldo, lom = self.getNewLDOandLOM(auth.simple, user=user, password=password)
        ldo.simple_bind_s.assert_called_once_with(user, password)

        # 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):
        def addOption(**kwargs):
            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)

        addOption(OPT_X_TLS=1, OPT_BOGUS=1, OPT_URI="ldaps://baz.bar")


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 refe

Context

StackExchange Code Review Q#71451, answer score: 2

Revisions (0)

No revisions yet.