From 25953bea063c74972410b676733414d4e932696d Mon Sep 17 00:00:00 2001 From: kovagoadi Date: Wed, 26 Aug 2026 20:57:56 +0200 Subject: [PATCH] Fix ruff lint findings across client, protocol, and tests Resolve 51 findings from the I/RUF/BLE/TRY002/S110/PLR0402 rule set: - Sort imports and __all__ (I001, RUF022, PLR0402). The sys.path.insert calls in check_public_calls.py and tests/test_e2e.py still precede the openttd imports that depend on them. - Replace unused unpacked values with _ (RUF059) and annotate the two timetable lookup tables as ClassVar (RUF012). - Narrow the best-effort excepts in OpenTTDClient.quit and OpenTTDAdminClient.quit to (OSError, SocketClosed) and log at debug rather than swallowing silently (BLE001, S110). The test doubles now raise an OSError subclass so they still exercise that branch. - Narrow the gamescript JSON fallback to json.JSONDecodeError. The broad catch in receive_packet keeps a noqa: it guards untrusted wire data and must degrade to a no-op packet instead of killing the connection. - Use contextlib.suppress instead of try/except/pass in tests. ruff check . is clean, 102 tests pass, coverage stays at 100%. Co-Authored-By: Claude --- check_public_calls.py | 9 +++--- lib/openttd/__init__.py | 4 +-- lib/openttd/client.py | 63 ++++++++++++++++++++++++++++++----------- lib/openttd/protocol.py | 18 ++++++------ main.py | 4 +-- main_admin.py | 8 +++--- tests/conftest.py | 4 ++- tests/test_admin.py | 13 ++++++--- tests/test_coverage.py | 13 ++++++--- tests/test_e2e.py | 22 +++++++------- tests/test_logic.py | 11 +++++-- tests/test_protocol.py | 18 ++++++------ tests/test_timetable.py | 23 ++++++++++----- 13 files changed, 136 insertions(+), 74 deletions(-) diff --git a/check_public_calls.py b/check_public_calls.py index 839ee61..2877105 100755 --- a/check_public_calls.py +++ b/check_public_calls.py @@ -1,16 +1,17 @@ #!/usr/bin/env python3 import ast import inspect -import sys import os +import sys # Add lib and tests to path sys.path.insert(0, os.path.join(os.path.dirname(__file__), 'lib')) sys.path.insert(0, os.path.dirname(__file__)) -from openttd import OpenTTDClient, OpenTTDAdminClient -from openttd.protocol import OpenTTDProtocol, OpenTTDAdminProtocol -import tests.test_e2e as test_e2e +from openttd import OpenTTDAdminClient, OpenTTDClient +from openttd.protocol import OpenTTDAdminProtocol, OpenTTDProtocol + +from tests import test_e2e # 1. Gather public functions dynamically at runtime using reflection classes = [OpenTTDClient, OpenTTDAdminClient, OpenTTDProtocol, OpenTTDAdminProtocol] diff --git a/lib/openttd/__init__.py b/lib/openttd/__init__.py index a24152d..33e0b04 100644 --- a/lib/openttd/__init__.py +++ b/lib/openttd/__init__.py @@ -1,4 +1,4 @@ +from .client import OpenTTDAdminClient, OpenTTDClient from .decorators import exclude_call_check -from .client import OpenTTDClient, OpenTTDAdminClient -__all__ = ['OpenTTDClient', 'OpenTTDAdminClient', 'exclude_call_check'] +__all__ = ['OpenTTDAdminClient', 'OpenTTDClient', 'exclude_call_check'] diff --git a/lib/openttd/client.py b/lib/openttd/client.py index 6687597..758ec44 100644 --- a/lib/openttd/client.py +++ b/lib/openttd/client.py @@ -1,18 +1,42 @@ import asyncio -import logging -import uuid -import monocypher -import os import hashlib -from openttd_protocol.wire.write import write_init, write_string, write_uint8, write_uint16, write_uint32, write_presend, SEND_TCP_MTU +import logging +import os +import uuid +from typing import ClassVar + +import monocypher +from openttd_protocol.wire.exceptions import SocketClosed from openttd_protocol.wire.read import read_uint8, read_uint16 -from .protocol import ( - PacketGameType, OpenTTDProtocol, PacketAdminType, OpenTTDAdminProtocol, NetworkAuthenticationMethod, - GameCommand, ModifyTimetableFlags, ModifyTimetableCtrlFlag, - OrderType, OrderStopLocation, INVALID_VEH_ORDER_ID, - write_varuint, read_varuint, write_varuint_signed, read_varuint_signed +from openttd_protocol.wire.write import ( + SEND_TCP_MTU, + write_init, + write_presend, + write_string, + write_uint8, + write_uint16, + write_uint32, ) + from .decorators import exclude_call_check +from .protocol import ( + INVALID_VEH_ORDER_ID, + GameCommand, + ModifyTimetableCtrlFlag, + ModifyTimetableFlags, + NetworkAuthenticationMethod, + OpenTTDAdminProtocol, + OpenTTDProtocol, + OrderStopLocation, + OrderType, + PacketAdminType, + PacketGameType, + read_varuint, + read_varuint_signed, + write_varuint, + write_varuint_signed, +) + class OpenTTDClient: """High-level OpenTTD client for easy integration.""" @@ -261,8 +285,9 @@ class OpenTTDClient: try: d = write_init(PacketGameType.ClientQuit) await self._protocol.send_packet(write_presend(d, SEND_TCP_MTU)) - except Exception: - pass + except (OSError, SocketClosed) as e: + # Best-effort courtesy packet: the socket may already be gone. + self.log.debug(f"Could not send quit packet: {e}") self._transport.close() self.shutdown_event.set() @@ -386,7 +411,7 @@ class OpenTTDClient: async def receive_ServerMapData(self, source, **kwargs): pass async def receive_ServerConfigurationUpdate(self, source, **kwargs): pass async def receive_ServerExternalChat(self, source, **kwargs): pass - _TIMETABLE_FIELD_BY_FLAG = { + _TIMETABLE_FIELD_BY_FLAG: ClassVar[dict[ModifyTimetableFlags, str]] = { ModifyTimetableFlags.WaitTime: "wait_time", ModifyTimetableFlags.TravelTime: "travel_time", ModifyTimetableFlags.TravelSpeed: "travel_speed", @@ -395,7 +420,10 @@ class OpenTTDClient: ModifyTimetableFlags.SetLeaveType: "leave_type", ModifyTimetableFlags.AssignSchedule: "assigned_schedule", } - _TIMETABLE_BOOL_FLAGS = {ModifyTimetableFlags.SetWaitFixed, ModifyTimetableFlags.SetTravelFixed} + _TIMETABLE_BOOL_FLAGS: ClassVar[set[ModifyTimetableFlags]] = { + ModifyTimetableFlags.SetWaitFixed, + ModifyTimetableFlags.SetTravelFixed, + } async def receive_ServerCommand(self, source, cmd, payload, **kwargs): if cmd == GameCommand.ChangeTimetable: @@ -512,8 +540,9 @@ class OpenTTDAdminClient: try: d = write_init(PacketAdminType.AdminQuit) await self._protocol.send_packet(write_presend(d, SEND_TCP_MTU)) - except Exception: - pass + except (OSError, SocketClosed) as e: + # Best-effort courtesy packet: the socket may already be gone. + self.log.debug(f"Could not send admin quit packet: {e}") self._transport.close() self.shutdown_event.set() @@ -597,7 +626,7 @@ class OpenTTDAdminClient: Raises asyncio.TimeoutError if no reply arrives within `timeout`, ValueError on an error reply, and ConnectionError if the admin connection drops while waiting. """ - from .protocol import AdminUpdateType, AdminUpdateFrequency + from .protocol import AdminUpdateFrequency, AdminUpdateType if not self._gs_subscribed: await self.update_frequency(AdminUpdateType.Gamescript, AdminUpdateFrequency.Automatic) self._gs_subscribed = True diff --git a/lib/openttd/protocol.py b/lib/openttd/protocol.py index 7449547..9b98aa0 100644 --- a/lib/openttd/protocol.py +++ b/lib/openttd/protocol.py @@ -1,9 +1,11 @@ import struct -import monocypher from enum import IntEnum -from openttd_protocol.wire.tcp import TCPProtocol -from openttd_protocol.wire.read import read_uint8, read_string, read_uint16, read_uint32 + +import monocypher from openttd_protocol.wire.exceptions import SocketClosed +from openttd_protocol.wire.read import read_string, read_uint8, read_uint16, read_uint32 +from openttd_protocol.wire.tcp import TCPProtocol + def write_varuint(buffer, value): """Encode a non-negative integer using OpenTTD's UTF-8-like varuint scheme.""" @@ -232,21 +234,21 @@ class OpenTTDProtocol(TCPProtocol): if self.handler.encryption_enabled: if not self.handler._recv_aead: self.handler._recv_aead = monocypher.IncrementalAuthenticatedEncryption(self.handler._session_key_recv, self.handler._encryption_nonce) - length, rest = read_uint16(data) + _, rest = read_uint16(data) payload = self.handler._recv_aead.unlock(bytes(rest[:16]), bytes(rest[16:])) if payload is None: raise SocketClosed("Decryption failed") data = memoryview(struct.pack(" 0: if len(data) >= 13: @@ -575,7 +577,7 @@ class OpenTTDAdminProtocol(OpenTTDProtocol): import json try: return {"data": json.loads(json_str)} - except Exception: + except json.JSONDecodeError: return {"raw_data": json_str} @staticmethod diff --git a/main.py b/main.py index aac3c8d..5dbb22f 100644 --- a/main.py +++ b/main.py @@ -1,7 +1,7 @@ import asyncio import logging -import sys import os +import sys # Add the lib directory to sys.path so we can import the openttd package sys.path.append(os.path.join(os.path.dirname(__file__), 'lib')) @@ -171,7 +171,7 @@ async def run_client(): print("--- Finished 10s stay, exiting gracefully ---") await client.quit() - except Exception as e: + except Exception as e: # noqa: BLE001 - top-level demo handler: report any failure instead of dumping a traceback print(f"!!! Error: {e}") if __name__ == "__main__": diff --git a/main_admin.py b/main_admin.py index 1d4f05a..baeccc0 100644 --- a/main_admin.py +++ b/main_admin.py @@ -1,13 +1,13 @@ import asyncio import logging -import sys import os +import sys # Add the lib directory to sys.path so we can import the openttd package sys.path.append(os.path.join(os.path.dirname(__file__), 'lib')) from openttd import OpenTTDAdminClient -from openttd.protocol import AdminUpdateType, AdminUpdateFrequency +from openttd.protocol import AdminUpdateFrequency, AdminUpdateType # Configuration SERVER_HOST = "127.0.0.1" @@ -82,14 +82,14 @@ async def run_admin(): print(f" waiting by next hop: {flow['waiting_by_via']}") print(f" planned by source: {flow['planned_by_from']}") print(f" planned by next hop: {flow['planned_by_via']}") - except Exception as e: + except Exception as e: # noqa: BLE001 - demo script: one failed station query should not abort the walk print(f"!!! station query failed: {e}") await asyncio.sleep(5) print("--- Quitting ---") await admin.quit() - except Exception as e: + except Exception as e: # noqa: BLE001 - top-level demo handler: report any failure instead of dumping a traceback print(f"!!! Error: {e}") if __name__ == "__main__": diff --git a/tests/conftest.py b/tests/conftest.py index 9b08adb..af2000c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,8 @@ -import pytest import os +import pytest + + @pytest.fixture(scope="session") def server_config(): """Provides server connection parameters from environment variables with defaults.""" diff --git a/tests/test_admin.py b/tests/test_admin.py index 92318be..5d35cb9 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -1,10 +1,15 @@ -import pytest import asyncio import json import os + import monocypher +import pytest from openttd import OpenTTDAdminClient -from openttd.protocol import PacketAdminType, AdminUpdateType, AdminUpdateFrequency +from openttd.protocol import AdminUpdateFrequency, AdminUpdateType, PacketAdminType + + +class FakeNetworkError(OSError): + """Stand-in for a socket-level failure, so it matches the client's narrowed handlers.""" def decode_gamescript_payload(packet): """Decode the JSON payload of an AdminGamescript packet (2-byte length + 1-byte type + string).""" @@ -84,7 +89,7 @@ async def test_admin_client_connect_and_actions(monkeypatch): # 3. Connect Exception async def mock_fail(*args, **kwargs): - raise Exception("Connection Failed") + raise FakeNetworkError("Connection Failed") monkeypatch.setattr(asyncio.get_running_loop(), "create_connection", mock_fail) with pytest.raises(Exception, match="Connection Failed"): await client.connect() @@ -196,7 +201,7 @@ async def test_admin_client_connect_and_actions(monkeypatch): # 8. Quit Exception class BadProtocol: async def send_packet(self, data): - raise Exception("Fail") + raise FakeNetworkError("Fail") client._transport = MockTransport() client._protocol = BadProtocol() await client.quit() diff --git a/tests/test_coverage.py b/tests/test_coverage.py index 820cc8d..1dd76b7 100644 --- a/tests/test_coverage.py +++ b/tests/test_coverage.py @@ -1,8 +1,10 @@ -import pytest import asyncio import struct -from openttd.protocol import OpenTTDProtocol, PacketGameType + +import pytest from openttd.client import OpenTTDClient +from openttd.protocol import OpenTTDProtocol, PacketGameType + class MockTransport: def __init__(self): self._closing = False @@ -10,16 +12,19 @@ class MockTransport: def close(self): self._closing = True def write(self, data): return len(data) +class FakeNetworkError(OSError): + """Stand-in for a socket-level failure, so it matches the client's narrowed handlers.""" + class MockProtocol: async def send_packet(self, data): - raise Exception("Send failed") + raise FakeNetworkError("Send failed") @pytest.mark.asyncio async def test_client_connect_exception(monkeypatch): # Coverage for client.py:51-53 client = OpenTTDClient(host="127.0.0.1") async def mock_fail(*args, **kwargs): - raise Exception("Async Failure") + raise FakeNetworkError("Async Failure") monkeypatch.setattr(asyncio.get_running_loop(), "create_connection", mock_fail) with pytest.raises(Exception, match="Async Failure"): await client.connect() diff --git a/tests/test_e2e.py b/tests/test_e2e.py index e37ebbe..1c20645 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -1,20 +1,22 @@ import asyncio -import pytest -import pytest_asyncio -import sys import os import random +import sys + +import pytest +import pytest_asyncio + # Add lib to path sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', 'lib')) -from openttd import OpenTTDClient, OpenTTDAdminClient +from openttd import OpenTTDAdminClient, OpenTTDClient from openttd.protocol import ( - OpenTTDProtocol, - OpenTTDAdminProtocol, - AdminUpdateType, AdminUpdateFrequency, + AdminUpdateType, + ModifyTimetableFlags, + OpenTTDAdminProtocol, + OpenTTDProtocol, PacketGameType, - ModifyTimetableFlags ) # These identify a vehicle/order that already exists in the local dev server's persisted @@ -738,11 +740,11 @@ async def test_e2e_protocol_public_functions_multiple_inputs(server_config): # 2. Test receive_packet() with multiple inputs # Input 1: Valid packet structure (length >= 3) - res_type1, res_data1 = proto_game.receive_packet(None, memoryview(b"\x03\x00\x05")) # type 5 is ServerUnused + res_type1, _ = proto_game.receive_packet(None, memoryview(b"\x03\x00\x05")) # type 5 is ServerUnused assert res_type1 == PacketGameType.ServerUnused # Input 2: Invalid/short packet structure (length < 3) - res_type2, res_data2 = proto_game.receive_packet(None, memoryview(b"\x01")) + res_type2, _ = proto_game.receive_packet(None, memoryview(b"\x01")) assert res_type2 == PacketGameType.ServerUnused # Falls back to ServerUnused on error # 3. Test send_packet() with multiple inputs (using mock transports) diff --git a/tests/test_logic.py b/tests/test_logic.py index 1a89002..fca9e76 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -1,8 +1,13 @@ -import pytest import asyncio import hashlib -from openttd.protocol import PacketGameType + +import pytest from openttd.client import OpenTTDClient +from openttd.protocol import PacketGameType + + +class FakeNetworkError(OSError): + """Stand-in for a socket-level failure, so it matches the client's narrowed handlers.""" def test_packet_game_type_values(): assert PacketGameType.ServerFull == 0 @@ -61,7 +66,7 @@ async def test_client_connect_success(monkeypatch): async def test_client_connect_failure(monkeypatch): client = OpenTTDClient(host="127.0.0.1") async def mock_fail(*args, **kwargs): - raise Exception("Async Failure") + raise FakeNetworkError("Async Failure") monkeypatch.setattr(asyncio.get_running_loop(), "create_connection", mock_fail) with pytest.raises(Exception, match="Async Failure"): await client.connect() diff --git a/tests/test_protocol.py b/tests/test_protocol.py index 403ada8..c60d579 100644 --- a/tests/test_protocol.py +++ b/tests/test_protocol.py @@ -1,9 +1,12 @@ -import pytest +import contextlib import struct + import monocypher +import pytest from openttd.protocol import OpenTTDProtocol, PacketGameType from openttd_protocol.wire.exceptions import SocketClosed + class MockTransport: def __init__(self): self._closing = False def is_closing(self): return self._closing @@ -59,10 +62,8 @@ def test_protocol_static_parsers(): assert OpenTTDProtocol.receive_ServerNeedCompanyPassword(None, memoryview(struct.pack("