From 3c3ffc09c203d1b2262f6a319cceadd727749761 Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Thu, 26 Mar 2020 18:18:56 -0500 Subject: [PATCH] Fix and add tests for some module_utils.common.validation (#67771) * Fix test_check_mutually_exclusive exception-checking Asserting inside of the `with` context of `pytest.raises` doesn't actually have any effect. So we move the assert out, using the exception that gets placed into the scope after we leave the context, and ensure that it actually gets checked. This is also what the pytest documentation says to do: https://docs.pytest.org/en/latest/assert.html#assertions-about-expected-exceptions Signed-off-by: Rick Elrod * Add some tests for check_required_together Signed-off-by: Rick Elrod * use to_native instead of str, for consistency Signed-off-by: Rick Elrod * Add newlines for pep8 Signed-off-by: Rick Elrod * Add tests for check_required_arguments Signed-off-by: Rick Elrod * Sort missing keys in error message, since hashes are unsorted and this can be random Signed-off-by: Rick Elrod * Add changelog entry Signed-off-by: Rick Elrod --- .../fragments/67771-validation-error.yml | 2 + lib/ansible/module_utils/common/validation.py | 2 +- .../test_check_mutually_exclusive.py | 7 +- .../test_check_required_arguments.py | 88 +++++++++++++++++++ .../test_check_required_together.py | 57 ++++++++++++ 5 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/67771-validation-error.yml create mode 100644 test/units/module_utils/common/validation/test_check_required_arguments.py create mode 100644 test/units/module_utils/common/validation/test_check_required_together.py diff --git a/changelogs/fragments/67771-validation-error.yml b/changelogs/fragments/67771-validation-error.yml new file mode 100644 index 00000000000..29e25f66b27 --- /dev/null +++ b/changelogs/fragments/67771-validation-error.yml @@ -0,0 +1,2 @@ +minor_changes: + - validation - Sort missing parameters in exception message thrown by check_required_arguments diff --git a/lib/ansible/module_utils/common/validation.py b/lib/ansible/module_utils/common/validation.py index 4c29c8b2347..fc13f4d0aab 100644 --- a/lib/ansible/module_utils/common/validation.py +++ b/lib/ansible/module_utils/common/validation.py @@ -189,7 +189,7 @@ def check_required_arguments(argument_spec, module_parameters): missing.append(k) if missing: - msg = "missing required arguments: %s" % ", ".join(missing) + msg = "missing required arguments: %s" % ", ".join(sorted(missing)) raise TypeError(to_native(msg)) return missing diff --git a/test/units/module_utils/common/validation/test_check_mutually_exclusive.py b/test/units/module_utils/common/validation/test_check_mutually_exclusive.py index 5d44f85151c..7bf90760b1a 100644 --- a/test/units/module_utils/common/validation/test_check_mutually_exclusive.py +++ b/test/units/module_utils/common/validation/test_check_mutually_exclusive.py @@ -34,11 +34,12 @@ def test_check_mutually_exclusive_found(mutually_exclusive_terms): 'fox': 'red', 'socks': 'blue', } - expected = "TypeError('parameters are mutually exclusive: string1|string2, box|fox|socks',)" + expected = "parameters are mutually exclusive: string1|string2, box|fox|socks" with pytest.raises(TypeError) as e: check_mutually_exclusive(mutually_exclusive_terms, params) - assert e.value == expected + + assert to_native(e.value) == expected def test_check_mutually_exclusive_none(): @@ -53,4 +54,4 @@ def test_check_mutually_exclusive_none(): def test_check_mutually_exclusive_no_params(mutually_exclusive_terms): with pytest.raises(TypeError) as te: check_mutually_exclusive(mutually_exclusive_terms, None) - assert "TypeError: 'NoneType' object is not iterable" in to_native(te.error) + assert "'NoneType' object is not iterable" in to_native(te.value) diff --git a/test/units/module_utils/common/validation/test_check_required_arguments.py b/test/units/module_utils/common/validation/test_check_required_arguments.py new file mode 100644 index 00000000000..1dd54584bcf --- /dev/null +++ b/test/units/module_utils/common/validation/test_check_required_arguments.py @@ -0,0 +1,88 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2020 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._text import to_native +from ansible.module_utils.common.validation import check_required_arguments + + +@pytest.fixture +def arguments_terms(): + return { + 'foo': { + 'required': True, + }, + 'bar': { + 'required': False, + }, + 'tomato': { + 'irrelevant': 72, + }, + } + + +@pytest.fixture +def arguments_terms_multiple(): + return { + 'foo': { + 'required': True, + }, + 'bar': { + 'required': True, + }, + 'tomato': { + 'irrelevant': 72, + }, + } + + +def test_check_required_arguments(arguments_terms): + params = { + 'foo': 'hello', + 'bar': 'haha', + } + assert check_required_arguments(arguments_terms, params) == [] + + +def test_check_required_arguments_missing(arguments_terms): + params = { + 'apples': 'woohoo', + } + expected = "missing required arguments: foo" + + with pytest.raises(TypeError) as e: + check_required_arguments(arguments_terms, params) + + assert to_native(e.value) == expected + + +def test_check_required_arguments_missing_multiple(arguments_terms_multiple): + params = { + 'apples': 'woohoo', + } + expected = "missing required arguments: bar, foo" + + with pytest.raises(TypeError) as e: + check_required_arguments(arguments_terms_multiple, params) + + assert to_native(e.value) == expected + + +def test_check_required_arguments_missing_none(): + terms = None + params = { + 'foo': 'bar', + 'baz': 'buzz', + } + assert check_required_arguments(terms, params) == [] + + +def test_check_required_arguments_no_params(arguments_terms): + with pytest.raises(TypeError) as te: + check_required_arguments(arguments_terms, None) + assert "'NoneType' is not iterable" in to_native(te.value) diff --git a/test/units/module_utils/common/validation/test_check_required_together.py b/test/units/module_utils/common/validation/test_check_required_together.py new file mode 100644 index 00000000000..8a2daab17f0 --- /dev/null +++ b/test/units/module_utils/common/validation/test_check_required_together.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2020 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._text import to_native +from ansible.module_utils.common.validation import check_required_together + + +@pytest.fixture +def together_terms(): + return [ + ['bananas', 'potatoes'], + ['cats', 'wolves'] + ] + + +def test_check_required_together(together_terms): + params = { + 'bananas': 'hello', + 'potatoes': 'this is here too', + 'dogs': 'haha', + } + assert check_required_together(together_terms, params) == [] + + +def test_check_required_together_missing(together_terms): + params = { + 'bananas': 'woohoo', + 'wolves': 'uh oh', + } + expected = "parameters are required together: bananas, potatoes" + + with pytest.raises(TypeError) as e: + check_required_together(together_terms, params) + + assert to_native(e.value) == expected + + +def test_check_required_together_missing_none(): + terms = None + params = { + 'foo': 'bar', + 'baz': 'buzz', + } + assert check_required_together(terms, params) == [] + + +def test_check_required_together_no_params(together_terms): + with pytest.raises(TypeError) as te: + check_required_together(together_terms, None) + + assert "'NoneType' object is not iterable" in to_native(te.value)