From 3da28067f306582a10b798ba527356d62c1f4f86 Mon Sep 17 00:00:00 2001 From: Ceridwen Date: Thu, 19 Oct 2017 16:01:26 -0700 Subject: [PATCH 1/2] Replace introspection in compat.getfuncargnames() with inspect/funcsigs.signature --- AUTHORS | 1 + _pytest/compat.py | 95 +++++++++++++++++++++------------------ _pytest/fixtures.py | 3 +- changelog/2267.feature | 4 ++ setup.py | 5 ++- testing/python/fixture.py | 5 +-- 6 files changed, 63 insertions(+), 50 deletions(-) create mode 100644 changelog/2267.feature diff --git a/AUTHORS b/AUTHORS index f1769116d..44d11ed32 100644 --- a/AUTHORS +++ b/AUTHORS @@ -30,6 +30,7 @@ Brianna Laugher Bruno Oliveira Cal Leeming Carl Friedrich Bolz +Ceridwen Charles Cloud Charnjit SiNGH (CCSJ) Chris Lamb diff --git a/_pytest/compat.py b/_pytest/compat.py index 99ec54c53..8499e8882 100644 --- a/_pytest/compat.py +++ b/_pytest/compat.py @@ -2,11 +2,12 @@ python version compatibility code """ from __future__ import absolute_import, division, print_function -import sys + +import codecs +import functools import inspect import re -import functools -import codecs +import sys import py @@ -25,6 +26,12 @@ _PY3 = sys.version_info > (3, 0) _PY2 = not _PY3 +if _PY3: + from inspect import signature, Parameter as Parameter +else: + from funcsigs import signature, Parameter as Parameter + + NoneType = type(None) NOTSET = object() @@ -32,12 +39,10 @@ PY35 = sys.version_info[:2] >= (3, 5) PY36 = sys.version_info[:2] >= (3, 6) MODULE_NOT_FOUND_ERROR = 'ModuleNotFoundError' if PY36 else 'ImportError' -if hasattr(inspect, 'signature'): - def _format_args(func): - return str(inspect.signature(func)) -else: - def _format_args(func): - return inspect.formatargspec(*inspect.getargspec(func)) + +def _format_args(func): + return str(signature(func)) + isfunction = inspect.isfunction isclass = inspect.isclass @@ -63,7 +68,6 @@ def iscoroutinefunction(func): def getlocation(function, curdir): - import inspect fn = py.path.local(inspect.getfile(function)) lineno = py.builtin._getcode(function).co_firstlineno if fn.relto(curdir): @@ -83,40 +87,45 @@ def num_mock_patch_args(function): return len(patchings) -def getfuncargnames(function, startindex=None, cls=None): +def getfuncargnames(function, is_method=False, cls=None): + """Returns the names of a function's mandatory arguments. + + This should return the names of all function arguments that: + * Aren't bound to an instance or type as in instance or class methods. + * Don't have default values. + * Aren't bound with functools.partial. + * Aren't replaced with mocks. + + The is_method and cls arguments indicate that the function should + be treated as a bound method even though it's not unless, only in + the case of cls, the function is a static method. + + @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. + """ - @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. - """ - if startindex is None and cls is not None: - is_staticmethod = isinstance(cls.__dict__.get(function.__name__, None), staticmethod) - startindex = 0 if is_staticmethod else 1 - # XXX merge with main.py's varnames - # assert not isclass(function) - realfunction = function - while hasattr(realfunction, "__wrapped__"): - realfunction = realfunction.__wrapped__ - if startindex is None: - startindex = inspect.ismethod(function) and 1 or 0 - if realfunction != function: - startindex += num_mock_patch_args(function) - function = realfunction - if isinstance(function, functools.partial): - argnames = inspect.getargs(_pytest._code.getrawcode(function.func))[0] - partial = function - argnames = argnames[len(partial.args):] - if partial.keywords: - for kw in partial.keywords: - argnames.remove(kw) - else: - argnames = inspect.getargs(_pytest._code.getrawcode(function))[0] - defaults = getattr(function, 'func_defaults', - getattr(function, '__defaults__', None)) or () - numdefaults = len(defaults) - if numdefaults: - return tuple(argnames[startindex:-numdefaults]) - return tuple(argnames[startindex:]) + # The parameters attribute of a Signature object contains an + # ordered mapping of parameter names to Parameter instances. This + # creates a tuple of the names of the parameters that don't have + # defaults. + arg_names = tuple( + p.name for p in signature(function).parameters.values() + if (p.kind is Parameter.POSITIONAL_OR_KEYWORD + or p.kind is Parameter.KEYWORD_ONLY) and + p.default is Parameter.empty) + # 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))): + arg_names = arg_names[1:] + # Remove any names that will be replaced with mocks. + if hasattr(function, "__wrapped__"): + arg_names = arg_names[num_mock_patch_args(function):] + return arg_names if _PY3: diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index af993f3f9..5ac93b1a9 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -728,8 +728,7 @@ class FixtureDef: where=baseid ) self.params = params - startindex = unittest and 1 or None - self.argnames = getfuncargnames(func, startindex=startindex) + self.argnames = getfuncargnames(func, is_method=unittest) self.unittest = unittest self.ids = ids self._finalizer = [] diff --git a/changelog/2267.feature b/changelog/2267.feature new file mode 100644 index 000000000..a2f14811e --- /dev/null +++ b/changelog/2267.feature @@ -0,0 +1,4 @@ +Replace the old introspection code in compat.py that determines the +available arguments of fixtures with inspect.signature on Python 3 and +funcsigs.signature on Python 2. This should respect __signature__ +declarations on functions. diff --git a/setup.py b/setup.py index 68b8ec065..61ae1587f 100644 --- a/setup.py +++ b/setup.py @@ -44,16 +44,19 @@ def has_environment_marker_support(): def main(): install_requires = ['py>=1.4.34', 'six>=1.10.0', 'setuptools'] + extras_require = {} # if _PYTEST_SETUP_SKIP_PLUGGY_DEP is set, skip installing pluggy; # used by tox.ini to test with pluggy master if '_PYTEST_SETUP_SKIP_PLUGGY_DEP' not in os.environ: install_requires.append('pluggy>=0.4.0,<0.5') - extras_require = {} if has_environment_marker_support(): + extras_require[':python_version<"3.0"'] = ['funcsigs'] extras_require[':sys_platform=="win32"'] = ['colorama'] else: if sys.platform == 'win32': install_requires.append('colorama') + if sys.version_info < (3, 0): + install_requires.append('funcsigs') setup( name='pytest', diff --git a/testing/python/fixture.py b/testing/python/fixture.py index 06b08d68e..fa5da3284 100644 --- a/testing/python/fixture.py +++ b/testing/python/fixture.py @@ -34,9 +34,6 @@ def test_getfuncargnames(): pass assert fixtures.getfuncargnames(A().f) == ('arg1',) - if sys.version_info < (3, 0): - assert fixtures.getfuncargnames(A.f) == ('arg1',) - assert fixtures.getfuncargnames(A.static, cls=A) == ('arg1', 'arg2') @@ -2826,7 +2823,7 @@ class TestShowFixtures(object): import pytest class TestClass: @pytest.fixture - def fixture1(): + def fixture1(self): """line1 line2 indented line From f7387e45ea0b163ba4944801fdb70c521d7b8cc8 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 19 Oct 2017 21:50:15 -0200 Subject: [PATCH 2/2] Fix linting --- testing/python/fixture.py | 1 - 1 file changed, 1 deletion(-) diff --git a/testing/python/fixture.py b/testing/python/fixture.py index fa5da3284..184a80374 100644 --- a/testing/python/fixture.py +++ b/testing/python/fixture.py @@ -2,7 +2,6 @@ from textwrap import dedent import _pytest._code import pytest -import sys from _pytest.pytester import get_public_names from _pytest.fixtures import FixtureLookupError from _pytest import fixtures