From d86d59c0f75f91f97bc2c9d6b4b494ea6c475e0b Mon Sep 17 00:00:00 2001 From: Timothy Allen Date: Sat, 10 Oct 2026 09:03:34 +0200 Subject: [PATCH] Stop the server thread at the end of each server test The server tests left main() in a thread after each test. These threads kept their patches active and delayed the suite. Each test now sends an invalid command, which ends the server loop, and then joins the thread. The suite time drops from about 8.5 seconds to about 2 seconds. Remove one unused line from a test. --- tests/test_server.py | 71 ++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 8e776ea..1f0d009 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -380,6 +380,24 @@ class TestServerMain: sock.close() return json.loads(resp_data) + def _stop_server(self, sock_path: str, thread: threading.Thread) -> None: + """End main(), so that its thread and its patches do not outlive the test. + + An invalid command breaks the server loop. No response follows. + """ + if thread.is_alive(): + sock = socket_module.socket(socket_module.AF_UNIX, socket_module.SOCK_STREAM) + try: + sock.connect(sock_path) + payload = json.dumps({"command": "STOP", "client": "test"}).encode() + sock.sendall(struct.pack("!I", len(payload)) + payload) + except OSError: + pass # The server ended between the check and the connection. + finally: + sock.close() + thread.join(timeout=5) + assert not thread.is_alive() + def test_register_command(self, tmp_path): """Test REGISTER command via server main().""" sock_path = str(tmp_path / "server.sock") @@ -396,20 +414,7 @@ class TestServerMain: response = self._send_command(sock_path, {"command": "REGISTER", "client": "test"}) assert response.get("status") == "REGISTERED" finally: - import os - # Trigger server shutdown by connecting and sending KeyboardInterrupt-triggering command - try: - import socket as _s, struct, json - s = _s.socket(_s.AF_UNIX, _s.SOCK_STREAM) - s.connect(sock_path) - # Send DEREGISTER - payload = json.dumps({"command": "DEREGISTER", "client": "test"}).encode() - s.sendall(struct.pack("!I", len(payload)) + payload) - s.recv(4) - s.close() - except Exception: - pass - t.join(timeout=0.5) + self._stop_server(sock_path, t) def test_get_command(self, tmp_path): """Test GET command via server main().""" @@ -427,7 +432,7 @@ class TestServerMain: response = self._send_command(sock_path, {"command": "GET", "client": "test"}) assert "ttyUSB0" in response or len(response) >= 0 finally: - t.join(timeout=0.5) + self._stop_server(sock_path, t) def test_deregister_command(self, tmp_path): """Test DEREGISTER command via server main().""" @@ -449,7 +454,7 @@ class TestServerMain: r2 = self._send_command(sock_path, {"command": "DEREGISTER", "client": "test"}) assert r2.get("status") == "DEREGISTERED" finally: - t.join(timeout=0.5) + self._stop_server(sock_path, t) def test_get_with_ups_filter(self, tmp_path): """Test GET command with ups filter.""" @@ -468,7 +473,7 @@ class TestServerMain: # Should get ttyUSB0 data or empty (ups filter) assert isinstance(response, dict) finally: - t.join(timeout=0.5) + self._stop_server(sock_path, t) def test_debug_mode_verbose(self, tmp_path, capsys): """Test server main() with debug=1 logs messages.""" @@ -487,7 +492,7 @@ class TestServerMain: except Exception: pass finally: - t.join(timeout=0.5) + self._stop_server(sock_path, t) def test_duplicate_device_name_skipped(self, tmp_path, capsys): """Test that duplicate UPS names are skipped.""" @@ -534,10 +539,12 @@ class TestServerMain: ready.wait(timeout=2) import time as _t _t.sleep(0.2) - # Server should have started with one device - response = self._send_command(sock_path, {"command": "GET", "client": "test"}) - assert "myups" in response - t.join(timeout=0.5) + try: + # Server should have started with one device + response = self._send_command(sock_path, {"command": "GET", "client": "test"}) + assert "myups" in response + finally: + self._stop_server(sock_path, t) def test_socket_dir_created_if_missing(self, tmp_path): """Test that socket dir is created when it doesn't exist.""" @@ -588,7 +595,7 @@ class TestServerMain: self._send_command(sock_path, {"command": "REGISTER", "client": "test"}) except Exception: pass - t.join(timeout=0.5) + self._stop_server(sock_path, t) assert len(makedirs_called) > 0 def test_debug_3_logs_startup(self, tmp_path, capsys): @@ -608,7 +615,7 @@ class TestServerMain: except Exception: pass finally: - t.join(timeout=0.5) + self._stop_server(sock_path, t) captured = capsys.readouterr() # debug>2 triggers "starting up" and "waiting for connection" assert "starting up" in captured.out.lower() or "waiting" in captured.out.lower() @@ -645,7 +652,7 @@ class TestServerMain: response = self._send_command(sock_path, {"command": "DEREGISTER", "client": "ghost"}) assert response.get("status") == "DEREGISTERED" finally: - t.join(timeout=0.5) + self._stop_server(sock_path, t) def test_keyboard_interrupt_closes_connection(self, tmp_path): """Test KeyboardInterrupt handler closes connection when connection is active.""" @@ -705,7 +712,7 @@ class TestServerMain: self._send_command(sock_path, {"command": "REGISTER", "client": "test2"}) except Exception: pass - t.join(timeout=2) + self._stop_server(sock_path, t) def test_socket_read_error_logs_and_continues(self, tmp_path, capsys): """Test that read_request errors are caught and logged.""" @@ -734,7 +741,7 @@ class TestServerMain: sock.close() _t.sleep(0.1) # Give server time to process finally: - t.join(timeout=0.5) + self._stop_server(sock_path, t) def test_root_user_socket_dir_created(self, tmp_path, capsys): """Test root user path when socket dir doesn't exist (chown/chmod triggered).""" @@ -797,7 +804,7 @@ class TestServerMain: self._send_command(sock_path, {"command": "REGISTER", "client": "test"}) except Exception: pass - t.join(timeout=0.5) + self._stop_server(sock_path, t) def test_root_user_setgid_error(self, tmp_path, capsys): """Test root user path when setgid raises OSError.""" @@ -808,8 +815,6 @@ class TestServerMain: from bmspy.server import main as server_main from bmspy.jbd_bms import JBDBMS - fake_bms = JGDBMS() if False else JBDBMS() - def _do_main(): argv = ["bmspy-server", "--socket", sock_path, "--device", "/dev/ttyUSB0"] @@ -857,7 +862,7 @@ class TestServerMain: self._send_command(sock_path, {"command": "REGISTER", "client": "test"}) except Exception: pass - t.join(timeout=0.5) + self._stop_server(sock_path, t) captured = capsys.readouterr() # Should log errors about setgid/setuid assert "gid" in captured.out.lower() or "uid" in captured.out.lower() @@ -920,7 +925,7 @@ class TestServerMain: self._send_command(sock_path, {"command": "REGISTER", "client": "test"}) except Exception: pass - t.join(timeout=0.5) + self._stop_server(sock_path, t) def test_invalid_command_breaks_loop(self, tmp_path, capsys): """Test that an invalid command logs an error.""" @@ -950,7 +955,7 @@ class TestServerMain: except Exception: pass finally: - t.join(timeout=0.5) + self._stop_server(sock_path, t) class TestSendResponseDebug: