cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:bug/fix-centos6-unittests into cloud-init:master.

Commit message:
unittests: fix unittests run on centos.

Some unit tests were broken when running on centos.
This fixes the unit test, with a small re-work of apt_configure.

Requested reviews:
  cloud init development team (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/323265
-- 
Your team cloud init development team is requested to review the proposed merge of ~smoser/cloud-init:bug/fix-centos6-unittests into cloud-init:master.

diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py
index 7e75177..78c4e06 100644
--- a/cloudinit/config/cc_apt_configure.py
+++ b/cloudinit/config/cc_apt_configure.py
@@ -282,16 +282,21 @@ def handle(name, ocfg, cloud, log, _):
     apply_apt(cfg, cloud, target)
 
 
+def _should_configure_on_empty_apt():
+    # if no config was provided, should apt configuration be done?
+    if util.system_is_snappy():
+        return False, "system is snappy."
+    if not (util.which('apt-get') or util.which('apt')):
+        return False, "no apt commands."
+    return True, "Apt is available."
+
+
 def apply_apt(cfg, cloud, target):
     # cfg is the 'apt' top level dictionary already in 'v3' format.
     if not cfg:
-        # no config was provided.  If apt configuration does not seem
-        # necessary on this system, then return.
-        if util.system_is_snappy():
-            LOG.debug("Nothing to do: No apt config and running on snappy")
-            return
-        if not (util.which('apt-get') or util.which('apt')):
-            LOG.debug("Nothing to do: No apt config and no apt commands")
+        should_config, msg = _should_configure_on_empty_apt()
+        if not should_config:
+            LOG.debug("Nothing to do: No apt config and %s", msg)
             return
 
     LOG.debug("handling apt config: %s", cfg)
diff --git a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v3.py b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v3.py
index 24e4523..a5bd02a 100644
--- a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v3.py
+++ b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v3.py
@@ -124,26 +124,45 @@ class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase):
     def _apt_source_list(self, cfg, expected, distro):
         "_apt_source_list - Test rendering from template (generic)"
 
+        cfg_on_empty = distro in ('debian', 'ubuntu')
+        pre="cloudinit.config.cc_apt_configure."
         # entry at top level now, wrap in 'apt' key
         cfg = {'apt': cfg}
         mycloud = self._get_cloud(distro)
-        with mock.patch.object(util, 'write_file') as mockwf:
-            with mock.patch.object(util, 'load_file',
-                                   return_value=MOCKED_APT_SRC_LIST) as mocklf:
-                with mock.patch.object(os.path, 'isfile',
-                                       return_value=True) as mockisfile:
-                    with mock.patch.object(util, 'rename'):
-                        cc_apt_configure.handle("test", cfg, mycloud,
-                                                LOG, None)
-
-        # check if it would have loaded the distro template
-        mockisfile.assert_any_call(
-            ('/etc/cloud/templates/sources.list.%s.tmpl' % distro))
-        mocklf.assert_any_call(
-            ('/etc/cloud/templates/sources.list.%s.tmpl' % distro))
-        # check expected content in result
-        mockwf.assert_called_once_with('/etc/apt/sources.list', expected,
-                                       mode=0o644)
+        unmocks = []
+        unmocks.append(mock.patch.object(util, 'write_file'))
+        mockwf = unmocks[-1].start()
+        unmocks.append(mock.patch.object(util, 'load_file',
+                                         return_value=MOCKED_APT_SRC_LIST))
+        mocklf = unmocks[-1].start()
+        unmocks.append(mock.patch.object(os.path, 'isfile', return_value=True))
+        mockisfile = unmocks[-1].start()
+        unmocks.append(mock.patch.object(util, 'rename'))
+        mockrename = unmocks[-1].start()
+        unmocks.append(mock.patch(pre + '_should_configure_on_empty_apt',
+                       return_value=(cfg_on_empty, "this is for test")))
+        mockshouldcfg = unmocks[-1].start()
+
+        try:
+            cc_apt_configure.handle("test", cfg, mycloud, LOG, None)
+        finally:
+            for u in unmocks:
+                u.stop()
+
+        if not cfg['apt']:
+            self.assertEqual(1, mockshouldcfg.call_count)
+
+        if cfg['apt'] or cfg_on_empty:
+            # check if it would have loaded the distro template
+            mockisfile.assert_any_call(
+                ('/etc/cloud/templates/sources.list.%s.tmpl' % distro))
+            mocklf.assert_any_call(
+                ('/etc/cloud/templates/sources.list.%s.tmpl' % distro))
+            # check expected content in result
+            mockwf.assert_called_once_with('/etc/apt/sources.list', expected,
+                                           mode=0o644)
+        else:
+            self.assertEqual(0, mockwf.call_count)
 
     def test_apt_v3_source_list_debian(self):
         """test_apt_v3_source_list_debian - without custom sources or parms"""
diff --git a/tests/unittests/test_handler/test_handler_yum_add_repo.py b/tests/unittests/test_handler/test_handler_yum_add_repo.py
index 4815bdb..c4396df 100644
--- a/tests/unittests/test_handler/test_handler_yum_add_repo.py
+++ b/tests/unittests/test_handler/test_handler_yum_add_repo.py
@@ -72,7 +72,7 @@ class TestConfig(helpers.FilesystemMockingTestCase):
         }
         for section in expected:
             self.assertTrue(parser.has_section(section),
-                            "Contains section {}".format(section))
+                            "Contains section {0}".format(section))
             for k, v in expected[section].items():
                 self.assertEqual(parser.get(section, k), v)
 
@@ -109,7 +109,7 @@ class TestConfig(helpers.FilesystemMockingTestCase):
         }
         for section in expected:
             self.assertTrue(parser.has_section(section),
-                            "Contains section {}".format(section))
+                            "Contains section {0}".format(section))
             for k, v in expected[section].items():
                 self.assertEqual(parser.get(section, k), v)
 
diff --git a/tox.ini b/tox.ini
index bf9046a..826f554 100644
--- a/tox.ini
+++ b/tox.ini
@@ -70,7 +70,7 @@ deps =
 
 [testenv:centos6]
 basepython = python2.6
-commands = nosetests {posargs:tests}
+commands = nosetests {posargs:tests/unittests}
 deps =
     # requirements
     argparse==1.2.1