testing: monkeypatch system_info call in unit tests (SC-533) (#1117)

testing: monkeypatch system_info call in unit tests

system_info can make calls that read or write from the filesystem, which
should require special mocking. It is also decorated with 'lru_cache',
which means test authors often don't realize they need to be mocking.
Also, we don't actually want the results from the user's local
machine, so monkeypatching it across all tests should be reasonable.

Additionally, moved some of 'system_info` into a helper function to
reduce the surface area of the monkeypatch, added tests for the new
function (and fixed a bug as a result), and removed related mocks that
should be no longer needed.

4 files changed, 63 insertions, 33 deletions

diff --git a/cloudinit/analyze/tests/test_boot.py b/cloudinit/analyze/tests/test_boot.py
index f69423c3..6b3afb5e 100644
--- a/cloudinit/analyze/tests/test_boot.py
+++ b/cloudinit/analyze/tests/test_boot.py
@@ -9,20 +9,11 @@ err_code = (FAIL_CODE, -1, -1, -1)
 
 class TestDistroChecker(CiTestCase):
 
-    @mock.patch('cloudinit.util.system_info', return_value={'dist': ('', '',
-                                                                     ''),
-                                                            'system': ''})
-    @mock.patch('cloudinit.util.get_linux_distro', return_value=('', '', ''))
-    @mock.patch('cloudinit.util.is_FreeBSD', return_value=False)
-    def test_blank_distro(self, m_sys_info, m_get_linux_distro, m_free_bsd):
+    def test_blank_distro(self):
         self.assertEqual(err_code, dist_check_timestamp())
 
-    @mock.patch('cloudinit.util.system_info', return_value={'dist': ('', '',
-                                                                     '')})
-    @mock.patch('cloudinit.util.get_linux_distro', return_value=('', '', ''))
     @mock.patch('cloudinit.util.is_FreeBSD', return_value=True)
-    def test_freebsd_gentoo_cant_find(self, m_sys_info,
-                                      m_get_linux_distro, m_is_FreeBSD):
+    def test_freebsd_gentoo_cant_find(self, m_is_FreeBSD):
         self.assertEqual(err_code, dist_check_timestamp())
 
     @mock.patch('cloudinit.subp.subp', return_value=(0, 1))
diff --git a/cloudinit/config/tests/test_set_passwords.py b/cloudinit/config/tests/test_set_passwords.py
index 79118a12..2a27f72f 100644
--- a/cloudinit/config/tests/test_set_passwords.py
+++ b/cloudinit/config/tests/test_set_passwords.py
@@ -121,13 +121,11 @@ class TestSetPasswordsHandle(CiTestCase):
             m_subp.call_args_list)
 
     @mock.patch(MODPATH + "util.multi_log")
-    @mock.patch(MODPATH + "util.is_BSD")
     @mock.patch(MODPATH + "subp.subp")
     def test_handle_on_chpasswd_list_creates_random_passwords(
-        self, m_subp, m_is_bsd, m_multi_log
+        self, m_subp, m_multi_log
     ):
         """handle parses command set random passwords."""
