From e1bc13b6342b6ec28ca69b0d506ef46ea5536f13 Mon Sep 17 00:00:00 2001
From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
Date: Sat, 3 Oct 2026 02:42:42 +0100
Subject: [PATCH] test: retain ownership of sockets and subprocess groups

---
 tests/cli/test_dev_cli_isolation.py | 10 +++++-----
 tests/test_chroma_client.py         | 22 ++++++++++------------
 tests/test_process_ownership.py     | 11 ++++++-----
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/tests/cli/test_dev_cli_isolation.py b/tests/cli/test_dev_cli_isolation.py
index 267d35994..be017ecf2 100644
--- a/tests/cli/test_dev_cli_isolation.py
+++ b/tests/cli/test_dev_cli_isolation.py
@@ -92,11 +92,11 @@ def test_a_chromadb_we_did_not_start_is_refused_not_adopted(cli, worktree, monke
     ports = cli.derive_ports(worktree)
 
     foreign = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-    foreign.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-    try:
-        foreign.bind(("127.0.0.1", ports["chroma"]))
-    except OSError:
-        pytest.skip(f"derived chroma port {ports['chroma']} is unavailable on this host")
+    foreign.bind(("127.0.0.1", 0))
+    ports["chroma"] = foreign.getsockname()[1]
+    # Port derivation is covered above. This refusal test owns a held socket
+    # rather than depending on a derived port being free on the host.
+    monkeypatch.setattr(cli, "derive_ports", lambda _root: ports)
     foreign.listen(1)
     try:
         with pytest.raises(SystemExit):
diff --git a/tests/test_chroma_client.py b/tests/test_chroma_client.py
index 0a57fee2a..bb8e34aec 100644
--- a/tests/test_chroma_client.py
+++ b/tests/test_chroma_client.py
@@ -12,19 +12,17 @@ import pytest
 import src.chroma_client as cc
 
 
-def _free_port() -> int:
-    """Bind to port 0, grab the assigned port, release it — nothing listens."""
-    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-    s.bind(("127.0.0.1", 0))
-    port = s.getsockname()[1]
-    s.close()
-    return port
+@pytest.fixture
+def closed_port():
+    """Reserve a port without listening, so another worker cannot take it."""
+    with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as reserved:
+        reserved.bind(("127.0.0.1", 0))
+        yield reserved.getsockname()[1]
 
 
-def test_port_open_false_for_closed_port_and_is_fast():
-    port = _free_port()
+def test_port_open_false_for_closed_port_and_is_fast(closed_port):
     t0 = time.monotonic()
-    assert cc._port_open("127.0.0.1", port, timeout=1.0) is False
+    assert cc._port_open("127.0.0.1", closed_port, timeout=1.0) is False
     # The whole point: we fail fast, nowhere near the 30-60s OS timeout.
     assert time.monotonic() - t0 < 5.0
 
@@ -40,11 +38,11 @@ def test_port_open_true_for_listening_socket():
         srv.close()
 
 
-def test_get_chroma_client_does_not_cache_when_unreachable(monkeypatch):
+def test_get_chroma_client_does_not_cache_when_unreachable(monkeypatch, closed_port):
     pytest.importorskip("chromadb")
     cc.reset_client()
     monkeypatch.setenv("CHROMADB_HOST", "127.0.0.1")
-    monkeypatch.setenv("CHROMADB_PORT", str(_free_port()))
+    monkeypatch.setenv("CHROMADB_PORT", str(closed_port))
     with pytest.raises(RuntimeError):
         cc.get_chroma_client()
     # A failed connection must leave the singleton unset so a later call
diff --git a/tests/test_process_ownership.py b/tests/test_process_ownership.py
index 571ff1247..8727cdfc3 100644
--- a/tests/test_process_ownership.py
+++ b/tests/test_process_ownership.py
@@ -15,6 +15,7 @@ because an absent mechanism read as a successful answer.
 """
 
 import os
+import signal
 import subprocess
 
 import pytest
@@ -35,11 +36,11 @@ def sleeper():
 
     yield _spawn
     for proc in procs:
-        try:
-            proc.kill()
-            proc.wait(timeout=5)
-        except Exception:
-            pass
+        # Each child owns a session/process group. Keep the leader unreaped
+        # until its group is signalled, so the group id cannot be recycled.
+        if proc.returncode is None:
+            os.killpg(proc.pid, signal.SIGKILL)
+        proc.wait(timeout=5)
 
 
 # ── Verdicts, against real processes ────────────────────────────────────────