Repository: pytest-dev/pytest-order (language: python)

Issue:
Ordering breaking with pytest-xdist + pytest-dependency
Hi,
I have the following test code:
```
class BaseTest:
   ...

    @pytest.mark.dependency(name="one", depends=["zero"])
    def test_one(self, fixtures...):
        ...

    @pytest.mark.dependency(name="two", depends=["one"])
    def test_two(self, fixtures...):
        ...

    @pytest.mark.dependency(name="three", depends=["two"])
    def test_three(self, fixtures...):
        ...

    @pytest.mark.dependency(name="zero")
    def test_zero(self, fixtures...):
        ...
```

Other tests inherit from the BaseTest. Every class has its own file.
```
class TestSomething(BaseTest):
    @pytest.fixture(scope="module")
    def something(self):
        return "something"
```
I've been running `python3 -m pytest -s --order-dependencies -ra -v --dist=loadfile -n auto`.

The first ordering works well - zero runs before one for every class that inherits the test. But, for some reason, pytest-order is consistently putting test_three before test_two (except for one class in particular - not sure if this is a red herring or not). I'm not sure if this is a bug or some behavior that I don't understand. I can confirm the order that the tests are being run in in the verbose output. Everything runs correctly when running without xdist: `python3 -m pytest -s --order-dependencies -ra -v -k something`.
Happy to share more details privately if they would help.
Thanks for maintaining such a useful plugin!



