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 <rick@elrod.me>

* Add some tests for check_required_together

Signed-off-by: Rick Elrod <rick@elrod.me>

* use to_native instead of str, for consistency

Signed-off-by: Rick Elrod <rick@elrod.me>

* Add newlines for pep8

Signed-off-by: Rick Elrod <rick@elrod.me>

* Add tests for check_required_arguments

Signed-off-by: Rick Elrod <rick@elrod.me>

* Sort missing keys in error message, since hashes are unsorted and this can be random

Signed-off-by: Rick Elrod <rick@elrod.me>

* Add changelog entry

Signed-off-by: Rick Elrod <rick@elrod.me>

changelogs/fragments/67771-validation-error.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - validation - Sort missing parameters in exception message thrown by check_required_arguments

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

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)

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)

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)