-
Notifications
You must be signed in to change notification settings - Fork 0
[08/10] refactor: separate weather adapters #99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,15 +7,15 @@ | |
| import pendulum | ||
| import pytest | ||
|
|
||
| from weather_briefing.data.resources import ReferenceDataError | ||
| from weather_briefing.models import ( | ||
| AirQualitySnapshot, | ||
| AirQualityTimeKind, | ||
| AllergenSnapshot, | ||
| WeatherContextSnapshot, | ||
| ) | ||
| from weather_briefing.reference_data import ReferenceDataError | ||
| from weather_briefing.time_utils import parse_aware_datetime | ||
| from weather_briefing.weather_context import ( | ||
| from weather_briefing.weather import ( | ||
| AirQualitySupplementingWeatherProvider, | ||
| FallbackWeatherContextProvider, | ||
| LoggedWeatherContextProvider, | ||
|
|
@@ -24,11 +24,15 @@ | |
| QWeatherProvider, | ||
| UnsupportedForecastDateError, | ||
| WeatherContextError, | ||
| _format_qweather_day, | ||
| _format_qweather_lifestyle, | ||
| snapshot_to_documents, | ||
| ) | ||
| from weather_briefing.weather.open_meteo import ( | ||
| _open_meteo_daily_peak_values, | ||
| _open_meteo_weather_description, | ||
| snapshot_to_documents, | ||
| ) | ||
| from weather_briefing.weather.qweather import ( | ||
| _format_qweather_day, | ||
| _format_qweather_lifestyle, | ||
| ) | ||
|
|
||
|
|
||
|
|
@@ -551,7 +555,7 @@ def test_open_meteo_weather_code_uses_readable_description() -> None: | |
|
|
||
| def test_open_meteo_weather_code_lookup_uses_cached_loader_boundary(monkeypatch) -> None: | ||
| monkeypatch.setattr( | ||
| "weather_briefing.weather_context.open_meteo_weather_code_descriptions", | ||
| "weather_briefing.weather.open_meteo_reference.open_meteo_weather_code_descriptions", | ||
| lambda: {53: "Reloaded description"}, | ||
| ) | ||
|
|
||
|
|
@@ -656,6 +660,38 @@ async def test_open_meteo_future_enrichment_rejects_non_object_hourly_payload() | |
| assert allergen is None | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("payload", "reason"), | ||
| ( | ||
| ([], "air-quality response must be an object"), | ||
| ({"current": []}, "current air quality must be an object"), | ||
| ), | ||
| ) | ||
| async def test_open_meteo_current_enrichment_rejects_non_object_payload( | ||
| caplog, | ||
| payload: object, | ||
| reason: str, | ||
| ) -> None: | ||
| async with httpx.AsyncClient( | ||
| transport=httpx.MockTransport(lambda request: httpx.Response(200, json=payload)) | ||
| ) as client: | ||
| provider = OpenMeteoProvider( | ||
| client, | ||
| air_quality_base_url="https://air.example.invalid", | ||
| ) | ||
|
|
||
| with caplog.at_level("WARNING", logger="weather_briefing.weather_context"): | ||
| air_quality, allergen = await provider._fetch_air_quality_and_allergen( | ||
| 39.9, | ||
| 116.3, | ||
| forecast_date=None, | ||
| ) | ||
|
|
||
| assert air_quality is None | ||
| assert allergen is None | ||
| assert f"operation=air-quality reason={reason}" in caplog.text | ||
|
|
||
|
|
||
|
Comment on lines
+663
to
+694
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win Use synthetic coordinates in the mocked test. The test passes the real location As per coding guidelines, “Never commit real credentials, locations, coordinates, private source URLs, generated content, or runtime state; use runtime configuration, ignored state paths, and public test data.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| def test_open_meteo_daily_peaks_skip_invalid_hourly_values() -> None: | ||
| hourly: dict[str, object] = { | ||
| "time": ["2026-07-15T06:00", "2026-07-15T09:00", "2026-07-15T18:00"], | ||
|
|
@@ -846,7 +882,8 @@ async def fetch_for_date( | |
|
|
||
| assert "Weather API call started provider=qweather" in caplog.text | ||
| assert "Weather API call failed provider=qweather" in caplog.text | ||
| assert "reason=QWeather weather forecast failed: HTTP 401" in caplog.text | ||
| assert "reason=WeatherContextError" in caplog.text | ||
| assert "QWeather weather forecast failed: HTTP 401" not in caplog.text | ||
| assert "Weather API call succeeded provider=open-meteo" in caplog.text | ||
| assert "source_id=weather:fallback" in caplog.text | ||
|
|
||
|
|
@@ -902,7 +939,7 @@ async def fetch(self, latitude: float, longitude: float) -> WeatherContextSnapsh | |
| def fail_elapsed(started_at: float) -> int: | ||
| raise RuntimeError("secondary failure") | ||
|
|
||
| monkeypatch.setattr("weather_briefing.weather_context._elapsed_milliseconds", fail_elapsed) | ||
| monkeypatch.setattr("weather_briefing.weather.base._elapsed_milliseconds", fail_elapsed) | ||
| else: | ||
| original_info = logging.getLogger("weather_briefing.weather_context").info | ||
|
|
||
|
|
@@ -911,7 +948,7 @@ def fail_skip_log(message: str, *args: object) -> None: | |
| raise RuntimeError("secondary failure") | ||
| original_info(message, *args) | ||
|
|
||
| monkeypatch.setattr("weather_briefing.weather_context._LOGGER.info", fail_skip_log) | ||
| monkeypatch.setattr("weather_briefing.weather.base._LOGGER.info", fail_skip_log) | ||
|
|
||
| provider = LoggedWeatherContextProvider("nea-sg", UndatedProvider()) | ||
|
|
||
|
|
@@ -1044,6 +1081,25 @@ def handler(request: httpx.Request) -> httpx.Response: | |
| ).fetch(1, 2) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("payload", "message"), | ||
| ( | ||
| ([], "weather response must be an object"), | ||
| ({"code": "200", "daily": {}}, "daily forecast must be an array"), | ||
| ), | ||
| ) | ||
| async def test_qweather_rejects_invalid_weather_response_shape(payload: object, message: str) -> None: | ||
| async with httpx.AsyncClient( | ||
| transport=httpx.MockTransport(lambda request: httpx.Response(200, json=payload)) | ||
| ) as client: | ||
| with pytest.raises(WeatherContextError, match=message): | ||
| await QWeatherProvider( | ||
| client, | ||
| authenticator=StaticAuthenticator(), | ||
| base_url="https://api.example.invalid", | ||
| ).fetch(1, 2) | ||
|
|
||
|
|
||
| async def test_qweather_does_not_log_untrusted_api_status(caplog) -> None: | ||
| untrusted_status = "400\nforged-log-entry" | ||
|
|
||
|
|
@@ -1066,7 +1122,7 @@ def handler(request: httpx.Request) -> httpx.Response: | |
| await provider.fetch(1, 2) | ||
|
|
||
| assert untrusted_status not in caplog.text | ||
| assert "reason=QWeather returned a non-success weather status code=invalid" in caplog.text | ||
| assert "reason=WeatherContextError" in caplog.text | ||
|
|
||
|
|
||
| async def test_qweather_rejects_empty_daily_forecast() -> None: | ||
|
|
@@ -1212,7 +1268,7 @@ def handler(request: httpx.Request) -> httpx.Response: | |
|
|
||
|
|
||
| async def test_fallback_weather_provider_requires_at_least_one_provider() -> None: | ||
| from weather_briefing.weather_context import FallbackWeatherContextProvider | ||
| from weather_briefing.weather import FallbackWeatherContextProvider | ||
|
|
||
| with pytest.raises(ValueError, match="At least one"): | ||
| FallbackWeatherContextProvider() | ||
|
|
@@ -1303,6 +1359,35 @@ def handler(request: httpx.Request) -> httpx.Response: | |
| assert "operation=lifestyle-indices reason=non-success indices status code=400" in caplog.text | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("payload", "reason"), | ||
| ( | ||
| ([], "indices response must be an object"), | ||
| ({"code": "200", "daily": {}}, "daily indices must be an array"), | ||
| ), | ||
| ) | ||
| async def test_qweather_invalid_indices_response_shape_is_optional(caplog, payload: object, reason: str) -> None: | ||
| def handler(request: httpx.Request) -> httpx.Response: | ||
| if request.url.path == "/v7/weather/3d": | ||
| return _qweather_weather_response() | ||
| if request.url.path == "/v7/indices/1d": | ||
| return httpx.Response(200, json=payload) | ||
| assert request.url.path == "/airquality/v1/current/1.00/2.00" | ||
| return httpx.Response(500) | ||
|
|
||
| async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: | ||
| with caplog.at_level("WARNING", logger="weather_briefing.weather_context"): | ||
| snapshot = await QWeatherProvider( | ||
| client, | ||
| authenticator=StaticAuthenticator(), | ||
| base_url="https://api.example.invalid", | ||
| ).fetch(1, 2) | ||
|
|
||
| assert snapshot.weather_forecast | ||
| assert snapshot.lifestyle_advice == () | ||
| assert f"operation=lifestyle-indices reason={reason}" in caplog.text | ||
|
|
||
|
|
||
| async def test_qweather_rejects_http_error() -> None: | ||
| async with httpx.AsyncClient(transport=httpx.MockTransport(lambda _: httpx.Response(500))) as client: | ||
| with pytest.raises(WeatherContextError, match="weather forecast failed: HTTP 500"): | ||
|
|
@@ -2016,7 +2101,7 @@ def handler(request: httpx.Request) -> httpx.Response: | |
| ) | ||
|
|
||
| monkeypatch.setattr( | ||
| "weather_briefing.weather_context.pollen_type_names", | ||
| "weather_briefing.allergen.pollen_type_names", | ||
| fail_to_load_pollen_types, | ||
| ) | ||
| async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.