Merge lp:~daniel-thewatkins/cloud-init/fix-multi_log into lp:~cloud-init-dev/cloud-init/trunk

[Dan Watkins]

This fixes the encoding problem that was causing warnings in cloud-init logs.

It also introduces unit tests for util.multi_log (the problematic part of the code), fixes a test that was failing, and makes it easier to run a single test file using tox.

[lp:~daniel-thewatkins/cloud-init/fix-multi_log]

1060. By Dan Watkins on 2015-02-11

Open /dev/console in text mode (so we don't have to encode strings to write them).

=== modified file 'cloudinit/util.py'
--- cloudinit/util.py	2015-02-11 01:50:23 +0000
+++ cloudinit/util.py	2015-02-11 17:25:15 +0000
@@ -404,7 +404,7 @@
     if console:
         conpath = "/dev/console"
         if os.path.exists(conpath):
-            with open(conpath, 'wb') as wfh:
+            with open(conpath, 'w') as wfh:
                 wfh.write(text)
                 wfh.flush()
         else:
@@ -1481,8 +1481,8 @@
                               device, mtype, exc)
                     pass
             if not mountpoint:
-                raise MountFailedError("Failed mounting %s to %s due to: %s" %
-                                       (device, tmpd, exc))
+                raise MountFailedError(
+                    "Failed mounting %s to %s".format(device, tmpd))
 
         # Be nice and ensure it ends with a slash
         if not mountpoint.endswith("/"):

=== modified file 'tests/unittests/helpers.py'
--- tests/unittests/helpers.py	2015-02-10 20:32:32 +0000
+++ tests/unittests/helpers.py	2015-02-11 17:25:15 +0000
@@ -254,6 +254,19 @@
                 self.patched_funcs.enter_context(
                     mock.patch.object(mod, f, trap_func))
 
+    def patchOpen(self, new_root):
+        trap_func = retarget_many_wrapper(new_root, 1, open)
+        name = 'builtins.open' if PY3 else '__builtin__.open'
+        self.patched_funcs.enter_context(mock.patch(name, trap_func))
+
+    def patchStdoutAndStderr(self, stdout=None, stderr=None):
+        if stdout is not None:
+            self.patched_funcs.enter_context(
+                mock.patch.object(sys, 'stdout', stdout))
+        if stderr is not None:
+            self.patched_funcs.enter_context(
+                mock.patch.object(sys, 'stderr', stderr))
+
 
 class HttprettyTestCase(TestCase):
     # necessary as http_proxy gets in the way of httpretty

=== modified file 'tests/unittests/test_util.py'
--- tests/unittests/test_util.py	2015-01-27 01:02:31 +0000
+++ tests/unittests/test_util.py	2015-02-11 17:25:15 +0000
@@ -1,21 +1,22 @@
 from __future__ import print_function
 
+import logging
 import os
+import shutil
 import stat
+import tempfile
+
+import six
 import yaml
-import shutil
-import tempfile
 
+from cloudinit import importer, util
 from . import helpers
-import six
 
 try:
     from unittest import mock
 except ImportError:
     import mock
 
-from cloudinit import importer
-from cloudinit import util
 
 
 class FakeSelinux(object):
@@ -377,4 +378,71 @@
         self.assertFalse(None, util.read_dmi_data("key"))
 
 
+class TestMultiLog(helpers.FilesystemMockingTestCase):
+
+    def _createConsole(self, root):
+        os.mkdir(os.path.join(root, 'dev'))
+        open(os.path.join(root, 'dev', 'console'), 'a').close()
+
+    def setUp(self):
+        super(TestMultiLog, self).setUp()
+        self.root = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.root)
+        self.patchOS(self.root)
+        self.patchUtils(self.root)
+        self.patchOpen(self.root)
+        self.stdout = six.StringIO()
+        self.stderr = six.StringIO()
+        self.patchStdoutAndStderr(self.stdout, self.stderr)
+
+    def test_stderr_used_by_default(self):
+        logged_string = 'test stderr output'
+        util.multi_log(logged_string)
+        self.assertEqual(logged_string, self.stderr.getvalue())
+
+    def test_stderr_not_used_if_false(self):
+        util.multi_log('should not see this', stderr=False)
+        self.assertEqual('', self.stderr.getvalue())
+
+    def test_logs_go_to_console_by_default(self):
+        self._createConsole(self.root)
+        logged_string = 'something very important'
+        util.multi_log(logged_string)
+        self.assertEqual(logged_string, open('/dev/console').read())
+
+    def test_logs_dont_go_to_stdout_if_console_exists(self):
+        self._createConsole(self.root)
+        util.multi_log('something')
+        self.assertEqual('', self.stdout.getvalue())
+
+    def test_logs_go_to_stdout_if_console_does_not_exist(self):
+        logged_string = 'something very important'
+        util.multi_log(logged_string)
+        self.assertEqual(logged_string, self.stdout.getvalue())
+
+    def test_logs_go_to_log_if_given(self):
+        log = mock.MagicMock()
+        logged_string = 'something very important'
+        util.multi_log(logged_string, log=log)
+        self.assertEqual([((mock.ANY, logged_string), {})],
+                         log.log.call_args_list)
+
+    def test_newlines_stripped_from_log_call(self):
+        log = mock.MagicMock()
+        expected_string = 'something very important'
+        util.multi_log('{0}\n'.format(expected_string), log=log)
+        self.assertEqual((mock.ANY, expected_string), log.log.call_args[0])
+
+    def test_log_level_defaults_to_debug(self):
+        log = mock.MagicMock()
+        util.multi_log('message', log=log)
+        self.assertEqual((logging.DEBUG, mock.ANY), log.log.call_args[0])
+
+    def test_given_log_level_used(self):
+        log = mock.MagicMock()
+        log_level = mock.Mock()
+        util.multi_log('message', log=log, log_level=log_level)
+        self.assertEqual((log_level, mock.ANY), log.log.call_args[0])
+
+
 # vi: ts=4 expandtab

=== modified file 'tox.ini'
--- tox.ini	2015-01-27 01:02:31 +0000
+++ tox.ini	2015-02-11 17:25:15 +0000
@@ -3,7 +3,7 @@
 recreate = True
 
 [testenv]
-commands = python -m nose tests
+commands = python -m nose {posargs:tests}
 deps =
      contextlib2
      httpretty>=0.7.1
@@ -13,7 +13,7 @@
      pyflakes
 
 [testenv:py26]
-commands = nosetests tests
+commands = nosetests {posargs:tests}
 deps =
      contextlib2
      httpretty>=0.7.1