Merge lp:~michael.nelson/charm-helpers/ensure_etc_salt_exists into lp:charm-helpers

[Michael Nelson]

When installing only the salt-common lib, the /etc/salt directory is not created (at least with the version I'm using from lamont).

This branch was just going to ensure that the dir exists before it writes out the grains for use in templates...

...but, while QAing this branch on my charm, I came across some changed behaviour in hookenv.relation_get(). When run in the context of no relation, it used to return None, now it returns Serializable(None).

What I was seeing was this http://paste.ubuntu.com/5783035/

So I added a test to demo the issue and fixed it. Just in terms of consistency, I'm not sure whether it's intended that hookenv.unit_get always returns a dict (result of json.loads) rather than a Serializable (when the result isn't None, that is).

Finally, I got sick of waiting for one specific test to finish when doing `make test`, so added `make ftest` for use while developing.

[Matthew Wedgwood (mew)]

Hi, Michael,

Looks like stub's submitted an MP that removes the automatic decoration of those values with Serializable. I think that's a sensible solution and I think it should simplify things for you. That MP needs a bit of work before it can be merged.

If stub doesn't have the time to fix up the tests, I can probably get to that early next week.

[Matthew Wedgwood (mew)]

Because of r42, this needs rebasing to trunk.

review: Needs Fixing

[lp:~michael.nelson/charm-helpers/ensure_etc_salt_exists]

37. By Michael Nelson on 2013-07-09

Merged trunk and resolved conflict.

38. By Michael Nelson on 2013-07-09

Removed an unnecessary change left from the merge.

39. By Michael Nelson on 2013-07-09

Remove no-longer-relevant test comment.

[Michael Nelson (michael.nelson)]

Thanks Matthew - just getting back from a week off. I've merged trunk and resolved the conflicts. Diff should look better now with r39.

I've left the new test in there that I added for hook_env, as I don't see another one there for relation_get returning None.

[Michael Nelson (michael.nelson)]

Ping? :)

[Matthew Wedgwood (mew)]

Thanks for the re-merge, Michael, and sorry for the delay. Looks good now.

review: Approve

=== modified file 'Makefile'
--- Makefile	2013-06-27 15:59:39 +0000
+++ Makefile	2013-07-09 10:04:27 +0000
@@ -35,6 +35,10 @@
 	@echo Starting tests...
 	@$(PYTHON) /usr/bin/nosetests --nologcapture tests/
 
+ftest:
+	@echo Starting fast tests...
+	@$(PYTHON) /usr/bin/nosetests --attr '!slow' --nologcapture tests/
+
 lint:
 	@echo Checking for Python syntax...
 	@flake8 --ignore=E123,E501 $(PROJECT) $(TESTS) && echo OK

=== modified file 'charmhelpers/contrib/saltstack/__init__.py'
--- charmhelpers/contrib/saltstack/__init__.py	2013-06-19 09:54:19 +0000
+++ charmhelpers/contrib/saltstack/__init__.py	2013-07-09 10:04:27 +0000
@@ -122,5 +122,10 @@
     yaml.add_representer(unicode, lambda dumper,
                          value: dumper.represent_scalar(
                              u'tag:yaml.org,2002:str', value))
+
+    grains_dir = os.path.dirname(salt_grains_path)
+    if not os.path.exists(grains_dir):
+        os.makedirs(grains_dir)
+
     with open(salt_grains_path, "w+") as fp:
         fp.write(config.yaml())

=== modified file 'tests/contrib/jujugui/test_utils.py'
--- tests/contrib/jujugui/test_utils.py	2013-05-19 18:17:25 +0000
+++ tests/contrib/jujugui/test_utils.py	2013-07-09 10:04:27 +0000
@@ -1,6 +1,7 @@
 #!/usr/bin/env python2
 
 from contextlib import contextmanager
+import nose.plugins.attrib
 import os
 import shutil
 from simplejson import dumps
@@ -445,6 +446,7 @@
         self.cert_file = os.path.join(self.cert_path, 'juju.crt')
         self.key_file = os.path.join(self.cert_path, 'juju.key')
 
+    @nose.plugins.attrib.attr('slow')
     def test_generation(self):
         # Ensure certificates are correctly generated.
         save_or_create_certificates(

=== modified file 'tests/contrib/saltstack/test_saltstates.py'
--- tests/contrib/saltstack/test_saltstates.py	2013-06-19 19:09:31 +0000
+++ tests/contrib/saltstack/test_saltstates.py	2013-07-09 10:04:27 +0000
@@ -3,6 +3,8 @@
 # Authors:
 #  Charm Helpers Developers <[e-mail]>
 import mock
+import os
+import shutil
 import tempfile
 import unittest
 import yaml
@@ -99,10 +101,9 @@
         self.addCleanup(patcher.stop)
 
         # patches specific to this test class.
-        grain_file = tempfile.NamedTemporaryFile()
-        self.grain_path = grain_file.name
-        self.addCleanup(grain_file.close)
-
+        etc_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, etc_dir)
+        self.grain_path = os.path.join(etc_dir, 'salt', 'grains')
         patcher = mock.patch.object(charmhelpers.contrib.saltstack,
                                     'salt_grains_path', self.grain_path)
         patcher.start()

=== modified file 'tests/core/test_hookenv.py'
--- tests/core/test_hookenv.py	2013-07-05 18:43:41 +0000
+++ tests/core/test_hookenv.py	2013-07-09 10:04:27 +0000
@@ -549,6 +549,14 @@
         self.assertEqual(result['foo'], 'BAR')
         check_output.assert_called_with(['relation-get', '--format=json', '-'])
 
+    @patch('charmhelpers.core.hookenv.subprocess')
+    def test_relation_get_none(self, mock_subprocess):
+        mock_subprocess.check_output.return_value = 'null'
+
+        result = hookenv.relation_get()
+
+        self.assertIsNone(result)
+
     @patch('subprocess.check_output')
     def test_gets_relation_with_scope(self, check_output):
         check_output.return_value = json.dumps('bar')