diff --git a/changelog/1362.bugfix.rst b/changelog/1362.bugfix.rst new file mode 100644 index 00000000..19fc7559 --- /dev/null +++ b/changelog/1362.bugfix.rst @@ -0,0 +1 @@ +Fixed an ``INTERNALERROR`` (``KeyError``) with ``--dist=loadscope``/``--dist=loadfile`` when a worker died before the collection completed: the dead worker's entry is now dropped from the scheduler's registered collections, so a still-collecting worker is no longer mistaken for a completed one. diff --git a/src/xdist/scheduler/loadscope.py b/src/xdist/scheduler/loadscope.py index 73162dcd..154e1cae 100644 --- a/src/xdist/scheduler/loadscope.py +++ b/src/xdist/scheduler/loadscope.py @@ -179,6 +179,10 @@ def remove_node(self, node: WorkerController) -> str | None: node has no more pending items. """ workload = self.assigned_work.pop(node) + # Drop the dead node's collection too. Leaving it behind keeps ``collection_is_completed`` + # counting a node that is gone, so a *later* still-collecting worker looks done and + # ``_assign_work_unit`` then raises ``KeyError`` indexing ``registered_collections`` for it. + self.registered_collections.pop(node, None) if not self._pending_of(workload): return None diff --git a/testing/test_dsession.py b/testing/test_dsession.py index 680b7ae0..b57f094b 100644 --- a/testing/test_dsession.py +++ b/testing/test_dsession.py @@ -15,6 +15,7 @@ from xdist.report import report_collection_diff from xdist.scheduler import EachScheduling from xdist.scheduler import LoadScheduling +from xdist.scheduler import LoadScopeScheduling from xdist.scheduler import WorkStealingScheduling from xdist.workermanage import WorkerController @@ -290,6 +291,37 @@ def pytest_collectreport(self, report: pytest.CollectReport) -> None: assert "Different tests were collected between" in rep.longrepr +class TestLoadScopeScheduling: + def test_remove_node_forgets_dead_worker_collection( + self, pytester: pytest.Pytester + ) -> None: + # A dead worker must not keep inflating ``collection_is_completed``: otherwise a still-collecting + # worker looks done, ``schedule()`` runs early, and ``_assign_work_unit`` crashes with a + # ``KeyError`` indexing ``registered_collections`` for the worker that never registered. + config = pytester.parseconfig("--tx=3*popen", "--dist=loadscope") + sched = LoadScopeScheduling(config) + node_a, node_b, node_c = MockNode(), MockNode(), MockNode() + for node in (node_a, node_b, node_c): + sched.add_node(node) + collection = ["test_a.py::test_1", "test_a.py::test_2"] + sched.add_node_collection(node_a, collection) + sched.add_node_collection(node_b, collection) + # node_c is still collecting, so the run is not ready to schedule. + assert not sched.collection_is_completed + + # node_a dies before the collection completed. + assert sched.remove_node(node_a) is None + assert node_a not in sched.registered_collections + + # A replacement worker joins and reports its collection. + node_d = MockNode() + sched.add_node(node_d) + sched.add_node_collection(node_d, collection) + + # With the dead node forgotten, we still wait for node_c instead of scheduling early. + assert not sched.collection_is_completed + + class TestWorkStealingScheduling: def test_ideal_case(self, pytester: pytest.Pytester) -> None: config = pytester.parseconfig("--tx=2*popen")