Add dictutils tests, clarify some var names

This change adds dictutils test coverage and also documents/clarifies
some of the functions provided.

No functionality should be changed, only exercised and documented.

Change-Id: I21ade46ffd620244708860b7f1243c3c3e5189cf

lib/dictutils.py

@@ -50,33 +50,43 @@ def append_if(array, item):
     return False
 
 
-def recursive_list_removal(inventory, purge_list):
+def recursive_list_removal(base_list, purge_list):
     """Remove items from a list.
 
-    :param inventory: ``dict`` Dictionary representing the inventory
+    If base_list and purge_list resolve to the same list in memory,
+    only the first item will be removed due to Python's handling of mutation
+    during iteration.
+
+    :param base_list: ``list`` List representing base_list entries
     :param purge_list: ``list`` List of items to remove
     """
     for item in purge_list:
-        for _item in inventory:
+        for _item in base_list:
             if item == _item:
-                inventory.pop(inventory.index(item))
+                base_list.pop(base_list.index(item))
 
 
+# TODO(nrb): this probably needs to be renamed, as it's not really recursive
 def recursive_dict_removal(inventory, purge_list):
     """Remove items from a dictionary.
 
-    Keyword arguments:
+    Only items in child dictionaries and lists are removed.
+
+    Dictionary keys can only be deleted at the 3rd level (e.g in
+    inventory['top']['middle']['bottom'], only 'bottom' would be targetted by
+    this function)
+
     :param inventory: ``dict`` Dictionary representing the inventory
     :param purge_list: ``list`` List of items to remove
     """
     for key, value in inventory.iteritems():
         if isinstance(value, dict):
-            for _key, _value in value.iteritems():
-                if isinstance(_value, dict):
+            for child_key, child_value in value.iteritems():
+                if isinstance(child_value, dict):
                     for item in purge_list:
-                        if item in _value:
-                            del(_value[item])
-                elif isinstance(_value, list):
-                    recursive_list_removal(_value, purge_list)
+                        if item in child_value:
+                            del(child_value[item])
+                elif isinstance(child_value, list):
+                    recursive_list_removal(child_value, purge_list)
         elif isinstance(value, list):
             recursive_list_removal(value, purge_list)

tests/test_dictutils.py

@@ -0,0 +1,155 @@
+#!/usr/bin/env python
+
+import os
+from os import path
+import sys
+import unittest
+
+LIB_DIR = 'lib'
+
+sys.path.append(path.join(os.getcwd(), LIB_DIR))
+
+import dictutils as du
+
+
+class TestMergeDictUnit(unittest.TestCase):
+    def test_merging_dict(self):
+        base = {'key1': 'value1'}
+        target = {'key2': 'value2'}
+
+        new = du.merge_dict(base, target)
+
+        self.assertIn('key2', new.keys())
+        self.assertEqual('value2', new['key2'])
+
+    def test_base_dict_is_modified(self):
+        base = {'key1': 'value1'}
+        target = {'key2': 'value2'}
+
+        new = du.merge_dict(base, target)
+
+        self.assertIs(base, new)
+
+    def test_merging_nested_dicts(self):
+        base = {'key1': 'value1'}
+        target = {'key2': {'key2.1': 'value2'}}
+
+        new = du.merge_dict(base, target)
+
+        self.assertIn('key2', new.keys())
+        self.assertIn('key2.1', new['key2'].keys())
+
+
+class TestAppendIfUnit(unittest.TestCase):
+    def test_appending_not_present(self):
+        base = ['foo', 'bar']
+        target = 'baz'
+
+        retval = du.append_if(base, target)
+
+        self.assertIn(target, base)
+        self.assertTrue(retval)
+
+    def test_appending_present(self):
+        base = ['foo', 'bar', 'baz']
+        target = 'baz'
+
+        retval = du.append_if(base, target)
+
+        self.assertFalse(retval)
+
+
+class TestListRemovalUnit(unittest.TestCase):
+    def setUp(self):
+        # Can't just use a member list, since it remains changed after each
+        # test
+        self.base = ['foo', 'bar']
+
+    def test_removing_one_target(self):
+        target = ['bar']
+
+        du.recursive_list_removal(self.base, target)
+
+        self.assertNotIn('bar', self.base)
+
+    def test_removing_entire_list(self):
+        # Use a copy so we're not hitting the exact same object in memory.
+        target = list(self.base)
+
+        du.recursive_list_removal(self.base, target)
+
+        self.assertEqual(0, len(self.base))
+
+    def test_using_base_as_target(self):
+        target = self.base
+
+        du.recursive_list_removal(self.base, target)
+
+        self.assertEqual(1, len(self.base))
+        self.assertEqual(1, len(target))
+        self.assertIn('bar', self.base)
+
+    def test_using_bare_string(self):
+        target = 'foo'
+
+        du.recursive_list_removal(self.base, target)
+
+        self.assertEqual(2, len(self.base))
+
+
+class TestDictRemovalUnit(unittest.TestCase):
+    def test_deleting_single_item_in_single_level_noop(self):
+        """The funtion only operates on nested dictionaries"""
+        base = {'key1': 'value1'}
+        target = ['value1']
+
+        du.recursive_dict_removal(base, target)
+
+        self.assertEqual('value1', base['key1'])
+
+    def test_deleting_single_item(self):
+        base = {'key1': {'key1.1': 'value1'}}
+        target = ['value1']
+
+        du.recursive_dict_removal(base, target)
+
+        self.assertEqual('value1', base['key1']['key1.1'])
+
+    def test_deleting_single_item_from_list(self):
+        base = {'key1': {'key1.1': ['value1']}}
+        target = ['value1']
+
+        du.recursive_dict_removal(base, target)
+
+        self.assertEqual(0, len(base['key1']['key1.1']))
+        self.assertNotIn('value1', base['key1']['key1.1'])
+
+    def test_deleting_single_item_from_nested_list(self):
+        """The function only operates on the 2nd level dictionary"""
+        base = {'key1': {'key1.1': {'key1.1.1': ['value1']}}}
+        target = ['value1']
+
+        du.recursive_dict_removal(base, target)
+
+        self.assertEqual(1, len(base['key1']['key1.1']['key1.1.1']))
+        self.assertIn('value1', base['key1']['key1.1']['key1.1.1'])
+
+    def test_deleting_single_item_top_level_list(self):
+        base = {'key1': ['value1']}
+        target = ['value1']
+
+        du.recursive_dict_removal(base, target)
+
+        self.assertEqual(0, len(base['key1']))
+
+    def test_deleting_single_nested_key(self):
+        base = {'key1': {'key1.1': {'key1.1.1': ['value1']}}}
+        target = ['key1.1.1']
+
+        du.recursive_dict_removal(base, target)
+
+        self.assertNotIn('key1.1.1', base['key1']['key1.1'])
+
+
+if __name__ == '__main__':
+    unittest.main()

tox.ini

@@ -151,6 +151,7 @@ commands =
     coverage erase
     coverage run -a {toxinidir}/tests/test_inventory.py
     coverage run -a {toxinidir}/tests/test_manage.py
+    coverage run -a {toxinidir}/tests/test_dictutils.py
     coverage run -a {toxinidir}/tests/test_ip.py
     coverage report --show-missing --include={toxinidir}/playbooks/inventory/*,{toxinidir}/lib/*