Xfconf tests (#1305) (#1313)

* Adjusted case in class names - transparent to users

* Adjustments to module code:

- No need to try/except everything, in fact it complicated debugging
- Replaced second call to xfconf.get() with xfconf.previous_value

* the actual test

* removed extraneous empty lines

* added changelog fragment

* rolled back removing the try/except around the main execution

* Update changelogs/fragments/1305-added-xfconf-tests.yaml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/system/xfconf.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Removed extraneous import

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit f4c63ede7f)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

changelogs/fragments/1305-added-xfconf-tests.yaml

@@ -0,0 +1,2 @@
+minor_changes:
+  - xfconf - removed unnecessary second execution of ``xfconf-query`` (https://github.com/ansible-collections/community.general/pull/1305).

plugins/modules/system/xfconf.py

@@ -112,15 +112,17 @@ RETURN = '''
     sample: "96"
 '''
 
+import traceback
+
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.six.moves import shlex_quote
 
 
-class XfConfException(Exception):
+class XFConfException(Exception):
     pass
 
 
-class XfConfProperty(object):
+class XFConfProperty(object):
     SET = "present"
     GET = "get"
     RESET = "absent"
@@ -162,12 +164,12 @@ class XfConfProperty(object):
             if err.rstrip() == self.does_not:
                 return None
             if rc or len(err):
-                raise XfConfException('xfconf-query failed with error (rc={0}): {1}'.format(rc, err))
+                raise XFConfException('xfconf-query failed with error (rc={0}): {1}'.format(rc, err))
 
             return out.rstrip()
 
         except OSError as exception:
-            XfConfException('xfconf-query failed with exception: {0}'.format(exception))
+            XFConfException('xfconf-query failed with exception: {0}'.format(exception))
 
     def get(self):
         previous_value = self._execute_xfconf_query()
@@ -216,7 +218,7 @@ class XfConfProperty(object):
         if self.value is None and self.value_type is None:
             return
         if (self.value is None) ^ (self.value_type is None):
-            raise XfConfException('Must set both "value" and "value_type"')
+            raise XFConfException('Must set both "value" and "value_type"')
 
         # stringify all values - in the CLI they will all be happy strings anyway
         # and by doing this here the rest of the code can be agnostic to it
@@ -230,7 +232,7 @@ class XfConfProperty(object):
             self.value_type = self.value_type * values_len
         elif types_len != values_len:
             # or complain if lists' lengths are different
-            raise XfConfException('Same number of "value" and "value_type" needed')
+            raise XFConfException('Same number of "value" and "value_type" needed')
 
         # fix boolean values
         self.value = [self._fix_bool(v[0]) if v[1] == 'bool' else v[0] for v in zip(self.value, self.value_type)]
@@ -247,18 +249,18 @@ def main():
     # Setup the Ansible module
     module = AnsibleModule(
         argument_spec=dict(
-            state=dict(default=XfConfProperty.SET,
+            state=dict(default=XFConfProperty.SET,
-                       choices=XfConfProperty.VALID_STATES,
+                       choices=XFConfProperty.VALID_STATES,
                        type='str'),
             channel=dict(required=True, type='str'),
             property=dict(required=True, type='str'),
             value_type=dict(required=False, type='list',
-                            elements='str', choices=XfConfProperty.VALID_VALUE_TYPES),
+                            elements='str', choices=XFConfProperty.VALID_VALUE_TYPES),
             value=dict(required=False, type='list', elements='raw'),
             force_array=dict(default=False, type='bool', aliases=['array']),
         ),
         required_if=[
-            ('state', XfConfProperty.SET, ['value', 'value_type'])
+            ('state', XFConfProperty.SET, ['value', 'value_type'])
         ],
         supports_check_mode=True
     )
@@ -269,11 +271,10 @@ def main():
     state = module.params['state']
 
     try:
-        # Create a Xfconf preference
+        xfconf = XFConfProperty(module)
-        xfconf = XfConfProperty(module)
         xfconf.sanitize()
 
-        previous_value = xfconf.get()
+        previous_value = xfconf.previous_value
         facts = {
             facts_name: dict(
                 channel=xfconf.channel,
@@ -283,9 +284,9 @@ def main():
             )
         }
 
-        if state == XfConfProperty.GET \
+        if state == XFConfProperty.GET \
                 or (previous_value is not None
-                    and (state, set(previous_value)) == (XfConfProperty.SET, set(xfconf.value))):
+                    and (state, set(previous_value)) == (XFConfProperty.SET, set(xfconf.value))):
             module.exit_json(changed=False, ansible_facts=facts)
             return
 
