diff --git a/src/accessiweather/notifications/notification_event_manager.py b/src/accessiweather/notifications/notification_event_manager.py index 7dbe5ca7..6dd5dbec 100644 --- a/src/accessiweather/notifications/notification_event_manager.py +++ b/src/accessiweather/notifications/notification_event_manager.py @@ -225,15 +225,28 @@ def _check_discussion_update( """ if not issuance_time: # No issuance time available (non-US location or API issue) + logger.debug( + "_check_discussion_update: no issuance_time (non-US location or fetch failed) " + "— skipping" + ) return None # First time seeing discussion - store but don't notify if self.state.last_discussion_issuance_time is None: self.state.last_discussion_issuance_time = issuance_time self.state.last_discussion_text = discussion_text - logger.debug("First discussion issuance time stored: %s", issuance_time) + logger.debug( + "_check_discussion_update: first-run — stored issuance_time=%s, no notification", + issuance_time, + ) return None + logger.debug( + "_check_discussion_update: last=%s current=%s", + self.state.last_discussion_issuance_time, + issuance_time, + ) + # Check if issuance time is newer (discussion was updated) if issuance_time > self.state.last_discussion_issuance_time: logger.info( @@ -249,7 +262,7 @@ def _check_discussion_update( self.state.last_discussion_text = discussion_text issued_label = ( - issuance_time.strftime("%-I:%M %p") + issuance_time.strftime("%I:%M %p").lstrip("0") if hasattr(issuance_time, "strftime") else str(issuance_time) ) @@ -265,6 +278,10 @@ def _check_discussion_update( ) self.state.last_discussion_text = discussion_text + logger.debug( + "_check_discussion_update: issuance_time unchanged (%s) — no notification", + issuance_time, + ) return None def _check_severe_risk_change( diff --git a/src/accessiweather/weather_client_base.py b/src/accessiweather/weather_client_base.py index bbb14306..e60dcfe9 100644 --- a/src/accessiweather/weather_client_base.py +++ b/src/accessiweather/weather_client_base.py @@ -424,12 +424,18 @@ async def get_notification_event_data(self, location: Location) -> WeatherData: try: if self._is_us_location(location): - discussion_task = asyncio.create_task( - self._get_nws_forecast_and_discussion(location) - ) + # Use discussion-only fetch so a forecast API failure can never + # silently suppress AFD update notifications. + discussion_task = asyncio.create_task(self._get_nws_discussion_only(location)) alerts_task = asyncio.create_task(self._get_nws_alerts(location)) - forecast, discussion, discussion_issuance_time = await discussion_task + discussion, discussion_issuance_time = await discussion_task alerts = await alerts_task + logger.debug( + "get_notification_event_data: discussion=%s issuance=%s alerts=%s", + "ok" if discussion else "None", + discussion_issuance_time, + len(alerts.alerts) if alerts and alerts.alerts else 0, + ) weather_data.discussion = discussion weather_data.discussion_issuance_time = discussion_issuance_time weather_data.alerts = alerts or WeatherAlerts(alerts=[]) @@ -1053,6 +1059,14 @@ async def _get_nws_forecast_and_discussion( location, self.nws_base_url, self.user_agent, self.timeout, self._get_http_client() ) + async def _get_nws_discussion_only( + self, location: Location + ) -> tuple[str | None, datetime | None]: + """Fetch only the NWS AFD discussion (no forecast). Used by the notification path.""" + return await nws_client.get_nws_discussion_only( + location, self.nws_base_url, self.user_agent, self.timeout, self._get_http_client() + ) + async def _get_nws_alerts(self, location: Location) -> WeatherAlerts | None: """Delegate to the NWS client module.""" alert_radius_type = getattr(self.settings, "alert_radius_type", "county") diff --git a/src/accessiweather/weather_client_nws.py b/src/accessiweather/weather_client_nws.py index 97841a8a..a1a6c021 100644 --- a/src/accessiweather/weather_client_nws.py +++ b/src/accessiweather/weather_client_nws.py @@ -580,6 +580,9 @@ async def get_nws_forecast_and_discussion( """ Fetch forecast and discussion from the NWS API for the given location. + Forecast and discussion fetches are independent: if the forecast fetch fails, + the discussion is still returned (and vice versa). + Returns: Tuple of (forecast, discussion_text, discussion_issuance_time) @@ -591,23 +594,37 @@ async def get_nws_forecast_and_discussion( # Use provided client or create a new one if client is not None: - # Fetch grid data if not provided + # Fetch grid data if not provided (needed by both forecast and discussion) if grid_data is None: grid_url = f"{nws_base_url}/points/{location.latitude},{location.longitude}" response = await _client_get(client, grid_url, headers=headers) response.raise_for_status() grid_data = response.json() - forecast_url = grid_data["properties"]["forecast"] - response = await _client_get(client, forecast_url, headers=feature_headers) - response.raise_for_status() - forecast_data = response.json() + # Fetch forecast independently so a failure doesn't kill the discussion + parsed_forecast: Forecast | None = None + try: + forecast_url = grid_data["properties"]["forecast"] + response = await _client_get(client, forecast_url, headers=feature_headers) + response.raise_for_status() + parsed_forecast = parse_nws_forecast(response.json()) + except Exception as forecast_exc: # noqa: BLE001 + logger.warning( + "Forecast fetch failed (discussion will still be returned): %s", forecast_exc + ) discussion, discussion_issuance_time = await get_nws_discussion( client, headers, grid_data, nws_base_url ) + logger.debug( + "get_nws_forecast_and_discussion: forecast=%s discussion_len=%s issuance=%s", + "ok" if parsed_forecast else "None", + len(discussion) if discussion else 0, + discussion_issuance_time, + ) + + return parsed_forecast, discussion, discussion_issuance_time - return parse_nws_forecast(forecast_data), discussion, discussion_issuance_time grid_url = f"{nws_base_url}/points/{location.latitude},{location.longitude}" async with httpx.AsyncClient(timeout=timeout, follow_redirects=True) as new_client: @@ -615,16 +632,29 @@ async def get_nws_forecast_and_discussion( response.raise_for_status() grid_data = response.json() - forecast_url = grid_data["properties"]["forecast"] - response = await new_client.get(forecast_url, headers=feature_headers) - response.raise_for_status() - forecast_data = response.json() + # Fetch forecast independently so a failure doesn't kill the discussion + parsed_forecast = None + try: + forecast_url = grid_data["properties"]["forecast"] + response = await new_client.get(forecast_url, headers=feature_headers) + response.raise_for_status() + parsed_forecast = parse_nws_forecast(response.json()) + except Exception as forecast_exc: # noqa: BLE001 + logger.warning( + "Forecast fetch failed (discussion will still be returned): %s", forecast_exc + ) discussion, discussion_issuance_time = await get_nws_discussion( new_client, headers, grid_data, nws_base_url ) + logger.debug( + "get_nws_forecast_and_discussion: forecast=%s discussion_len=%s issuance=%s", + "ok" if parsed_forecast else "None", + len(discussion) if discussion else 0, + discussion_issuance_time, + ) - return parse_nws_forecast(forecast_data), discussion, discussion_issuance_time + return parsed_forecast, discussion, discussion_issuance_time except Exception as exc: # noqa: BLE001 logger.error(f"Failed to get NWS forecast and discussion: {exc}") @@ -633,6 +663,71 @@ async def get_nws_forecast_and_discussion( return None, None, None +@async_retry_with_backoff(max_attempts=3, base_delay=1.0, timeout=20.0) +async def get_nws_discussion_only( + location: Location, + nws_base_url: str, + user_agent: str, + timeout: float, + client: httpx.AsyncClient | None = None, +) -> tuple[str | None, datetime | None]: + """ + Fetch only the NWS Area Forecast Discussion for a location. + + Lighter-weight than get_nws_forecast_and_discussion — skips the forecast + fetch entirely. Used by the notification event path so that a transient + forecast API error never silently suppresses AFD update notifications. + + Returns: + Tuple of (discussion_text, discussion_issuance_time). + Returns (None, None) on unrecoverable error. + + """ + try: + headers = {"User-Agent": user_agent} + logger.debug( + "get_nws_discussion_only: fetching grid data for %s,%s", + location.latitude, + location.longitude, + ) + + if client is not None: + grid_url = f"{nws_base_url}/points/{location.latitude},{location.longitude}" + response = await _client_get(client, grid_url, headers=headers) + response.raise_for_status() + grid_data = response.json() + discussion, issuance_time = await get_nws_discussion( + client, headers, grid_data, nws_base_url + ) + logger.debug( + "get_nws_discussion_only: discussion_len=%s issuance=%s", + len(discussion) if discussion else 0, + issuance_time, + ) + return discussion, issuance_time + + grid_url = f"{nws_base_url}/points/{location.latitude},{location.longitude}" + async with httpx.AsyncClient(timeout=timeout, follow_redirects=True) as new_client: + response = await new_client.get(grid_url, headers=headers) + response.raise_for_status() + grid_data = response.json() + discussion, issuance_time = await get_nws_discussion( + new_client, headers, grid_data, nws_base_url + ) + logger.debug( + "get_nws_discussion_only: discussion_len=%s issuance=%s", + len(discussion) if discussion else 0, + issuance_time, + ) + return discussion, issuance_time + + except Exception as exc: # noqa: BLE001 + logger.error("Failed to fetch NWS discussion only: %s", exc) + if isinstance(exc, RETRYABLE_EXCEPTIONS) or is_retryable_http_error(exc): + raise + return None, None + + async def get_nws_discussion( client: httpx.AsyncClient, headers: dict[str, str], diff --git a/tests/test_nws_afd_notification.py b/tests/test_nws_afd_notification.py new file mode 100644 index 00000000..4bf46920 --- /dev/null +++ b/tests/test_nws_afd_notification.py @@ -0,0 +1,474 @@ +""" +Tests for AFD notification failure isolation. + +Covers: +- get_nws_forecast_and_discussion: forecast failure does not kill discussion +- get_nws_discussion_only: happy-path fetch and error handling +- get_notification_event_data: uses discussion-only path, returns issuance_time +""" + +from __future__ import annotations + +from datetime import datetime, timezone +from unittest.mock import AsyncMock, MagicMock, patch + +import httpx +import pytest + +from accessiweather.models import Location, WeatherAlerts +from accessiweather.weather_client import WeatherClient +from accessiweather.weather_client_nws import ( + get_nws_discussion_only, + get_nws_forecast_and_discussion, +) + +NWS_BASE = "https://api.weather.gov" +US_LOCATION = Location(name="NYC", latitude=40.7128, longitude=-74.0060, country_code="US") + +GRID_DATA = { + "properties": { + "forecast": f"{NWS_BASE}/gridpoints/OKX/36,38/forecast", + } +} + +PRODUCTS_LIST = { + "@graph": [ + { + "id": "test-afd-product-001", + "issuanceTime": "2026-01-20T19:01:00+00:00", + } + ] +} + +PRODUCT_TEXT = {"productText": "THIS IS A TEST AFD ISSUED 701 PM EST MON JAN 20 2026."} + +FORECAST_DATA = { + "properties": { + "periods": [ + { + "name": "Tonight", + "temperature": 32, + "temperatureUnit": "F", + "windSpeed": "5 mph", + "windDirection": "NW", + "shortForecast": "Clear", + "detailedForecast": "Clear skies.", + "isDaytime": False, + "startTime": "2026-01-20T18:00:00-05:00", + "endTime": "2026-01-21T06:00:00-05:00", + } + ], + "generatedAt": "2026-01-20T19:01:00+00:00", + } +} + + +def _make_mock_response(json_data, status_code=200): + """Build a synchronous mock httpx.Response.""" + resp = MagicMock(spec=httpx.Response) + resp.status_code = status_code + resp.json.return_value = json_data + resp.raise_for_status = MagicMock() + return resp + + +def _make_mock_response_error(status_code=503): + """Build a mock response whose raise_for_status raises HTTPStatusError.""" + resp = MagicMock(spec=httpx.Response) + resp.status_code = status_code + error = httpx.HTTPStatusError( + f"HTTP {status_code}", + request=MagicMock(), + response=resp, + ) + resp.raise_for_status.side_effect = error + return resp + + +def _build_client_for_discussion_happy(include_forecast_response=True): + """ + Return a mock AsyncClient whose .get() returns appropriate responses. + + Covers the full forecast+discussion fetch path. + """ + responses = { + f"{NWS_BASE}/points/{US_LOCATION.latitude},{US_LOCATION.longitude}": _make_mock_response( + GRID_DATA + ), + f"{NWS_BASE}/gridpoints/OKX/36,38/forecast": _make_mock_response(FORECAST_DATA), + f"{NWS_BASE}/products/types/AFD/locations/OKX": _make_mock_response(PRODUCTS_LIST), + f"{NWS_BASE}/products/test-afd-product-001": _make_mock_response(PRODUCT_TEXT), + } + + def side_effect(url, **kwargs): + return responses.get(url, _make_mock_response({}, status_code=404)) + + client = MagicMock(spec=httpx.AsyncClient) + client.get.side_effect = side_effect + return client + + +def _build_client_forecast_fails(): + """ + Return a mock AsyncClient where the forecast endpoint returns 503. + + Discussion endpoints still work fine. + """ + responses = { + f"{NWS_BASE}/points/{US_LOCATION.latitude},{US_LOCATION.longitude}": _make_mock_response( + GRID_DATA + ), + f"{NWS_BASE}/gridpoints/OKX/36,38/forecast": _make_mock_response_error(503), + f"{NWS_BASE}/products/types/AFD/locations/OKX": _make_mock_response(PRODUCTS_LIST), + f"{NWS_BASE}/products/test-afd-product-001": _make_mock_response(PRODUCT_TEXT), + } + + def side_effect(url, **kwargs): + return responses.get(url, _make_mock_response({}, status_code=404)) + + client = MagicMock(spec=httpx.AsyncClient) + client.get.side_effect = side_effect + return client + + +class TestGetNwsForecastAndDiscussionIsolation: + """Forecast failure must not suppress AFD discussion data.""" + + @pytest.mark.asyncio + async def test_forecast_failure_still_returns_discussion(self): + """When forecast fetch raises a non-retryable HTTP error, discussion is still returned.""" + mock_client = _build_client_forecast_fails() + + forecast, discussion, issuance_time = await get_nws_forecast_and_discussion( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=mock_client, + ) + + # Forecast should be None because it failed + assert forecast is None, "Expected forecast to be None when HTTP 503 returned" + + # Discussion MUST still be populated — this is the core bug fix + assert discussion is not None, "Discussion must not be None even when forecast fetch fails" + assert "AFD" in discussion + + # issuance_time MUST be set for AFD update notifications to fire + assert issuance_time is not None, ( + "issuance_time must not be None even when forecast fetch fails" + ) + assert issuance_time == datetime(2026, 1, 20, 19, 1, 0, tzinfo=timezone.utc) + + @pytest.mark.asyncio + async def test_forecast_and_discussion_both_succeed(self): + """When everything works, both forecast and discussion are returned.""" + mock_client = _build_client_for_discussion_happy() + + forecast, discussion, issuance_time = await get_nws_forecast_and_discussion( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=mock_client, + ) + + assert forecast is not None + assert discussion is not None + assert issuance_time is not None + + @pytest.mark.asyncio + async def test_grid_data_failure_returns_all_none(self): + """If the /points grid fetch fails (retryable), the function raises to let retry handle it.""" + # 503 on /points is a retryable HTTP error — the retry decorator will re-raise it + error_response = _make_mock_response_error(503) + mock_client = MagicMock(spec=httpx.AsyncClient) + mock_client.get.return_value = error_response + + # The retry decorator catches retryable errors and re-raises after max attempts. + # After exhausting retries it should propagate the exception. + with pytest.raises(httpx.HTTPStatusError): + await get_nws_forecast_and_discussion( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=mock_client, + ) + + +class TestGetNwsDiscussionOnly: + """Tests for the lightweight discussion-only fetch.""" + + @pytest.mark.asyncio + async def test_happy_path_returns_discussion_and_issuance_time(self): + """Happy path: returns (discussion_text, issuance_time).""" + responses = { + f"{NWS_BASE}/points/{US_LOCATION.latitude},{US_LOCATION.longitude}": ( + _make_mock_response(GRID_DATA) + ), + f"{NWS_BASE}/products/types/AFD/locations/OKX": _make_mock_response(PRODUCTS_LIST), + f"{NWS_BASE}/products/test-afd-product-001": _make_mock_response(PRODUCT_TEXT), + } + + def side_effect(url, **kwargs): + return responses.get(url, _make_mock_response({}, status_code=404)) + + mock_client = MagicMock(spec=httpx.AsyncClient) + mock_client.get.side_effect = side_effect + + discussion, issuance_time = await get_nws_discussion_only( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=mock_client, + ) + + assert discussion is not None + assert "AFD" in discussion + assert issuance_time == datetime(2026, 1, 20, 19, 1, 0, tzinfo=timezone.utc) + + @pytest.mark.asyncio + async def test_no_forecast_call_is_made(self): + """Discussion-only fetch must never call the forecast endpoint.""" + responses = { + f"{NWS_BASE}/points/{US_LOCATION.latitude},{US_LOCATION.longitude}": ( + _make_mock_response(GRID_DATA) + ), + f"{NWS_BASE}/products/types/AFD/locations/OKX": _make_mock_response(PRODUCTS_LIST), + f"{NWS_BASE}/products/test-afd-product-001": _make_mock_response(PRODUCT_TEXT), + } + + def side_effect(url, **kwargs): + return responses.get(url, _make_mock_response({}, status_code=404)) + + mock_client = MagicMock(spec=httpx.AsyncClient) + mock_client.get.side_effect = side_effect + + await get_nws_discussion_only( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=mock_client, + ) + + called_urls = [call.args[0] for call in mock_client.get.call_args_list] + assert all("forecast" not in url.split("/")[-1] for url in called_urls), ( + f"Forecast endpoint must not be called by get_nws_discussion_only, " + f"but called: {called_urls}" + ) + + @pytest.mark.asyncio + async def test_unrecoverable_error_returns_none_none(self): + """Non-retryable error on /points returns (None, None).""" + # 404 is not in RETRYABLE_EXCEPTIONS so it should return (None, None) + error_response = _make_mock_response_error(404) + mock_client = MagicMock(spec=httpx.AsyncClient) + mock_client.get.return_value = error_response + + discussion, issuance_time = await get_nws_discussion_only( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=mock_client, + ) + + assert discussion is None + assert issuance_time is None + + +class TestGetNotificationEventDataDiscussionPath: + """WeatherClient.get_notification_event_data must use the discussion-only path.""" + + @pytest.fixture + def client(self): + return WeatherClient() + + @pytest.fixture + def us_location(self): + return US_LOCATION + + @pytest.mark.asyncio + async def test_discussion_issuance_time_populated(self, client, us_location): + """get_notification_event_data populates discussion_issuance_time for US locations.""" + issuance = datetime(2026, 1, 20, 19, 1, 0, tzinfo=timezone.utc) + + client._get_nws_discussion_only = AsyncMock(return_value=("AFD text", issuance)) + client._get_nws_alerts = AsyncMock(return_value=WeatherAlerts(alerts=[])) + client._fetch_nws_cancel_references = AsyncMock(return_value=set()) + + weather_data = await client.get_notification_event_data(us_location) + + assert weather_data.discussion_issuance_time == issuance + assert weather_data.discussion == "AFD text" + + @pytest.mark.asyncio + async def test_discussion_only_method_is_called_not_forecast_and_discussion( + self, client, us_location + ): + """Notification path calls _get_nws_discussion_only, not _get_nws_forecast_and_discussion.""" + issuance = datetime(2026, 1, 20, 19, 1, 0, tzinfo=timezone.utc) + + client._get_nws_discussion_only = AsyncMock(return_value=("AFD text", issuance)) + client._get_nws_forecast_and_discussion = AsyncMock( + return_value=(None, "should not be called", None) + ) + client._get_nws_alerts = AsyncMock(return_value=WeatherAlerts(alerts=[])) + client._fetch_nws_cancel_references = AsyncMock(return_value=set()) + + await client.get_notification_event_data(us_location) + + client._get_nws_discussion_only.assert_called_once_with(us_location) + client._get_nws_forecast_and_discussion.assert_not_called() + + @pytest.mark.asyncio + async def test_discussion_fetch_failure_does_not_crash(self, client, us_location): + """If discussion fetch returns (None, None), weather_data is returned without crashing.""" + client._get_nws_discussion_only = AsyncMock(return_value=(None, None)) + client._get_nws_alerts = AsyncMock(return_value=WeatherAlerts(alerts=[])) + client._fetch_nws_cancel_references = AsyncMock(return_value=set()) + + weather_data = await client.get_notification_event_data(us_location) + + assert weather_data is not None + assert weather_data.discussion_issuance_time is None + + +def _make_async_client_mock(responses): + """Build a mock AsyncClient usable as an async context manager.""" + + def side_effect(url, **kwargs): + return responses.get(url, _make_mock_response({}, status_code=404)) + + mock_client = MagicMock(spec=httpx.AsyncClient) + mock_client.get.side_effect = side_effect + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + return mock_client + + +class TestGetNwsForecastAndDiscussionNoClientBranch: + """Covers the no-client (new AsyncClient) branch of get_nws_forecast_and_discussion.""" + + @pytest.mark.asyncio + async def test_happy_path_no_client(self): + """client=None creates its own AsyncClient and returns forecast+discussion.""" + responses = { + f"{NWS_BASE}/points/{US_LOCATION.latitude},{US_LOCATION.longitude}": ( + _make_mock_response(GRID_DATA) + ), + f"{NWS_BASE}/gridpoints/OKX/36,38/forecast": _make_mock_response(FORECAST_DATA), + f"{NWS_BASE}/products/types/AFD/locations/OKX": _make_mock_response(PRODUCTS_LIST), + f"{NWS_BASE}/products/test-afd-product-001": _make_mock_response(PRODUCT_TEXT), + } + mock_client = _make_async_client_mock(responses) + + with patch("accessiweather.weather_client_nws.httpx.AsyncClient", return_value=mock_client): + forecast, discussion, issuance_time = await get_nws_forecast_and_discussion( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=None, + ) + + assert forecast is not None + assert discussion is not None + assert issuance_time == datetime(2026, 1, 20, 19, 1, 0, tzinfo=timezone.utc) + + @pytest.mark.asyncio + async def test_forecast_failure_no_client(self): + """client=None: forecast 503 does not suppress discussion.""" + responses = { + f"{NWS_BASE}/points/{US_LOCATION.latitude},{US_LOCATION.longitude}": ( + _make_mock_response(GRID_DATA) + ), + f"{NWS_BASE}/gridpoints/OKX/36,38/forecast": _make_mock_response_error(503), + f"{NWS_BASE}/products/types/AFD/locations/OKX": _make_mock_response(PRODUCTS_LIST), + f"{NWS_BASE}/products/test-afd-product-001": _make_mock_response(PRODUCT_TEXT), + } + mock_client = _make_async_client_mock(responses) + + with patch("accessiweather.weather_client_nws.httpx.AsyncClient", return_value=mock_client): + forecast, discussion, issuance_time = await get_nws_forecast_and_discussion( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=None, + ) + + assert forecast is None + assert discussion is not None + assert issuance_time is not None + + +class TestGetNwsDiscussionOnlyNoClientBranch: + """Covers the no-client branch of get_nws_discussion_only.""" + + @pytest.mark.asyncio + async def test_happy_path_no_client(self): + """client=None creates its own AsyncClient and returns discussion+issuance_time.""" + responses = { + f"{NWS_BASE}/points/{US_LOCATION.latitude},{US_LOCATION.longitude}": ( + _make_mock_response(GRID_DATA) + ), + f"{NWS_BASE}/products/types/AFD/locations/OKX": _make_mock_response(PRODUCTS_LIST), + f"{NWS_BASE}/products/test-afd-product-001": _make_mock_response(PRODUCT_TEXT), + } + mock_client = _make_async_client_mock(responses) + + with patch("accessiweather.weather_client_nws.httpx.AsyncClient", return_value=mock_client): + discussion, issuance_time = await get_nws_discussion_only( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=None, + ) + + assert discussion is not None + assert issuance_time == datetime(2026, 1, 20, 19, 1, 0, tzinfo=timezone.utc) + + @pytest.mark.asyncio + async def test_retryable_error_raises_no_client(self): + """client=None: 503 on grid fetch raises after retry exhaustion (covers line 727).""" + error_response = _make_mock_response_error(503) + mock_client = MagicMock(spec=httpx.AsyncClient) + mock_client.get.return_value = error_response + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + + with ( + patch("accessiweather.weather_client_nws.httpx.AsyncClient", return_value=mock_client), + pytest.raises(httpx.HTTPStatusError), + ): + await get_nws_discussion_only( + location=US_LOCATION, + nws_base_url=NWS_BASE, + user_agent="Test/1.0", + timeout=10.0, + client=None, + ) + + +class TestWeatherClientGetNwsDiscussionOnlyDelegation: + """Covers WeatherClient._get_nws_discussion_only (base line 1066).""" + + @pytest.mark.asyncio + async def test_delegates_to_nws_client_module(self): + """_get_nws_discussion_only calls nws_client.get_nws_discussion_only.""" + issuance = datetime(2026, 1, 20, 19, 1, 0, tzinfo=timezone.utc) + + weather_client = WeatherClient() + with patch( + "accessiweather.weather_client_nws.get_nws_discussion_only", + new=AsyncMock(return_value=("AFD text", issuance)), + ) as mock_fn: + result = await weather_client._get_nws_discussion_only(US_LOCATION) + + assert result == ("AFD text", issuance) + mock_fn.assert_called_once() diff --git a/tests/test_split_notification_timers.py b/tests/test_split_notification_timers.py index b27f5c80..fdf4e320 100644 --- a/tests/test_split_notification_timers.py +++ b/tests/test_split_notification_timers.py @@ -296,14 +296,11 @@ def client(self): @pytest.mark.asyncio async def test_us_location_fetches_nws_data(self, client, us_location): - forecast = MagicMock() discussion = "Discussion text" issuance_time = MagicMock() alerts = WeatherAlerts(alerts=[]) - client._get_nws_forecast_and_discussion = AsyncMock( - return_value=(forecast, discussion, issuance_time) - ) + client._get_nws_discussion_only = AsyncMock(return_value=(discussion, issuance_time)) client._get_nws_alerts = AsyncMock(return_value=alerts) result = await client.get_notification_event_data(us_location) @@ -311,12 +308,12 @@ async def test_us_location_fetches_nws_data(self, client, us_location): assert result.discussion == discussion assert result.discussion_issuance_time == issuance_time assert result.alerts == alerts - client._get_nws_forecast_and_discussion.assert_awaited_once_with(us_location) + client._get_nws_discussion_only.assert_awaited_once_with(us_location) client._get_nws_alerts.assert_awaited_once_with(us_location) @pytest.mark.asyncio async def test_us_location_none_alerts_becomes_empty(self, client, us_location): - client._get_nws_forecast_and_discussion = AsyncMock(return_value=(None, None, None)) + client._get_nws_discussion_only = AsyncMock(return_value=(None, None)) client._get_nws_alerts = AsyncMock(return_value=None) result = await client.get_notification_event_data(us_location) @@ -352,7 +349,7 @@ async def test_intl_location_without_vc_client(self, intl_location): @pytest.mark.asyncio async def test_tracks_alert_lifecycle_diff(self, client, us_location): alerts = WeatherAlerts(alerts=[]) - client._get_nws_forecast_and_discussion = AsyncMock(return_value=(None, None, None)) + client._get_nws_discussion_only = AsyncMock(return_value=(None, None)) client._get_nws_alerts = AsyncMock(return_value=alerts) result = await client.get_notification_event_data(us_location) @@ -361,7 +358,7 @@ async def test_tracks_alert_lifecycle_diff(self, client, us_location): @pytest.mark.asyncio async def test_exception_returns_empty_alerts(self, client, us_location): - client._get_nws_forecast_and_discussion = AsyncMock(side_effect=RuntimeError("API down")) + client._get_nws_discussion_only = AsyncMock(side_effect=RuntimeError("API down")) client._get_nws_alerts = AsyncMock(side_effect=RuntimeError("API down")) result = await client.get_notification_event_data(us_location)