Merge pull request #544 from pytest-dev/fix-542

Fix step function resolution logic

Author: youtux

.coveragerc

@@ -4,8 +4,10 @@ Changelog
 Unreleased
 ----------
 - Fix bug where steps without parsers would take precedence over steps with parsers. `#534 <https://github.com/pytest-dev/pytest-bdd/pull/534>`_
-- Step functions can now be decorated multiple times with @given, @when, @then. Previously every decorator would override ``converters`` and ``target_fixture`` every at every application. `#534 <https://github.com/pytest-dev/pytest-bdd/pull/534>`_ `#525 <https://github.com/pytest-dev/pytest-bdd/issues/525>`_
+- Step functions can now be decorated multiple times with @given, @when, @then. Previously every decorator would override ``converters`` and ``target_fixture`` every at every application. `#534 <https://github.com/pytest-dev/pytest-bdd/pull/534>`_ `#544 <https://github.com/pytest-dev/pytest-bdd/pull/544>`_ `#525 <https://github.com/pytest-dev/pytest-bdd/issues/525>`_
 - ``parsers.re`` now does a `fullmatch <https://docs.python.org/3/library/re.html#re.fullmatch>`_ instead of a partial match. This is to make it work just like the other parsers, since they don't ignore non-matching characters at the end of the string. `#539 <https://github.com/pytest-dev/pytest-bdd/pull/539>`_
+- Require pytest>=6.2 `#534 <https://github.com/pytest-dev/pytest-bdd/pull/534>`_
+
 
 6.0.1
 -----

CHANGES.rst

@@ -39,14 +39,15 @@ glob2 = "*"
 Mako = "*"
 parse = "*"
 parse-type = "*"
-pytest = ">=5.0"
+pytest = ">=6.2.0"
 typing-extensions = "*"
 
 [tool.poetry.dev-dependencies]
 tox = "^3.25.1"
 mypy = "^0.961"
 types-setuptools = "^57.4.18"
 pytest-xdist = "^2.5.0"
+coverage = {extras = ["toml"], version = "^6.4.2"}
 
 [build-system]
 requires = ["poetry-core>=1.0.0"]
@@ -61,6 +62,19 @@ profile = "black"
 line_length = 120
 multi_line_output = 3
 
+[tool.coverage.report]
+exclude_lines = [
+    "if TYPE_CHECKING:",
+    "if typing\\.TYPE_CHECKING:",
+]
+[tool.coverage.run]
+branch = true
+include =[
+    "pytest_bdd/*",
+    "tests/*",
+]
+
+
 [tool.mypy]
 python_version = "3.7"
 warn_return_any = true

poetry.lock

@@ -9,7 +9,7 @@
 from mako.lookup import TemplateLookup
 
 from .feature import get_features
-from .scenario import find_argumented_step_function, make_python_docstring, make_python_name, make_string_literal
+from .scenario import inject_fixturedefs_for_step, make_python_docstring, make_python_name, make_string_literal
 from .steps import get_step_fixture_name
 from .types import STEP_TYPES
 
@@ -124,18 +124,12 @@ def print_missing_code(scenarios: list[ScenarioTemplate], steps: list[Step]) ->
 
 
 def _find_step_fixturedef(
-    fixturemanager: FixtureManager, item: Function, name: str, type_: str
+    fixturemanager: FixtureManager, item: Function, step: Step
 ) -> Sequence[FixtureDef[Any]] | None:
     """Find step fixturedef."""
-    step_fixture_name = get_step_fixture_name(name, type_)
-    fixturedefs = fixturemanager.getfixturedefs(step_fixture_name, item.nodeid)
-    if fixturedefs is not None:
-        return fixturedefs
-
-    step_func_context = find_argumented_step_function(name, type_, fixturemanager)
-    if step_func_context is not None:
-        return fixturemanager.getfixturedefs(step_func_context.name, item.nodeid)
-    return None
+    with inject_fixturedefs_for_step(step=step, fixturemanager=fixturemanager, nodeid=item.nodeid):
+        bdd_name = get_step_fixture_name(step=step)
+        return fixturemanager.getfixturedefs(bdd_name, item.nodeid)
 
 
 def parse_feature_files(paths: list[str], **kwargs: Any) -> tuple[list[Feature], list[ScenarioTemplate], list[Step]]:
@@ -192,7 +186,7 @@ def _show_missing_code_main(config: Config, session: Session) -> None:
             if scenario in scenarios:
                 scenarios.remove(scenario)
             for step in scenario.steps:
-                fixturedefs = _find_step_fixturedef(fm, item, step.name, step.type)
+                fixturedefs = _find_step_fixturedef(fm, item, step=step)
                 if fixturedefs:
                     try:
                         steps.remove(step)

pyproject.toml

