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 <[email protected]>

* Update doc/en/deprecations.rst

Co-authored-by: Bruno Oliveira <[email protected]>

* 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 <[email protected]>

Author: symonk

changelog/8948.deprecation.rst

@@ -0,0 +1,5 @@
+:func:`pytest.skip(msg=...) <pytest.skip>`, :func:`pytest.fail(msg=...) <pytest.fail>` and :func:`pytest.exit(msg=...) <pytest.exit>`
+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 <pytest.mark.skip>` and  :func:`pytest.mark.xfail <pytest.mark.xfail>` which both accept
+``reason`` as an argument.

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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

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
 ~~~~~~~~~~~

src/_pytest/compat.py

@@ -20,9 +20,6 @@
 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:

src/_pytest/deprecated.py

@@ -114,6 +114,11 @@
     " 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):

src/_pytest/outcomes.py

@@ -1,13 +1,16 @@
 """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
 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 decorate(func: _F) -> _WithException[_F, _ET]:
 
 
 @_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):

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)

src/_pytest/scope.py

@@ -71,14 +71,16 @@ def from_user(
         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(
                     descr, f"from {where} " if where else "", scope_name
                 ),
                 pytrace=False,
             )
+        return scope
 
 
 _ALL_SCOPES = list(Scope)

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(
         """

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
         )