Merge pull request #748 from pytest-dev/raise-error-if-step-uses-reserved-arguments

Raise an error if a step defines reserved argument names (`datatable`, `docstring`)

Author: jsa34

CHANGES.rst

@@ -14,6 +14,7 @@ Added
 
 Changed
 +++++++
+* Step arguments ``"datatable"`` and ``"docstring"`` are now reserved, and they can't be used as step argument names.
 
 Deprecated
 ++++++++++

src/pytest_bdd/exceptions.py

@@ -3,6 +3,10 @@
 from __future__ import annotations
 
 
+class StepImplementationError(Exception):
+    """Step implementation error."""
+
+
 class ScenarioIsDecoratorOnly(Exception):
     """Scenario can be only used as decorator."""
 

src/pytest_bdd/scenario.py

@@ -28,7 +28,7 @@
 from .compat import getfixturedefs, inject_fixture
 from .feature import get_feature, get_features
 from .steps import StepFunctionContext, get_step_fixture_name
-from .utils import CONFIG_STACK, get_args, get_caller_module_locals, get_caller_module_path
+from .utils import CONFIG_STACK, get_args, get_caller_module_locals, get_caller_module_path, identity
 
 if TYPE_CHECKING:
     from _pytest.mark.structures import ParameterSet
@@ -41,10 +41,13 @@
 
 logger = logging.getLogger(__name__)
 
-
 PYTHON_REPLACE_REGEX = re.compile(r"\W")
 ALPHA_REGEX = re.compile(r"^\d+_*")
 
+STEP_ARGUMENT_DATATABLE = "datatable"
+STEP_ARGUMENT_DOCSTRING = "docstring"
+STEP_ARGUMENTS_RESERVED_NAMES = {STEP_ARGUMENT_DATATABLE, STEP_ARGUMENT_DOCSTRING}
+
 
 def find_fixturedefs_for_step(step: Step, fixturemanager: FixtureManager, node: Node) -> Iterable[FixtureDef[Any]]:
     """Find the fixture defs that can parse a step."""
@@ -172,6 +175,27 @@ def get_step_function(request: FixtureRequest, step: Step) -> StepFunctionContex
             return None
 
 
+def parse_step_arguments(step: Step, context: StepFunctionContext) -> dict[str, object]:
+    """Parse step arguments."""
+    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}"
+    )
+
+    reserved_args = set(parsed_args.keys()) & STEP_ARGUMENTS_RESERVED_NAMES
+    if reserved_args:
+        reserved_arguments_str = ", ".join(repr(arg) for arg in reserved_args)
+        raise exceptions.StepImplementationError(
+            f"Step {step.name!r} defines argument names that are reserved: {reserved_arguments_str}. "
+            "Please use different names."
+        )
+
+    converted_args = {key: (context.converters.get(key, identity)(value)) for key, value in parsed_args.items()}
+
+    return converted_args
+
+
 def _execute_step_function(
     request: FixtureRequest, scenario: Scenario, step: Step, context: StepFunctionContext
 ) -> None:
@@ -185,30 +209,17 @@ def _execute_step_function(
         "step_func": context.step_func,
         "step_func_args": {},
     }
-
     request.config.hook.pytest_bdd_before_step(**kw)
-
-    # Get the step argument values.
-    converters = context.converters
-    kwargs = {}
     args = get_args(context.step_func)
 
     try:
-        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
+        kwargs = parse_step_arguments(step=step, context=context)
 
         if step.datatable is not None:
-            kwargs["datatable"] = step.datatable.raw()
+            kwargs[STEP_ARGUMENT_DATATABLE] = step.datatable.raw()
 
         if step.docstring is not None:
-            kwargs["docstring"] = step.docstring
+            kwargs[STEP_ARGUMENT_DOCSTRING] = step.docstring
 
         kwargs = {arg: kwargs[arg] if arg in kwargs else request.getfixturevalue(arg) for arg in args}
 

src/pytest_bdd/utils.py

@@ -83,3 +83,8 @@ def setdefault(obj: object, name: str, default: T) -> T:
     except AttributeError:
         setattr(obj, name, default)
         return default
+
+
+def identity(x: T) -> T:
+    """Return the argument."""
+    return x

tests/datatable/test_datatable.py

@@ -210,3 +210,48 @@ def test_datatable():
     )
     result = pytester.runpytest("-s")
     result.assert_outcomes(passed=1)
+
+
+def test_datatable_step_argument_is_reserved_and_cannot_be_used(pytester):
+    pytester.makefile(
+        ".feature",
+        reserved_datatable_arg=textwrap.dedent(
+            """\
+            Feature: Reserved datatable argument
+
+              Scenario: Reserved datatable argument
+                Given this step has a {datatable} argument
+                Then the test fails
+            """
+        ),
+    )
+
+    pytester.makepyfile(
+        textwrap.dedent(
+            """\
+        from pytest_bdd import scenario, given, then, parsers
+
+        @scenario("reserved_datatable_arg.feature", "Reserved datatable argument")
+        def test_datatable():
+            pass
+
+
+        @given(parsers.parse("this step has a {datatable} argument"))
+        def _(datatable):
+            pass
+
+
+        @then("the test fails")
+        def _():
+            pass
+        """
+        )
+    )
+
+    result = pytester.runpytest()
+    result.assert_outcomes(failed=1)
+    result.stdout.fnmatch_lines(
+        [
+            "*Step 'this step has a {datatable} argument' defines argument names that are reserved: 'datatable'. Please use different names.*"
+        ]
+    )

tests/steps/test_docstring.py

@@ -193,3 +193,48 @@ def _():
     )
     result = pytester.runpytest("-s")
     result.assert_outcomes(passed=1)
+
+
+def test_docstring_step_argument_is_reserved_and_cannot_be_used(pytester):
+    pytester.makefile(
+        ".feature",
+        reserved_docstring_arg=textwrap.dedent(
+            """\
+            Feature: Reserved docstring argument
+
+              Scenario: Reserved docstring argument
+                Given this step has a {docstring} argument
+                Then the test fails
+            """
+        ),
+    )
+
+    pytester.makepyfile(
+        textwrap.dedent(
+            """\
+        from pytest_bdd import scenario, given, then, parsers
+
+        @scenario("reserved_docstring_arg.feature", "Reserved docstring argument")
+        def test_docstring():
+            pass
+
+
+        @given(parsers.parse("this step has a {docstring} argument"))
+        def _(docstring):
+            pass
+
+
+        @then("the test fails")
+        def _():
+            pass
+        """
+        )
+    )
+
+    result = pytester.runpytest()
+    result.assert_outcomes(failed=1)
+    result.stdout.fnmatch_lines(
+        [
+            "*Step 'this step has a {docstring} argument' defines argument names that are reserved: 'docstring'. Please use different names.*"
+        ]
+    )