@@ -299,7 +300,7 @@ def main():
         module.exit_json(changed=True, ansible_facts=facts)
 
     except Exception as e:
-        module.fail_json(msg="Failed with exception: {0}".format(e))
+        module.fail_json(msg="Failed with exception: {0}".format(e), exception=traceback.format_exc())
 
 
 if __name__ == '__main__':

tests/unit/plugins/modules/system/test_xfconf.py

@@ -0,0 +1,193 @@
+# Author: Alexei Znamensky (russoz@gmail.com)
+# Largely adapted from test_redhat_subscription by
+# Jiri Hnidek (jhnidek@redhat.com)
+#
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+
+from ansible.module_utils import basic
+from ansible_collections.community.general.plugins.modules.system import xfconf
+
+import pytest
+
+TESTED_MODULE = xfconf.__name__
+
+
+@pytest.fixture
+def patch_xfconf(mocker):
+    """
+    Function used for mocking some parts of redhat_subscribtion module
+    """
+    mocker.patch('ansible_collections.community.general.plugins.modules.system.xfconf.AnsibleModule.get_bin_path',
+                 return_value='/testbin/xfconf-query')
+
+
+@pytest.mark.parametrize('patch_ansible_module', [{}], indirect=['patch_ansible_module'])
+@pytest.mark.usefixtures('patch_ansible_module')
+def test_without_required_parameters(capfd, patch_xfconf):
+    """
+    Failure must occurs when all parameters are missing
+    """
+    with pytest.raises(SystemExit):
+        xfconf.main()
+    out, err = capfd.readouterr()
+    results = json.loads(out)
+    assert results['failed']
+    assert 'missing required arguments' in results['msg']
+
+
+TEST_CASES = [
+    # Test the case, when the system is already registered
+    [
+        {'channel': 'xfwm4', 'property': '/general/inactive_opacity', 'state': 'get'},
+        {
+            'id': 'test_simple_property_get',
+            'run_command.calls': [
+                (
+                    # Calling of following command will be asserted
+                    '/testbin/xfconf-query --channel xfwm4 --property /general/inactive_opacity',
+                    # Was return code checked?
+                    {'check_rc': False},
+                    # Mock of returned code, stdout and stderr
+                    (0, '100/n', '',),
+                ),
+            ],
+            'changed': False,
+            'value': '100'
+        }
+    ],
+    [
+        {'channel': 'xfwm4', 'property': '/general/workspace_names', 'state': 'get'},
+        {
+            'id': 'test_property_get_array',
+            'run_command.calls': [
+                (
+                    # Calling of following command will be asserted
+                    '/testbin/xfconf-query --channel xfwm4 --property /general/workspace_names',
+                    # Was return code checked?
+                    {'check_rc': False},
+                    # Mock of returned code, stdout and stderr
+                    (0, 'Value is an array with 3 items:\n\nMain\nWork\nTmp\n', '',),
+                ),
+            ],
+            'changed': False,
+            'value': ['Main', 'Work', 'Tmp']
+        },
+    ],
+    [
+        {'channel': 'xfwm4', 'property': '/general/use_compositing', 'state': 'get'},
+        {
+            'id': 'test_property_get_bool',
+            'run_command.calls': [
+                (
+                    # Calling of following command will be asserted
+                    '/testbin/xfconf-query --channel xfwm4 --property /general/use_compositing',
+                    # Was return code checked?
+                    {'check_rc': False},
+                    # Mock of returned code, stdout and stderr
+                    (0, 'true', '',),
+                ),
+            ],
+            'changed': False,
+            'value': True
+        },
+    ],
+    [
+        {'channel': 'xfwm4', 'property': '/general/use_compositing', 'state': 'get'},
+        {
+            'id': 'test_property_get_bool_false',
+            'run_command.calls': [
+                (
+                    # Calling of following command will be asserted
+                    '/testbin/xfconf-query --channel xfwm4 --property /general/use_compositing',
+                    # Was return code checked?
+                    {'check_rc': False},
+                    # Mock of returned code, stdout and stderr
+                    (0, 'false', '',),
+                ),
+            ],
+            'changed': False,
+            'value': False
+        },
+    ],
+    [
+        {
+            'channel': 'xfwm4',
+            'property': '/general/workspace_names',
+            'state': 'present',
+            'value_type': 'string',
+            'value': ['A', 'B', 'C'],
+        },
+        {
+            'id': 'test_property_set_array',
+            'run_command.calls': [
+                (
+                    # Calling of following command will be asserted
+                    '/testbin/xfconf-query --channel xfwm4 --property /general/workspace_names',
+                    # Was return code checked?
+                    {'check_rc': False},
+                    # Mock of returned code, stdout and stderr
+                    (0, 'Value is an array with 3 items:\n\nMain\nWork\nTmp\n', '',),
+                ),
+                (
+                    # Calling of following command will be asserted
+                    "/testbin/xfconf-query --channel xfwm4 --property /general/workspace_names --create "
+                    "--force-array --type 'string' --set 'A' --type 'string' --set 'B' --type 'string' --set 'C'",
+                    # Was return code checked?
+                    {'check_rc': False},
+                    # Mock of returned code, stdout and stderr
+                    (0, '', '',),
+                ),
+            ],
+            'changed': True,
+            'previous_value': ['Main', 'Work', 'Tmp'],
+            'value': ['A', 'B', 'C'],
+        },
+    ],
+]
+TEST_CASES_IDS = [item[1]['id'] for item in TEST_CASES]
+
+
+@pytest.mark.parametrize('patch_ansible_module, testcase',
+                         TEST_CASES,
+                         ids=TEST_CASES_IDS,
+                         indirect=['patch_ansible_module'])
+@pytest.mark.usefixtures('patch_ansible_module')
+def test_xfconf(mocker, capfd, patch_xfconf, testcase):
+    """
+    Run unit tests for test cases listen in TEST_CASES
+    """
+
+    # Mock function used for running commands first
+    call_results = [item[2] for item in testcase['run_command.calls']]
+    mock_run_command = mocker.patch(
+        'ansible_collections.community.general.plugins.modules.system.xfconf.AnsibleModule.run_command',
+        side_effect=call_results)
+
+    # Try to run test case
+    with pytest.raises(SystemExit):
+        xfconf.main()
+
+    out, err = capfd.readouterr()
+    results = json.loads(out)
+    print("results = %s" % results)
+    assert 'changed' in results
+    assert results['changed'] == testcase['changed']
+    if 'msg' in results:
+        assert results.get('msg') == testcase['msg']
+    if 'value' in results:
+        assert results['value'] == testcase['value']
+    if 'previous_value' in results:
+        assert results['previous_value'] == testcase['previous_value']
+
+    assert mock_run_command.call_count == len(testcase['run_command.calls'])
+    if mock_run_command.call_count:
+        call_args_list = [(item[0][0], item[1]) for item in mock_run_command.call_args_list]
+        expected_call_args_list = [(item[0], item[1]) for item in testcase['run_command.calls']]
+        print("call args list =\n%s" % call_args_list)
+        print("expected args list =\n%s" % expected_call_args_list)
+        assert call_args_list == expected_call_args_list