5/5 fix: stabilise duration_based_chunks across collection orders

.github/workflows/test.yml

@@ -44,7 +44,8 @@ jobs:
       - uses: ./.github/actions/python-poetry-env
         with:
           python-version: ${{ matrix.python-version }}
-      - run: poetry run pytest
+      - run: poetry run coverage run -m pytest
+      - run: poetry run coverage report
 
   docs:
     runs-on: ubuntu-latest

CHANGELOG.md

@@ -4,6 +4,28 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Changed
+- `AlgorithmBase.__call__` now takes a single `durations: dict[Item, float]`
+  argument instead of separate `items` and `durations` arguments. Custom
+  algorithm subclasses must update their signature. Use the new public
+  `pytest_split.algorithms.compute_durations(items, cached_durations)` helper to
+  build the dict the same way the plugin does.
+- Algorithms now own only group membership; the order of `selected` items in
+  the returned `TestGroup`s is implementation-defined. The plugin rebuilds the
+  chosen group's `selected` and `deselected` lists in pytest's collection
+  order before the test session executes, so end-to-end behaviour is
+  unchanged.
+- `duration_based_chunks` now computes group membership from a canonical
+  (nodeid-sorted) traversal, so splits are stable across pytest collection
+  orders (e.g. when parametrising over a `set`/`frozenset`/`dict.keys()` whose
+  iteration order is hash-randomised). Within-group execution order is
+  unchanged: pytest still runs each group's tests in the order it collected
+  them. Notebook cell node-ids that contain numeric suffixes
+  (`nb.ipynb::cell_0..cell_10`) sort alphabetically (`cell_0, cell_1, cell_10,
+  cell_11, cell_2, …`) for the membership pass; `ensure_ipynb_compatibility`
+  still pulls all sibling cells into the same group, so user-visible behaviour
+  is correct.
+
 ### Fixed
 - Fix malformed bullet points rendering in GitHub Pages documentation
 

README.md

