From f92e99fd8f6b49cdb24e8863f56225feab544ca2 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Sat, 2 Nov 2024 10:36:45 -0700 Subject: test: fix check_required_by (#84153) * Update the documentation for check_required_by * Fix return value for check_required_by (now returns empty list on success) Signed-off-by: Abhijeet Kasurde --- lib/ansible/module_utils/common/validation.py | 19 +++++--------- .../common/validation/test_check_required_by.py | 29 ++++++---------------- 2 files changed, 14 insertions(+), 34 deletions(-) diff --git a/lib/ansible/module_utils/common/validation.py b/lib/ansible/module_utils/common/validation.py index 399767e775..1098f27336 100644 --- a/lib/ansible/module_utils/common/validation.py +++ b/lib/ansible/module_utils/common/validation.py @@ -185,7 +185,7 @@ def check_required_by(requirements, parameters, options_context=None): :kwarg options_context: List of strings of parent key names if ``requirements`` are in a sub spec. - :returns: Empty dictionary or raises :class:`TypeError` if the + :returns: Empty dictionary or raises :class:`TypeError` if the check fails. """ result = {} @@ -195,22 +195,15 @@ def check_required_by(requirements, parameters, options_context=None): for (key, value) in requirements.items(): if key not in parameters or parameters[key] is None: continue - result[key] = [] # Support strings (single-item lists) if isinstance(value, string_types): value = [value] - for required in value: - if required not in parameters or parameters[required] is None: - result[key].append(required) - - if result: - for key, missing in result.items(): - if len(missing) > 0: - msg = "missing parameter(s) required by '%s': %s" % (key, ', '.join(missing)) - if options_context: - msg = "{0} found in {1}".format(msg, " -> ".join(options_context)) - raise TypeError(to_native(msg)) + if missing := [required for required in value if required not in parameters or parameters[required] is None]: + msg = f"missing parameter(s) required by '{key}': {', '.join(missing)}" + if options_context: + msg = f"{msg} found in {' -> '.join(options_context)}" + raise TypeError(to_native(msg)) return result diff --git a/test/units/module_utils/common/validation/test_check_required_by.py b/test/units/module_utils/common/validation/test_check_required_by.py index 053c30a143..8ac10474e3 100644 --- a/test/units/module_utils/common/validation/test_check_required_by.py +++ b/test/units/module_utils/common/validation/test_check_required_by.py @@ -4,10 +4,10 @@ from __future__ import annotations +import re import pytest -from ansible.module_utils.common.text.converters import to_native from ansible.module_utils.common.validation import check_required_by @@ -19,9 +19,7 @@ def path_arguments_terms(): def test_check_required_by(): - arguments_terms = {} - params = {} - assert check_required_by(arguments_terms, params) == {} + assert check_required_by({}, {}) == {} def test_check_required_by_missing(): @@ -30,12 +28,9 @@ def test_check_required_by_missing(): } params = {"force": True} expected = "missing parameter(s) required by 'force': force_reason" - - with pytest.raises(TypeError) as e: + with pytest.raises(TypeError, match=re.escape(expected)): check_required_by(arguments_terms, params) - assert to_native(e.value) == expected - def test_check_required_by_multiple(path_arguments_terms): params = { @@ -43,21 +38,17 @@ def test_check_required_by_multiple(path_arguments_terms): } expected = "missing parameter(s) required by 'path': mode, owner" - with pytest.raises(TypeError) as e: + with pytest.raises(TypeError, match=re.escape(expected)): check_required_by(path_arguments_terms, params) - assert to_native(e.value) == expected - def test_check_required_by_single(path_arguments_terms): params = {"path": "/foo/bar", "mode": "0700"} expected = "missing parameter(s) required by 'path': owner" - with pytest.raises(TypeError) as e: + with pytest.raises(TypeError, match=re.escape(expected)): check_required_by(path_arguments_terms, params) - assert to_native(e.value) == expected - def test_check_required_by_missing_none(path_arguments_terms): params = { @@ -65,7 +56,7 @@ def test_check_required_by_missing_none(path_arguments_terms): "mode": "0700", "owner": "root", } - assert check_required_by(path_arguments_terms, params) + assert check_required_by(path_arguments_terms, params) == {} def test_check_required_by_options_context(path_arguments_terms): @@ -75,11 +66,9 @@ def test_check_required_by_options_context(path_arguments_terms): expected = "missing parameter(s) required by 'path': owner found in foo_context" - with pytest.raises(TypeError) as e: + with pytest.raises(TypeError, match=re.escape(expected)): check_required_by(path_arguments_terms, params, options_context) - assert to_native(e.value) == expected - def test_check_required_by_missing_multiple_options_context(path_arguments_terms): params = { @@ -91,7 +80,5 @@ def test_check_required_by_missing_multiple_options_context(path_arguments_terms "missing parameter(s) required by 'path': mode, owner found in foo_context" ) - with pytest.raises(TypeError) as e: + with pytest.raises(TypeError, match=re.escape(expected)): check_required_by(path_arguments_terms, params, options_context) - - assert to_native(e.value) == expected -- cgit v1.2.3