Repository: ESSS/pytest-replay (language: python)

Issue:
pytest-replay does not respect the order of the replay file
Currently, pytest-replay does not respect the order in the replay file, but it just filters the tests in the order collected by pytest:

```
λ cat replay.txt
{"nodeid": "tests/foo_test.py::test_foo_1"}
{"nodeid": "tests/bar_test.py::test_bar_1"}

λ pytest --replay=replay.txt -v
================ test session start ==============
platform win32 -- Python 3.10.12, pytest-7.4.4, pluggy-1.3.0 -- C:\Users\CantonDavide\source\prove\prova_pytest\.venv\Scripts\python.exe
cachedir: .pytest_cache
rootdir: C:\Users\CantonDavide\source\prove\prova_pytest
plugins: replay-1.4.0
collected 4 items / 2 deselected / 2 selected

tests/bar_test.py::test_bar_1 PASSED                 [ 50%]
tests/foo_test.py::test_foo_1 PASSED                 [100%]

============== 2 passed, 2 deselected in 0.01s ==============

```

After reading the implementation of `pytest_collection_modifyitems` in the plugin, I've seen that this is how the plugin is implemented now, since it just loads a set of nodeids from the replay, but it is used just to filter away the unselected items.

Tweaking a bit the function I've managed to reproduce what I expected:

```py
def pytest_collection_modifyitems(self, items, config):
    replay_file = config.getoption("replay_file")
    if not replay_file:
        return

    with open(replay_file, "r", encoding="UTF-8") as f:
        all_lines = f.readlines()
        nodeids = dict.fromkeys(
            json.loads(line)["nodeid"]
            for line in all_lines
            if not line.strip().startswith(("#", "//"))
        )
    

    remaining = []
    inds = set()
    for n in nodeids:
        ind = -1
        for i, item in enumerate(items):
            if item.nodeid == n:
                ind = i
                break
        if ind >= 0:
            remaining.append(items[i])
            inds.add(ind)


    deselected = [item for i, item in enumerate(items) if i not in inds]
    if deselected:
        config.hook.pytest_deselected(items=deselected)
    items[:] = remaining
```

The implementation is a bit raw since it was just a test.

What about adding this feature, maybe with it being optional and disabled by default? I can provide a PR if it's feasible to add as a feature.


Discussion / hints:
Hey @DavideCanton thanks for the detailed problem description!

Indeed it seems you are correct, it definitely seems like an oversight on our part -- the intention is to reproduce exactly what was executed, so the order of course matters.

Would you like to open a PR? I would be happy to review one.
Sure, in the next days, I'll create a PR!
Awesome, thanks!

Fix patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 1ef5272..d386003 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,5 +1,5 @@
 repos:
--   repo: https://github.com/psf/black
+-   repo: https://github.com/psf/black-pre-commit-mirror
     rev: 23.12.1
     hooks:
     -   id: black
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 8000273..35cd516 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,3 +1,10 @@
+UNRELEASED
+==========
+
+* Test execution order using ``--replay`` now follows the recorded order, not the collection order, as was always intended (`#52`_).
+
+.. _`#52`: https://github.com/ESSS/pytest-replay/pull/53
+
 1.4.0 (2021-06-09)
 ==================
 


Test patch:
diff --git a/src/pytest_replay/__init__.py b/src/pytest_replay/__init__.py
index 7e3a3d8..cf6c371 100644
--- a/src/pytest_replay/__init__.py
+++ b/src/pytest_replay/__init__.py
@@ -106,22 +106,26 @@ def pytest_collection_modifyitems(self, items, config):
 
         with open(replay_file, "r", encoding="UTF-8") as f:
             all_lines = f.readlines()
-            nodeids = {
+            # Use a dict to deduplicate the node ids while keeping the order.
+            nodeids = dict.fromkeys(
                 json.loads(line)["nodeid"]
                 for line in all_lines
                 if not line.strip().startswith(("#", "//"))
-            }
+            )
+
+        items_dict = {item.nodeid: item for item in items}
         remaining = []
-        deselected = []
-        for item in items:
-            if item.nodeid in nodeids:
+        # Make sure to respect the order from the JSON file (#52).
+        for nodeid in nodeids:
+            item = items_dict.pop(nodeid)
+            if item:
                 remaining.append(item)
-            else:
-                deselected.append(item)
+        deselected = list(items_dict.values())
 
         if deselected:
             config.hook.pytest_deselected(items=deselected)
-            items[:] = remaining
+
+        items[:] = remaining
 
     def append_test_to_script(self, nodeid, line):
         suffix = "-" + self.xdist_worker_name if self.xdist_worker_name else ""
diff --git a/tests/test_replay.py b/tests/test_replay.py
index ed63215..cebc6d2 100644
--- a/tests/test_replay.py
+++ b/tests/test_replay.py
@@ -1,3 +1,4 @@
+import itertools as it
 import json
 
 import pytest
@@ -216,3 +217,56 @@ def test_2():
     expected = {"test_cwd_changed.py::test_1", "test_cwd_changed.py::test_2"}
     assert contents == expected
     assert result.ret == 0
+
+
+@pytest.mark.usefixtures("suite")
+def test_execution_different_order(testdir):
+    """Ensure tests execute in the order defined by the JSON file, not collection (#52)."""
+    dir = testdir.tmpdir / "replay"
+    options = [f"--replay-record-dir={dir}"]
+    result = testdir.runpytest(*options)
+
+    replay_file = dir / ".pytest-replay.txt"
+
+    with replay_file.open("r+") as f:
+        content = f.readlines()
+
+        # pairwise shuffle of replay file
+        pairs = [(content[i], content[i + 1]) for i in range(0, len(content), 2)]
+        pairs = [pairs[2], pairs[0], pairs[3], pairs[1]]
+        content = list(it.chain.from_iterable(pairs))
+
+        f.seek(0)
+        f.writelines(content)
+
+    result = testdir.runpytest(f"--replay={replay_file}", "-v")
+    assert result.ret == 0
+    result.stdout.fnmatch_lines(
+        [
+            "test_2.py::test_zz*25%*",
+            "test_1.py::test_foo*50%*",
+            "test_3.py::test_foobar*75%*",
+            "test_1.py::test_bar*100%*",
+        ],
+        consecutive=True,
+    )
+
+
+@pytest.mark.usefixtures("suite")
+def test_filter_out_tests_not_in_file(testdir):
+    """Tests not found in the JSON file should not run."""
+    dir = testdir.tmpdir / "replay"
+    options = [f"--replay-record-dir={dir}", "-k", "foo"]
+    result = testdir.runpytest(*options)
+
+    replay_file = dir / ".pytest-replay.txt"
+
+    result = testdir.runpytest(f"--replay={replay_file}", "-v")
+    assert result.ret == 0
+    result.stdout.fnmatch_lines(
+        [
+            "test_1.py::test_foo*50%*",
+            "test_3.py::test_foobar*100%*",
+        ],
+        consecutive=True,
+    )