Merge pull request #531 from pytest-dev/remove-clutter

Remove clutter from _step_decorator

Author: youtux

pytest_bdd/parsers.py

@@ -3,7 +3,7 @@
 
 import abc
 import re as base_re
-from typing import Any, Dict, cast
+from typing import Any, Dict, TypeVar, cast
 
 import parse as base_parse
 from parse_type import cfparse as base_cfparse
@@ -99,7 +99,10 @@ def is_matching(self, name: str) -> bool:
         return self.name == name
 
 
-def get_parser(step_name: Any) -> StepParser:
+TStepParser = TypeVar("TStepParser", bound=StepParser)
+
+
+def get_parser(step_name: str | TStepParser) -> TStepParser:
     """Get parser by given name."""
 
     if isinstance(step_name, StepParser):

pytest_bdd/scenario.py

@@ -36,30 +36,25 @@
 ALPHA_REGEX = re.compile(r"^\d+_*")
 
 
-def find_argumented_step_fixture_name(
-    name: str, type_: str, fixturemanager: FixtureManager, request: FixtureRequest | None = None
-) -> str | None:
+def find_argumented_step_fixture_name(name: str, type_: str, fixturemanager: FixtureManager) -> str | None:
     """Find argumented step fixture name."""
     # happens to be that _arg2fixturedefs is changed during the iteration so we use a copy
     for fixturename, fixturedefs in list(fixturemanager._arg2fixturedefs.items()):
         for fixturedef in fixturedefs:
-            parsers = getattr(fixturedef.func, "_pytest_bdd_parsers", [])
-            for parser in parsers:
-                match = parser.is_matching(name)
-                if not match:
-                    continue
-
-                parser_name = get_step_fixture_name(parser.name, type_)
-                if request:
-                    try:
-                        request.getfixturevalue(parser_name)
-                    except FixtureLookupError:
-                        continue
-                return parser_name
+            parser = getattr(fixturedef.func, "_pytest_bdd_parser", None)
+            if parser is None:
+                continue
+
+            match = parser.is_matching(name)
+            if not match:
+                continue
+
+            parser_name = get_step_fixture_name(parser.name, type_)
+            return parser_name
     return None
 
 
-def _find_step_function(request: FixtureRequest, step: Step, scenario: Scenario) -> Any:
+def _find_step_function(request: FixtureRequest, step: Step, scenario: Scenario) -> Callable[..., Any]:
     """Match the step defined by the regular expression pattern.
 
     :param request: PyTest request object.
@@ -76,7 +71,7 @@ def _find_step_function(request: FixtureRequest, step: Step, scenario: Scenario)
     except FixtureLookupError as e:
         try:
             # Could not find a fixture with the same name, let's see if there is a parser involved
-            argumented_name = find_argumented_step_fixture_name(name, step.type, request._fixturemanager, request)
+            argumented_name = find_argumented_step_fixture_name(name, step.type, request._fixturemanager)
             if argumented_name:
                 return request.getfixturevalue(argumented_name)
             raise e
@@ -87,7 +82,9 @@ def _find_step_function(request: FixtureRequest, step: Step, scenario: Scenario)
             ) from e2
 
 
-def _execute_step_function(request: FixtureRequest, scenario: Scenario, step: Step, step_func: Callable) -> None:
+def _execute_step_function(
+    request: FixtureRequest, scenario: Scenario, step: Step, step_func: Callable[..., Any]
+) -> None:
     """Execute step function.
 
     :param request: PyTest request.
