From 137255816ea18b0ce8ea8f18c19fb8232d3f77a2 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Tue, 6 Aug 2019 15:02:46 +0100 Subject: [PATCH] Fix collection of staticmethods defined with functools.partial Related to #5701 --- changelog/5701.bugfix.rst | 1 + src/_pytest/compat.py | 12 ++++++---- src/_pytest/fixtures.py | 4 ++-- testing/python/collect.py | 46 -------------------------------------- testing/python/fixtures.py | 46 +++++++++++++++++++++++++++++++++++--- testing/test_compat.py | 11 +++++++++ 6 files changed, 65 insertions(+), 55 deletions(-) create mode 100644 changelog/5701.bugfix.rst diff --git a/changelog/5701.bugfix.rst b/changelog/5701.bugfix.rst new file mode 100644 index 000000000..b654e7447 --- /dev/null +++ b/changelog/5701.bugfix.rst @@ -0,0 +1 @@ +Fix collection of ``staticmethod`` objects defined with ``functools.partial``. diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index 52ffc36bc..97c06e3ff 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -78,7 +78,7 @@ def num_mock_patch_args(function): ) -def getfuncargnames(function, is_method=False, cls=None): +def getfuncargnames(function, *, name: str = "", is_method=False, cls=None): """Returns the names of a function's mandatory arguments. This should return the names of all function arguments that: @@ -91,11 +91,12 @@ def getfuncargnames(function, is_method=False, cls=None): be treated as a bound method even though it's not unless, only in the case of cls, the function is a static method. + The name parameter should be the original name in which the function was collected. + @RonnyPfannschmidt: This function should be refactored when we revisit fixtures. The fixture mechanism should ask the node for the fixture names, and not try to obtain directly from the function object well after collection has occurred. - """ # The parameters attribute of a Signature object contains an # ordered mapping of parameter names to Parameter instances. This @@ -118,11 +119,14 @@ def getfuncargnames(function, is_method=False, cls=None): ) and p.default is Parameter.empty ) + if not name: + name = function.__name__ + # If this function should be treated as a bound method even though # it's passed as an unbound method or function, remove the first # parameter name. if is_method or ( - cls and not isinstance(cls.__dict__.get(function.__name__, None), staticmethod) + cls and not isinstance(cls.__dict__.get(name, None), staticmethod) ): arg_names = arg_names[1:] # Remove any names that will be replaced with mocks. @@ -245,7 +249,7 @@ def get_real_method(obj, holder): try: is_method = hasattr(obj, "__func__") obj = get_real_func(obj) - except Exception: + except Exception: # pragma: no cover return obj if is_method and hasattr(obj, "__get__") and callable(obj.__get__): obj = obj.__get__(holder) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 965a2e6e9..6a3e82907 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -828,7 +828,7 @@ class FixtureDef: where=baseid, ) self.params = params - self.argnames = getfuncargnames(func, is_method=unittest) + self.argnames = getfuncargnames(func, name=argname, is_method=unittest) self.unittest = unittest self.ids = ids self._finalizers = [] @@ -1143,7 +1143,7 @@ class FixtureManager: def getfixtureinfo(self, node, func, cls, funcargs=True): if funcargs and not getattr(node, "nofuncargs", False): - argnames = getfuncargnames(func, cls=cls) + argnames = getfuncargnames(func, name=node.name, cls=cls) else: argnames = () diff --git a/testing/python/collect.py b/testing/python/collect.py index 61fc58579..e6dd3e870 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -1143,52 +1143,6 @@ def test_unorderable_types(testdir): assert result.ret == ExitCode.NO_TESTS_COLLECTED -def test_collect_functools_partial(testdir): - """ - Test that collection of functools.partial object works, and arguments - to the wrapped functions are dealt with correctly (see #811). - """ - testdir.makepyfile( - """ - import functools - import pytest - - @pytest.fixture - def fix1(): - return 'fix1' - - @pytest.fixture - def fix2(): - return 'fix2' - - def check1(i, fix1): - assert i == 2 - assert fix1 == 'fix1' - - def check2(fix1, i): - assert i == 2 - assert fix1 == 'fix1' - - def check3(fix1, i, fix2): - assert i == 2 - assert fix1 == 'fix1' - assert fix2 == 'fix2' - - test_ok_1 = functools.partial(check1, i=2) - test_ok_2 = functools.partial(check1, i=2, fix1='fix1') - test_ok_3 = functools.partial(check1, 2) - test_ok_4 = functools.partial(check2, i=2) - test_ok_5 = functools.partial(check3, i=2) - test_ok_6 = functools.partial(check3, i=2, fix1='fix1') - - test_fail_1 = functools.partial(check2, 2) - test_fail_2 = functools.partial(check3, 2) - """ - ) - result = testdir.inline_run() - result.assertoutcome(passed=6, failed=2) - - @pytest.mark.filterwarnings("default") def test_dont_collect_non_function_callable(testdir): """Test for issue https://github.com/pytest-dev/pytest/issues/331 diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index c0c230ccf..a85a0d731 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -10,7 +10,9 @@ from _pytest.pytester import get_public_names from _pytest.warnings import SHOW_PYTEST_WARNINGS_ARG -def test_getfuncargnames(): +def test_getfuncargnames_functions(): + """Test getfuncargnames for normal functions""" + def f(): pass @@ -31,18 +33,56 @@ def test_getfuncargnames(): assert fixtures.getfuncargnames(j) == ("arg1", "arg2") + +def test_getfuncargnames_methods(): + """Test getfuncargnames for normal methods""" + class A: def f(self, arg1, arg2="hello"): pass + assert fixtures.getfuncargnames(A().f) == ("arg1",) + + +def test_getfuncargnames_staticmethod(): + """Test getfuncargnames for staticmethods""" + + class A: @staticmethod - def static(arg1, arg2): + def static(arg1, arg2, x=1): pass - assert fixtures.getfuncargnames(A().f) == ("arg1",) assert fixtures.getfuncargnames(A.static, cls=A) == ("arg1", "arg2") +def test_getfuncargnames_partial(): + """Check getfuncargnames for methods defined with functools.partial (#5701)""" + import functools + + def check(arg1, arg2, i): + pass + + class T: + test_ok = functools.partial(check, i=2) + + values = fixtures.getfuncargnames(T().test_ok, name="test_ok") + assert values == ("arg1", "arg2") + + +def test_getfuncargnames_staticmethod_partial(): + """Check getfuncargnames for staticmethods defined with functools.partial (#5701)""" + import functools + + def check(arg1, arg2, i): + pass + + class T: + test_ok = staticmethod(functools.partial(check, i=2)) + + values = fixtures.getfuncargnames(T().test_ok, name="test_ok") + assert values == ("arg1", "arg2") + + @pytest.mark.pytester_example_path("fixtures/fill_fixtures") class TestFillFixtures: def test_fillfuncargs_exposed(self): diff --git a/testing/test_compat.py b/testing/test_compat.py index 9e7d05c5d..fb2470d07 100644 --- a/testing/test_compat.py +++ b/testing/test_compat.py @@ -1,4 +1,5 @@ import sys +from functools import partial from functools import wraps import pytest @@ -72,6 +73,16 @@ def test_get_real_func(): assert get_real_func(wrapped_func2) is wrapped_func +def test_get_real_func_partial(): + """Test get_real_func handles partial instances correctly""" + + def foo(x): + return x + + assert get_real_func(foo) is foo + assert get_real_func(partial(foo)) is foo + + def test_is_generator_asyncio(testdir): testdir.makepyfile( """