cleanup tests

* use nose test generator
* more comments
* move facts import inside the skipped function, fix python3 warning

lib/ansible/module_utils/facts.py

@@ -282,6 +282,11 @@ class Facts(object):
     # platform.dist() is deprecated in 2.6
     # in 2.6 and newer, you should use platform.linux_distribution()
     def get_distribution_facts(self):
+        """
+        Fills facts about the distribution name and version.
+
+        This is unit tested. Please extend the tests to cover all distributions if you have them available.
+        """
 
         # A list with OS Family members
         OS_FAMILY = dict(

test/units/module_utils/gen_distribution_version_testcase.py

@@ -3,7 +3,7 @@
 """
 This script generated test_cases for test_distribution_version.py.
 
-To do so it outputs the relevant files from /etc/*release, the ouput of platform.dist() and the current ansible_facts regarding the distribution version.
+To do so it outputs the relevant files from /etc/*release, the output of platform.dist() and the current ansible_facts regarding the distribution version.
 
 This assumes a working ansible version in the path.
 """

test/units/module_utils/test_distribution_version.py

@@ -29,66 +29,34 @@ from ansible.utils.unicode import to_bytes
 # for testing
 from ansible.compat.tests import unittest
 from ansible.compat.tests.mock import patch
+from nose.tools import assert_in, assert_equal
 
-import ansible.module_utils.facts
+# the module we are actually testing
 
 
-@unittest.skipIf(sys.version_info[0] >= 3, "Python 3 is not supported on targets (yet)")
+# to generate the testcase data, you can use the script gen_distribution_version_testcase.py in hacking/tests
-class TestModuleUtilsFactsDistribution(unittest.TestCase):
+TESTSETS = [
-
-    def setUp(self):
-        self.real_stdin = sys.stdin
-        from ansible.module_utils import basic
-
-        args = json.dumps(dict(ANSIBLE_MODULE_ARGS={}, ANSIBLE_MODULE_CONSTANTS={}))
-        if PY3:
-            sys.stdin = StringIO(args)
-            sys.stdin.buffer = BytesIO(to_bytes(args))
-        else:
-            sys.stdin = BytesIO(to_bytes(args))
-        self.module = basic.AnsibleModule(argument_spec=dict())
-
-    def tearDown(self):
-        sys.stdin = self.real_stdin
-
-    def clear_modules(self, mods):
-        for mod in mods:
-            if mod in sys.modules:
-                del sys.modules[mod]
-
-    def test_distribution_from_files(self):
-        """tests the distribution parsing code of the Facts class
-
-        testsets have
-        * a name (for output/debugging only)
-        * input files that are faked
-          * those should be complete and also include "irrelevant" files that might be mistaken as coming from other distributions
-          * all files that are not listed here are assumed to not exist at all
-        * the output of pythons platform.dist()
-        * results for the ansible variables distribution*
-        """
-
-        testsets = [
     {
         "name" : "openSUSE Leap 42.1",
         "input": {
             "/etc/os-release":
-                    """NAME="openSUSE Leap"
+            """
-                    VERSION="42.1"
+NAME="openSUSE Leap"
-                    VERSION_ID="42.1"
+VERSION="42.1"
-                    PRETTY_NAME="openSUSE Leap 42.1 (x86_64)"
+VERSION_ID="42.1"
-                    ID=opensuse
+PRETTY_NAME="openSUSE Leap 42.1 (x86_64)"
-                    ANSI_COLOR="0;32"
+ID=opensuse
-                    CPE_NAME="cpe:/o:opensuse:opensuse:42.1"
+ANSI_COLOR="0;32"
-                    BUG_REPORT_URL="https://bugs.opensuse.org"
+CPE_NAME="cpe:/o:opensuse:opensuse:42.1"
-                    HOME_URL="https://opensuse.org/"
+BUG_REPORT_URL="https://bugs.opensuse.org"
-                    ID_LIKE="suse"
+HOME_URL="https://opensuse.org/"
+ID_LIKE="suse"
             """,
             "/etc/SuSE-release":"""
-                    openSUSE 42.1 (x86_64)
+openSUSE 42.1 (x86_64)
-                    VERSION = 42.1
+VERSION = 42.1
-                    CODENAME = Malachite
+CODENAME = Malachite
-                    # /etc/SuSE-release is deprecated and will be removed in the future, use /etc/os-release instead
+# /etc/SuSE-release is deprecated and will be removed in the future, use /etc/os-release instead
             """
         },
         "platform.dist": ['SuSE', '42.1', 'x86_64'],
@@ -101,8 +69,22 @@ class TestModuleUtilsFactsDistribution(unittest.TestCase):
     },
                 {
         'name': 'openSUSE 13.2',
-                'input': {'/etc/SuSE-release': 'openSUSE 13.2 (x86_64)\nVERSION = 13.2\nCODENAME = Harlequin\n# /etc/SuSE-release is deprecated and will be removed in the future, use /etc/os-release instead\n',
+        'input': {'/etc/SuSE-release': """openSUSE 13.2 (x86_64)
-                          '/etc/os-release': 'NAME=openSUSE\nVERSION="13.2 (Harlequin)"\nVERSION_ID="13.2"\nPRETTY_NAME="openSUSE 13.2 (Harlequin) (x86_64)"\nID=opensuse\nANSI_COLOR="0;32"\nCPE_NAME="cpe:/o:opensuse:opensuse:13.2"\nBUG_REPORT_URL="https://bugs.opensuse.org"\nHOME_URL="https://opensuse.org/"\nID_LIKE="suse"\n'},
+VERSION = 13.2
+CODENAME = Harlequin
+# /etc/SuSE-release is deprecated and will be removed in the future, use /etc/os-release instead
+""",
+                  '/etc/os-release': """NAME=openSUSE
+VERSION="13.2 (Harlequin)"
+VERSION_ID="13.2"
+PRETTY_NAME="openSUSE 13.2 (Harlequin) (x86_64)"
+ID=opensuse
+ANSI_COLOR="0;32"
+CPE_NAME="cpe:/o:opensuse:opensuse:13.2"
+BUG_REPORT_URL="https://bugs.opensuse.org"
+HOME_URL="https://opensuse.org/"
+ID_LIKE="suse"
+"""},
         'platform.dist': ('SuSE', '13.2', 'x86_64'),
         'result': {'distribution': u'openSUSE',
                    'distribution_major_version': u'13',
@@ -235,10 +217,16 @@ BUG_REPORT_URL="https://bugs.debian.org/"
     },
     {
         'name': "Debian 7.9",
-                'input': {'/etc/os-release': 'PRETTY_NAME="Debian GNU/Linux 7 (wheezy)"\nNAME="Debian GNU/Linux"'
+        'input': {'/etc/os-release': """PRETTY_NAME="Debian GNU/Linux 7 (wheezy)"
-                                             '\nVERSION_ID="7"\nVERSION="7 (wheezy)"\nID=debian\nANSI_COLOR="1;31"\n'
+NAME="Debian GNU/Linux"
-                                             'HOME_URL="http://www.debian.org/"\nSUPPORT_URL="http://www.debian.org/support/'
+VERSION_ID="7"
-                                             '"\nBUG_REPORT_URL="http://bugs.debian.org/"\n'},
+VERSION="7 (wheezy)"
+ID=debian
+ANSI_COLOR="1;31"
+HOME_URL="http://www.debian.org/"
+SUPPORT_URL="http://www.debian.org/support/"
+BUG_REPORT_URL="http://bugs.debian.org/"
+"""},
         'platform.dist': ('debian', '7.9', ''),
         'result': {'distribution': u'Debian',
                    'distribution_major_version': u'7',
@@ -247,8 +235,21 @@ BUG_REPORT_URL="https://bugs.debian.org/"
     },
     {
         'name': "Ubuntu 14.04",
-                'input': {'/etc/lsb-release': 'DISTRIB_ID=Ubuntu\nDISTRIB_RELEASE=14.04\nDISTRIB_CODENAME=trusty\nDISTRIB_DESCRIPTION="Ubuntu 14.04.4 LTS"\n',
+        'input': {'/etc/lsb-release': """DISTRIB_ID=Ubuntu
-                          '/etc/os-release': 'NAME="Ubuntu"\nVERSION="14.04.4 LTS, Trusty Tahr"\nID=ubuntu\nID_LIKE=debian\nPRETTY_NAME="Ubuntu 14.04.4 LTS"\nVERSION_ID="14.04"\nHOME_URL="http://www.ubuntu.com/"\nSUPPORT_URL="http://help.ubuntu.com/"\nBUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/"\n'},
+DISTRIB_RELEASE=14.04
+DISTRIB_CODENAME=trusty
+DISTRIB_DESCRIPTION="Ubuntu 14.04.4 LTS"
+""",
+                  '/etc/os-release': """NAME="Ubuntu"
+VERSION="14.04.4 LTS, Trusty Tahr"
+ID=ubuntu
+ID_LIKE=debian
+PRETTY_NAME="Ubuntu 14.04.4 LTS"
+VERSION_ID="14.04"
+HOME_URL="http://www.ubuntu.com/"
+SUPPORT_URL="http://help.ubuntu.com/"
+BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/"
+"""},
         'platform.dist': ('Ubuntu', '14.04', 'trusty'),
         'result': {'distribution': u'Ubuntu',
                    'distribution_major_version': u'14',
@@ -257,8 +258,18 @@ BUG_REPORT_URL="https://bugs.debian.org/"
     },
     {
         'name': "Ubuntu 12.04",
-                'input': {'/etc/lsb-release': 'DISTRIB_ID=Ubuntu\nDISTRIB_RELEASE=12.04\nDISTRIB_CODENAME=precise\nDISTRIB_DESCRIPTION="Ubuntu 12.04.5 LTS"\n',
+        'input': {'/etc/lsb-release': """DISTRIB_ID=Ubuntu
-                          '/etc/os-release': 'NAME="Ubuntu"\nVERSION="12.04.5 LTS, Precise Pangolin"\nID=ubuntu\nID_LIKE=debian\nPRETTY_NAME="Ubuntu precise (12.04.5 LTS)"\nVERSION_ID="12.04"\n'},
+DISTRIB_RELEASE=12.04
+DISTRIB_CODENAME=precise
+DISTRIB_DESCRIPTION="Ubuntu 12.04.5 LTS"
+""",
+                  '/etc/os-release': """NAME="Ubuntu"
+VERSION="12.04.5 LTS, Precise Pangolin"
+ID=ubuntu
+ID_LIKE=debian
+PRETTY_NAME="Ubuntu precise (12.04.5 LTS)"
+VERSION_ID="12.04"
+"""},
         'platform.dist': ('Ubuntu', '12.04', 'precise'),
         'result': {'distribution': u'Ubuntu',
                    'distribution_major_version': u'12',
@@ -279,7 +290,11 @@ ANSI_COLOR="1;32"
 HOME_URL="https://coreos.com/"
 BUG_REPORT_URL="https://github.com/coreos/bugs/issues"
             """,
-                    '/etc/lsb-release':'DISTRIB_ID=CoreOS\nDISTRIB_RELEASE=976.0.0\nDISTRIB_CODENAME="Coeur Rouge"\nDISTRIB_DESCRIPTION="CoreOS 976.0.0 (Coeur Rouge)"\n',
+            '/etc/lsb-release':"""DISTRIB_ID=CoreOS
+DISTRIB_RELEASE=976.0.0
+DISTRIB_CODENAME="Coeur Rouge"
+DISTRIB_DESCRIPTION="CoreOS 976.0.0 (Coeur Rouge)"
+""",
         },
         'platform.dist': ('', '', ''),
         'result' : {
@@ -291,38 +306,87 @@ BUG_REPORT_URL="https://github.com/coreos/bugs/issues"
     }
 
 
