loadscope: forget a dead worker's collection so a still-collecting worker isn't mistaken for done

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.

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
 

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")