Added docstrings. Removed unnecessary returns.

Created fixture to simplify the tests.
Better comments.

bigchaindb/backend/mongodb/admin.py

@@ -18,13 +18,20 @@ def add_replicas(connection, replicas):
     """Add a set of replicas to the replicaset
 
     Args:
-        replicas list of strings: of the form "hostname:port".
+        connection (:class:`~bigchaindb.backend.connection.Connection`):
+            A connection to the database.
+        replicas (:obj:`list` of :obj:`str`): replica addresses in the
+            form "hostname:port".
+
+    Raises:
+        DatabaseOpFailedError: If the reconfiguration fails due to a MongoDB
+            :exc:`OperationFailure`
     """
     # get current configuration
     conf = connection.conn.admin.command('replSetGetConfig')
 
-    # MongoDB does not automatically add and id for the members so we need
+    # MongoDB does not automatically add an id for the members so we need
-    # to chose one that does not exists yet. The safest way is to use
+    # to choose one that does not exists yet. The safest way is to use
     # incrementing ids, so we first check what is the highest id already in
     # the set and continue from there.
     cur_id = max([member['_id'] for member in conf['config']['members']])
@@ -35,11 +42,13 @@ def add_replicas(connection, replicas):
         conf['config']['members'].append({'_id': cur_id, 'host': replica})
 
     # increase the configuration version number
+    # when reconfiguring, mongodb expects a version number higher than the one
+    # it currently has
     conf['config']['version'] += 1
 
     # apply new configuration
     try:
-        return connection.conn.admin.command('replSetReconfig', conf['config'])
+        connection.conn.admin.command('replSetReconfig', conf['config'])
     except OperationFailure as exc:
         raise DatabaseOpFailedError(exc.details['errmsg'])
 
@@ -48,6 +57,15 @@ def add_replicas(connection, replicas):
 def remove_replicas(connection, replicas):
     """Remove a set of replicas from the replicaset
 
+    Args:
+        connection (:class:`~bigchaindb.backend.connection.Connection`):
+            A connection to the database.
+        replicas (:obj:`list` of :obj:`str`): replica addresses in the
+            form "hostname:port".
+
+    Raises:
+        DatabaseOpFailedError: If the reconfiguration fails due to a MongoDB
+            :exc:`OperationFailure`
     """
     # get the current configuration
     conf = connection.conn.admin.command('replSetGetConfig')
@@ -63,6 +81,6 @@ def remove_replicas(connection, replicas):
 
     # apply new configuration
     try:
-        return connection.conn.admin.command('replSetReconfig', conf['config'])
+        connection.conn.admin.command('replSetReconfig', conf['config'])
     except OperationFailure as exc:
         raise DatabaseOpFailedError(exc.details['errmsg'])

tests/backend/mongodb/test_admin.py

@@ -30,14 +30,22 @@ def mock_replicaset_config():
     }
 
 
-def test_add_replicas(mock_replicaset_config):
+@pytest.fixture
+def connection():
     from bigchaindb.backend import connect
-    from bigchaindb.backend.admin import add_replicas
-
     connection = connect()
-    # force the connection object to setup a connection to the database
+    # connection is a lazy object. It only actually creates a connection to
-    # before we mock `Database.command`
+    # the database when its first used.
-    connection.conn
+    # During the setup of a MongoDBConnection some `Database.command` are
+    # executed to make sure that the replica set is correctly initialized.
+    # Here we force the the connection setup so that all required
+    # `Database.command` are executed before we mock them it in the tests.
+    connection._connect()
+    return connection
+
+
+def test_add_replicas(mock_replicaset_config, connection):
+    from bigchaindb.backend.admin import add_replicas
 
     expected_config = copy.deepcopy(mock_replicaset_config)
     expected_config['config']['members'] += [
@@ -54,16 +62,10 @@ def test_add_replicas(mock_replicaset_config):
                                     expected_config['config'])
 
 
-def test_add_replicas_raises(mock_replicaset_config):
+def test_add_replicas_raises(mock_replicaset_config, connection):
-    from bigchaindb.backend import connect
     from bigchaindb.backend.admin import add_replicas
     from bigchaindb.backend.exceptions import DatabaseOpFailedError
 
-    connection = connect()
-    # force the connection object to setup a connection to the database
-    # before we mock `Database.command`
-    connection.conn
-
     with mock.patch.object(Database, 'command') as mock_command:
         mock_command.side_effect = [
             mock_replicaset_config,
@@ -73,15 +75,9 @@ def test_add_replicas_raises(mock_replicaset_config):
             add_replicas(connection, ['localhost:27018'])
 
 
-def test_remove_replicas(mock_replicaset_config):
+def test_remove_replicas(mock_replicaset_config, connection):
-    from bigchaindb.backend import connect
     from bigchaindb.backend.admin import remove_replicas
 
-    connection = connect()
-    # force the connection object to setup a connection to the database
-    # before we mock `Database.command`
-    connection.conn
-
     expected_config = copy.deepcopy(mock_replicaset_config)
     expected_config['config']['version'] += 1
 
@@ -99,16 +95,10 @@ def test_remove_replicas(mock_replicaset_config):
                                     expected_config['config'])
 
 
-def test_remove_replicas_raises(mock_replicaset_config):
+def test_remove_replicas_raises(mock_replicaset_config, connection):
-    from bigchaindb.backend import connect
     from bigchaindb.backend.admin import remove_replicas
     from bigchaindb.backend.exceptions import DatabaseOpFailedError
 
-    connection = connect()
-    # force the connection object to setup a connection to the database
-    # before we mock `Database.command`
-    connection.conn
-
     with mock.patch.object(Database, 'command') as mock_command:
         mock_command.side_effect = [
             mock_replicaset_config,