Skip to content

Commit

Permalink
Fix incorrect value from get weather (#42)
Browse files Browse the repository at this point in the history
* style: removed pylint: disable=line-too-long

* fix: log update errors as UpdateFailed

* style: addressed PT006 warning

* style: addressed W7451 warning

Platforms must be sorted alphabetically

* test: fixed failing test
  • Loading branch information
iprak authored Aug 26, 2024
1 parent 11364f0 commit 4405642
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 40 deletions.
2 changes: 1 addition & 1 deletion custom_components/weatherapi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from .coordinator import WeatherAPIUpdateCoordinator, WeatherAPIUpdateCoordinatorConfig
from .sensor import SENSOR_DESCRIPTIONS

PLATFORMS: Final = [Platform.WEATHER, Platform.SENSOR]
PLATFORMS: Final = [Platform.SENSOR, Platform.WEATHER]
_LOGGER = logging.getLogger(__name__)


Expand Down
33 changes: 14 additions & 19 deletions custom_components/weatherapi/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.aiohttp_client import async_get_clientsession
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
import homeassistant.util.dt as dt_util

from .const import (
Expand Down Expand Up @@ -214,12 +214,10 @@ async def get_weather(self):
"aqi": "yes",
}

# pylint: disable=line-too-long
headers = {
"accept": "application/json",
"user-agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/117.0.0.0 Safari/537.36",
}
# pylint: enable=line-too-long

try:
session: ClientSession = async_get_clientsession(self.hass)
Expand All @@ -234,26 +232,21 @@ async def get_weather(self):
# Deciode only if 200 status. This should avoid
# JSONDecodeError: Expecting value: line 1 column 1 (char 0)
if response.status != HTTPStatus.OK:
_LOGGER.error(
"WeatherAPI responded with HTTP error %s: %s",
response.status,
response.reason,
raise UpdateFailed(
f"WeatherAPI responded with status={response.status}, reason={response.reason}"
)
return False

json_data = await response.json(content_type=None)
if json_data is None:
_LOGGER.warning("No data received")
return False
raise UpdateFailed(
f"WeatherAPI responded with HTTP error {response.status} but no data was provided."
)

error = json_data.get("error")
if error:
_LOGGER.error(
"WeatherAPI responded with error %s: %s",
error.get("code"),
error.get("message"),
raise UpdateFailed(
f"WeatherAPI responded with error={error.get("code")}, message={error.get("message")}"
)
return False

# Using timeZome from location falling back to local timezone
location = json_data.get("location", {})
Expand All @@ -275,11 +268,13 @@ async def get_weather(self):
return result

except asyncio.TimeoutError as exception:
_LOGGER.error("Timeout invoking WeatherAPI end point: %s", exception)
raise CannotConnect from exception
raise UpdateFailed(
f"Timeout invoking WeatherAPI end point: {exception}"
) from exception
except aiohttp.ClientError as exception:
_LOGGER.error("Error invoking WeatherAPI end point: %s", exception)
raise CannotConnect from exception
raise UpdateFailed(
f"Error invoking WeatherAPI end point: {exception}"
) from exception

def populate_time_zone(self, zone: str):
"""Define timzeone for the forecasts."""
Expand Down
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ def load_json(filename):
@pytest.fixture
def mock_json():
"""Return sample JSON data with 24 hourly forecast and 1 day forecast."""
yield json.loads(load_json("response.json"))
return json.loads(load_json("response.json"))


@pytest.fixture
def coordinator_config():
"""Return a mock coordinator configuration."""
yield coordinator.WeatherAPIUpdateCoordinatorConfig(
return coordinator.WeatherAPIUpdateCoordinatorConfig(
api_key="api_key",
location="latitude,longitude",
name="Place",
Expand Down
51 changes: 33 additions & 18 deletions tests/test_coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@
ATTR_CONDITION_SUNNY,
Forecast,
)
from homeassistant.core import HomeAssistant
from homeassistant.helpers.update_coordinator import UpdateFailed


@pytest.mark.parametrize(
"value, result",
("value", "result"),
[
("1", 1),
(None, None),
Expand All @@ -29,7 +31,7 @@ def test_to_int(value, result) -> None:


@pytest.mark.parametrize(
"value, result",
("value", "result"),
[
("1", 1),
(None, None),
Expand All @@ -45,7 +47,7 @@ def test_to_float(value, result) -> None:


@pytest.mark.parametrize(
"value, result",
("value", "result"),
[
(None, None),
# Unit test are not in local timezone
Expand All @@ -58,7 +60,7 @@ def test_datetime_to_iso(value, result) -> None:


@pytest.mark.parametrize(
"value,is_day,result",
("value", "is_day", "result"),
[
("1000", True, ATTR_CONDITION_SUNNY),
("1000", False, ATTR_CONDITION_CLEAR_NIGHT),
Expand All @@ -72,14 +74,16 @@ def test_parse_condition_code(value, is_day, result) -> None:


@pytest.mark.parametrize(
"response_status, json,result",
("response_status", "json", "result"),
[
(HTTPStatus.OK, {"error": {"code": "12345"}}, False),
(HTTPStatus.OK, {}, True),
(HTTPStatus.OK, {"error": {}}, True),
],
)
async def test_is_valid_api_key(hass, response_status, json, result) -> None:
async def test_is_valid_api_key(
hass: HomeAssistant, response_status, json, result
) -> None:
"""Test is_valid_api_key function."""
session = Mock()
response = Mock()
Expand All @@ -95,13 +99,13 @@ async def test_is_valid_api_key(hass, response_status, json, result) -> None:
assert await coordinator.is_valid_api_key(hass, "api_key") == result


async def test_is_valid_api_key_raises_missing_key(hass) -> None:
async def test_is_valid_api_key_raises_missing_key(hass: HomeAssistant) -> None:
"""Test missing key input for is_valid_api_key."""
with pytest.raises(coordinator.InvalidApiKey):
await coordinator.is_valid_api_key(hass, "")


async def test_is_valid_api_key_raises_cannotconnect(hass) -> None:
async def test_is_valid_api_key_raises_cannotconnect(hass: HomeAssistant) -> None:
"""Test connection issues for is_valid_api_key."""
session = Mock()
session.get = AsyncMock(side_effect=asyncio.TimeoutError)
Expand All @@ -115,7 +119,7 @@ async def test_is_valid_api_key_raises_cannotconnect(hass) -> None:


async def test_async_update_data_http_error(
hass, mock_json, coordinator_config
hass: HomeAssistant, mock_json, coordinator_config
) -> None:
"""Test failed coordinator data update."""
session = Mock()
Expand All @@ -133,7 +137,9 @@ async def test_async_update_data_http_error(
assert result is None


async def test_get_weather_raises_cannotconnect(hass, coordinator_config) -> None:
async def test_get_weather_raises_cannotconnect(
hass: HomeAssistant, coordinator_config
) -> None:
"""Test failed connection for coordinator data update."""
session = Mock()
session.get = AsyncMock(side_effect=asyncio.TimeoutError)
Expand All @@ -142,12 +148,13 @@ async def test_get_weather_raises_cannotconnect(hass, coordinator_config) -> Non
coordinator,
"async_get_clientsession",
return_value=session,
), pytest.raises(coordinator.CannotConnect), patch.object(coordinator, "_LOGGER"):
), patch.object(coordinator, "_LOGGER"):
coord = coordinator.WeatherAPIUpdateCoordinator(hass, coordinator_config)
await coord.get_weather()
with pytest.raises(UpdateFailed):
await coord.get_weather()


async def test_get_weather(hass, mock_json, coordinator_config) -> None:
async def test_get_weather(hass: HomeAssistant, mock_json, coordinator_config) -> None:
"""Test coordinator data update."""
session = Mock()
response = Mock()
Expand All @@ -167,7 +174,9 @@ async def test_get_weather(hass, mock_json, coordinator_config) -> None:
assert result[HOURLY_FORECAST]


async def test_get_weather_hourly_forecast(hass, mock_json, coordinator_config) -> None:
async def test_get_weather_hourly_forecast(
hass: HomeAssistant, mock_json, coordinator_config
) -> None:
"""Test hourly forecast in coordinator data update."""
session = Mock()
response = Mock()
Expand All @@ -189,7 +198,7 @@ async def test_get_weather_hourly_forecast(hass, mock_json, coordinator_config)


async def test_get_weather_hourly_forecast_missing_data(
hass, mock_json, coordinator_config
hass: HomeAssistant, mock_json, coordinator_config
) -> None:
"""Test hourly forecast with data missing."""
session = Mock()
Expand Down Expand Up @@ -218,7 +227,9 @@ async def test_get_weather_hourly_forecast_missing_data(
assert len(coordinator.get_logger().warning.mock_calls) == 1


async def test_get_weather_no_forecast_data(hass, coordinator_config) -> None:
async def test_get_weather_no_forecast_data(
hass: HomeAssistant, coordinator_config
) -> None:
"""Test missing forecast data."""
session = Mock()
response = Mock()
Expand Down Expand Up @@ -248,7 +259,9 @@ async def test_get_weather_no_forecast_data(hass, coordinator_config) -> None:
)


async def test_get_weather_no_forecastday_data(hass, coordinator_config) -> None:
async def test_get_weather_no_forecastday_data(
hass: HomeAssistant, coordinator_config
) -> None:
"""Test missing forecast data."""
session = Mock()
response = Mock()
Expand Down Expand Up @@ -344,7 +357,9 @@ async def test_get_weather_no_forecastday_data(hass, coordinator_config) -> None
),
],
)
def test_parse_hour_forecast(hass, coordinator_config, zone, data, expected) -> None:
def test_parse_hour_forecast(
hass: HomeAssistant, coordinator_config, zone, data, expected
) -> None:
"""Test parse_hour_forecast function."""

coord = coordinator.WeatherAPIUpdateCoordinator(hass, coordinator_config)
Expand Down

0 comments on commit 4405642

Please sign in to comment.