From eb6c4493b205c5b709e1a2323eb9ffd2397e4e79 Mon Sep 17 00:00:00 2001 From: Simon K Date: Mon, 8 Nov 2021 14:31:14 +0000 Subject: [PATCH] Deprecation of `msg=` for both `pytest.skip()` and `pytest.fail()`. (#8950) * porting pytest.skip() to use reason=, adding tests * avoid adding **kwargs, it breaks other functionality, use optional msg= instead * deprecation of `pytest.fail(msg=...)` * fix bug with not capturing the returned reason value * pass reason= in acceptance async tests instead of msg= * finalising deprecations of `msg` in `pytest.skip()` and `pytest.fail()` * Update doc/en/deprecations.rst Co-authored-by: Bruno Oliveira * Update doc/en/deprecations.rst Co-authored-by: Bruno Oliveira * fix failing test after upstream merge * adding deprecation to `pytest.exit(msg=...)` * add docs for pytest.exit deprecations * finalising deprecation of msg for pytest.skip, pytest.exit and pytest.fail * hold a reference to the Scope instance to please mypy Co-authored-by: Bruno Oliveira --- changelog/8948.deprecation.rst | 5 ++ doc/en/deprecations.rst | 32 +++++++++++ doc/en/reference/reference.rst | 6 +-- src/_pytest/compat.py | 7 +-- src/_pytest/deprecated.py | 5 ++ src/_pytest/outcomes.py | 98 ++++++++++++++++++++++++++++++---- src/_pytest/python.py | 2 +- src/_pytest/scope.py | 4 +- testing/deprecated_test.py | 58 ++++++++++++++++++++ testing/test_main.py | 2 +- testing/test_skipping.py | 89 ++++++++++++++++++++++++++++++ 11 files changed, 288 insertions(+), 20 deletions(-) create mode 100644 changelog/8948.deprecation.rst diff --git a/changelog/8948.deprecation.rst b/changelog/8948.deprecation.rst new file mode 100644 index 000000000..a39a92bc7 --- /dev/null +++ b/changelog/8948.deprecation.rst @@ -0,0 +1,5 @@ +:func:`pytest.skip(msg=...) `, :func:`pytest.fail(msg=...) ` and :func:`pytest.exit(msg=...) ` +signatures now accept a ``reason`` argument instead of ``msg``. Using ``msg`` still works, but is deprecated and will be removed in a future release. + +This was changed for consistency with :func:`pytest.mark.skip ` and :func:`pytest.mark.xfail ` which both accept +``reason`` as an argument. diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index c47e9a703..67859d226 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -56,6 +56,38 @@ In order to support the transition from ``py.path.local`` to :mod:`pathlib`, the The accompanying ``py.path.local`` based paths have been deprecated: plugins which manually invoke those hooks should only pass the new ``pathlib.Path`` arguments, and users should change their hook implementations to use the new ``pathlib.Path`` arguments. +Passing ``msg=`` to ``pytest.skip``, ``pytest.fail`` or ``pytest.exit`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. deprecated:: 7.0 + +Passing the keyword argument ``msg`` to :func:`pytest.skip`, :func:`pytest.fail` or :func:`pytest.exit` +is now deprecated and ``reason`` should be used instead. This change is to bring consistency between these +functions and the``@pytest.mark.skip`` and ``@pytest.mark.xfail`` markers which already accept a ``reason`` argument. + +.. code-block:: python + + def test_fail_example(): + # old + pytest.fail(msg="foo") + # new + pytest.fail(reason="bar") + + + def test_skip_example(): + # old + pytest.skip(msg="foo") + # new + pytest.skip(reason="bar") + + + def test_exit_example(): + # old + pytest.exit(msg="foo") + # new + pytest.exit(reason="bar") + + Implementing the ``pytest_cmdline_preparse`` hook ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/doc/en/reference/reference.rst b/doc/en/reference/reference.rst index 5750f81a5..638bd9568 100644 --- a/doc/en/reference/reference.rst +++ b/doc/en/reference/reference.rst @@ -22,12 +22,12 @@ pytest.fail **Tutorial**: :ref:`skipping` -.. autofunction:: pytest.fail +.. autofunction:: pytest.fail(reason, [pytrace=True, msg=None]) pytest.skip ~~~~~~~~~~~ -.. autofunction:: pytest.skip(msg, [allow_module_level=False]) +.. autofunction:: pytest.skip(reason, [allow_module_level=False, msg=None]) .. _`pytest.importorskip ref`: @@ -44,7 +44,7 @@ pytest.xfail pytest.exit ~~~~~~~~~~~ -.. autofunction:: pytest.exit +.. autofunction:: pytest.exit(reason, [returncode=False, msg=None]) pytest.main ~~~~~~~~~~~ diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index 1cf3ded01..7703dee8c 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -20,9 +20,6 @@ from typing import Union import attr import py -from _pytest.outcomes import fail -from _pytest.outcomes import TEST_OUTCOME - if TYPE_CHECKING: from typing import NoReturn from typing_extensions import Final @@ -152,6 +149,8 @@ def getfuncargnames( try: parameters = signature(function).parameters except (ValueError, TypeError) as e: + from _pytest.outcomes import fail + fail( f"Could not determine arguments of {function!r}: {e}", pytrace=False, @@ -324,6 +323,8 @@ def safe_getattr(object: Any, name: str, default: Any) -> Any: are derived from BaseException instead of Exception (for more details check #2707). """ + from _pytest.outcomes import TEST_OUTCOME + try: return getattr(object, name, default) except TEST_OUTCOME: diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 452128d07..ad1dfcb80 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -114,6 +114,11 @@ WARNS_NONE_ARG = PytestDeprecationWarning( " Replace pytest.warns(None) by simply pytest.warns()." ) +KEYWORD_MSG_ARG = UnformattedWarning( + PytestDeprecationWarning, + "pytest.{func}(msg=...) is now deprecated, use pytest.{func}(reason=...) instead", +) + # You want to make some `__init__` or function "private". # # def my_private_function(some, args): diff --git a/src/_pytest/outcomes.py b/src/_pytest/outcomes.py index 00ffe2b45..25206fe0e 100644 --- a/src/_pytest/outcomes.py +++ b/src/_pytest/outcomes.py @@ -1,6 +1,7 @@ """Exception classes and constants handling test outcomes as well as functions creating them.""" import sys +import warnings from typing import Any from typing import Callable from typing import cast @@ -8,6 +9,8 @@ from typing import Optional from typing import Type from typing import TypeVar +from _pytest.deprecated import KEYWORD_MSG_ARG + TYPE_CHECKING = False # Avoid circular import through compat. if TYPE_CHECKING: @@ -110,28 +113,56 @@ def _with_exception(exception_type: _ET) -> Callable[[_F], _WithException[_F, _E @_with_exception(Exit) -def exit(msg: str, returncode: Optional[int] = None) -> "NoReturn": +def exit( + reason: str = "", returncode: Optional[int] = None, *, msg: Optional[str] = None +) -> "NoReturn": """Exit testing process. - :param str msg: Message to display upon exit. - :param int returncode: Return code to be used when exiting pytest. + :param reason: + The message to show as the reason for exiting pytest. reason has a default value + only because `msg` is deprecated. + + :param returncode: + Return code to be used when exiting pytest. + + :param msg: + Same as ``reason``, but deprecated. Will be removed in a future version, use ``reason`` instead. """ __tracebackhide__ = True - raise Exit(msg, returncode) + from _pytest.config import UsageError + + if reason and msg: + raise UsageError( + "cannot pass reason and msg to exit(), `msg` is deprecated, use `reason`." + ) + if not reason: + if msg is None: + raise UsageError("exit() requires a reason argument") + warnings.warn(KEYWORD_MSG_ARG.format(func="exit"), stacklevel=2) + reason = msg + raise Exit(reason, returncode) @_with_exception(Skipped) -def skip(msg: str = "", *, allow_module_level: bool = False) -> "NoReturn": +def skip( + reason: str = "", *, allow_module_level: bool = False, msg: Optional[str] = None +) -> "NoReturn": """Skip an executing test with the given message. This function should be called only during testing (setup, call or teardown) or during collection by using the ``allow_module_level`` flag. This function can be called in doctests as well. - :param bool allow_module_level: + :param reason: + The message to show the user as reason for the skip. + + :param allow_module_level: Allows this function to be called at module level, skipping the rest of the module. Defaults to False. + :param msg: + Same as ``reason``, but deprecated. Will be removed in a future version, use ``reason`` instead. + .. note:: It is better to use the :ref:`pytest.mark.skipif ref` marker when possible to declare a test to be skipped under certain conditions @@ -140,21 +171,66 @@ def skip(msg: str = "", *, allow_module_level: bool = False) -> "NoReturn": to skip a doctest statically. """ __tracebackhide__ = True - raise Skipped(msg=msg, allow_module_level=allow_module_level) + reason = _resolve_msg_to_reason("skip", reason, msg) + raise Skipped(msg=reason, allow_module_level=allow_module_level) @_with_exception(Failed) -def fail(msg: str = "", pytrace: bool = True) -> "NoReturn": +def fail( + reason: str = "", pytrace: bool = True, msg: Optional[str] = None +) -> "NoReturn": """Explicitly fail an executing test with the given message. - :param str msg: + :param reason: The message to show the user as reason for the failure. - :param bool pytrace: + + :param pytrace: If False, msg represents the full failure information and no python traceback will be reported. + + :param msg: + Same as ``reason``, but deprecated. Will be removed in a future version, use ``reason`` instead. """ __tracebackhide__ = True - raise Failed(msg=msg, pytrace=pytrace) + reason = _resolve_msg_to_reason("fail", reason, msg) + raise Failed(msg=reason, pytrace=pytrace) + + +def _resolve_msg_to_reason( + func_name: str, reason: str, msg: Optional[str] = None +) -> str: + """ + Handles converting the deprecated msg parameter if provided into + reason, raising a deprecation warning. This function will be removed + when the optional msg argument is removed from here in future. + + :param str func_name: + The name of the offending function, this is formatted into the deprecation message. + + :param str reason: + The reason= passed into either pytest.fail() or pytest.skip() + + :param str msg: + The msg= passed into either pytest.fail() or pytest.skip(). This will + be converted into reason if it is provided to allow pytest.skip(msg=) or + pytest.fail(msg=) to continue working in the interim period. + + :returns: + The value to use as reason. + + """ + __tracebackhide__ = True + if msg is not None: + + if reason: + from pytest import UsageError + + raise UsageError( + f"Passing both ``reason`` and ``msg`` to pytest.{func_name}(...) is not permitted." + ) + warnings.warn(KEYWORD_MSG_ARG.format(func=func_name), stacklevel=3) + reason = msg + return reason class XFailed(Failed): diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 184c22143..b5f5811b9 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -175,7 +175,7 @@ def async_warn_and_skip(nodeid: str) -> None: msg += " - pytest-trio\n" msg += " - pytest-twisted" warnings.warn(PytestUnhandledCoroutineWarning(msg.format(nodeid))) - skip(msg="async def function and no async plugin installed (see warnings)") + skip(reason="async def function and no async plugin installed (see warnings)") @hookimpl(trylast=True) diff --git a/src/_pytest/scope.py b/src/_pytest/scope.py index 4f37a0f3b..7a746fb9f 100644 --- a/src/_pytest/scope.py +++ b/src/_pytest/scope.py @@ -71,7 +71,8 @@ class Scope(Enum): from _pytest.outcomes import fail try: - return Scope(scope_name) + # Holding this reference is necessary for mypy at the moment. + scope = Scope(scope_name) except ValueError: fail( "{} {}got an unexpected scope value '{}'".format( @@ -79,6 +80,7 @@ class Scope(Enum): ), pytrace=False, ) + return scope _ALL_SCOPES = list(Scope) diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 0f5e483ce..5fa50e5cd 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -200,6 +200,64 @@ def test_warns_none_is_deprecated(): pass +class TestSkipMsgArgumentDeprecated: + def test_skip_with_msg_is_deprecated(self, pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest + + def test_skipping_msg(): + pytest.skip(msg="skippedmsg") + """ + ) + result = pytester.runpytest(p) + result.stdout.fnmatch_lines( + [ + "*PytestDeprecationWarning: pytest.skip(msg=...) is now deprecated, " + "use pytest.skip(reason=...) instead", + '*pytest.skip(msg="skippedmsg")*', + ] + ) + result.assert_outcomes(skipped=1, warnings=1) + + def test_fail_with_msg_is_deprecated(self, pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest + + def test_failing_msg(): + pytest.fail(msg="failedmsg") + """ + ) + result = pytester.runpytest(p) + result.stdout.fnmatch_lines( + [ + "*PytestDeprecationWarning: pytest.fail(msg=...) is now deprecated, " + "use pytest.fail(reason=...) instead", + '*pytest.fail(msg="failedmsg")', + ] + ) + result.assert_outcomes(failed=1, warnings=1) + + def test_exit_with_msg_is_deprecated(self, pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest + + def test_exit_msg(): + pytest.exit(msg="exitmsg") + """ + ) + result = pytester.runpytest(p) + result.stdout.fnmatch_lines( + [ + "*PytestDeprecationWarning: pytest.exit(msg=...) is now deprecated, " + "use pytest.exit(reason=...) instead", + ] + ) + result.assert_outcomes(warnings=1) + + def test_deprecation_of_cmdline_preparse(pytester: Pytester) -> None: pytester.makeconftest( """ diff --git a/testing/test_main.py b/testing/test_main.py index 445002934..6a13633f6 100644 --- a/testing/test_main.py +++ b/testing/test_main.py @@ -70,7 +70,7 @@ def test_wrap_session_exit_sessionfinish( """ import pytest def pytest_sessionfinish(): - pytest.exit(msg="exit_pytest_sessionfinish", returncode={returncode}) + pytest.exit(reason="exit_pytest_sessionfinish", returncode={returncode}) """.format( returncode=returncode ) diff --git a/testing/test_skipping.py b/testing/test_skipping.py index d50a16c90..308c91abe 100644 --- a/testing/test_skipping.py +++ b/testing/test_skipping.py @@ -1444,3 +1444,92 @@ def test_relpath_rootdir(pytester: Pytester) -> None: result.stdout.fnmatch_lines( ["SKIPPED [[]1[]] tests/test_1.py:2: unconditional skip"] ) + + +def test_skip_using_reason_works_ok(pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest + + def test_skipping_reason(): + pytest.skip(reason="skippedreason") + """ + ) + result = pytester.runpytest(p) + result.stdout.no_fnmatch_line("*PytestDeprecationWarning*") + result.assert_outcomes(skipped=1) + + +def test_fail_using_reason_works_ok(pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest + + def test_failing_reason(): + pytest.fail(reason="failedreason") + """ + ) + result = pytester.runpytest(p) + result.stdout.no_fnmatch_line("*PytestDeprecationWarning*") + result.assert_outcomes(failed=1) + + +def test_fail_fails_with_msg_and_reason(pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest + + def test_fail_both_arguments(): + pytest.fail(reason="foo", msg="bar") + """ + ) + result = pytester.runpytest(p) + result.stdout.fnmatch_lines( + "*UsageError: Passing both ``reason`` and ``msg`` to pytest.fail(...) is not permitted.*" + ) + result.assert_outcomes(failed=1) + + +def test_skip_fails_with_msg_and_reason(pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest + + def test_skip_both_arguments(): + pytest.skip(reason="foo", msg="bar") + """ + ) + result = pytester.runpytest(p) + result.stdout.fnmatch_lines( + "*UsageError: Passing both ``reason`` and ``msg`` to pytest.skip(...) is not permitted.*" + ) + result.assert_outcomes(failed=1) + + +def test_exit_with_msg_and_reason_fails(pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest + + def test_exit_both_arguments(): + pytest.exit(reason="foo", msg="bar") + """ + ) + result = pytester.runpytest(p) + result.stdout.fnmatch_lines( + "*UsageError: cannot pass reason and msg to exit(), `msg` is deprecated, use `reason`.*" + ) + result.assert_outcomes(failed=1) + + +def test_exit_with_reason_works_ok(pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest + + def test_exit_reason_only(): + pytest.exit(reason="foo") + """ + ) + result = pytester.runpytest(p) + result.stdout.fnmatch_lines("*_pytest.outcomes.Exit: foo*")