cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Thanks for the patience on this. I figured out the issue I had with my calls to helpers.wrap_and_call resulted from relying on mock.patch(return_value) instead of mock.patch(new)


Diff comments:

> diff --git a/tests/unittests/test_handler/test_handler_landscape.py b/tests/unittests/test_handler/test_handler_landscape.py
> new file mode 100644
> index 0000000..d35ba60
> --- /dev/null
> +++ b/tests/unittests/test_handler/test_handler_landscape.py
> @@ -0,0 +1,129 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +from cloudinit.config import cc_landscape
> +from cloudinit.sources import DataSourceNone
> +from cloudinit import (distros, helpers, cloud, util)
> +from ..helpers import FilesystemMockingTestCase, mock
> +
> +from configobj import ConfigObj
> +import logging
> +import os
> +
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +class TestLandscape(FilesystemMockingTestCase):
> +
> +    with_logs = True
> +
> +    def setUp(self):
> +        super(TestLandscape, self).setUp()
> +        self.new_root = self.tmp_dir()
> +        self.conf = os.path.join(self.new_root, 'client.conf')
> +        self.default_file = os.path.join(self.new_root, 'default/landscape')
> +
> +    def _get_cloud(self, distro):
> +        self.patchUtils(self.new_root)
> +        paths = helpers.Paths({'templates_dir': self.new_root})
> +        cls = distros.fetch(distro)
> +        mydist = cls(distro, {}, paths)
> +        myds = DataSourceNone.DataSourceNone({}, mydist, paths)
> +        return cloud.Cloud(myds, paths, {}, mydist, None)
> +
> +    def test_handler_skips_empty_landscape_cloudconfig(self):
> +        """Empty landscape cloud-config section does no work."""
> +        mycloud = self._get_cloud('ubuntu')
> +        mycloud.distro = mock.MagicMock()
> +        cfg = {'landscape': {}}
> +        cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> +        self.assertFalse(mycloud.distro.install_packages.called)
> +
> +    def test_handler_error_on_invalid_landscape_type(self):
> +        """Raise an error when landscape configuraiton option is invalid."""
> +        mycloud = self._get_cloud('ubuntu')
> +        cfg = {'landscape': 'wrongtype'}
> +        with self.assertRaises(RuntimeError) as context_manager:
> +            cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> +        self.assertIn(
> +            "'landscape' key existed in config, but not a dict",
> +            str(context_manager.exception))
> +
> +    @mock.patch('cloudinit.config.cc_landscape.util')
> +    def test_handler_restarts_landscape_client(self, m_util):
> +        """handler restarts lansdscape-client after install."""
> +        mycloud = self._get_cloud('ubuntu')
> +        cfg = {'landscape': {'client': {}}}
> +        with mock.patch('cloudinit.config.cc_landscape.LSC_CLIENT_CFG_FILE',
> +                        self.conf):
> +                cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> +        self.assertEqual(
> +            [mock.call(['service', 'landscape-client', 'restart'])],
> +            m_util.subp.call_args_list)
> +
> +    def test_handler_installs_client_and_creates_config_file(self):
> +        """Write landscape client.conf and install landscape-client."""
> +        self.assertFalse(os.path.exists(self.conf))
> +        mycloud = self._get_cloud('ubuntu')
> +        cfg = {'landscape': {'client': {}}}
> +        expected = {'client': {
> +            'log_level': 'info',
> +            'url': 'https://landscape.canonical.com/message-system',
> +            'ping_url': 'http://landscape.canonical.com/ping',
> +            'data_path': '/var/lib/landscape/client'}}
> +        mycloud.distro = mock.MagicMock()
> +        with mock.patch('cloudinit.config.cc_landscape.LSC_CLIENT_CFG_FILE',

@Scott, fixed it. I was calling it incorrectly. When passing mocks to wrap_and_call and mocking a string value instead of a function return_value I needed to specify 'new' instead of relying on 'return_value' for the mocked string globals.

Wrong:
wrap_and_call('cloudinit.config.cc_landscape', {'LSC_CLIENT_CFG_FILE': self.conf}, ...)

Correct:
wrap_and_call('cloudinit.config.cc_landscape', {'LSC_CLIENT_CFG_FILE': {'new': self.conf}, ...)

> +                        self.conf):
> +            with mock.patch('cloudinit.config.cc_landscape.LS_DEFAULT_FILE',
> +                            self.default_file):
> +                cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> +        self.assertEqual(
> +            [mock.call('landscape-client')],
> +            mycloud.distro.install_packages.call_args)
> +        self.assertEqual(expected, dict(ConfigObj(self.conf)))
> +        self.assertIn(
> +            'Wrote landscape config file to {0}'.format(self.conf),
> +            self.logs.getvalue())
> +        with open(self.default_file) as stream:
> +            default_content = stream.read()
> +        self.assertEqual('RUN=1\n', default_content)
> +
> +    def test_handler_writes_merged_client_config_file_with_defaults(self):
> +        """Merge and write options from LSC_CLIENT_CFG_FILE with defaults."""
> +        # Write existing sparse client.conf file
> +        util.write_file(self.conf, '[client]\ncomputer_title = My PC\n')
> +        mycloud = self._get_cloud('ubuntu')
> +        cfg = {'landscape': {'client': {}}}
> +        expected = {'client': {
> +            'log_level': 'info',
> +            'url': 'https://landscape.canonical.com/message-system',
> +            'ping_url': 'http://landscape.canonical.com/ping',
> +            'data_path': '/var/lib/landscape/client',
> +            'computer_title': 'My PC'}}
> +        with mock.patch("cloudinit.config.cc_landscape.LSC_CLIENT_CFG_FILE",
> +                        self.conf):
> +            cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> +        self.assertEqual(expected, dict(ConfigObj(self.conf)))
> +        self.assertIn(
> +            'Wrote landscape config file to {0}'.format(self.conf),
> +            self.logs.getvalue())
> +
> +    def test_handler_writes_merged_provided_cloudconfig_with_defaults(self):
> +        """Merge and write options from cloud-config options with defaults."""
> +        # Write empty sparse client.conf file
> +        util.write_file(self.conf, '')
> +        mycloud = self._get_cloud('ubuntu')
> +        cfg = {'landscape': {'client': {'computer_title': 'My PC'}}}
> +        expected = {'client': {
> +            'log_level': 'info',
> +            'url': 'https://landscape.canonical.com/message-system',
> +            'ping_url': 'http://landscape.canonical.com/ping',
> +            'data_path': '/var/lib/landscape/client',
> +            'computer_title': 'My PC'}}
> +        with mock.patch("cloudinit.config.cc_landscape.LSC_CLIENT_CFG_FILE",
> +                        self.conf):
> +            cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> +        self.assertEqual(expected, dict(ConfigObj(self.conf)))
> +        self.assertIn(
> +            'Wrote landscape config file to {0}'.format(self.conf),
> +            self.logs.getvalue())


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/329087
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:cc-landscape-py3-config-fix into cloud-init:master.