From 63a4268b9c94b4563e2d56f766868ea2320c0033 Mon Sep 17 00:00:00 2001
From: Alessio Bogon <778703+youtux@users.noreply.github.com>
Date: Wed, 27 Jul 2022 17:02:35 +0200
Subject: [PATCH] Fix bug when 2 steps have the same step name...

... but only one step impl was kept.
---
 pytest_bdd/scenario.py     |  2 +-
 pytest_bdd/steps.py        | 38 ++++++++++++++++--------------
 tests/steps/test_common.py | 48 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 19 deletions(-)

diff --git a/pytest_bdd/scenario.py b/pytest_bdd/scenario.py
index 8f4a817..1b6b5f7 100644
--- a/pytest_bdd/scenario.py
+++ b/pytest_bdd/scenario.py
@@ -48,7 +48,7 @@ def iter_argumented_step_function(
     """Iterate over argumented step functions."""
     # happens to be that _arg2fixturedefs is changed during the iteration so we use a copy
     fixture_def_by_name = list(fixturemanager._arg2fixturedefs.items())
-    for i, (fixturename, fixturedefs) in enumerate(reversed(fixture_def_by_name)):
+    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:
diff --git a/pytest_bdd/steps.py b/pytest_bdd/steps.py
index 9f37192..19175b4 100644
--- a/pytest_bdd/steps.py
+++ b/pytest_bdd/steps.py
@@ -37,7 +37,8 @@ def _(article):
 from __future__ import annotations
 
 from dataclasses import dataclass, field
-from typing import Any, Callable, TypeVar
+from itertools import count
+from typing import Any, Callable, Iterable, TypeVar
 
 import pytest
 from _pytest.fixtures import FixtureDef, FixtureRequest
@@ -52,7 +53,6 @@ TCallable = TypeVar("TCallable", bound=Callable[..., Any])
 
 @dataclass
 class StepFunctionContext:
-    name: str
     type: Literal["given", "when", "then"]
     step_func: Callable[..., Any]
     parser: StepParser
@@ -60,17 +60,6 @@ class StepFunctionContext:
     target_fixture: str | None = None
 
 
-def get_step_fixture_name(name: str, type_: str) -> str:
-    """Get step fixture name.
-
-    :param name: string
-    :param type: step type
-    :return: step fixture name
-    :rtype: string
-    """
-    return f"pytestbdd_{type_}_{name}"
-
-
 def get_parsed_step_fixture_name(name: str, type_: str) -> str:
     """Get step fixture name.
 
@@ -129,6 +118,23 @@ def then(
     return _step_decorator(THEN, name, converters=converters, target_fixture=target_fixture)
 
 
+def find_unique_name(name: str, seen: Iterable[str]) -> str:
+    """Find unique name.
+
+    :param name: string
+    :param seen: iterable of strings
+    :return: unique string
+    """
+    seen = set(seen)
+    if name not in seen:
+        return name
+
+    for i in count(1):
+        new_name = f"{name}_{i}"
+        if new_name not in seen:
+            return new_name
+
+
 def _step_decorator(
     step_type: Literal["given", "when", "then"],
     step_name: str | StepParser,
@@ -149,12 +155,8 @@ def _step_decorator(
 
     def decorator(func: TCallable) -> TCallable:
         parser = get_parser(step_name)
-        parsed_step_name = parser.name
-
-        fixture_step_name = get_step_fixture_name(parsed_step_name, step_type)
 
         context = StepFunctionContext(
-            name=fixture_step_name,
             type=step_type,
             step_func=func,
             parser=parser,
@@ -162,13 +164,13 @@ def _step_decorator(
             target_fixture=target_fixture,
         )
 
-        # TODO: Probably we can keep on returning None here instead
         def step_function_marker() -> StepFunctionContext:
             return context
 
         step_function_marker._pytest_bdd_step_context = context
 
         caller_locals = get_caller_module_locals()
+        fixture_step_name = find_unique_name(f"pytestbdd_stepdef_{step_type}_{parser.name}", seen=caller_locals.keys())
         caller_locals[fixture_step_name] = pytest.fixture(name=fixture_step_name)(step_function_marker)
         return func
 
diff --git a/tests/steps/test_common.py b/tests/steps/test_common.py
index e11cef1..0a3fc8c 100644
--- a/tests/steps/test_common.py
+++ b/tests/steps/test_common.py
@@ -49,3 +49,51 @@ def test_step_function_multiple_target_fixtures(testdir):
     [foo, bar] = collect_dumped_objects(result)
     assert foo == "test foo"
     assert bar == "test bar"
+
+
+def test_step_functions_same_parser(testdir):
+    testdir.makefile(
+        ".feature",
+        target_fixture=textwrap.dedent(
+            """\
+            Feature: A feature
+                Scenario: A scenario
+                    Given there is a foo with value "(?P<value>\\w+)"
+                    And there is a foo with value "testfoo"
+                    When pass
+                    Then pass
+            """
+        ),
+    )
+    testdir.makepyfile(
+        textwrap.dedent(
+            """\
+        import pytest
+        from pytest_bdd import given, when, then, scenarios, parsers
+        from pytest_bdd.utils import dump_obj
+
+        scenarios("target_fixture.feature")
+
+        STEP = 'there is a foo with value "(?P<value>\\w+)"'
+
+        @given(STEP)
+        def _():
+            dump_obj(('str',))
+
+        @given(parsers.re(STEP))
+        def _(value):
+            dump_obj(('re', value))
+
+        @when("pass")
+        @then("pass")
+        def _():
+            pass
+        """
+        )
+    )
+    result = testdir.runpytest("-s")
+    result.assert_outcomes(passed=1)
+
+    [first_given, second_given] = collect_dumped_objects(result)
+    assert first_given == ("str",)
+    assert second_given == ("re", "testfoo")