-        m_is_bsd.return_value = False
         cloud = self.tmp_cloud(distro='ubuntu')
         valid_random_pwds = [
             'root:R',
diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py
index ab5eb35c..7a3175f3 100644
--- a/cloudinit/tests/test_util.py
+++ b/cloudinit/tests/test_util.py
@@ -815,6 +815,44 @@ class TestGetLinuxDistro(CiTestCase):
         self.assertEqual(('foo', '1.1', 'aarch64'), dist)
 
 
+class TestGetVariant:
+    @pytest.mark.parametrize('info, expected_variant', [
+        ({'system': 'Linux', 'dist': ('almalinux',)}, 'almalinux'),
+        ({'system': 'linux', 'dist': ('alpine',)}, 'alpine'),
+        ({'system': 'linux', 'dist': ('arch',)}, 'arch'),
+        ({'system': 'linux', 'dist': ('centos',)}, 'centos'),
+        ({'system': 'linux', 'dist': ('cloudlinux',)}, 'cloudlinux'),
+        ({'system': 'linux', 'dist': ('debian',)}, 'debian'),
+        ({'system': 'linux', 'dist': ('eurolinux',)}, 'eurolinux'),
+        ({'system': 'linux', 'dist': ('fedora',)}, 'fedora'),
+        ({'system': 'linux', 'dist': ('openEuler',)}, 'openeuler'),
+        ({'system': 'linux', 'dist': ('photon',)}, 'photon'),
+        ({'system': 'linux', 'dist': ('rhel',)}, 'rhel'),
+        ({'system': 'linux', 'dist': ('rocky',)}, 'rocky'),
+        ({'system': 'linux', 'dist': ('suse',)}, 'suse'),
+        ({'system': 'linux', 'dist': ('virtuozzo',)}, 'virtuozzo'),
+        ({'system': 'linux', 'dist': ('ubuntu',)}, 'ubuntu'),
+        ({'system': 'linux', 'dist': ('linuxmint',)}, 'ubuntu'),
+        ({'system': 'linux', 'dist': ('mint',)}, 'ubuntu'),
+        ({'system': 'linux', 'dist': ('redhat',)}, 'rhel'),
+        ({'system': 'linux', 'dist': ('opensuse',)}, 'suse'),
+        ({'system': 'linux', 'dist': ('opensuse-tumbleweed',)}, 'suse'),
+        ({'system': 'linux', 'dist': ('opensuse-leap',)}, 'suse'),
+        ({'system': 'linux', 'dist': ('sles',)}, 'suse'),
+        ({'system': 'linux', 'dist': ('sle_hpc',)}, 'suse'),
+        ({'system': 'linux', 'dist': ('my_distro',)}, 'linux'),
+        ({'system': 'Windows', 'dist': ('dontcare',)}, 'windows'),
+        ({'system': 'Darwin', 'dist': ('dontcare',)}, 'darwin'),
+        ({'system': 'Freebsd', 'dist': ('dontcare',)}, 'freebsd'),
+        ({'system': 'Netbsd', 'dist': ('dontcare',)}, 'netbsd'),
+        ({'system': 'Openbsd', 'dist': ('dontcare',)}, 'openbsd'),
+        ({'system': 'Dragonfly', 'dist': ('dontcare',)}, 'dragonfly'),
+    ])
+    def test_get_variant(self, info, expected_variant):
+        """Verify we get the correct variant name"""
+        assert util._get_variant(info) == expected_variant
+
+
 class TestJsonDumps(CiTestCase):
     def test_is_str(self):
         """json_dumps should return a string."""
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 575a1fef..2045a6ab 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -533,42 +533,45 @@ def get_linux_distro():
     return (distro_name, distro_version, flavor)
 
 
-@lru_cache()
-def system_info():
-    info = {
-        'platform': platform.platform(),
-        'system': platform.system(),
-        'release': platform.release(),
-        'python': platform.python_version(),
-        'uname': list(platform.uname()),
-        'dist': get_linux_distro()
-    }
+def _get_variant(info):
     system = info['system'].lower()
-    var = 'unknown'
+    variant = 'unknown'
     if system == "linux":
         linux_dist = info['dist'][0].lower()
         if linux_dist in (
                 'almalinux', 'alpine', 'arch', 'centos', 'cloudlinux',
-                'debian', 'eurolinux', 'fedora', 'openEuler', 'photon',
+                'debian', 'eurolinux', 'fedora', 'openeuler', 'photon',
                 'rhel', 'rocky', 'suse', 'virtuozzo'):
-            var = linux_dist
+            variant = linux_dist
         elif linux_dist in ('ubuntu', 'linuxmint', 'mint'):
-            var = 'ubuntu'
+            variant = 'ubuntu'
         elif linux_dist == 'redhat':
-            var = 'rhel'
+            variant = 'rhel'
         elif linux_dist in (
                 'opensuse', 'opensuse-tumbleweed', 'opensuse-leap',
                 'sles', 'sle_hpc'):
-            var = 'suse'
+            variant = 'suse'
         else:
-            var = 'linux'
+            variant = 'linux'
     elif system in (
             'windows', 'darwin', "freebsd", "netbsd",
             "openbsd", "dragonfly"):
-        var = system
+        variant = system
+
+    return variant
 
-    info['variant'] = var
 
+@lru_cache()
+def system_info():
+    info = {
+        'platform': platform.platform(),
+        'system': platform.system(),
+        'release': platform.release(),
+        'python': platform.python_version(),
+        'uname': list(platform.uname()),
+        'dist': get_linux_distro()
+    }
+    info['variant'] = _get_variant(info)
     return info