Skip to content

Commit f08afd7

Browse files
committed
Fix KeyError when replacement worker has mismatched collection
When a worker crashes and is replaced, if the replacement worker's collection doesn't match the original, add_node_collection() returns early without adding the node to registered_collections. Later, when schedule() calls _reschedule() on all nodes, _assign_work_unit() crashes with KeyError accessing registered_collections[node]. Add a guard in _reschedule() to skip nodes that aren't in registered_collections. Fixes #1189 Fixes #714
1 parent 04c3ca8 commit f08afd7

2 files changed

Lines changed: 90 additions & 0 deletions

File tree

src/xdist/scheduler/loadscope.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,11 @@ def _reschedule(self, node: WorkerController) -> None:
320320
if node.shutting_down:
321321
return
322322

323+
# Skip nodes that aren't properly registered (e.g., collection mismatch)
324+
# See https://github.com/pytest-dev/pytest-xdist/issues/1189
325+
if node not in self.registered_collections:
326+
return
327+
323328
# Check that more work is available
324329
if not self.workqueue:
325330
node.shutdown()

testing/test_loadscope.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
"""Tests for LoadScopeScheduling."""
2+
from __future__ import annotations
3+
4+
from collections.abc import Sequence
5+
6+
import pytest
7+
8+
from xdist.scheduler import LoadScopeScheduling
9+
10+
11+
class MockGateway:
12+
_count = 0
13+
14+
def __init__(self) -> None:
15+
self.id = str(MockGateway._count)
16+
MockGateway._count += 1
17+
18+
19+
class MockNode:
20+
def __init__(self) -> None:
21+
self.sent: list[int] = []
22+
self.gateway = MockGateway()
23+
self._shutdown = False
24+
25+
def send_runtest_some(self, indices: Sequence[int]) -> None:
26+
self.sent.extend(indices)
27+
28+
def shutdown(self) -> None:
29+
self._shutdown = True
30+
31+
@property
32+
def shutting_down(self) -> bool:
33+
return self._shutdown
34+
35+
36+
@pytest.fixture(autouse=True)
37+
def reset_mock_gateway_counter() -> None:
38+
MockGateway._count = 0
39+
40+
41+
class TestLoadScopeScheduling:
42+
def test_replacement_worker_with_mismatched_collection_is_skipped(
43+
self, pytester: pytest.Pytester
44+
) -> None:
45+
"""Regression test for https://github.com/pytest-dev/pytest-xdist/issues/1189"""
46+
config = pytester.parseconfig("--tx=2*popen")
47+
sched = LoadScopeScheduling(config)
48+
49+
node1, node2 = MockNode(), MockNode()
50+
sched.add_node(node1)
51+
sched.add_node(node2)
52+
53+
collection = [
54+
"test_mod.py::test_a",
55+
"test_mod.py::test_b",
56+
"test_other.py::test_c",
57+
"test_other.py::test_d",
58+
]
59+
sched.add_node_collection(node1, collection)
60+
sched.add_node_collection(node2, collection)
61+
sched.schedule()
62+
63+
# Simulate node1 crashing
64+
sched.remove_node(node1)
65+
66+
# Replacement worker collects different tests (e.g., due to test file changes)
67+
replacement_node = MockNode()
68+
sched.add_node(replacement_node)
69+
different_collection = [
70+
"test_mod.py::test_a",
71+
"test_mod.py::test_b",
72+
"test_mod.py::test_NEW", # Different
73+
"test_other.py::test_d",
74+
]
75+
sched.add_node_collection(replacement_node, different_collection)
76+
77+
# Replacement node should not be in registered_collections due to mismatch
78+
assert replacement_node not in sched.registered_collections
79+
assert replacement_node in sched.assigned_work
80+
81+
# schedule() should skip unregistered nodes rather than crashing
82+
sched.schedule()
83+
84+
assert replacement_node.sent == []
85+
assert node2 in sched.registered_collections

0 commit comments

Comments
 (0)