diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 35d4061..d3d4ac0 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -19,7 +19,7 @@ jobs: - name: Checkout code uses: actions/checkout@v7 with: - fetch-depth: 0 + fetch-depth: 0 # hatch-vcs derives the release version from git history and tags. - name: Set up uv uses: astral-sh/setup-uv@v10.3.0 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ab3e0eb..5d50c80 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,7 +28,4 @@ jobs: enable-cache: true - name: Run tests - run: uv run --locked coverage run -m pytest - - - name: Enforce coverage threshold - run: uv run --locked coverage report + run: uv run --locked pytest --cov=pyrail --cov-report=term-missing:skip-covered diff --git a/README.md b/README.md index 55c4365..a06f97a 100644 --- a/README.md +++ b/README.md @@ -284,7 +284,7 @@ uv run pytest CI also enforces a coverage threshold: coverage over `pyrail/`, statements and branches together, must stay at or above 92 percent. Run it locally with: ```bash -uv run coverage run -m pytest && uv run coverage report +uv run pytest --cov=pyrail --cov-report=term-missing:skip-covered ``` To run the live suite, which needs network access: diff --git a/pyproject.toml b/pyproject.toml index 3309df3..454b61b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,7 +32,7 @@ Changelog = "https://github.com/tjorim/pyrail/releases" test = [ "pytest>=9.1.1", "pytest-asyncio>=1.4.0", - "coverage[toml]>=7.16.2", + "pytest-cov>=7.1.0", ] dev = [ "ruff>=0.16.10", diff --git a/pyrail/irail.py b/pyrail/irail.py index 1c92ea5..b858af7 100644 --- a/pyrail/irail.py +++ b/pyrail/irail.py @@ -35,6 +35,7 @@ _API_BASE_URL = "https://api.irail.be/v1" +# Retry transient failures with exponential backoff, capped at the maximum delay. _RETRY_BASE_DELAY = 1.0 _RETRY_MAX_DELAY = 10.0 @@ -50,6 +51,7 @@ _LANGUAGES: frozenset[str] = frozenset({"en", "nl", "fr", "de"}) +# The API silently substitutes defaults for unknown values, so validate them locally. _ALLOWED_VALUES: Mapping[str, frozenset[str]] = MappingProxyType( { "arrdep": frozenset({"departure", "arrival"}), @@ -59,28 +61,6 @@ ) -type _EndpointSpec = Mapping[str, tuple[str, ...]] - - -def _frozen_spec(spec: dict[str, tuple[str, ...]]) -> _EndpointSpec: - """Freeze one endpoint spec, so the endpoint table cannot be mutated at any level.""" - return MappingProxyType(spec) - - -_ENDPOINTS: Mapping[str, _EndpointSpec] = MappingProxyType( - { - "stations": _frozen_spec({}), - "liveboard": _frozen_spec({"xor": ("station", "id"), "optional": ("date", "time", "arrdep", "alerts")}), - "connections": _frozen_spec( - {"required": ("from", "to"), "optional": ("date", "time", "timesel", "typeOfTransport")} - ), - "vehicle": _frozen_spec({"required": ("id",), "optional": ("date", "alerts")}), - "composition": _frozen_spec({"required": ("id",), "optional": ("data",)}), - "disturbances": _frozen_spec({"optional": ("lineBreakCharacter",)}), - } -) - - def _describe(error: BaseException) -> str: """Describe an exception for log and error messages, including its type when it has no message.""" return f"{type(error).__name__}: {error}" if str(error) else type(error).__name__ @@ -389,51 +369,6 @@ def _validate_values(self, params: dict[str, Any]) -> None: msg = f"Invalid {name}: expected one of {sorted(allowed)}, got: {params[name]}" raise ValueError(msg) - def _validate_params(self, method: str, params: dict[str, Any] | None = None) -> None: - """Validate parameters for a specific iRail API endpoint based on predefined requirements. - - Checks that the method is a known endpoint, that all required parameters are present, - that the XOR parameter logic holds, that no unexpected parameters are given, and that - date, time and fixed-value parameters hold accepted values. - - Args: - method (str): The API endpoint method to validate parameters for. - params (dict[str, Any], optional): Dictionary of parameters to validate. Defaults to None. - - Raises: - ValueError: If any of the checks fails. - - """ - if method not in _ENDPOINTS: - msg = f"Unknown API endpoint: {method}" - raise ValueError(msg) - - endpoint = _ENDPOINTS[method] - required = endpoint.get("required", ()) - xor = endpoint.get("xor", ()) - optional = endpoint.get("optional", ()) - - params = params or {} - - self._validate_values(params) - - for param in required: - if param not in params or params[param] is None: - msg = f"Missing required parameter: {param} for endpoint: {method}" - raise ValueError(msg) - - if xor: - xor_values = [params.get(param) is not None for param in xor] - if sum(xor_values) != 1: - msg = f"Exactly one of the XOR parameters {xor} must be provided for endpoint: {method}" - raise ValueError(msg) - - all_params = required + xor + optional - for param in params: - if param not in all_params: - msg = f"Unexpected parameter: {param} for endpoint: {method}" - raise ValueError(msg) - async def _handle_success_response( self, response: ClientResponse, method: str ) -> tuple[dict[str, Any], str | None]: @@ -556,9 +491,10 @@ async def _do_request[T]( ) -> T | None: """Send an asynchronous request to the specified iRail API endpoint. - This method handles API requests with rate limiting, parameter validation, - and ETag-based caching. It supports conditional requests and manages - various HTTP response scenarios. Network errors, timeouts and server errors (5xx) + This method handles API requests with rate limiting and ETag-based caching. + Endpoint methods validate their parameters before calling this helper. + It supports conditional requests and manages various HTTP response scenarios. + Network errors, timeouts and server errors (5xx) are retried up to ``max_retries`` times with exponential backoff. The response is deserialized with ``from_dict``, and its ETag is cached only after the response is fully accepted, so a rejected response never leaves a cache entry. @@ -574,7 +510,6 @@ async def _do_request[T]( not modified (304). Raises: - ValueError: If the method is not a known endpoint or the parameters are invalid. RuntimeError: If the active session has been closed. RateLimitError: If the API responds with HTTP 429. The exception's ``retry_after`` holds the Retry-After delay in seconds, if given. The request is not retried. @@ -597,7 +532,6 @@ async def _do_request[T]( """ logger.debug("Starting request to endpoint: %s", method) - self._validate_params(method, args or {}) url: str = f"{_API_BASE_URL}/{method}/" params = {"format": "json", "lang": self.lang} @@ -609,6 +543,7 @@ async def _do_request[T]( max_attempts = self.max_retries + 1 attempt = 1 while True: + # Retries must consume a token too, or failures could exceed the API limits. async with self._lock: await self._handle_rate_limit() @@ -652,7 +587,7 @@ def _parse_response[T](self, method: str, data: dict[str, Any], from_dict: Calla return from_dict(data) except (LookupError, ValueError, OverflowError) as e: msg = f"Response from {method} does not match the expected model: {_describe(e)}" - raise InvalidResponseError(msg) from e + raise InvalidResponseError(msg, status=200) from e async def get_stations(self) -> StationsApiResponse | None: """Retrieve a list of all train stations from the iRail API. @@ -730,10 +665,16 @@ async def get_liveboard( "arrdep": arrdep, "alerts": "true" if alerts else "false", } + params = {k: v for k, v in extra_params.items() if v is not None} + self._validate_values(params) + if (station is None) == (station_id is None): + msg = "Provide exactly one of station or station_id" + raise ValueError(msg) + return await self._do_request( "liveboard", LiveboardApiResponse.from_dict, - {k: v for k, v in extra_params.items() if v is not None}, + params, ) async def get_connections( @@ -783,10 +724,16 @@ async def get_connections( "timesel": timesel, "typeOfTransport": type_of_transport, } + params = {k: v for k, v in extra_params.items() if v is not None} + self._validate_values(params) + if from_station is None or to_station is None: + msg = "Both from_station and to_station must be provided" + raise ValueError(msg) + return await self._do_request( "connections", ConnectionsApiResponse.from_dict, - {k: v for k, v in extra_params.items() if v is not None}, + params, ) async def get_vehicle( @@ -822,10 +769,16 @@ async def get_vehicle( """ extra_params: dict[str, Any] = {"id": vehicle_id, "date": date, "alerts": "true" if alerts else "false"} + params = {k: v for k, v in extra_params.items() if v is not None} + self._validate_values(params) + if vehicle_id is None: + msg = "vehicle_id must be provided" + raise ValueError(msg) + return await self._do_request( "vehicle", VehicleApiResponse.from_dict, - {k: v for k, v in extra_params.items() if v is not None}, + params, ) async def get_composition( @@ -857,10 +810,15 @@ async def get_composition( """ extra_params: dict[str, str | None] = {"id": vehicle_id, "data": data} + params = {k: v for k, v in extra_params.items() if v is not None} + if vehicle_id is None: + msg = "vehicle_id must be provided" + raise ValueError(msg) + return await self._do_request( "composition", CompositionApiResponse.from_dict, - {k: v for k, v in extra_params.items() if v is not None}, + params, ) async def get_disturbances(self, line_break_character: str | None = None) -> DisturbancesApiResponse | None: diff --git a/pyrail/models.py b/pyrail/models.py index adfeac5..07f03b7 100644 --- a/pyrail/models.py +++ b/pyrail/models.py @@ -350,7 +350,7 @@ class VehicleStop(DataClassORJSONMixin): left: bool = field(metadata=field_options(deserialize=_str_to_bool)) arrived: bool = field(metadata=field_options(deserialize=_str_to_bool)) is_extra_stop: bool = field(metadata=field_options(alias="isExtraStop", deserialize=_str_to_bool)) - occupancy: Occupancy | None = field(default=None) + occupancy: Occupancy | None = field(default=None) # Not present on the last stop. departure_connection: str | None = field(default=None, metadata=field_options(alias="departureConnection")) diff --git a/tests/live/live_api.py b/tests/live/live_api.py index 5749551..1a58d71 100644 --- a/tests/live/live_api.py +++ b/tests/live/live_api.py @@ -16,6 +16,8 @@ from pyrail.exceptions import InvalidRequestError, InvalidResponseError, NotFoundError, PyRailError from pyrail.irail import IRail +# The fixture disables client retries, so this is the only retry layer; client +# rate limiting still applies to every attempt. ATTEMPTS = 3 BACKOFF_SECONDS = (5, 15) REQUEST_TIMEOUT = ClientTimeout(total=30) diff --git a/tests/test_irail.py b/tests/test_irail.py index d43b8f6..c351229 100644 --- a/tests/test_irail.py +++ b/tests/test_irail.py @@ -8,7 +8,7 @@ import json import logging from pathlib import Path -from typing import Any, cast +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch from aiohttp import ClientError, ClientSession @@ -25,7 +25,7 @@ PyRailError, RateLimitError, ) -from pyrail.irail import _ENDPOINTS, _ETAG_CACHE_LIMIT, IRail, _parse_retry_after +from pyrail.irail import _ETAG_CACHE_LIMIT, IRail, _parse_retry_after from pyrail.models import ( Alert, ApiResponse, @@ -405,11 +405,13 @@ async def test_get_liveboard_by_id(mock_get: MagicMock) -> None: assert params["id"] == "BE.NMBS.008813003" assert "station" not in params - with pytest.raises(ValueError, match="Exactly one of the XOR parameters"): + with pytest.raises(ValueError, match="exactly one of station or station_id"): await api.get_liveboard(station="Brussels-Central", station_id="BE.NMBS.008813003") - with pytest.raises(ValueError, match="Exactly one of the XOR parameters"): + with pytest.raises(ValueError, match="exactly one of station or station_id"): await api.get_liveboard() + assert mock_get.call_count == 1 + @patch("pyrail.irail.ClientSession.get") async def test_get_liveboard_alerts_parameter(mock_get: MagicMock) -> None: @@ -471,23 +473,28 @@ async def test_get_connections_timesel_and_type_of_transport(mock_get: MagicMock @patch("pyrail.irail.ClientSession.get") -async def test_fixed_value_parameters_reject_unknown_values(mock_get: MagicMock) -> None: +@pytest.mark.parametrize( + ("call", "message"), + [ + ( + lambda api: api.get_connections("A", "B", type_of_transport="nointernationaltrains"), + "Invalid typeOfTransport", + ), + (lambda api: api.get_connections("A", "B", type_of_transport="Trains"), "Invalid typeOfTransport"), + (lambda api: api.get_connections("A", "B", timesel="arrivals"), "Invalid timesel"), + (lambda api: api.get_liveboard(station="Brussels-Central", arrdep="arrivals"), "Invalid arrdep"), + ], +) +async def test_fixed_value_parameters_reject_unknown_values( + mock_get: MagicMock, call: Callable[[IRail], Awaitable[object]], message: str +) -> None: """Values the API would silently replace by a default are rejected before any request is sent.""" - mock_get.return_value.__aenter__.return_value = _mock_response(data=_load_example("connections.json")) - - async with IRail() as api: - with pytest.raises(ValueError, match="Invalid typeOfTransport"): - await api._do_request( - "connections", dict, {"from": "A", "to": "B", "typeOfTransport": "nointernationaltrains"} - ) - with pytest.raises(ValueError, match="Invalid typeOfTransport"): - await api._do_request("connections", dict, {"from": "A", "to": "B", "typeOfTransport": "Trains"}) - with pytest.raises(ValueError, match="Invalid timesel"): - await api._do_request("connections", dict, {"from": "A", "to": "B", "timesel": "arrivals"}) - with pytest.raises(ValueError, match="Invalid arrdep"): - await api._do_request("liveboard", dict, {"station": "Brussels-Central", "arrdep": "arrivals"}) + api = IRail() + with pytest.raises(ValueError, match=message): + await call(api) mock_get.assert_not_called() + assert api.session is None @patch("pyrail.irail.ClientSession.get") @@ -1165,24 +1172,49 @@ def test_invalid_language_rejected() -> None: assert api.lang == "nl" -def test_validate_params_rejects_bad_requests() -> None: - """Test that unknown endpoints, missing required parameters and unexpected parameters raise ValueError.""" +@patch("pyrail.irail.ClientSession.get") +@pytest.mark.parametrize( + "call", + [ + pytest.param(lambda api: api.get_connections(None, "B"), id="missing-origin"), + pytest.param(lambda api: api.get_connections("A", None), id="missing-destination"), + pytest.param(lambda api: api.get_vehicle(None), id="missing-vehicle-id"), + pytest.param(lambda api: api.get_composition(None), id="missing-composition-id"), + ], +) +async def test_endpoint_methods_reject_missing_required_values( + mock_get: MagicMock, call: Callable[[IRail], Awaitable[object]] +) -> None: + """Required values are checked at runtime even when a caller bypasses type checking.""" api = IRail() - with pytest.raises(ValueError, match="Unknown API endpoint"): - api._validate_params("unknown", {}) - with pytest.raises(ValueError, match="Missing required parameter"): - api._validate_params("connections", {"from": "A"}) - with pytest.raises(ValueError, match="Unexpected parameter"): - api._validate_params("disturbances", {"unexpected": True}) + with pytest.raises(ValueError, match="must be provided"): + await call(api) + mock_get.assert_not_called() + assert api.session is None -def test_endpoint_table_is_frozen() -> None: - """The endpoint table rejects mutation at both levels.""" - endpoints = cast("Any", _ENDPOINTS) - with pytest.raises(TypeError): - endpoints["liveboard"]["optional"] = () - with pytest.raises(TypeError): - endpoints["new"] = {} + +@patch("pyrail.irail.ClientSession.get") +@pytest.mark.parametrize( + ("call", "message"), + [ + (lambda api: api.get_liveboard(station="A", date="320923"), "Invalid date format"), + (lambda api: api.get_liveboard(station="A", time="2460"), "Invalid time format"), + (lambda api: api.get_connections("A", "B", date="320923"), "Invalid date format"), + (lambda api: api.get_connections("A", "B", time="2460"), "Invalid time format"), + (lambda api: api.get_vehicle("IC538", date="320923"), "Invalid date format"), + ], +) +async def test_endpoint_methods_validate_dates_and_times_before_requesting( + mock_get: MagicMock, call: Callable[[IRail], Awaitable[object]], message: str +) -> None: + """All endpoints accepting dates or times reject invalid formats before opening a session.""" + api = IRail() + with pytest.raises(ValueError, match=message): + await call(api) + + mock_get.assert_not_called() + assert api.session is None def test_non_positive_request_timeout_rejected() -> None: @@ -1217,8 +1249,9 @@ async def test_deserialization_failure_raises_invalid_response(mock_get: MagicMo """Test that a body that does not match the models raises InvalidResponseError.""" mock_get.return_value.__aenter__.return_value = _mock_response(data={"unexpected": True}) async with IRail() as api: - with pytest.raises(InvalidResponseError, match="does not match the expected model"): + with pytest.raises(InvalidResponseError, match="does not match the expected model") as exc_info: await api.get_stations() + assert exc_info.value.status == 200 @patch("pyrail.irail.ClientSession.get") diff --git a/uv.lock b/uv.lock index cda0128..6393548 100644 --- a/uv.lock +++ b/uv.lock @@ -745,9 +745,9 @@ dev = [ { name = "ty" }, ] test = [ - { name = "coverage" }, { name = "pytest" }, { name = "pytest-asyncio" }, + { name = "pytest-cov" }, ] [package.metadata] @@ -762,9 +762,9 @@ dev = [ { name = "ty", specifier = ">=0.0.84" }, ] test = [ - { name = "coverage", extras = ["toml"], specifier = ">=7.16.2" }, { name = "pytest", specifier = ">=9.1.1" }, { name = "pytest-asyncio", specifier = ">=1.4.0" }, + { name = "pytest-cov", specifier = ">=7.1.0" }, ] [[package]] @@ -796,6 +796,20 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/03/e2/08a497ef684b88559c9cc5f4ad53a37e7b99e727094a86d6ea32536d5d3c/pytest_asyncio-1.4.0-py3-none-any.whl", hash = "sha256:933ca923a23075a87fb7070c0ec272a6848489824d887c85c812670932835aa1", size = 16930, upload-time = "2026-05-26T09:56:02.576Z" }, ] +[[package]] +name = "pytest-cov" +version = "7.1.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "coverage" }, + { name = "pluggy" }, + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/b1/51/a849f96e117386044471c8ec2bd6cfebacda285da9525c9106aeb28da671/pytest_cov-7.1.0.tar.gz", hash = "sha256:30674f2b5f6351aa09702a9c8c364f6a01c27aae0c1366ae8016160d1efc56b2", size = 55592, upload-time = "2026-03-21T20:11:16.284Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/9d/7a/d968e294073affff457b041c2be9868a40c1c71f4a35fcc1e45e5493067b/pytest_cov-7.1.0-py3-none-any.whl", hash = "sha256:a0461110b7865f9a271aa1b51e516c9a95de9d696734a2f71e3e78f46e1d4678", size = 22876, upload-time = "2026-03-21T20:11:14.438Z" }, +] + [[package]] name = "ruff" version = "0.17.1"