Merge lp:~stub/charm-helpers/fix-config into lp:charm-helpers

[Stuart Bishop]

The charmhelpers.core.hookenv.Config class loads state from disk into its
'previous' snapshot, and then selectively copies items into its 'current'
snapshot. It failed to deepcopy, so dicts and lists were copied by reference
and mutating them would not flag them as changed.

Fix this.

[Stuart Bishop (stub)]

Also fix some test log spam that should never have been landed

[Tim Van Steenburgh (tvansteenburgh)]

+1, LGTM.

review: Approve

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-06-30 15:28:49 +0000
+++ charmhelpers/core/hookenv.py	2015-07-22 06:02:10 +0000
@@ -21,6 +21,7 @@
 #  Charm Helpers Developers <[e-mail]>
 
 from __future__ import print_function
+import copy
 from distutils.version import LooseVersion
 from functools import wraps
 import glob
@@ -263,7 +264,7 @@
         self.path = path or self.path
         with open(self.path) as f:
             self._prev_dict = json.load(f)
-        for k, v in self._prev_dict.items():
+        for k, v in copy.deepcopy(self._prev_dict).items():
             if k not in self:
                 self[k] = v
 

=== modified file 'tests/core/test_hookenv.py'
--- tests/core/test_hookenv.py	2015-06-26 14:17:43 +0000
+++ tests/core/test_hookenv.py	2015-07-22 06:02:10 +0000
@@ -127,6 +127,17 @@
             self.assertEqual(c, json.load(f))
             self.assertEqual(c, dict(foo='bar', a='b'))
 
+    def test_deep_change(self):
+        # After loading stored data into our previous dictionary,
+        # it gets copied into our current dictionary. If this is not
+        # a deep copy, then mappings and lists will be shared instances
+        # and changes will not be detected.
+        c = hookenv.Config(dict(l=[]))
+        c.save()
+        c = hookenv.Config()
+        c['l'].append(42)
+        self.assertTrue(c.changed('l'), 'load_previous() did not deepcopy')
+
     def test_getitem(self):
         c = hookenv.Config(dict(foo='bar'))
         c.save()

=== modified file 'tests/fetch/test_fetch.py'
--- tests/fetch/test_fetch.py	2015-07-14 18:12:04 +0000
+++ tests/fetch/test_fetch.py	2015-07-22 06:02:10 +0000
@@ -336,8 +336,9 @@
             "garbage",
         )
 
+    @patch('charmhelpers.fetch.log')
     @patch('charmhelpers.fetch.plugins')
-    def test_installs_remote(self, _plugins):
+    def test_installs_remote(self, _plugins, _log):
         h1 = MagicMock(name="h1")
         h1.can_handle.return_value = "Nope"