@@ -60,9 +60,12 @@ Lists the slowest tests based on the information stored in the test durations fi
 
 ## Interactions with other pytest plugins
 * [`pytest-random-order`](https://github.com/jbasko/pytest-random-order) and [`pytest-randomly`](https://github.com/pytest-dev/pytest-randomly):
-   ⚠️ `pytest-split` running with the `duration_based_chunks` algorithm is **incompatible** with test-order-randomization plugins.
-  Test selection in the groups happens after randomization, potentially causing some tests to be selected in several groups and others not at all.
-  Instead, a global random seed needs to be computed before running the tests (for example using `$RANDOM` from the shell) and that single seed then needs to be used for all groups by setting the `--random-order-seed` option.
+  Group **membership** is a pure function of the (nodeid, duration) multiset
+  and the requested split count for both algorithms, so test-order-randomisation
+  plugins no longer cause tests to drift between groups. Within-group execution
+  order still follows pytest's collection order, so randomisation plugins
+  continue to influence the order in which a group's tests run — typically
+  the desired interaction.
 
 * [`nbval`](https://github.com/computationalmodelling/nbval): `pytest-split` could, in principle, break up a single IPython Notebook into different test groups. This most likely causes broken up pieces to fail (for the very least, package `import`s are usually done at Cell 1, and so, any broken up piece that doesn't contain Cell 1 will certainly fail). To avoid this, after splitting step is done, test groups are reorganized based on a simple algorithm illustrated in the following cartoon:
 
@@ -74,18 +77,17 @@ where the letters (A to E) refer to individual IPython Notebooks, and the number
 The plugin supports multiple algorithms to split tests into groups.
 Each algorithm makes different tradeoffs, but generally `least_duration` should give more balanced groups.
 
-| Algorithm      | Maintains Absolute Order | Maintains Relative Order | Split Quality | Works with random ordering |
-|----------------|--------------------------|--------------------------|---------------|----------------------------|
-| duration_based_chunks | ✅                | ✅                       | Good          | ❌                         |
-| least_duration | ❌                       | ✅                       | Better        | ✅                         |
+| Algorithm      | Maintains Relative Order | Split Quality | Stable across collection orders |
+|----------------|--------------------------|---------------|---------------------------------|
+| duration_based_chunks | ✅                | Good          | ✅                              |
+| least_duration | ✅                       | Better        | ✅                              |
 
 Explanation of the terms in the table:
 
-* Absolute Order: whether each group contains all tests between first and last element in the same order as the original list of tests
-* Relative Order: whether each test in each group has the same relative order to its neighbours in the group as in the original list of tests
-* Works with random ordering: whether the algorithm works with test-shuffling tools such as [`pytest-randomly`](https://github.com/pytest-dev/pytest-randomly)
+* Relative Order: whether each test in each group has the same relative order to its neighbours in the group as in the original (collection) list of tests.
+* Stable across collection orders: whether each group's *membership* is a pure function of the (nodeid, duration) multiset and the requested split count, irrespective of pytest's collection order. Both algorithms satisfy this; within-group execution order continues to follow pytest's collection order.
 
-The `duration_based_chunks` algorithm aims to find optimal boundaries for the list of tests and every test group contains all tests between the start and end boundary.
+The `duration_based_chunks` algorithm aims to find optimal boundaries by walking the tests in canonical (nodeid-sorted) order and packing them into contiguous slices; the chosen group is then emitted to pytest in collection order.
 The `least_duration` algorithm walks the list of tests and assigns each test to the group with the smallest current duration.
 
 

pyproject.toml

@@ -63,16 +63,12 @@ pytest-split = "pytest_split.plugin"
 target-version = ["py310", "py311", "py312", "py313", "py314"]
 include = '\.pyi?$'
 
-[tool.pytest.ini_options]
-addopts = """\
-    --cov pytest_split \
-    --cov tests \
-    --cov-report term-missing \
-    --no-cov-on-fail \
-"""
+[tool.coverage.run]
+source = ["pytest_split"]
 
 [tool.coverage.report]
-fail_under = 90
+fail_under = 95
+show_missing = true
 exclude_lines = [
     'if TYPE_CHECKING:',
     'pragma: no cover'

src/pytest_split/algorithms.py

@@ -19,7 +19,7 @@ class AlgorithmBase(ABC):
 
     @abstractmethod
     def __call__(
-        self, splits: int, items: "list[nodes.Item]", durations: "dict[str, float]"
+        self, splits: int, durations: "dict[nodes.Item, float]"
     ) -> "list[TestGroup]":
         pass
 
@@ -42,47 +42,39 @@ class LeastDurationAlgorithm(AlgorithmBase):
     maintaining the original order of items. It is therefore important that the order of items be identical on all nodes
     that use this plugin. Due to issue #25 this might not always be the case.
 
+    The order of ``selected`` items in each returned group is implementation-defined; the plugin reorders the chosen
+    group in pytest's collection order before execution.
+
     :param splits: How many groups we're splitting in.
-    :param items: Test items passed down by Pytest.
-    :param durations: Our cached test runtimes. Assumes contains timings only of relevant tests
+    :param durations: Mapping from each test item to its duration. Build it with :func:`compute_durations`.
     :return:
         List of groups
     """
 
     def __call__(
-        self, splits: int, items: "list[nodes.Item]", durations: "dict[str, float]"
+        self, splits: int, durations: "dict[nodes.Item, float]"
     ) -> "list[TestGroup]":
-        items_with_durations = _get_items_with_durations(items, durations)
-
-        # add index of item in list
-        items_with_durations_indexed = [
-            (*tup, i) for i, tup in enumerate(items_with_durations)
-        ]
-
-        # Sort by name to ensure it's always the same order
-        items_with_durations_indexed = sorted(
-            items_with_durations_indexed, key=lambda tup: str(tup[0])
-        )
-
-        # sort in ascending order
+        # Sort by duration descending so the heap-based assignment
+        # processes the largest tests first - greedy first-fit-decreasing
+        # for the smallest-running-total bin.
         sorted_items_with_durations = sorted(
-            items_with_durations_indexed, key=lambda tup: tup[1], reverse=True
+            durations.items(), key=itemgetter(1), reverse=True
         )
 
-        selected: list[list[tuple[nodes.Item, int]]] = [[] for _ in range(splits)]
+        selected: list[list[nodes.Item]] = [[] for _ in range(splits)]
         deselected: list[list[nodes.Item]] = [[] for _ in range(splits)]
         duration: list[float] = [0 for _ in range(splits)]
 
         # create a heap of the form (summed_durations, group_index)
         heap: list[tuple[float, int]] = [(0, i) for i in range(splits)]
         heapq.heapify(heap)
-        for item, item_duration, original_index in sorted_items_with_durations:
+        for item, item_duration in sorted_items_with_durations:
             # get group with smallest sum
             summed_durations, group_idx = heapq.heappop(heap)
             new_group_durations = summed_durations + item_duration
 
             # store assignment
-            selected[group_idx].append((item, original_index))
+            selected[group_idx].append(item)
             duration[group_idx] = new_group_durations
             for i in range(splits):
                 if i != group_idx:
@@ -91,19 +83,12 @@ def __call__(
             # store new duration - in case of ties it sorts by the group_idx
             heapq.heappush(heap, (new_group_durations, group_idx))
 
-        groups = []
-        for i in range(splits):
-            # sort the items by their original index to maintain relative ordering
-            # we don't care about the order of deselected items
-            s = [
-                item
-                for item, original_index in sorted(selected[i], key=lambda tup: tup[1])
-            ]
-            group = TestGroup(
-                selected=s, deselected=deselected[i], duration=duration[i]
+        return [
+            TestGroup(
+                selected=selected[i], deselected=deselected[i], duration=duration[i]
             )
-            groups.append(group)
-        return groups
+            for i in range(splits)
+        ]
 
 
 class DurationBasedChunksAlgorithm(AlgorithmBase):
@@ -114,23 +99,21 @@ class DurationBasedChunksAlgorithm(AlgorithmBase):
     and creating group_1 = items[0:i_0], group_2 = items[i_0, i_1], group_3 = items[i_1, i_2], ...
 
     :param splits: How many groups we're splitting in.
-    :param items: Test items passed down by Pytest.
-    :param durations: Our cached test runtimes. Assumes contains timings only of relevant tests
+    :param durations: Mapping from each test item to its duration. Build it with :func:`compute_durations`.
     :return: List of TestGroup
     """
 
     def __call__(
-        self, splits: int, items: "list[nodes.Item]", durations: "dict[str, float]"
+        self, splits: int, durations: "dict[nodes.Item, float]"
     ) -> "list[TestGroup]":
-        items_with_durations = _get_items_with_durations(items, durations)
-        time_per_group = sum(map(itemgetter(1), items_with_durations)) / splits
+        time_per_group = sum(durations.values()) / splits
 
         selected: list[list[nodes.Item]] = [[] for i in range(splits)]
         deselected: list[list[nodes.Item]] = [[] for i in range(splits)]
         duration: list[float] = [0 for i in range(splits)]
 
         group_idx = 0
-        for item, item_duration in items_with_durations:
+        for item, item_duration in durations.items():
             if duration[group_idx] >= time_per_group:
                 group_idx += 1
 
@@ -148,33 +131,43 @@ def __call__(
         ]
 
 
-def _get_items_with_durations(
-    items: "list[nodes.Item]", durations: "dict[str, float]"
-) -> "list[tuple[nodes.Item, float]]":
-    durations = _remove_irrelevant_durations(items, durations)
-    avg_duration_per_test = _get_avg_duration_per_test(durations)
-    items_with_durations = [
-        (item, durations.get(item.nodeid, avg_duration_per_test)) for item in items
-    ]
-    return items_with_durations
-
+def compute_durations(
+    items: "list[nodes.Item]", cached_durations: "dict[str, float]"
+) -> "dict[nodes.Item, float]":
+    """
+    Build the splitting input from collected items and their cached durations.
 
-def _get_avg_duration_per_test(durations: "dict[str, float]") -> float:
-    if durations:
-        avg_duration_per_test = sum(durations.values()) / len(durations)
+    Items missing from ``cached_durations`` get the average duration of the
+    cached entries that are relevant to this suite; with no cached data at
+    all, every item gets ``1`` as a placeholder.
+    """
+    # Filtering down durations to relevant ones ensures the avg isn't skewed by irrelevant data
+    relevant = {
+        item.nodeid: cached_durations[item.nodeid]
+        for item in items
+        if item.nodeid in cached_durations
+    }
+    if relevant:
+        avg = sum(relevant.values()) / len(relevant)
     else:
         # If there are no durations, give every test the same arbitrary value
-        avg_duration_per_test = 1
-    return avg_duration_per_test
+        avg = 1
+    return {item: relevant.get(item.nodeid, avg) for item in items}
 
 
-def _remove_irrelevant_durations(
-    items: "list[nodes.Item]", durations: "dict[str, float]"
-) -> "dict[str, float]":
-    # Filtering down durations to relevant ones ensures the avg isn't skewed by irrelevant data
-    test_ids = [item.nodeid for item in items]
-    durations = {name: durations[name] for name in test_ids if name in durations}
-    return durations
+def select_in_collection_order(
+    group: TestGroup, items: "list[nodes.Item]"
+) -> TestGroup:
+    """
+    Rebuild ``group`` so that ``selected`` and ``deselected`` filter
+    ``items`` in their original collection order, keyed on nodeid.
+    """
+    selected_ids = {it.nodeid for it in group.selected}
+    return TestGroup(
+        selected=[it for it in items if it.nodeid in selected_ids],
+        deselected=[it for it in items if it.nodeid not in selected_ids],
+        duration=group.duration,
+    )
 
 
 class Algorithms(enum.Enum):

src/pytest_split/plugin.py

@@ -160,8 +160,15 @@ def pytest_collection_modifyitems(
         group_idx: int = config.option.group
 
         algo = algorithms.Algorithms[config.option.splitting_algorithm].value
-        groups = algo(splits, items, self.cached_durations)
-        group = groups[group_idx - 1]
+        durations = algorithms.compute_durations(items, self.cached_durations)
+        # Pass items in canonical (nodeid-sorted) order so group membership is
+        # independent of the order pytest happened to collect them.
+        durations = {
+            item: durations[item]
+            for item in sorted(durations, key=lambda item: item.nodeid)
+        }
+        groups = algo(splits, durations)
+        group = algorithms.select_in_collection_order(groups[group_idx - 1], items)
 
         ensure_ipynb_compatibility(group, items)