@@ -96,19 +93,16 @@ def _execute_step_function(request: FixtureRequest, scenario: Scenario, step: St
     :param function step_func: Step function.
     :param example: Example table.
     """
-    kw = dict(request=request, feature=scenario.feature, scenario=scenario, step=step, step_func=step_func)
+    kw = {"request": request, "feature": scenario.feature, "scenario": scenario, "step": step, "step_func": step_func}
 
     request.config.hook.pytest_bdd_before_step(**kw)
-
     kw["step_func_args"] = {}
     try:
         # Get the step argument values.
-        converters = getattr(step_func, "converters", {})
+        converters = step_func._pytest_bdd_converters
         kwargs = {}
 
-        parsers = getattr(step_func, "_pytest_bdd_parsers", [])
-
-        for parser in parsers:
+        for parser in step_func._pytest_bdd_parsers:
             if not parser.is_matching(step.name):
                 continue
             for arg, value in parser.parse_arguments(step.name).items():
@@ -117,11 +111,12 @@ def _execute_step_function(request: FixtureRequest, scenario: Scenario, step: St
                 kwargs[arg] = value
             break
 
-        kwargs = {arg: kwargs[arg] if arg in kwargs else request.getfixturevalue(arg) for arg in get_args(step_func)}
+        args = get_args(step_func)
+        kwargs = {arg: kwargs[arg] if arg in kwargs else request.getfixturevalue(arg) for arg in args}
         kw["step_func_args"] = kwargs
 
         request.config.hook.pytest_bdd_before_step_call(**kw)
-        target_fixture = getattr(step_func, "target_fixture", None)
+        target_fixture = step_func._pytest_bdd_target_fixture
 
         # Execute the step as if it was a pytest fixture, so that we can allow "yield" statements in it
         return_value = call_fixture_func(fixturefunc=step_func, request=request, kwargs=kwargs)

pytest_bdd/steps.py

@@ -36,17 +36,16 @@ def given_beautiful_article(article):
 """
 from __future__ import annotations
 
-import typing
+from typing import Any, Callable, TypeVar
 
 import pytest
 from _pytest.fixtures import FixtureDef, FixtureRequest
 
-from .parsers import get_parser
+from .parsers import StepParser, get_parser
 from .types import GIVEN, THEN, WHEN
 from .utils import get_caller_module_locals, setdefault
 
-if typing.TYPE_CHECKING:
-    from typing import Any, Callable
+TCallable = TypeVar("TCallable", bound=Callable[..., Any])
 
 
 def get_step_fixture_name(name: str, type_: str) -> str:
@@ -61,7 +60,7 @@ def get_step_fixture_name(name: str, type_: str) -> str:
 
 
 def given(
-    name: Any,
+    name: str | StepParser,
     converters: dict[str, Callable] | None = None,
     target_fixture: str | None = None,
 ) -> Callable:
@@ -77,7 +76,9 @@ def given(
     return _step_decorator(GIVEN, name, converters=converters, target_fixture=target_fixture)
 
 
-def when(name: Any, converters: dict[str, Callable] | None = None, target_fixture: str | None = None) -> Callable:
+def when(
+    name: str | StepParser, converters: dict[str, Callable] | None = None, target_fixture: str | None = None
+) -> Callable:
     """When step decorator.
 
     :param name: Step name or a parser object.
@@ -90,7 +91,9 @@ def when(name: Any, converters: dict[str, Callable] | None = None, target_fixtur
     return _step_decorator(WHEN, name, converters=converters, target_fixture=target_fixture)
 
 
-def then(name: Any, converters: dict[str, Callable] | None = None, target_fixture: str | None = None) -> Callable:
+def then(
+    name: str | StepParser, converters: dict[str, Callable] | None = None, target_fixture: str | None = None
+) -> Callable:
     """Then step decorator.
 
     :param name: Step name or a parser object.
@@ -105,7 +108,7 @@ def then(name: Any, converters: dict[str, Callable] | None = None, target_fixtur
 
 def _step_decorator(
     step_type: str,
-    step_name: Any,
+    step_name: str | StepParser,
     converters: dict[str, Callable] | None = None,
     target_fixture: str | None = None,
 ) -> Callable:
@@ -118,38 +121,26 @@ def _step_decorator(
 
     :return: Decorator function for the step.
     """
+    if converters is None:
+        converters = {}
 
-    def decorator(func: Callable) -> Callable:
-        step_func = func
+    def decorator(func: TCallable) -> TCallable:
         parser_instance = get_parser(step_name)
         parsed_step_name = parser_instance.name
 
-        # TODO: Try to not attach to both step_func and lazy_step_func
-
-        step_func.__name__ = str(parsed_step_name)
-
-        def lazy_step_func() -> Callable:
-            return step_func
-
-        step_func.step_type = step_type
-        lazy_step_func.step_type = step_type
-
-        # Preserve the docstring
-        lazy_step_func.__doc__ = func.__doc__
-
-        setdefault(step_func, "_pytest_bdd_parsers", []).append(parser_instance)
-        setdefault(lazy_step_func, "_pytest_bdd_parsers", []).append(parser_instance)
+        def lazy_step_func() -> TCallable:
+            return func
 
-        if converters:
-            step_func.converters = lazy_step_func.converters = converters
+        lazy_step_func._pytest_bdd_parser = parser_instance
 
-        step_func.target_fixture = lazy_step_func.target_fixture = target_fixture
+        setdefault(func, "_pytest_bdd_parsers", []).append(parser_instance)
+        func._pytest_bdd_converters = converters
+        func._pytest_bdd_target_fixture = target_fixture
 
-        lazy_step_func = pytest.fixture()(lazy_step_func)
         fixture_step_name = get_step_fixture_name(parsed_step_name, step_type)
 
         caller_locals = get_caller_module_locals()
-        caller_locals[fixture_step_name] = lazy_step_func
+        caller_locals[fixture_step_name] = pytest.fixture(name=fixture_step_name)(lazy_step_func)
         return func
 
     return decorator

tests/steps/test_steps.py

@@ -2,8 +2,6 @@
 
 import textwrap
 
-import pytest
-
 
 def test_when_then(testdir):
     """Test when and then steps are callable functions.
@@ -40,32 +38,3 @@ def test_when_then(request):
     )
     result = testdir.runpytest()
     result.assert_outcomes(passed=1)
-
-
-@pytest.mark.parametrize(
-    ("step", "keyword"),
-    [("given", "Given"), ("when", "When"), ("then", "Then")],
-)
-def test_preserve_decorator(testdir, step, keyword):
-    """Check that we preserve original function attributes after decorating it."""
-    testdir.makepyfile(
-        textwrap.dedent(
-            '''\
-            from pytest_bdd import {step}
-            from pytest_bdd.steps import get_step_fixture_name
-
-            @{step}("{keyword}")
-            def func():
-                """Doc string."""
-
-            def test_decorator():
-                assert globals()[get_step_fixture_name("{keyword}", {step}.__name__)].__doc__ == "Doc string."
-
-
-        '''.format(
-                step=step, keyword=keyword
-            )
-        )
-    )
-    result = testdir.runpytest()
-    result.assert_outcomes(passed=1)