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

Issue:
Relative test naming / syntax is confusing
Hello o/

So, I've wanted to give this library a shot. I'm in the middle of moving from `pytest-dependency`, and I really can't get my tests to work with it. All I want, is to specify that certain tests are allowed to run only after others specified, which is why I'm pretty much exclusively using `@pytest.mark.order(after=...)` everywhere.

Here's the code: https://github.com/DevilXD/aRez/compare/dev...order

And the error I'm getting:
```
WARNING: cannot execute test relative to others: test_misc.test_enum test_misc.test_cache tests.test_endpoint.test_session test_player.test_player_history test_api.test_get_match test_player.test_player_history test_player.test_player_status test_server_status_merge test_endpoint.test_session test_endpoint.test_session test_enum test_endpoint.test_session test_cache test_api.test_bounty test_api.test_get_match test_match.test_live_match test_player.test_player_history test_player.test_player_loadouts test_player.test_player_champion_stats test_cache test_player.test_player_history
tests.test_player.test_player_expand - ignoring the marker.
```
Python 3.8.8, pytest 6.2.3 and latest this lib (0.10.0) versions.

Here are the test names I've tried, for `tests/test_misc.py::test_enum` as an example:
```py
test_enum
test_misc.test_enum
tests.test_misc.test_enum
```
Again, none of them appear to make the warning go away, even when I rigorously change them everywhere, making sure to not miss anything.

Regardless, this leads me to the main topic, which I wanted to ask about here. Why are the tests referred to like this? Why is `tests/test_misc.py::test_enum` turned into `tests.test_misc.test_enum` or `test_misc.test_enum` (or idk, I never got it to work), especially since the library itself appears to have to convert it back to the previous form here: https://github.com/pytest-dev/pytest-order/blob/70b3f55aa46b79034a16acfaabca9d9eee1d8482/pytest_order/sorter.py#L201

Additionally, why a space as a separator? It's quite hard to notice exactly where each dependency starts and ends here:
`pytest.mark.order(after="test_misc.test_enum test_misc.test_cache")`
..., not even talking about something like this:
```py
@pytest.mark.order(after="test_cache test_api.test_bounty test_api.test_get_match test_match.test_live_match test_player.test_player_history test_player.test_player_loadouts test_player.test_player_champion_stats")
```
Fortunately, this one can be broken down a little, utilizing Python's ability to concat strings at compile time:
```py
@pytest.mark.order(after=(
    "test_cache "  #  notice a space after each element
    "test_api.test_bounty "
    "test_api.test_get_match "
    "test_match.test_live_match "
    "test_player.test_player_history "
    "test_player.test_player_loadouts "
    "test_player.test_player_champion_stats"
))
```
But still. Wouldn't it be better and easier for the user, to refer to tests by their ID labels? They're displayed when you run `pytest` with the `-v` flag, making it easy to just copy'n'paste where you'd want an order dependency to exist. Also, wouldn't a list of strings work better than a "space separated" single string? This is how it would/could look like:

```py
@pytest.mark.order(after=["tests/test_misc.py::test_enum", "tests/test_misc.py::test_cache"])
def test_thing():
    ...
# can be easily broken down if the line would get long too, not everyone knows that you can concat strings at compile time
@pytest.mark.order(after=[
    "tests/test_misc.py::test_enum",
    "tests/test_misc.py::test_cache",
])
def test_thing():
    ...
```

I know a change like this would be breaking a little, but it'd be easy to adapt it. Doing some quick searching, it appears that no one tried to use the `before` and `after` kwargs from this library yet, as the relevant results point at this repo only. This doesn't take into account private repos, or things not hosted on github, but still. Note - these are github-wide searches.

https://github.com/search?q=pytest.mark.order%28before&type=code
https://github.com/search?q=pytest.mark.order%28after&type=code

Please consider changing this as shown.


Discussion / hints:
Thanks for the extensive input and the suggestions. 

