From 1e6ffc1d02559a26def6c9c3b07baf27032865a2 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Fri, 18 Oct 2024 14:11:33 -0700 Subject: [PATCH] Fixed broken tests (#84088) * Add `match=` in pytest.raises * Remove redundant assert statements Signed-off-by: Abhijeet Kasurde Co-authored-by: Matt Clay --- .../module_utils/basic/test_exit_json.py | 2 +- .../common/parameters/test_handle_aliases.py | 12 +-- test/units/module_utils/facts/test_timeout.py | 47 +++++------- .../module_utils/parsing/test_convert_bool.py | 76 +++++++------------ test/units/plugins/cache/test_cache.py | 2 +- test/units/utils/display/test_curses.py | 4 +- 6 files changed, 53 insertions(+), 90 deletions(-) diff --git a/test/units/module_utils/basic/test_exit_json.py b/test/units/module_utils/basic/test_exit_json.py index 0f6844b24a1..5fec45deb6a 100644 --- a/test/units/module_utils/basic/test_exit_json.py +++ b/test/units/module_utils/basic/test_exit_json.py @@ -158,7 +158,7 @@ class TestAnsibleModuleExitValuesRemoved: monkeypatch.setattr(warnings, '_global_deprecations', []) expected['failed'] = True with pytest.raises(SystemExit): - am.fail_json(**return_val) == expected + am.fail_json(**return_val) out, err = capfd.readouterr() assert json.loads(out) == expected diff --git a/test/units/module_utils/common/parameters/test_handle_aliases.py b/test/units/module_utils/common/parameters/test_handle_aliases.py index 550d10ff5c0..b5b9e6033b2 100644 --- a/test/units/module_utils/common/parameters/test_handle_aliases.py +++ b/test/units/module_utils/common/parameters/test_handle_aliases.py @@ -8,7 +8,6 @@ from __future__ import annotations import pytest from ansible.module_utils.common.parameters import _handle_aliases -from ansible.module_utils.common.text.converters import to_native def test_handle_aliases_no_aliases(): @@ -21,10 +20,7 @@ def test_handle_aliases_no_aliases(): 'path': 'bar' } - expected = {} - result = _handle_aliases(argument_spec, params) - - assert expected == result + assert _handle_aliases(argument_spec, params) == {} def test_handle_aliases_basic(): @@ -54,9 +50,8 @@ def test_handle_aliases_value_error(): 'name': 'foo', } - with pytest.raises(ValueError) as ve: + with pytest.raises(ValueError, match='internal error: required and default are mutually exclusive'): _handle_aliases(argument_spec, params) - assert 'internal error: aliases must be a list or tuple' == to_native(ve.error) def test_handle_aliases_type_error(): @@ -68,6 +63,5 @@ def test_handle_aliases_type_error(): 'name': 'foo', } - with pytest.raises(TypeError) as te: + with pytest.raises(TypeError, match='internal error: aliases must be a list or tuple'): _handle_aliases(argument_spec, params) - assert 'internal error: required and default are mutually exclusive' in to_native(te.error) diff --git a/test/units/module_utils/facts/test_timeout.py b/test/units/module_utils/facts/test_timeout.py index d187cedf22c..cc64f9f0492 100644 --- a/test/units/module_utils/facts/test_timeout.py +++ b/test/units/module_utils/facts/test_timeout.py @@ -1,20 +1,7 @@ # -*- coding: utf-8 -*- -# (c) 2017, Toshio Kuratomi -# -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . +# Copyright: (c) 2017, Toshio Kuratomi +# Copyright: Contributors to the Ansible project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import annotations @@ -83,8 +70,8 @@ def test_implicit_file_default_timesout(monkeypatch): monkeypatch.setattr(timeout, 'DEFAULT_GATHER_TIMEOUT', 1) # sleep_time is greater than the default sleep_time = timeout.DEFAULT_GATHER_TIMEOUT + 1 - with pytest.raises(timeout.TimeoutError): - assert sleep_amount_implicit(sleep_time) == '(Not expected to succeed)' + with pytest.raises(timeout.TimeoutError, match=r"^Timer expired after"): + sleep_amount_implicit(sleep_time) def test_implicit_file_overridden_succeeds(set_gather_timeout_higher): @@ -96,8 +83,8 @@ def test_implicit_file_overridden_succeeds(set_gather_timeout_higher): def test_implicit_file_overridden_timesout(set_gather_timeout_lower): # Set sleep_time greater than our new timeout but less than the default sleep_time = 3 - with pytest.raises(timeout.TimeoutError): - assert sleep_amount_implicit(sleep_time) == '(Not expected to Succeed)' + with pytest.raises(timeout.TimeoutError, match=r"^Timer expired after"): + sleep_amount_implicit(sleep_time) def test_explicit_succeeds(monkeypatch): @@ -110,8 +97,8 @@ def test_explicit_succeeds(monkeypatch): def test_explicit_timeout(): # Set sleep_time greater than our new timeout but less than the default sleep_time = 3 - with pytest.raises(timeout.TimeoutError): - assert sleep_amount_explicit_lower(sleep_time) == '(Not expected to succeed)' + with pytest.raises(timeout.TimeoutError, match=r"^Timer expired after"): + sleep_amount_explicit_lower(sleep_time) # @@ -149,21 +136,21 @@ def function_catches_all_exceptions(): def test_timeout_raises_timeout(): - with pytest.raises(timeout.TimeoutError): - assert function_times_out() == '(Not expected to succeed)' + with pytest.raises(timeout.TimeoutError, match=r"^Timer expired after"): + function_times_out() @pytest.mark.parametrize('stdin', ({},), indirect=['stdin']) def test_timeout_raises_timeout_integration_test(am): - with pytest.raises(timeout.TimeoutError): - assert function_times_out_in_run_command(am) == '(Not expected to succeed)' + with pytest.raises(timeout.TimeoutError, match=r"^Timer expired after"): + function_times_out_in_run_command(am) def test_timeout_raises_other_exception(): - with pytest.raises(ZeroDivisionError): - assert function_raises() == '(Not expected to succeed)' + with pytest.raises(ZeroDivisionError, match=r"^division by"): + function_raises() def test_exception_not_caught_by_called_code(): - with pytest.raises(timeout.TimeoutError): - assert function_catches_all_exceptions() == '(Not expected to succeed)' + with pytest.raises(timeout.TimeoutError, match=r"^Timer expired after"): + function_catches_all_exceptions() diff --git a/test/units/module_utils/parsing/test_convert_bool.py b/test/units/module_utils/parsing/test_convert_bool.py index d0ade8bd085..68c98f9071c 100644 --- a/test/units/module_utils/parsing/test_convert_bool.py +++ b/test/units/module_utils/parsing/test_convert_bool.py @@ -9,50 +9,32 @@ import pytest from ansible.module_utils.parsing.convert_bool import boolean -class TestBoolean: - def test_bools(self): - assert boolean(True) is True - assert boolean(False) is False - - def test_none(self): - with pytest.raises(TypeError): - assert boolean(None, strict=True) is False - assert boolean(None, strict=False) is False - - def test_numbers(self): - assert boolean(1) is True - assert boolean(0) is False - assert boolean(0.0) is False - -# Current boolean() doesn't consider these to be true values -# def test_other_numbers(self): -# assert boolean(2) is True -# assert boolean(-1) is True -# assert boolean(0.1) is True - - def test_strings(self): - assert boolean("true") is True - assert boolean("TRUE") is True - assert boolean("t") is True - assert boolean("yes") is True - assert boolean("y") is True - assert boolean("on") is True - - def test_junk_values_nonstrict(self): - assert boolean("flibbity", strict=False) is False - assert boolean(42, strict=False) is False - assert boolean(42.0, strict=False) is False - assert boolean(object(), strict=False) is False - - def test_junk_values_strict(self): - with pytest.raises(TypeError): - assert boolean("flibbity", strict=True) is False - - with pytest.raises(TypeError): - assert boolean(42, strict=True) is False - - with pytest.raises(TypeError): - assert boolean(42.0, strict=True) is False - - with pytest.raises(TypeError): - assert boolean(object(), strict=True) is False +junk_values = ("flibbity", 42, 42.0, object(), None, 2, -1, 0.1) + + +@pytest.mark.parametrize(("value", "expected"), [ + (True, True), + (False, False), + (1, True), + (0, False), + (0.0, False), + ("true", True), + ("TRUE", True), + ("t", True), + ("yes", True), + ("y", True), + ("on", True), +]) +def test_boolean(value: object, expected: bool) -> None: + assert boolean(value) is expected + + +@pytest.mark.parametrize("value", junk_values) +def test_junk_values_non_strict(value: object) -> None: + assert boolean(value, strict=False) is False + + +@pytest.mark.parametrize("value", junk_values) +def test_junk_values_strict(value: object) -> None: + with pytest.raises(TypeError, match=f"^The value '{value}' is not"): + boolean(value, strict=True) diff --git a/test/units/plugins/cache/test_cache.py b/test/units/plugins/cache/test_cache.py index edd50486b68..61662ad58c2 100644 --- a/test/units/plugins/cache/test_cache.py +++ b/test/units/plugins/cache/test_cache.py @@ -71,7 +71,7 @@ class TestCachePluginAdjudicator(unittest.TestCase): def test_pop_without_default(self): with pytest.raises(KeyError): - assert self.cache.pop('foo') + self.cache.pop('foo') def test_pop(self): v = self.cache.pop('cache_key_2') diff --git a/test/units/utils/display/test_curses.py b/test/units/utils/display/test_curses.py index 6816b715ee3..e2474b5409f 100644 --- a/test/units/utils/display/test_curses.py +++ b/test/units/utils/display/test_curses.py @@ -60,8 +60,8 @@ def test_pause_missing_curses(mocker, monkeypatch): assert mod.HAS_CURSES is False - with pytest.raises(AttributeError): - assert mod.curses + with pytest.raises(AttributeError, match=r"^module 'ansible\.utils\.display' has no attribute"): + mod.curses # pylint: disable=pointless-statement assert mod.HAS_CURSES is False assert mod.MOVE_TO_BOL == b'\r'