-        ]
+]
 
-        for t in testsets:
+@unittest.skipIf(sys.version_info[0] >= 3, "Python 3 is not supported on targets (yet)")
+def test_distribution_version():
+    """tests the distribution parsing code of the Facts class
+
+    testsets have
+    * a name (for output/debugging only)
+    * input files that are faked
+      * those should be complete and also include "irrelevant" files that might be mistaken as coming from other distributions
+      * all files that are not listed here are assumed to not exist at all
+    * the output of pythons platform.dist()
+    * results for the ansible variables distribution*
+    """
+
+    # needs to be in here, because the import fails with python3 still
+    import ansible.module_utils.facts
+
+    real_stdin = sys.stdin
+    from ansible.module_utils import basic
+
+    args = json.dumps(dict(ANSIBLE_MODULE_ARGS={}, ANSIBLE_MODULE_CONSTANTS={}))
+    if PY3:
+        sys.stdin = StringIO(args)
+        sys.stdin.buffer = BytesIO(to_bytes(args))
+    else:
+        sys.stdin = BytesIO(to_bytes(args))
+    module = basic.AnsibleModule(argument_spec=dict())
+
+    for t in TESTSETS:
+        # run individual tests via generator
+        # set nicer stdout output for nosetest
+        _test_one_distribution.description = "check distribution_version for %s" % t['name']
+        yield _test_one_distribution, module, t
+
+
+    sys.stdin = real_stdin
+
+
+def _test_one_distribution(module, testcase):
+    """run the test on one distribution testcase
+    
+    * prepare some mock functions to get the testdata in
+    * run Facts()
+    * compare with the expected output
+    """
 
     def mock_get_file_content(fname, default=None, strip=True):
