From 5b83417afcce36c42e5c6cd51649da55101c0d86 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 12 Dec 2018 19:12:44 -0200 Subject: [PATCH] Deprecate the 'message' parameter of pytest.raises Fix #3974 --- changelog/3974.deprecation.rst | 8 ++++ doc/en/deprecations.rst | 13 ++++++ src/_pytest/deprecated.py | 7 ++++ src/_pytest/python_api.py | 73 ++++++++++++++++++---------------- testing/deprecated_test.py | 6 +++ testing/python/raises.py | 5 ++- testing/test_pytester.py | 2 +- 7 files changed, 77 insertions(+), 37 deletions(-) create mode 100644 changelog/3974.deprecation.rst diff --git a/changelog/3974.deprecation.rst b/changelog/3974.deprecation.rst new file mode 100644 index 000000000..070ecb8b2 --- /dev/null +++ b/changelog/3974.deprecation.rst @@ -0,0 +1,8 @@ +Passing the ``message`` parameter of ``pytest.raises`` now issues a ``DeprecationWarning``. + +It is a common mistake to think this parameter will match the exception message, while in fact +it only serves to provide a custom message in case the ``pytest.raises`` check fails. To avoid this +mistake and because it is believed to be little used, pytest is deprecating it without providing +an alternative for the moment. + +If you have concerns about this, please comment on `issue #3974 `__. diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 3ee5ca0d4..a2f16d974 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -14,6 +14,19 @@ Below is a complete list of all pytest features which are considered deprecated. :class:`_pytest.warning_types.PytestWarning` or subclasses, which can be filtered using :ref:`standard warning filters `. +``"message"`` parameter of ``pytest.raises`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. deprecated:: 4.1 + +It is a common mistake to think this parameter will match the exception message, while in fact +it only serves to provide a custom message in case the ``pytest.raises`` check fails. To avoid this +mistake and because it is believed to be little used, pytest is deprecating it without providing +an alternative for the moment. + +If you have concerns about this, please comment on `issue #3974 `__. + + ``pytest.config`` global ~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 30173e6b1..426533a0c 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -51,6 +51,13 @@ GETFUNCARGVALUE = RemovedInPytest4Warning( "getfuncargvalue is deprecated, use getfixturevalue" ) +RAISES_MESSAGE_PARAMETER = PytestDeprecationWarning( + "The 'message' parameter is deprecated.\n" + "(did you mean to use `match='some regex'` to check the exception message?)\n" + "Please comment on https://github.com/pytest-dev/pytest/issues/3974 " + "if you have concerns about removal of this parameter." +) + RESULT_LOG = PytestDeprecationWarning( "--result-log is deprecated and scheduled for removal in pytest 5.0.\n" "See https://docs.pytest.org/en/latest/deprecations.html#result-log-result-log for more information." diff --git a/src/_pytest/python_api.py b/src/_pytest/python_api.py index 7e5dc74a8..33e88b409 100644 --- a/src/_pytest/python_api.py +++ b/src/_pytest/python_api.py @@ -13,11 +13,11 @@ from six.moves import filterfalse from six.moves import zip import _pytest._code +from _pytest import deprecated from _pytest.compat import isclass from _pytest.compat import Mapping from _pytest.compat import Sequence from _pytest.compat import STRING_TYPES -from _pytest.deprecated import RAISES_EXEC from _pytest.outcomes import fail BASE_TYPE = (type, STRING_TYPES) @@ -551,29 +551,47 @@ def _is_numpy_array(obj): def raises(expected_exception, *args, **kwargs): r""" Assert that a code block/function call raises ``expected_exception`` - and raise a failure exception otherwise. + or raise a failure exception otherwise. - :arg message: if specified, provides a custom failure message if the - exception is not raised - :arg match: if specified, asserts that the exception matches a text or regex + :kwparam match: if specified, asserts that the exception matches a text or regex - This helper produces a ``ExceptionInfo()`` object (see below). + :kwparam message: **(deprecated since 4.1)** if specified, provides a custom failure message + if the exception is not raised - You may use this function as a context manager:: + .. currentmodule:: _pytest._code + + Use ``pytest.raises`` as a context manager, which will capture the exception of the given + type:: >>> with raises(ZeroDivisionError): ... 1/0 - .. versionchanged:: 2.10 + If the code block does not raise the expected exception (``ZeroDivisionError`` in the example + above), or no exception at all, the check will fail instead. - In the context manager form you may use the keyword argument - ``message`` to specify a custom failure message:: + You can also use the keyword argument ``match`` to assert that the + exception matches a text or regex:: - >>> with raises(ZeroDivisionError, message="Expecting ZeroDivisionError"): - ... pass - Traceback (most recent call last): - ... - Failed: Expecting ZeroDivisionError + >>> with raises(ValueError, match='must be 0 or None'): + ... raise ValueError("value must be 0 or None") + + >>> with raises(ValueError, match=r'must be \d+$'): + ... raise ValueError("value must be 42") + + The context manager produces an :class:`ExceptionInfo` object which can be used to inspect the + details of the captured exception:: + + >>> with raises(ValueError) as exc_info: + ... raise ValueError("value must be 42") + >>> assert exc_info.type is ValueError + >>> assert exc_info.value.args[0] == "value must be 42" + + .. deprecated:: 4.1 + + In the context manager form you may use the keyword argument + ``message`` to specify a custom failure message that will be displayed + in case the ``pytest.raises`` check fails. This has been deprecated as it + is considered error prone as users often mean to use ``match`` instead. .. note:: @@ -587,7 +605,7 @@ def raises(expected_exception, *args, **kwargs): >>> with raises(ValueError) as exc_info: ... if value > 10: ... raise ValueError("value must be <= 10") - ... assert exc_info.type == ValueError # this will not execute + ... assert exc_info.type is ValueError # this will not execute Instead, the following approach must be taken (note the difference in scope):: @@ -596,23 +614,10 @@ def raises(expected_exception, *args, **kwargs): ... if value > 10: ... raise ValueError("value must be <= 10") ... - >>> assert exc_info.type == ValueError - - - Since version ``3.1`` you can use the keyword argument ``match`` to assert that the - exception matches a text or regex:: - - >>> with raises(ValueError, match='must be 0 or None'): - ... raise ValueError("value must be 0 or None") - - >>> with raises(ValueError, match=r'must be \d+$'): - ... raise ValueError("value must be 42") + >>> assert exc_info.type is ValueError **Legacy form** - The form below is fully supported but discouraged for new code because the - context manager form is regarded as more readable and less error-prone. - It is possible to specify a callable by passing a to-be-called lambda:: >>> raises(ZeroDivisionError, lambda: 1/0) @@ -627,9 +632,8 @@ def raises(expected_exception, *args, **kwargs): >>> raises(ZeroDivisionError, f, x=0) - .. currentmodule:: _pytest._code - - Consult the API of ``excinfo`` objects: :class:`ExceptionInfo`. + The form above is fully supported but discouraged for new code because the + context manager form is regarded as more readable and less error-prone. .. note:: Similar to caught exception objects in Python, explicitly clearing @@ -660,6 +664,7 @@ def raises(expected_exception, *args, **kwargs): if not args: if "message" in kwargs: message = kwargs.pop("message") + warnings.warn(deprecated.RAISES_MESSAGE_PARAMETER, stacklevel=2) if "match" in kwargs: match_expr = kwargs.pop("match") if kwargs: @@ -668,7 +673,7 @@ def raises(expected_exception, *args, **kwargs): raise TypeError(msg) return RaisesContext(expected_exception, message, match_expr) elif isinstance(args[0], str): - warnings.warn(RAISES_EXEC, stacklevel=2) + warnings.warn(deprecated.RAISES_EXEC, stacklevel=2) code, = args assert isinstance(code, str) frame = sys._getframe(1) diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 111ff629f..4353ec2be 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -136,6 +136,12 @@ def test_pytest_catchlog_deprecated(testdir, plugin): ) +def test_raises_message_argument_deprecated(): + with pytest.warns(pytest.PytestDeprecationWarning): + with pytest.raises(RuntimeError, message="foobar"): + raise RuntimeError + + def test_pytest_plugins_in_non_top_level_conftest_deprecated(testdir): from _pytest.deprecated import PYTEST_PLUGINS_FROM_NON_TOP_LEVEL_CONFTEST diff --git a/testing/python/raises.py b/testing/python/raises.py index 52ad6cfa6..aad60d775 100644 --- a/testing/python/raises.py +++ b/testing/python/raises.py @@ -121,8 +121,9 @@ class TestRaises(object): def test_custom_raise_message(self): message = "TEST_MESSAGE" try: - with pytest.raises(ValueError, message=message): - pass + with pytest.warns(PytestDeprecationWarning): + with pytest.raises(ValueError, message=message): + pass except pytest.raises.Exception as e: assert e.msg == message else: diff --git a/testing/test_pytester.py b/testing/test_pytester.py index 669da6e17..0b66acbf2 100644 --- a/testing/test_pytester.py +++ b/testing/test_pytester.py @@ -280,7 +280,7 @@ def test_assert_outcomes_after_pytest_error(testdir): testdir.makepyfile("def test_foo(): assert True") result = testdir.runpytest("--unexpected-argument") - with pytest.raises(ValueError, message="Pytest terminal report not found"): + with pytest.raises(ValueError, match="Pytest terminal report not found"): result.assert_outcomes(passed=0)