First, the currently released version (0.10.0) does not support more than one relative marker - I have added this only [recently](https://github.com/pytest-dev/pytest-order/blob/main/CHANGELOG.md). You have probably checked the [dev branch documentation](https://pytest-dev.github.io/pytest-order/dev/) instead of the [release documentation](https://pytest-dev.github.io/pytest-order/stable/). Can you please check if you have the same problem with the main branch? 

About your suggestions:
Changing the syntax for several markers: I was unsure about that myself. The main reason I did it this way was to make the default case (only one marker) most convenient, so I didn't want to add the list notation as mandatory (this is a common pitfall in `pytest-dependency`). I still think that the main usage will be a single dependency, so I don't want to change that, but I could change the syntax for several markers to the list syntax as in `pytest-dependency`, which was what I actually considered first. I can make this change without problems, as the version is not released yet.

The notation of the markers: I found this syntax more readable at the time, but I agree that the nodeid may be a better (more standard) way to use. I have considered this while I was adding support for `pytest-dependency`, I just wasn't sure about it, also because it is a breaking change. But you are probably right that this is not used yet, so it should hopefully not have a big impact on existing code. I would like to retain the current behavior with regard to scopes though, e.g. you have to define only as much as needed of the nodeid for the current scope, without the need to explicitly defining a scope.

Nevertheless, as a breaking change this would demand a new major version (1.0) - I try to follow semantic versioning here. I also plan to drop Python 2.7 and 3.5 support for that version. 

But before doing any changes I would first like to understand why it does not work for you, and fix any bugs. 

> First, the currently released version (0.10.0) does not support more than one relative marker - I have added this only recently.
> Can you please check if you have the same problem with the main branch?
> But before doing any changes I would first like to understand why it does not work for you, and fix any bugs.

I've reinstalled the library from the github repo, and used the same naming I went with the first time I tried - and everything worked! Looks like that was the problem 😅

>You have probably checked the dev branch documentation instead of the release documentation.

Honestly, I'm not sure how did that even happen. I swear I was looking at the release documentation, but then somehow ended up switching to dev documentation mid-way, I guess. That's my bad, apologies.

> make the default case (only one marker) most convenient, so I didn't want to add the list notation as mandatory
> I can make this change without problems, as the version is not released yet.

First of all, thank you for considering =) That'd be pretty cool. It should be as easy as:

```py
after_value: Union[str, List[str]]  # the contents of the 'after' kwarg and the thing you'll be iterating over later

if not isinstance(after_value, list):
    # make it iterable by wrapping it in a single-item tuple (faster than a single-item list, but your choice)
    after_value= (after_value,)
    # after_value= [after_value]

# iterate over it
for thing in after_value:
    ...
```

> I would like to retain the current behavior with regard to scopes though, e.g. you have to define only as much as needed of the nodeid for the current scope, without the need to explicitly defining a scope.

I'm perfectly fine with that, as long as it's going to make sense. In my case, I'll probably be using only session and local/module scopes anyway. The nicest thing about node IDs, is that one can just copy them directly from pytest (and optionally trim them, to change the scope), while the current syntax required me to do a bunch of replacing slashes, double colons and `.py` with dots, and it never ended up working in the end anyway (but now I know why 😅), so I think that'll be a much better solution in the long run 👍
As you may have noticed, I have meanwhile changed the behavior for multiple markers in main as you have suggested. I will probably make a new minor release with that addition (I wanted to first fix any bug you found - but that is not needed, as it seems now 😄), and handle the change to nodeid later.
Sounds good! For the record, I'd still recommend using tuples for wrapping iterables, as they're about 5x faster to construct than a list: https://i.imgur.com/dX5iMFk.png

It's not that necessary (both work), but it's a simple speed optimization, and a small tip for the future 😊
Yeah, I missed that - already did this locally now, thanks. It doesn't really matter performance-wise in this case (most time is spent elsewhere), but it's a good practice, anyway.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0a38ae8..2eef14f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,13 +2,11 @@
 
 ## Unreleased
 
-_Breaking changes in next major version_:
-With that version the notation of relative markers in other modules is also 
-planned to change - instead of using the dot notation, the standard pytest
-nodeid will be used. 
-
 ### Breaking changes
 - removed support for Python 2.7 and 3.5