+        """give fake content if it exists, otherwise pretend the file is empty"""
         data = default
-                if fname in t['input']:
+        if fname in testcase['input']:
             # for debugging
-                    print('faked '+fname+' for '+t['name'])
+            print('faked '+fname+' for '+testcase['name'])
-                    data = t['input'][fname].strip()
+            data = testcase['input'][fname].strip()
         if strip and data is not None:
             data = data.strip()
         return data
 
     def mock_path_exists(fname):
-                return fname in t['input']
+        return fname in testcase['input']
 
     def mock_path_getsize(fname):
-                if fname in t['input']:
+        if fname in testcase['input']:
-                    return len(t['input'][fname])
+            # the len is not used, but why not be honest if you can be?
+            return len(testcase['input'][fname])
         else:
             return 0
 
+    @patch('ansible.module_utils.facts.get_file_content', mock_get_file_content)
+    @patch('os.path.exists', mock_path_exists)
+    @patch('os.path.getsize', mock_path_getsize)
+    @patch('platform.dist', lambda: testcase['platform.dist'])
+    @patch('platform.system', lambda: 'Linux')
+    def get_facts(testcase):
+        return ansible.module_utils.facts.Facts(module).populate()
     
-            with patch('ansible.module_utils.facts.get_file_content', side_effect=mock_get_file_content):
+    generated_facts = get_facts(testcase)
-                with patch('os.path.exists', side_effect=mock_path_exists):
+
-                    with patch('os.path.getsize', side_effect=mock_path_getsize):
+    # testcase['result'] has a list of variables and values it expects Facts() to set
-                        with patch('platform.dist', return_value=t['platform.dist']):
+    for key, val in testcase['result'].items():
-                            with patch('platform.system', return_value='Linux'):
+        assert_in(key, generated_facts)
-                                generated_facts = ansible.module_utils.facts.Facts(self.module).populate()
-                                for key, val in t['result'].items():
-                                    self.assertIn(key, generated_facts)
         msg = 'Comparing value of %s on %s, should: %s, is: %s' %\
-                                        (key, t['name'], val, generated_facts[key])
+            (key, testcase['name'], val, generated_facts[key])
-                                    self.assertEqual(generated_facts[key], val, msg)
+        assert_equal(generated_facts[key], val, msg)