Discussion / hints:
Thanks for the report! 
At first glance, I see nothing that would cause this - you use `--dist=loadfile`, and there are no dependencies between your modules. The setup with the base class is a bit unusual though, and certainly nothing I had tested so far - I suspect that the problem is somehow related to that, but that is only a guess.
I will try to reproduce the problem, and in case I can't, ask for more information.
A couple of questions:
- Do you also use the option `--order-scope=module`? You probably should, otherwise the ordering will be done globally (though actually this is something that could be done automatically if `xdist` is detected - haven't thought about this before)
- Do you get any warnings from `pytest-order`, e.g. `WARNING: cannot execute test relative to others: ...`? Maybe you can show the relevant part of the output.
- Do you derived tests just execute the base tests, with some changed fixtures? How is that `something` fixture in your example used? I tried this, and couldn't get the ordering work even without `pytest-xdist`...
Thanks for getting back to me so quickly. I tried it with the various order scopes, and I didn't get that warning.
Unfortunately, for other reasons, we stopped using pytest-dependency so I won't be able to provide much more information. I appreciate your help!
Thank you - I will see if I can find out anything, though that will take some time (won't have access to a computer for at least a week, and there are other issues to handle first afterwards). Anyway, if you don't use it anymore, you will be in no rush to have this fixed...
I had forgotten about this one... I just had another look and found that the problem may appear even without `xdist`. 

The root cause is that `pytest-order` does not correctly handle several markers with the same dependency name (in this case "zero", "one" etc, which are used in each derived class). I made the assumption that the test name would always be unique, but this is obviously not the case here.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 159c056..9bf4eb4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # pytest-order Release Notes
 
+## Fixes
+- handle dependency markers with the same alias name (see [#71](https://github.com/pytest-dev/pytest-order/issues/71))
+
 ## Infrastructure
 - avoid unknown marker warning in tests (see [#101](https://github.com/pytest-dev/pytest-order/issues/101))
 - added pytest 8 to CI tests


Test patch:
diff --git a/pytest_order/sorter.py b/pytest_order/sorter.py
index 3f57474..1c67305 100644
--- a/pytest_order/sorter.py
+++ b/pytest_order/sorter.py
@@ -1,7 +1,9 @@
+import re
 import sys
-from warnings import warn
+from collections import OrderedDict
 from contextlib import suppress
 from typing import Optional, List, Dict, Tuple, cast
+from warnings import warn
 
 from _pytest.config import Config
 from _pytest.mark import Mark
@@ -10,20 +12,6 @@
 from .item import Item, ItemList, ItemGroup, filter_marks, move_item, RelativeMark
 from .settings import Settings, Scope
 
-try:
-    from typing import OrderedDict
-except ImportError:
-    # In Python <3.7.2, we need to stub it
-    from collections import OrderedDict as OrderedDict_cls
-    from typing import MutableMapping, TypeVar
-
-    KT = TypeVar("KT")
-    VT = TypeVar("VT")
-
-    class OrderedDict(OrderedDict_cls, MutableMapping[KT, VT]):  # type: ignore
-        pass
-
-
 orders_map = {
     "first": 0,
     "second": 1,
@@ -112,7 +100,7 @@ def mark_binning(
         self,
         item: Item,
         dep_marks: Dict[Tuple[str, Scope, str], List[Item]],
-        aliases: Dict[str, Item],
+        aliases: Dict[str, List[Item]],
     ) -> None:
         """
         Collect relevant markers for the given item.
@@ -138,7 +126,7 @@ def handle_dependency_mark(
         item: Item,
         has_order: bool,
         dep_marks: Dict[Tuple[str, Scope, str], List[Item]],
-        aliases: Dict[str, Item],
+        aliases: Dict[str, List[Item]],
     ) -> None:
         # always order dependencies if an order mark is present
         # otherwise only if order-dependencies is set
@@ -159,7 +147,7 @@ def handle_dependency_mark(
         # of the nodeid, depending on the scope
         if not name_mark:
             name_mark = item.node_id
-        aliases[name_mark] = item
+        aliases.setdefault(name_mark, []).append(item)
 
     def handle_order_marks(self, item: Item) -> None:
         marks = item.item.iter_markers("order")
@@ -289,7 +277,7 @@ def warn_about_unknown_test(item: Item, rel_mark: str) -> None:
         )
 
     def collect_markers(self) -> None:
-        aliases: Dict[str, Item] = {}
+        aliases: Dict[str, List[Item]] = {}
         dep_marks: Dict[Tuple[str, Scope, str], List[Item]] = {}
         for item in self.items:
             self.mark_binning(item, dep_marks, aliases)
@@ -298,20 +286,20 @@ def collect_markers(self) -> None:
     def resolve_dependency_markers(
         self,
         dep_marks: Dict[Tuple[str, Scope, str], List[Item]],
-        aliases: Dict[str, Item],
+        aliases: Dict[str, List[Item]],
     ) -> None:
         for (name, _, prefix), items in dep_marks.items():
             if name in aliases:
                 for item in items:
-                    self.dep_marks.append(
-                        RelativeMark(aliases[name], item, move_after=True)
-                    )
+                    alias = self.matching_alias(aliases[name], item)
+                    self.dep_marks.append(RelativeMark(alias, item, move_after=True))
             else:
                 label = "::".join((prefix, name))
                 if label in aliases:
                     for item in items:
+                        alias = self.matching_alias(aliases[label], item)
                         self.dep_marks.append(
-                            RelativeMark(aliases[label], item, move_after=True)
+                            RelativeMark(alias, item, move_after=True)
                         )
                 else:
                     sys.stdout.write(
@@ -319,6 +307,28 @@ def resolve_dependency_markers(
                         "- ignoring it.".format(name)
                     )
 
+    @staticmethod
+    def matching_alias(aliases: List[Item], item: Item) -> Item:
+        if len(aliases) == 1:
+            return aliases[0]
+
+        # handle the rare case that several tests have the same alias name
+        # we use the item that best matches the node id of the dependent item
+        max_matching_parts = 0
+        node_id_parts = re.split("(::|/)", item.node_id)
+        matching_item = aliases[0]
+        for alias_item in aliases:
+            alias_node_id_parts = re.split("(::|/)", alias_item.node_id)
+            nr_matching_parts = 0
+            for n, a in zip(node_id_parts, alias_node_id_parts):
+                if n != a:
+                    break
+                nr_matching_parts += 1
+            if nr_matching_parts > max_matching_parts:
+                max_matching_parts = nr_matching_parts
+                matching_item = alias_item
+        return matching_item
+
 
 def module_item_groups(items: List[Item]) -> Dict[str, List[Item]]:
     """
diff --git a/tests/test_dependency.py b/tests/test_dependency.py
index b0dfdf9..7d30a95 100644
--- a/tests/test_dependency.py
+++ b/tests/test_dependency.py
@@ -399,6 +399,61 @@ def test_two():
     )
 
 
+def test_equally_named_dependency_in_modules(test_path):
+    # regression test for #71
+    test_path.makepyfile(
+        test_ndep1=(
+            """
+            import pytest
+
+            class TestOne:
+
+                @pytest.mark.dependency(name="one", depends=["zero"])
+                def test_one(self):
+                    assert True
+
+                @pytest.mark.dependency(name="two", depends=["one"])
+                def test_two(self):
+                    assert True
+
+                @pytest.mark.dependency(name="three", depends=["two"])
+                def test_three(self):
+                    assert True
+
+                @pytest.mark.dependency(name="zero")
+                def test_zero(self):
+                    assert True
+            """
+        ),
+        test_ndep2=(
+            """
+            import pytest
+
+            class TestTwo:
+
+                @pytest.mark.dependency(name="one", depends=["zero"])
+                def test_one(self):
+                    assert True
+
+                @pytest.mark.dependency(name="two", depends=["one"])
+                def test_two(self):
+                    assert True
+
+                @pytest.mark.dependency(name="three", depends=["two"])
+                def test_three(self):
+                    assert True
+
+                @pytest.mark.dependency(name="zero")
+                def test_zero(self):
+                    assert True
+            """
+        ),
+    )
+
+    result = test_path.runpytest("-v", "--order-dependencies")
+    result.assert_outcomes(passed=8, failed=0, skipped=0)
+
+
 @pytest.mark.skipif(
     pytest.__version__.startswith("3.7."),
     reason="pytest-dependency < 0.5 does not support session scope",