+- changed notation of relative markers in other modules - instead of using
+  the dot notation, the standard pytest nodeid is used,
+  see [#24](https://github.com/pytest-dev/pytest-order/issues/24)
 
 ## [Version 0.11.0](https://pypi.org/project/pytest-order/0.11.0/) (2021-04-11)
 Adds support for multiple relative markers for the same test.
diff --git a/docs/source/index.rst b/docs/source/index.rst
index c850757..91630bd 100644
--- a/docs/source/index.rst
+++ b/docs/source/index.rst
@@ -356,9 +356,11 @@ with a ``::`` suffix has to be prepended to the test name:
      def test_c(self):
          assert True
 
-If the referenced test lives in another module, the test name has to be
-prepended by the module path to be uniquely identifiable. Let's say we have
-the following module and test layout:
+If the referenced test lives in another module, you have to use the nodeid
+of the test, or a part of the nodeid that is sufficient to make it uniquely
+identifiable (the nodeid is the test ID that pytest prints if you run it with
+the ``-v`` option).
+Let's say we have the following module and test layout:
 
 ::
 
@@ -383,11 +385,11 @@ modules, this could be expressed like:
 
  import pytest
 
- @pytest.mark.order(after="test_module_a.TestA::test_a")
+ @pytest.mark.order(after="test_module_a.py::TestA::test_a")
  def test_a():
      assert True
 
- @pytest.mark.order(before="test_module_c.test_submodule.test_2")
+ @pytest.mark.order(before="test_module_c/test_submodule.py::test_2")
  def test_b():
      assert True
 
@@ -466,7 +468,7 @@ attributes by using a list or tuple of test names:
 
  import pytest
 
- @pytest.mark.order(after=["test_second", "other_module.test_other"])
+ @pytest.mark.order(after=["test_second", "other_module.py::test_other"])
  def test_first():
      assert True
 
@@ -474,7 +476,7 @@ attributes by using a list or tuple of test names:
      assert True
 
 This will ensure that ``test_first`` is executed both after ``test_second``
-and after ``test_other`` which resides in the module ``other_module``.
+and after ``test_other`` which resides in the module ``other_module.py``.
 
 Configuration
 =============
@@ -784,7 +786,7 @@ Here is a similar example using relative markers:
 
   import pytest
 
-  @pytest.mark.order(after="test_module2.test1")
+  @pytest.mark.order(after="test_module2.py::test1")
   def test1():
       pass
 
diff --git a/setup.cfg b/setup.cfg
index 28e3225..e8810f7 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -2,6 +2,6 @@
 description-file = README.md
 license_files = LICENSE
 [bdist_wheel]
-universal=1
+universal=0
 [tool:pytest]
 testpaths = tests


Test patch:
diff --git a/example/test_modules_b.py b/example/test_modules_b.py
index 196bb66..0c645e3 100644
--- a/example/test_modules_b.py
+++ b/example/test_modules_b.py
@@ -4,11 +4,11 @@
 import pytest
 
 
-@pytest.mark.order(after="test_module_a.TestA::test_a")
+@pytest.mark.order(after="test_module_a.py::TestA::test_a")
 def test_a():
     assert True
 
 
-@pytest.mark.order(before="test_module_c.test_submodule.test_2")
+@pytest.mark.order(before="test_module_c.py::test_submodule.test_2")
 def test_b():
     assert True
diff --git a/pytest_order/item.py b/pytest_order/item.py
index 8d30e79..80b0b79 100644
--- a/pytest_order/item.py
+++ b/pytest_order/item.py
@@ -37,10 +37,6 @@ def node_id(self) -> str:
             self._node_id = self.item.nodeid.replace("::()", "")
         return self._node_id
 
-    @property
-    def label(self) -> str:
-        return self.node_id.replace(".py::", ".").replace("/", ".")
-
 
 class ItemList:
     """Handles a group of items with the same scope."""
@@ -99,8 +95,8 @@ def sort_numbered_items(self) -> List[Item]:
         return sorted_list
 
     def print_unhandled_items(self):
-        msg = " ".join([mark.item.label for mark in self.rel_marks] +
-                       [mark.item.label for mark in self.dep_marks])
+        msg = " ".join([mark.item.node_id for mark in self.rel_marks] +
+                       [mark.item.node_id for mark in self.dep_marks])
         if msg:
             sys.stdout.write(
                 "\nWARNING: cannot execute test relative to others: ")
diff --git a/pytest_order/sorter.py b/pytest_order/sorter.py
index da00bf1..a637e0d 100644
--- a/pytest_order/sorter.py
+++ b/pytest_order/sorter.py
@@ -141,7 +141,6 @@ def item_from_label(
             self, label: str,
             item: Item,
             is_cls_mark: bool) -> Optional[Item]:
-        label = self.node_id_from_label(label)
         item_id = item.node_id
         label_len = len(label)
         last_comp = label.split("/")[-1].split("::")[-1]
@@ -159,7 +158,6 @@ def item_from_label(
 
     def items_from_class_label(self, label: str, item: Item) -> List[Item]:
         items = []
-        label = self.node_id_from_label(label)
         item_id = item.node_id
         label_len = len(label)
         for node_id in self.node_ids:
@@ -171,13 +169,6 @@ def items_from_class_label(self, label: str, item: Item) -> List[Item]:
                         items.append(self.node_ids[node_id])
         return items
 
-    @staticmethod
-    def node_id_from_label(label: str) -> str:
-        if "." in label:
-            label_comp = label.split(".")
-            label = ".py::".join(["/".join(label_comp[:-1]), label_comp[-1]])
-        return label
-
     def handle_before_or_after_mark(
             self,
             item: Item,
diff --git a/tests/test_order_group_scope_relative.py b/tests/test_order_group_scope_relative.py
index b3c511d..ddda4c4 100644
--- a/tests/test_order_group_scope_relative.py
+++ b/tests/test_order_group_scope_relative.py
@@ -22,7 +22,7 @@ class Test2:
         def test_one(self):
             assert True
 
-        @pytest.mark.order(after='test_rel3.test_two')
+        @pytest.mark.order(after='test_rel3.py::test_two')
         def test_two(self):
             assert True
     """,
@@ -39,7 +39,7 @@ def test_two():
     def test_one():
         assert True
 
-    @pytest.mark.order(before='test_rel2.test_one')
+    @pytest.mark.order(before='test_rel2.py::test_one')
     def test_two():
         assert True
     """,
diff --git a/tests/test_relative_ordering.py b/tests/test_relative_ordering.py
index 9328932..cb913ca 100644
--- a/tests/test_relative_ordering.py
+++ b/tests/test_relative_ordering.py
@@ -138,11 +138,11 @@ def fixture_path(test_path):
     import pytest
 
     class TestA:
-        @pytest.mark.order(after="mod2_test.TestB::test_b")
+        @pytest.mark.order(after="mod2_test.py::TestB::test_b")
         def test_a(self):
             pass
 
-        @pytest.mark.order(after="sub.mod3_test.test_b")
+        @pytest.mark.order(after="sub/mod3_test.py::test_b")
         def test_b(self):
             pass
 
@@ -153,7 +153,7 @@ def test_c(self):
     import pytest
 
     class TestB:
-        @pytest.mark.order(before="mod1_test.TestA::test_c")
+        @pytest.mark.order(before="mod1_test.py::TestA::test_c")
         def test_a(self):
             pass
 
@@ -168,7 +168,7 @@ def test_c(self):
     path.write("""
 import pytest
 
-@pytest.mark.order(before="mod2_test.TestB::test_c")
+@pytest.mark.order(before="mod2_test.py::TestB::test_c")
 def test_a():
     pass
 
@@ -356,7 +356,7 @@ def test_dependency_after_unknown_test(item_names_for, capsys):
     test_content = """
     import pytest
 
-    @pytest.mark.order(after="some_module.test_2")
+    @pytest.mark.order(after="some_module.py::test_2")
     def test_1():
         pass
 
@@ -366,7 +366,7 @@ def test_2():
     assert item_names_for(test_content) == ["test_1", "test_2"]
     out, err = capsys.readouterr()
     warning = ("cannot execute test relative to others: "
-               "some_module.test_2 - ignoring the marker")
+               "some_module.py::test_2 - ignoring the marker")
     assert warning in out
 
 
@@ -434,5 +434,5 @@ def test_3():
     assert item_names_for(test_content) == ["test_2", "test_1", "test_3"]
     out, err = capsys.readouterr()
     warning = ("cannot execute test relative to others: "
-               "test_dependency_loop.test_3")
+               "test_dependency_loop.py::test_3")
     assert warning in out