From ca97eb6f93d43b2c262356ab32d6f3ad2bbff38c Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Mon, 16 Nov 2020 12:49:38 +0100 Subject: [PATCH] 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 * Update plugins/modules/system/xfconf.py Co-authored-by: Felix Fontein * Removed extraneous import Co-authored-by: Felix Fontein (cherry picked from commit f4c63ede7f9b86547bb59239405e2ef17f7952e7) Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com> --- .../fragments/1305-added-xfconf-tests.yaml | 2 + plugins/modules/system/xfconf.py | 33 +-- .../plugins/modules/system/test_xfconf.py | 193 ++++++++++++++++++ 3 files changed, 212 insertions(+), 16 deletions(-) create mode 100644 changelogs/fragments/1305-added-xfconf-tests.yaml create mode 100644 tests/unit/plugins/modules/system/test_xfconf.py diff --git a/changelogs/fragments/1305-added-xfconf-tests.yaml b/changelogs/fragments/1305-added-xfconf-tests.yaml new file mode 100644 index 0000000000..f90ab5f70b --- /dev/null +++ b/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). diff --git a/plugins/modules/system/xfconf.py b/plugins/modules/system/xfconf.py index faa86c4810..1f604256a4 100644 --- a/plugins/modules/system/xfconf.py +++ b/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, - choices=XfConfProperty.VALID_STATES, + state=dict(default=XFConfProperty.SET, + 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__': diff --git a/tests/unit/plugins/modules/system/test_xfconf.py b/tests/unit/plugins/modules/system/test_xfconf.py new file mode 100644 index 0000000000..91fe839ef9 --- /dev/null +++ b/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