From fba008cf29ebf03049f1116f7296b100c50c5feb Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sun, 17 Sep 2023 09:54:15 +0330 Subject: [PATCH] Apply review comments --- doc/en/example/simple.rst | 4 +- src/_pytest/deprecated.py | 8 ++++ src/_pytest/python.py | 14 +++--- testing/deprecated_test.py | 90 ++++++++++++++++++++++++++++++++++++++ testing/python/fixtures.py | 6 +-- 5 files changed, 106 insertions(+), 16 deletions(-) diff --git a/doc/en/example/simple.rst b/doc/en/example/simple.rst index 64420874e..c908cbe3a 100644 --- a/doc/en/example/simple.rst +++ b/doc/en/example/simple.rst @@ -818,8 +818,8 @@ case we just write some information out to a ``failures`` file: mode = "a" if os.path.exists("failures") else "w" with open("failures", mode, encoding="utf-8") as f: # let's also access a fixture for the fun of it - if "tmp_path" in item.fixturenames: - extra = " ({})".format(item._request.getfixturevalue("tmp_path")) + if "tmp_path" in item.funcargs: + extra = " ({})".format(item.funcargs["tmp_path"]) else: extra = "" diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 1bc2cf57e..c513675a7 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -11,6 +11,7 @@ in case of warnings which need to format their messages. from warnings import warn from _pytest.warning_types import PytestDeprecationWarning +from _pytest.warning_types import PytestRemovedIn8Warning from _pytest.warning_types import PytestRemovedIn9Warning from _pytest.warning_types import UnformattedWarning @@ -48,6 +49,13 @@ MARKED_FIXTURE = PytestRemovedIn9Warning( "See docs: https://docs.pytest.org/en/stable/deprecations.html#applying-a-mark-to-a-fixture-function" ) +ITEM_FUNCARGS_MEMBERS = PytestRemovedIn9Warning( + "Access to names other than initialnames i.e., direct args," + " the ones with `usefixture` or the ones with `autouse` through " + "`item.funcargs` is deprecated and will raise `KeyError` from " + "pytest 9. Please use `request.getfixturevalue` instead." +) + # You want to make some `__init__` or function "private". # # def my_private_function(some, args): diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 2a7cd4979..52f4a8881 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -15,6 +15,7 @@ from pathlib import Path from typing import Any from typing import Callable from typing import Dict +from typing import Final from typing import final from typing import Generator from typing import Iterable @@ -55,6 +56,7 @@ from _pytest.config import ExitCode from _pytest.config import hookimpl from _pytest.config.argparsing import Parser from _pytest.deprecated import check_ispytest +from _pytest.deprecated import ITEM_FUNCARGS_MEMBERS from _pytest.fixtures import FixtureDef from _pytest.fixtures import FixtureRequest from _pytest.fixtures import FuncFixtureInfo @@ -1660,19 +1662,13 @@ def write_docstring(tw: TerminalWriter, doc: str, indent: str = " ") -> None: class DeprecatingFuncArgs(Dict[str, object]): - def __init__(self, initialnames): - self.initialnames = initialnames + def __init__(self, initialnames: Sequence[str]) -> None: super().__init__() + self.initialnames: Final = initialnames def __getitem__(self, key: str) -> object: if key not in self.initialnames: - warnings.warn( - "Accessing to names other than initialnames i.e., direct args," - " the ones with `usefixture` or the ones with `autouse` through " - "`item.funcargs` is deprecated and will raise `KeyError` from " - "pytest 9. Please use `request.getfixturevalue` instead.", - DeprecationWarning, - ) + warnings.warn(ITEM_FUNCARGS_MEMBERS) return super().__getitem__(key) diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index ebff49ce6..36b878df9 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -133,3 +133,93 @@ def test_fixture_disallowed_between_marks(): raise NotImplementedError() assert len(record) == 2 # one for each mark decorator + + +@pytest.mark.filterwarnings("default") +def test_nose_deprecated_with_setup(pytester: Pytester) -> None: + pytest.importorskip("nose") + pytester.makepyfile( + """ + from nose.tools import with_setup + + def setup_fn_no_op(): + ... + + def teardown_fn_no_op(): + ... + + @with_setup(setup_fn_no_op, teardown_fn_no_op) + def test_omits_warnings(): + ... + """ + ) + output = pytester.runpytest("-Wdefault::pytest.PytestRemovedIn8Warning") + message = [ + "*PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.", + "*test_nose_deprecated_with_setup.py::test_omits_warnings is using nose method: `setup_fn_no_op` (setup)", + "*PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.", + "*test_nose_deprecated_with_setup.py::test_omits_warnings is using nose method: `teardown_fn_no_op` (teardown)", + ] + output.stdout.fnmatch_lines(message) + output.assert_outcomes(passed=1) + + +@pytest.mark.filterwarnings("default") +def test_nose_deprecated_setup_teardown(pytester: Pytester) -> None: + pytest.importorskip("nose") + pytester.makepyfile( + """ + class Test: + + def setup(self): + ... + + def teardown(self): + ... + + def test(self): + ... + """ + ) + output = pytester.runpytest("-Wdefault::pytest.PytestRemovedIn8Warning") + message = [ + "*PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.", + "*test_nose_deprecated_setup_teardown.py::Test::test is using nose-specific method: `setup(self)`", + "*To remove this warning, rename it to `setup_method(self)`", + "*PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.", + "*test_nose_deprecated_setup_teardown.py::Test::test is using nose-specific method: `teardown(self)`", + "*To remove this warning, rename it to `teardown_method(self)`", + ] + output.stdout.fnmatch_lines(message) + output.assert_outcomes(passed=1) + + +def test_deprecated_access_to_item_funcargs(pytester: Pytester) -> None: + module = pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def fixture1(): + return None + + @pytest.fixture + def fixture2(fixture1): + return None + + def test(fixture2): + pass + """ + ) + test = pytester.genitems((pytester.getmodulecol(module),))[0] + assert isinstance(test, pytest.Function) + test.session._setupstate.setup(test) + test.setup() + with pytest.warns( + pytest.PytestRemovedIn9Warning, + match=r"Access to names other than initialnames", + ) as record: + test.funcargs["fixture1"] + assert len(record) == 1 + test.funcargs["fixture2"] + assert len(record) == 1 diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 199636ef7..81aa2bcc7 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -122,20 +122,16 @@ class TestFillFixtures: ["*recursive dependency involving fixture 'fix1' detected*"] ) - def test_funcarg_basic(self, recwarn, pytester: Pytester) -> None: + def test_funcarg_basic(self, pytester: Pytester) -> None: pytester.copy_example() item = pytester.getitem(Path("test_funcarg_basic.py")) assert isinstance(item, Function) # Execute's item's setup, which fills fixtures. item.session._setupstate.setup(item) - assert len(recwarn) == 0 - item.funcargs["request"] - assert len(recwarn) == 1 and recwarn[0].category is DeprecationWarning del item.funcargs["request"] assert len(get_public_names(item.funcargs)) == 2 assert item.funcargs["some"] == "test_func" assert item.funcargs["other"] == 42 - assert len(recwarn) == 1 def test_funcarg_lookup_modulelevel(self, pytester: Pytester) -> None: pytester.copy_example()