@@ -12,16 +12,19 @@
 """
 from __future__ import annotations
 
+import contextlib
+import logging
 import os
 import re
-from typing import TYPE_CHECKING, Callable, cast
+from typing import TYPE_CHECKING, Callable, Iterator, cast
 
 import pytest
-from _pytest.fixtures import FixtureManager, FixtureRequest, call_fixture_func
+from _pytest.fixtures import FixtureDef, FixtureManager, FixtureRequest, call_fixture_func
+from _pytest.nodes import iterparentnodeids
 
 from . import exceptions
 from .feature import get_feature, get_features
-from .steps import StepFunctionContext, inject_fixture
+from .steps import StepFunctionContext, get_step_fixture_name, inject_fixture
 from .utils import CONFIG_STACK, get_args, get_caller_module_locals, get_caller_module_path
 
 if TYPE_CHECKING:
@@ -31,28 +34,89 @@
 
     from .parser import Feature, Scenario, ScenarioTemplate, Step
 
+
+logger = logging.getLogger(__name__)
+
+
 PYTHON_REPLACE_REGEX = re.compile(r"\W")
 ALPHA_REGEX = re.compile(r"^\d+_*")
 
 
-def find_argumented_step_function(name: str, type_: str, fixturemanager: FixtureManager) -> StepFunctionContext | None:
-    """Find argumented step fixture name."""
+def find_fixturedefs_for_step(step: Step, fixturemanager: FixtureManager, nodeid: str) -> Iterable[FixtureDef[Any]]:
+    """Find the fixture defs that can parse a step."""
     # 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 reversed(fixturedefs):
+    fixture_def_by_name = list(fixturemanager._arg2fixturedefs.items())
+    for i, (fixturename, fixturedefs) in enumerate(fixture_def_by_name):
+        for pos, fixturedef in enumerate(fixturedefs):
             step_func_context = getattr(fixturedef.func, "_pytest_bdd_step_context", None)
             if step_func_context is None:
                 continue
 
-            if step_func_context.type != type_:
+            if step_func_context.type != step.type:
                 continue
 
-            match = step_func_context.parser.is_matching(name)
+            match = step_func_context.parser.is_matching(step.name)
             if not match:
                 continue
 
-            return step_func_context
-    return None
+            if fixturedef not in (fixturemanager.getfixturedefs(fixturename, nodeid) or []):
+                continue
+
+            yield fixturedef
+
+
+@contextlib.contextmanager
+def inject_fixturedefs_for_step(step: Step, fixturemanager: FixtureManager, nodeid: str) -> Iterator[None]:
+    """Inject fixture definitions that can parse a step.
+
+    We fist iterate over all the fixturedefs that can parse the step.
+
+    Then we sort them by their "path" (list of parent IDs) so that we respect the fixture scoping rules.
+
+    Finally, we inject them into the request.
+    """
+    bdd_name = get_step_fixture_name(step=step)
+
+    fixturedefs = list(find_fixturedefs_for_step(step=step, fixturemanager=fixturemanager, nodeid=nodeid))
+
+    # Sort the fixture definitions by their "path", so that the `bdd_name` fixture will
+    # respect the fixture scope
+
+    def get_fixture_path(fixture_def: FixtureDef) -> list[str]:
+        return list(iterparentnodeids(fixture_def.baseid))
+
+    fixturedefs.sort(key=lambda x: get_fixture_path(x))
+
+    if not fixturedefs:
+        yield
+        return
+
+    logger.debug("Adding providers for fixture %r: %r", bdd_name, fixturedefs)
+    fixturemanager._arg2fixturedefs[bdd_name] = fixturedefs
+
+    try:
+        yield
+    finally:
+        del fixturemanager._arg2fixturedefs[bdd_name]
+
+
+def get_step_function(request, step: Step) -> StepFunctionContext | None:
+    """Get the step function (context) for the given step.
+
+    We first figure out what's the step fixture name that we have to inject.
+
+    Then we let `patch_argumented_step_functions` find out what step definition fixtures can parse the current step,
+    and it will inject them for the step fixture name.
+
+    Finally we let request.getfixturevalue(...) fetch the step definition fixture.
+    """
+    bdd_name = get_step_fixture_name(step=step)
+
+    with inject_fixturedefs_for_step(step=step, fixturemanager=request._fixturemanager, nodeid=request.node.nodeid):
+        try:
+            return cast(StepFunctionContext, request.getfixturevalue(bdd_name))
+        except pytest.FixtureLookupError:
+            return None
 
 
 def _execute_step_function(
@@ -76,7 +140,11 @@ def _execute_step_function(
     args = get_args(context.step_func)
 
     try:
-        for arg, value in context.parser.parse_arguments(step.name).items():
+        parsed_args = context.parser.parse_arguments(step.name)
+        assert parsed_args is not None, (
+            f"Unexpected `NoneType` returned from " f"parse_arguments(...) in parser: {context.parser!r}"
+        )
+        for arg, value in parsed_args.items():
             if arg in converters:
                 value = converters[arg](value)
             kwargs[arg] = value
@@ -108,7 +176,7 @@ def _execute_scenario(feature: Feature, scenario: Scenario, request: FixtureRequ
     request.config.hook.pytest_bdd_before_scenario(request=request, feature=feature, scenario=scenario)
 
     for step in scenario.steps:
-        context = find_argumented_step_function(step.name, step.type, request._fixturemanager)
+        context = get_step_function(request=request, step=step)
         if context is None:
             exc = exceptions.StepDefinitionNotFoundError(
                 f"Step definition is not found: {step}. "