Improve ArgumentSpecValidator unit tests (#73642)

* Add more scenarios to basic valid testing
* Update invalid tests
* Fix test for Python 2
* Condense data
* Add tests for missing required and invalid-elements
* Update aliases tests
* Add invalid scenarios for aliases
* Add tests for _add_error() method
* Fix sanity test failure

7 files changed, 553 insertions, 190 deletions

diff --git a/test/units/module_utils/common/arg_spec/test__add_error.py b/test/units/module_utils/common/arg_spec/test__add_error.py
new file mode 100644
index 0000000000..bf98580398
--- /dev/null
+++ b/test/units/module_utils/common/arg_spec/test__add_error.py
@@ -0,0 +1,28 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2021 Ansible Project
+# 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 pytest
+
+from ansible.module_utils.common.arg_spec import ArgumentSpecValidator
+
+
+def test_add_sequence():
+    v = ArgumentSpecValidator({}, {})
+    errors = [
+        'one error',
+        'another error',
+    ]
+    v._add_error(errors)
+
+    assert v.error_messages == errors
+
+
+def test_invalid_error_message():
+    v = ArgumentSpecValidator({}, {})
+
+    with pytest.raises(ValueError, match="Error messages must be a string or sequence not a"):
+        v._add_error(None)
diff --git a/test/units/module_utils/common/arg_spec/test_aliases.py b/test/units/module_utils/common/arg_spec/test_aliases.py
new file mode 100644
index 0000000000..4dcdf938a7
--- /dev/null
+++ b/test/units/module_utils/common/arg_spec/test_aliases.py
@@ -0,0 +1,119 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2021 Ansible Project
+# 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 pytest
+
+from ansible.module_utils.common.arg_spec import ArgumentSpecValidator
+from ansible.module_utils.common.warnings import get_deprecation_messages, get_warning_messages
+
+# id, argument spec, parameters, expected parameters, expected pass/fail, error, deprecation, warning
+ALIAS_TEST_CASES = [
+    (
+        "alias",
+        {'path': {'aliases': ['dir', 'directory']}},
+        {'dir': '/tmp'},
+        {
+            'dir': '/tmp',
+            'path': '/tmp',
+        },
+        True,
+        "",
+        "",
+        "",
+    ),
+    (
+        "alias-invalid",
+        {'path': {'aliases': 'bad'}},
+        {},
+        {'path': None},
+        False,
+        "internal error: aliases must be a list or tuple",
+        "",
+        "",
+    ),
+    (
+        # This isn't related to aliases, but it exists in the alias handling code
+        "default-and-required",
+        {'name': {'default': 'ray', 'required': True}},
+        {},
+        {'name': 'ray'},
+        False,
+        "internal error: required and default are mutually exclusive for name",
+        "",
+        "",
+    ),
+    (
+        "alias-duplicate-warning",
+        {'path': {'aliases': ['dir', 'directory']}},
+        {
+            'dir': '/tmp',
+            'directory': '/tmp',
+        },
+        {
+            'dir': '/tmp',
+            'directory': '/tmp',
+            'path': '/tmp',
+        },
+        True,
+        "",
+        "",
+        "Both option path and its alias directory are set",
+    ),
+    (
+        "deprecated-alias",
+        {
+            'path': {
+                'aliases': ['not_yo_path'],
+                'deprecated_aliases': [
+                    {
+                        'name': 'not_yo_path',
+                        'version': '1.7',
+                    }
+                ]
+            }
+        },
+        {'not_yo_path': '/tmp'},
+        {
+            'path': '/tmp',
+            'not_yo_path': '/tmp',
+        },
+        True,
+        "",
+        "Alias 'not_yo_path' is deprecated.",
+        "",
+    )
+]
+
+
+@pytest.mark.parametrize(
+    ('arg_spec', 'parameters', 'expected', 'passfail', 'error', 'deprecation', 'warning'),
+    ((i[1], i[2], i[3], i[4], i[5], i[6], i[7]) for i in ALIAS_TEST_CASES),
+    ids=[i[0] for i in ALIAS_TEST_CASES]
+)
+def test_aliases(arg_spec, parameters, expected, passfail, error, deprecation, warning):
+    v = ArgumentSpecValidator(arg_spec, parameters)
+    passed = v.validate()
+
+    assert passed is passfail
+    assert v.validated_parameters == expected
+
+    if not error:
+        assert v.error_messages == []
+    else:
+        assert error in v.error_messages[0]
+
+    deprecations = get_deprecation_messages()
+    if not deprecations:
+        assert deprecations == ()
+    else:
+        assert deprecation in get_deprecation_messages()[0]['msg']
+
+    warnings = get_warning_messages()
+    if not warning:
+        assert warnings == ()
+    else:
+        assert warning in warnings[0]
diff --git a/test/units/module_utils/common/arg_spec/test_validate_aliases.py b/test/units/module_utils/common/arg_spec/test_validate_aliases.py
deleted file mode 100644
index a2a36cf4c3..0000000000
--- a/test/units/module_utils/common/arg_spec/test_validate_aliases.py
+++ /dev/null
@@ -1,61 +0,0 @@
-# -*- coding: utf-8 -*-
-# Copyright (c) 2021 Ansible Project
-# 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
-
-from ansible.module_utils.common.arg_spec import ArgumentSpecValidator
-from ansible.module_utils.common.warnings import get_deprecation_messages
-
-
-def test_spec_with_aliases():
-    arg_spec = {
-        'path': {'aliases': ['dir', 'directory']}
-    }
-
-    parameters = {
-        'dir': '/tmp',
-        'directory': '/tmp',
-    }
-
-    expected = {
-        'dir': '/tmp',
-        'directory': '/tmp',
-        'path': '/tmp',
-    }
-
-    v = ArgumentSpecValidator(arg_spec, parameters)
-    passed = v.validate()
-
-    assert passed is True
-    assert v.validated_parameters == expected
-
-
-def test_alias_deprecation():
-    arg_spec = {
-        'path': {
-            'aliases': ['not_yo_path'],
-            'deprecated_aliases': [{
-                'name': 'not_yo_path',
-                'version': '1.7',
-            }]
-        }
-    }
-
-    parameters = {
-        'not_yo_path': '/tmp',
-    }
-
-    expected = {
-        'path': '/tmp',
-        'not_yo_path': '/tmp',
-    }
-
-    v = ArgumentSpecValidator(arg_spec, parameters)
-    passed = v.validate()
-
-    assert passed is True
-    assert v.validated_parameters == expected
-    assert v.error_messages == []
-    assert "Alias 'not_yo_path' is deprecated." in get_deprecation_messages()[0]['msg']
diff --git a/test/units/module_utils/common/arg_spec/test_validate_basic.py b/test/units/module_utils/common/arg_spec/test_validate_basic.py
deleted file mode 100644
index 344fb2b38c..0000000000
--- a/test/units/module_utils/common/arg_spec/test_validate_basic.py
+++ /dev/null
@@ -1,100 +0,0 @@
-# -*- coding: utf-8 -*-
-# Copyright (c) 2021 Ansible Project
-# 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
-
-from ansible.module_utils.common.arg_spec import ArgumentSpecValidator
-
-
-def test_basic_spec():
-    arg_spec = {
-        'param_str': {'type': 'str'},
-        'param_list': {'type': 'list'},
-        'param_dict': {'type': 'dict'},
-        'param_bool': {'type': 'bool'},
-        'param_int': {'type': 'int'},
-        'param_float': {'type': 'float'},
-        'param_path': {'type': 'path'},
-        'param_raw': {'type': 'raw'},
-        'param_bytes': {'type': 'bytes'},
-        'param_bits': {'type': 'bits'},
-
-    }
-
-    parameters = {
-        'param_str': 22,
-        'param_list': 'one,two,three',
-        'param_dict': 'first=star,last=lord',
-        'param_bool': True,
-        'param_int': 22,
-        'param_float': 1.5,
-        'param_path': '/tmp',
-        'param_raw': 'raw',
-        'param_bytes': '2K',
-        'param_bits': '1Mb',
-    }
-
-    expected = {
-        'param_str': '22',
-        'param_list': ['one', 'two', 'three'],
-        'param_dict': {'first': 'star', 'last': 'lord'},
-        'param_bool': True,
-        'param_float': 1.5,
-        'param_int': 22,
-        'param_path': '/tmp',
-        'param_raw': 'raw',
-        'param_bits': 1048576,
-        'param_bytes': 2048,
-    }
-
-    v = ArgumentSpecValidator(arg_spec, parameters)
-    passed = v.validate()
-
-    assert passed is True
-    assert v.validated_parameters == expected
-    assert v.error_messages == []
-
-
-def test_spec_with_defaults():
-    arg_spec = {
-        'param_str': {'type': 'str', 'default': 'DEFAULT'},
-    }
-
-    parameters = {}
-
-    expected = {
-        'param_str': 'DEFAULT',
-    }
-
-    v = ArgumentSpecValidator(arg_spec, parameters)
-    passed = v.validate()
-
-    assert passed is True
-    assert v.validated_parameters == expected
-    assert v.error_messages == []
-
-
-def test_spec_with_elements():
-    arg_spec = {
-        'param_list': {
-            'type': 'list',
-            'elements': 'int',
-        }
-    }
-
-    parameters = {
-        'param_list': [55, 33, 34, '22'],
-    }
-
-    expected = {
-        'param_list': [55, 33, 34, 22],
-    }
-
-    v = ArgumentSpecValidator(arg_spec, parameters)
-    passed = v.validate()
-
-    assert passed is True
-    assert v.error_messages == []
-    assert v.validated_parameters == expected
diff --git a/test/units/module_utils/common/arg_spec/test_validate_failures.py b/test/units/module_utils/common/arg_spec/test_validate_failures.py
deleted file mode 100644
index e0af0159e3..0000000000
--- a/test/units/module_utils/common/arg_spec/test_validate_failures.py
+++ /dev/null
@@ -1,29 +0,0 @@
-# -*- coding: utf-8 -*-
-# Copyright (c) 2021 Ansible Project
-# 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
-
-from ansible.module_utils.common.arg_spec import ArgumentSpecValidator
-
-
-def test_required_and_default():
-    arg_spec = {
-        'param_req': {'required': True, 'default': 'DEFAULT'},
-    }
-
-    v = ArgumentSpecValidator(arg_spec, {})
-    passed = v.validate()
-
-    expected = {
-        'param_req': 'DEFAULT'
-    }
-
-    expected_errors = [
-        'internal error: required and default are mutually exclusive for param_req',
-    ]
-
-    assert passed is False
-    assert v.validated_parameters == expected
-    assert v.error_messages == expected_errors
diff --git a/test/units/module_utils/common/arg_spec/test_validate_invalid.py b/test/units/module_utils/common/arg_spec/test_validate_invalid.py
new file mode 100644
index 0000000000..99ab62b18d
--- /dev/null
+++ b/test/units/module_utils/common/arg_spec/test_validate_invalid.py
@@ -0,0 +1,110 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2021 Ansible Project
+# 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 pytest
+
+from ansible.module_utils.common.arg_spec import ArgumentSpecValidator
+from ansible.module_utils.six import PY2
+
+
+# Each item is id, argument_spec, parameters, expected, error test string
+INVALID_SPECS = [
+    (
+        'invalid-list',
+        {'packages': {'type': 'list'}},
+        {'packages': {'key': 'value'}},
+        {'packages': {'key': 'value'}},
+        "unable to convert to list: <class 'dict'> cannot be converted to a list",
+    ),
+    (
+        'invalid-dict',
+        {'users': {'type': 'dict'}},
+        {'users': ['one', 'two']},
+        {'users': ['one', 'two']},
+        "unable to convert to dict: <class 'list'> cannot be converted to a dict",
+    ),
+    (
+        'invalid-bool',
+        {'bool': {'type': 'bool'}},
+        {'bool': {'k': 'v'}},
+        {'bool': {'k': 'v'}},
+        "unable to convert to bool: <class 'dict'> cannot be converted to a bool",
+    ),
+    (
+        'invalid-float',
+        {'float': {'type': 'float'}},
+        {'float': 'hello'},
+        {'float': 'hello'},
+        "unable to convert to float: <class 'str'> cannot be converted to a float",
+    ),
+    (
+        'invalid-bytes',
+        {'bytes': {'type': 'bytes'}},
+        {'bytes': 'one'},
+        {'bytes': 'one'},
+        "unable to convert to bytes: <class 'str'> cannot be converted to a Byte value",
+    ),
+    (
+        'invalid-bits',
+        {'bits': {'type': 'bits'}},
+        {'bits': 'one'},
+        {'bits': 'one'},
+        "unable to convert to bits: <class 'str'> cannot be converted to a Bit value",
+    ),
+    (
+        'invalid-jsonargs',
+        {'some_json': {'type': 'jsonarg'}},
+        {'some_json': set()},
+        {'some_json': set()},
+        "unable to convert to jsonarg: <class 'set'> cannot be converted to a json string",
+    ),
+    (
+        'invalid-parameter',
+        {'name': {}},
+        {
+            'badparam': '',
+            'another': '',
+        },
+        {
+            'name': None,
+            'badparam': '',
+            'another': '',
+        },
+        "Unsupported parameters: another, badparam",
+    ),
+    (
+        'invalid-elements',
+        {'numbers': {'type': 'list', 'elements': 'int'}},
+        {'numbers': [55, 33, 34, {'key': 'value'}]},
+        {'numbers': [55, 33, 34]},
+        "Elements value for option 'numbers' is of type <class 'dict'> and we were unable to convert to int: <class 'dict'> cannot be converted to an int"
+    ),
+    (
+        'required',
+        {'req': {'required': True}},
+        {},
+        {'req': None},
+        "missing required arguments: req"
+    )
+]
+
+
+@pytest.mark.parametrize(
+    ('arg_spec', 'parameters', 'expected', 'error'),
+    ((i[1], i[2], i[3], i[4]) for i in INVALID_SPECS),
+    ids=[i[0] for i in INVALID_SPECS]
+)
+def test_invalid_spec(arg_spec, parameters, expected, error):
+    v = ArgumentSpecValidator(arg_spec, parameters)
+    passed = v.validate()
+
+    if PY2:
+        error = error.replace('class', 'type')
+
+    assert error in v.error_messages[0]
+    assert v.validated_parameters == expected
+    assert passed is False
diff --git a/test/units/module_utils/common/arg_spec/test_validate_valid.py b/test/units/module_utils/common/arg_spec/test_validate_valid.py
new file mode 100644
index 0000000000..0b139aff5d
--- /dev/null
+++ b/test/units/module_utils/common/arg_spec/test_validate_valid.py
@@ -0,0 +1,296 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2021 Ansible Project
+# 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 pytest
+
+from ansible.module_utils.common.arg_spec import ArgumentSpecValidator
+
+# Each item is id, argument_spec, parameters, expected
+VALID_SPECS = [
+    (
+        'str-no-type-specified',
+        {'name': {}},
+        {'name': 'rey'},
+        {'name': 'rey'},
+    ),
+    (
+        'str',
+        {'name': {'type': 'str'}},
+        {'name': 'rey'},
+        {'name': 'rey'},
+    ),
+    (
+        'str-convert',
+        {'name': {'type': 'str'}},
+        {'name': 5},
+        {'name': '5'},
+    ),
+    (
+        'list',
+        {'packages': {'type': 'list'}},
+        {'packages': ['vim', 'python']},
+        {'packages': ['vim', 'python']},
+    ),
+    (
+        'list-comma-string',
+        {'packages': {'type': 'list'}},
+        {'packages': 'vim,python'},
+        {'packages': ['vim', 'python']},
+    ),
+    (
+        'list-comma-string-space',
+        {'packages': {'type': 'list'}},
+        {'packages': 'vim, python'},
+        {'packages': ['vim', ' python']},
+    ),
+    (
+        'dict',
+        {'user': {'type': 'dict'}},
+        {
+            'user':
+            {
+                'first': 'rey',
+                'last': 'skywalker',
+            }
+        },
+        {
+            'user':
+            {
+                'first': 'rey',
+                'last': 'skywalker',
+            }
+        },
+    ),
+    (
+        'dict-k=v',
+        {'user': {'type': 'dict'}},
+        {'user': 'first=rey,last=skywalker'},
+        {
+            'user':
+            {
+                'first': 'rey',
+                'last': 'skywalker',
+            }
+        },
+    ),
+    (
+        'dict-k=v-spaces',
+        {'user': {'type': 'dict'}},
+        {'user': 'first=rey,   last=skywalker'},
+        {
+            'user':
+            {
+                'first': 'rey',
+                'last': 'skywalker',
+            }
+        },
+    ),
+    (
+        'bool',
+        {
+            'enabled': {'type': 'bool'},
+            'disabled': {'type': 'bool'},
+        },
+        {
+            'enabled': True,
+            'disabled': False,
+        },
+        {
+            'enabled': True,
+            'disabled': False,
+        },
+    ),
+    (
+        'bool-ints',
+        {
+            'enabled': {'type': 'bool'},
+            'disabled': {'type': 'bool'},
+        },
+        {
+            'enabled': 1,
+            'disabled': 0,
+        },
+        {
+            'enabled': True,
+            'disabled': False,
+        },
+    ),
+    (
+        'bool-true-false',
+        {
+            'enabled': {'type': 'bool'},
+            'disabled': {'type': 'bool'},
+        },
+        {
+            'enabled': 'true',
+            'disabled': 'false',
+        },
+        {
+            'enabled': True,
+            'disabled': False,
+        },
+    ),
+    (
+        'bool-yes-no',
+        {
+            'enabled': {'type': 'bool'},
+            'disabled': {'type': 'bool'},
+        },
+        {
+            'enabled': 'yes',
+            'disabled': 'no',
+        },
+        {
+            'enabled': True,
+            'disabled': False,
+        },
+    ),
+    (
+        'bool-y-n',
+        {
+            'enabled': {'type': 'bool'},
+            'disabled': {'type': 'bool'},
+        },
+        {
+            'enabled': 'y',
+            'disabled': 'n',
+        },
+        {
+            'enabled': True,
+            'disabled': False,
+        },
+    ),
+    (
+        'bool-on-off',
+        {
+            'enabled': {'type': 'bool'},
+            'disabled': {'type': 'bool'},
+        },
+        {
+            'enabled': 'on',
+            'disabled': 'off',
+        },
+        {
+            'enabled': True,
+            'disabled': False,
+        },
+    ),
+    (
+        'bool-1-0',
+        {
+            'enabled': {'type': 'bool'},
+            'disabled': {'type': 'bool'},
+        },
+        {
+            'enabled': '1',
+            'disabled': '0',
+        },
+        {
+            'enabled': True,
+            'disabled': False,
+        },
+    ),
+    (
+        'bool-float',
+        {
+            'enabled': {'type': 'bool'},
+            'disabled': {'type': 'bool'},
+        },
+        {
+            'enabled': 1.0,
+            'disabled': 0.0,
+        },
+        {
+            'enabled': True,
+            'disabled': False,
+        },
+    ),
+    (
+        'float',
+        {'digit': {'type': 'float'}},
+        {'digit': 3.14159},
+        {'digit': 3.14159},
+    ),
+    (
+        'float-str',
+        {'digit': {'type': 'float'}},
+        {'digit': '3.14159'},
+        {'digit': 3.14159},
+    ),
+    (
+        'path',
+        {'path': {'type': 'path'}},
+        {'path': '~/bin'},
+        {'path': '/home/ansible/bin'},
+    ),
+    (
+        'raw',
+        {'raw': {'type': 'raw'}},
+        {'raw': 0x644},
+        {'raw': 0x644},
+    ),
+    (
+        'bytes',
+        {'bytes': {'type': 'bytes'}},
+        {'bytes': '2K'},
+        {'bytes': 2048},
+    ),
+    (
+        'bits',
+        {'bits': {'type': 'bits'}},
+        {'bits': '1Mb'},
+        {'bits': 1048576},
+    ),
+    (
+        'jsonarg',
+        {'some_json': {'type': 'jsonarg'}},
+        {'some_json': '{"users": {"bob": {"role": "accountant"}}}'},
+        {'some_json': '{"users": {"bob": {"role": "accountant"}}}'},
+    ),
+    (
+        'jsonarg-list',
+        {'some_json': {'type': 'jsonarg'}},
+        {'some_json': ['one', 'two']},
+        {'some_json': '["one", "two"]'},
+    ),
+    (
+        'jsonarg-dict',
+        {'some_json': {'type': 'jsonarg'}},
+        {'some_json': {"users": {"bob": {"role": "accountant"}}}},
+        {'some_json': '{"users": {"bob": {"role": "accountant"}}}'},
+    ),
+    (
+        'defaults',
+        {'param': {'default': 'DEFAULT'}},
+        {},
+        {'param': 'DEFAULT'},
+    ),
+    (
+        'elements',
+        {'numbers': {'type': 'list', 'elements': 'int'}},
+        {'numbers': [55, 33, 34, '22']},
+        {'numbers': [55, 33, 34, 22]},
+    ),
+]
+
+
+@pytest.mark.parametrize(
+    ('arg_spec', 'parameters', 'expected'),
+    ((i[1], i[2], i[3]) for i in VALID_SPECS),
+    ids=[i[0] for i in VALID_SPECS]
+)
+def test_valid_spec(arg_spec, parameters, expected, mocker):
+
+    mocker.patch('ansible.module_utils.common.validation.os.path.expanduser', return_value='/home/ansible/bin')
+    mocker.patch('ansible.module_utils.common.validation.os.path.expandvars', return_value='/home/ansible/bin')
+
+    v = ArgumentSpecValidator(arg_spec, parameters)
+    passed = v.validate()
+
+    assert v.validated_parameters == expected
+    assert v.error_messages == []
+    assert passed is True