mirror of
https://github.com/agessaman/meshcore-bot.git
synced 2026-08-28 13:24:09 +00:00
feat(location): enhance geocoding and error handling in location resolution
- Added asynchronous support for location resolution, improving performance and responsiveness. - Introduced detailed error handling for invalid latitude and longitude inputs, providing specific feedback to users. - Implemented a caching mechanism for geocoding results to optimize repeated lookups. - Refactored the AQI command to streamline location handling and improve clarity in error messages. - Expanded unit tests to cover new geocoding features and error scenarios, ensuring robust functionality.
This commit is contained in:
agessaman
@@ -4,22 +4,27 @@ AQI command for the MeshCore Bot
|
||||
Provides Air Quality Index information using OpenMeteo API
|
||||
"""
|
||||
|
||||
from typing import Optional
|
||||
|
||||
import openmeteo_requests
|
||||
import requests_cache
|
||||
from retry_requests import retry
|
||||
|
||||
from ..models import MeshMessage
|
||||
from ..location import (
|
||||
OPTIONS_AQI,
|
||||
ResolveOptions,
|
||||
classify_location,
|
||||
geocode_city_best_effort,
|
||||
get_neighborhood_queries as location_neighborhood_queries,
|
||||
resolve_location,
|
||||
)
|
||||
from ..location import (
|
||||
get_neighborhood_queries as location_neighborhood_queries,
|
||||
)
|
||||
from ..models import MeshMessage
|
||||
from ..utils import (
|
||||
abbreviate_location,
|
||||
get_nominatim_geocoder,
|
||||
is_valid_timezone,
|
||||
normalize_us_state,
|
||||
)
|
||||
from .base_command import BaseCommand
|
||||
|
||||
@@ -178,16 +183,12 @@ class AqiCommand(BaseCommand):
|
||||
await self.send_response(message, self.astronomical_responses[location_lower])
|
||||
return True
|
||||
|
||||
location, location_type = classify_location(
|
||||
location, use_international_cities=True
|
||||
)
|
||||
|
||||
try:
|
||||
# Record execution for this user
|
||||
self.record_execution(message.sender_id)
|
||||
|
||||
# Get AQI data for the location
|
||||
aqi_data = await self.get_aqi_for_location(location, location_type)
|
||||
# Get AQI data for the location (single resolve with intl rewrite)
|
||||
aqi_data = await self.get_aqi_for_location(location)
|
||||
|
||||
# Send the response
|
||||
await self.send_response(message, aqi_data)
|
||||
@@ -198,12 +199,29 @@ class AqiCommand(BaseCommand):
|
||||
await self.send_response(message, f"Error getting AQI data: {e}")
|
||||
return True
|
||||
|
||||
async def get_aqi_for_location(self, location: str, location_type: str) -> str:
|
||||
"""Get AQI data for a location (city or coordinates).
|
||||
def _resolved_state_differs_from_default(self, address_info: Optional[dict]) -> bool:
|
||||
"""True when reverse-geocode state/country differs from bot default_state."""
|
||||
if not address_info or not self.default_state:
|
||||
return False
|
||||
country = address_info.get("country", "")
|
||||
state = address_info.get("state", "")
|
||||
default_abbr, default_full = normalize_us_state(self.default_state)
|
||||
defaults = {d for d in (self.default_state, default_abbr, default_full) if d}
|
||||
if country in ("United States", "US", "United States of America"):
|
||||
abbr, full = normalize_us_state(state) if state else (None, None)
|
||||
actuals = {a for a in (abbr, full, state) if a}
|
||||
return bool(actuals) and actuals.isdisjoint(defaults)
|
||||
actual_state = country or address_info.get("province") or ""
|
||||
return bool(actual_state) and actual_state not in defaults
|
||||
|
||||
async def get_aqi_for_location(
|
||||
self, location: str, location_type: Optional[str] = None
|
||||
) -> str:
|
||||
"""Get AQI data for a location (city, ZIP, or coordinates).
|
||||
|
||||
Args:
|
||||
location: Location string (city name, ZIP, or "lat,lon").
|
||||
location_type: Type of location ("city", "zipcode", "coordinates").
|
||||
location: Raw location string (city name, ZIP, or "lat,lon").
|
||||
location_type: Unused; kept for call-site/test compatibility.
|
||||
|
||||
Returns:
|
||||
str: Formatted AQI string or error message.
|
||||
@@ -212,25 +230,30 @@ class AqiCommand(BaseCommand):
|
||||
opts = ResolveOptions(
|
||||
default_state=self.default_state,
|
||||
default_country=self.default_country,
|
||||
use_international_cities=False, # already applied in execute/classify
|
||||
use_neighborhoods=True,
|
||||
use_structured_zip=True,
|
||||
label_style="abbreviated",
|
||||
use_international_cities=OPTIONS_AQI.use_international_cities,
|
||||
use_neighborhoods=OPTIONS_AQI.use_neighborhoods,
|
||||
use_structured_zip=OPTIONS_AQI.use_structured_zip,
|
||||
label_style=OPTIONS_AQI.label_style,
|
||||
include_address_info=True,
|
||||
timeout=10,
|
||||
)
|
||||
# Pass already-classified input: resolve will re-classify coords/ZIP/city.
|
||||
resolved = resolve_location(self.bot, location, options=opts)
|
||||
|
||||
if resolved.error == "invalid_latitude":
|
||||
detail = resolved.error_detail or location
|
||||
return f"Invalid latitude: {detail}. Must be between -90 and 90."
|
||||
if resolved.error == "invalid_longitude":
|
||||
detail = resolved.error_detail or location
|
||||
return f"Invalid longitude: {detail}. Must be between -180 and 180."
|
||||
if resolved.error == "invalid_coordinates":
|
||||
return f"Invalid coordinates format: {location}. Use format: lat,lon (e.g., 47.6,-122.3)"
|
||||
if resolved.error == "no_location_zipcode":
|
||||
return f"Could not find ZIP code '{location.strip()}'"
|
||||
if resolved.error == "no_location_city":
|
||||
if "," in location:
|
||||
return f"Could not find city '{location}'"
|
||||
if "," in (resolved.query or location):
|
||||
return f"Could not find city '{resolved.query or location}'"
|
||||
region = self.default_state or self.default_country
|
||||
return f"Could not find city '{location}' in {region}"
|
||||
return f"Could not find city '{resolved.query or location}' in {region}"
|
||||
if resolved.lat is None or resolved.lon is None:
|
||||
return f"Could not find location '{location}'"
|
||||
|
||||
@@ -252,11 +275,10 @@ class AqiCommand(BaseCommand):
|
||||
location_prefix = f"{city_display}: "
|
||||
elif location_type == "zipcode":
|
||||
location_prefix = f"{location.strip()}: "
|
||||
else:
|
||||
# Fall back to shorter abbreviation when over budget
|
||||
elif self._resolved_state_differs_from_default(address_info):
|
||||
# Over budget: only keep a short prefix when outside default region
|
||||
short = abbreviate_location(city_display, max_length=20)
|
||||
if len(f"{short}: {aqi_data}") <= 130:
|
||||
location_prefix = f"{short}: "
|
||||
location_prefix = f"{short}: "
|
||||
elif location_type == "zipcode":
|
||||
location_prefix = f"{location.strip()}: "
|
||||
|
||||
|
||||
+56
-81
@@ -9,6 +9,7 @@ city | optional repeater / region capitals / neighborhoods) with opt-in
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import re
|
||||
from dataclasses import dataclass, replace
|
||||
from typing import Any, Literal, Mapping, Optional, Union
|
||||
@@ -18,11 +19,8 @@ import requests
|
||||
from .region_capitals import REGION_DEFAULT_NOTE, region_capital_query
|
||||
from .utils import (
|
||||
abbreviate_location,
|
||||
geocode_city,
|
||||
geocode_city_sync,
|
||||
geocode_zipcode,
|
||||
geocode_zipcode_sync,
|
||||
is_country_name,
|
||||
normalize_us_state,
|
||||
rate_limited_nominatim_geocode_sync,
|
||||
rate_limited_nominatim_reverse_sync,
|
||||
@@ -40,11 +38,29 @@ _COUNTRY_WORD_INDICATORS = frozenset({
|
||||
"spain", "australia", "japan", "china", "india", "brazil",
|
||||
})
|
||||
|
||||
_COUNTRY_NICKNAMES = frozenset({
|
||||
"uk", "uae", "united kingdom", "south korea", "north korea", "czech republic",
|
||||
"sri lanka", "south africa", "puerto rico", "san marino", "vatican",
|
||||
# Explicit allowlist for "city, country" intl geocode shortcut (legacy AQI list).
|
||||
# Do not use utils.is_country_name here — its len>2 heuristic is too broad.
|
||||
_COUNTRY_TOKENS = frozenset({
|
||||
"canada", "mexico", "uk", "united kingdom", "france", "germany", "italy",
|
||||
"spain", "australia", "japan", "china", "india", "brazil", "uae", "russia",
|
||||
"korea", "thailand", "singapore", "egypt", "turkey", "israel", "south africa",
|
||||
"kenya", "nigeria", "argentina", "peru", "chile", "colombia", "venezuela",
|
||||
"cuba", "jamaica", "puerto rico", "iceland", "norway", "sweden", "denmark",
|
||||
"finland", "poland", "czech republic", "hungary", "romania", "bulgaria",
|
||||
"croatia", "serbia", "greece", "portugal", "ireland", "belgium",
|
||||
"netherlands", "switzerland", "austria", "monaco", "andorra", "san marino",
|
||||
"vatican", "luxembourg", "malta", "cyprus", "albania", "macedonia",
|
||||
"montenegro", "bosnia", "slovenia", "slovakia", "lithuania", "latvia",
|
||||
"estonia", "belarus", "ukraine", "moldova", "georgia", "armenia",
|
||||
"azerbaijan", "kazakhstan", "uzbekistan", "kyrgyzstan", "tajikistan",
|
||||
"turkmenistan", "afghanistan", "pakistan", "bangladesh", "sri lanka",
|
||||
"nepal", "bhutan", "myanmar", "laos", "cambodia", "vietnam", "malaysia",
|
||||
"indonesia", "philippines", "taiwan", "north korea", "south korea",
|
||||
"mongolia",
|
||||
})
|
||||
|
||||
GEOCODE_CACHE_CAP = 256
|
||||
|
||||
# Bare place → "city, country" (full-string match, including multi-word keys).
|
||||
INTERNATIONAL_CITIES: dict[str, str] = {
|
||||
"beijing": "beijing, china",
|
||||
@@ -243,6 +259,7 @@ class ResolvedLocation:
|
||||
display_name: Optional[str]
|
||||
address_info: Optional[dict]
|
||||
error: Optional[str] = None
|
||||
error_detail: Optional[str] = None
|
||||
region_note: Optional[str] = None
|
||||
|
||||
|
||||
@@ -336,6 +353,15 @@ def reverse_geocode_region(
|
||||
return city, suffix
|
||||
|
||||
|
||||
def cache_put(
|
||||
cache: dict, key: Any, value: Any, *, cap: int = GEOCODE_CACHE_CAP
|
||||
) -> None:
|
||||
"""Insert into a size-capped cache, evicting the oldest entry when full."""
|
||||
if key not in cache and len(cache) >= cap:
|
||||
cache.pop(next(iter(cache)))
|
||||
cache[key] = value
|
||||
|
||||
|
||||
def zip_to_city_string(
|
||||
zipcode: str, *, timeout: int = 10, cache: Optional[dict[str, str]] = None, logger: Any = None
|
||||
) -> Optional[str]:
|
||||
@@ -356,21 +382,32 @@ def zip_to_city_string(
|
||||
if logger:
|
||||
logger.debug(f"Zippopotam ZIP lookup failed for {z}: {e}")
|
||||
if name and cache is not None:
|
||||
cache[z] = name
|
||||
cache_put(cache, z, name)
|
||||
return name
|
||||
|
||||
|
||||
def parse_coordinates(raw: str) -> Optional[tuple[float, float]]:
|
||||
"""Return (lat, lon) when valid; None for bad format or out-of-range."""
|
||||
coords, _error, _detail = parse_coordinates_detailed(raw)
|
||||
return coords
|
||||
|
||||
|
||||
def parse_coordinates_detailed(
|
||||
raw: str,
|
||||
) -> tuple[Optional[tuple[float, float]], Optional[str], Optional[str]]:
|
||||
"""Return ((lat, lon)|None, error_code|None, error_detail|None)."""
|
||||
if not COORD_RE.match(raw or ""):
|
||||
return None
|
||||
return None, "invalid_coordinates", None
|
||||
try:
|
||||
a, b = raw.split(",", 1)
|
||||
lat, lon = float(a.strip()), float(b.strip())
|
||||
except ValueError:
|
||||
return None
|
||||
if not (-90 <= lat <= 90) or not (-180 <= lon <= 180):
|
||||
return None
|
||||
return lat, lon
|
||||
return None, "invalid_coordinates", None
|
||||
if not (-90 <= lat <= 90):
|
||||
return None, "invalid_latitude", str(lat)
|
||||
if not (-180 <= lon <= 180):
|
||||
return None, "invalid_longitude", str(lon)
|
||||
return (lat, lon), None, None
|
||||
|
||||
|
||||
def _rewrite_space_country(location: str) -> str:
|
||||
@@ -419,10 +456,7 @@ def get_neighborhood_queries(city: str) -> list[str]:
|
||||
|
||||
|
||||
def _is_country_token(text: str) -> bool:
|
||||
t = text.strip().lower()
|
||||
if t in _COUNTRY_NICKNAMES:
|
||||
return True
|
||||
return is_country_name(text)
|
||||
return text.strip().lower() in _COUNTRY_TOKENS
|
||||
|
||||
|
||||
def _address_from_result(bot: Any, lat: float, lon: float, timeout: int) -> dict:
|
||||
@@ -751,11 +785,12 @@ def resolve_location(
|
||||
query, _ = classify_location(query, use_international_cities=True)
|
||||
|
||||
if location_type == "coordinates":
|
||||
parsed = parse_coordinates(query)
|
||||
if not parsed:
|
||||
parsed, err, detail = parse_coordinates_detailed(query)
|
||||
if err or not parsed:
|
||||
return ResolvedLocation(
|
||||
lat=None, lon=None, location_type="coordinates", query=query,
|
||||
display_name=None, address_info=None, error="invalid_coordinates",
|
||||
display_name=None, address_info=None,
|
||||
error=err or "invalid_coordinates", error_detail=detail,
|
||||
)
|
||||
lat, lon = parsed
|
||||
display = _display_from_address(
|
||||
@@ -818,69 +853,9 @@ async def resolve_location_async(
|
||||
*,
|
||||
options: Optional[ResolveOptions] = None,
|
||||
) -> ResolvedLocation:
|
||||
"""Async twin for prefix/solarforecast; falls back to sync best-effort when needed."""
|
||||
"""Async twin of ``resolve_location`` (same behavior via thread offload)."""
|
||||
opts = options or OPTIONS_PREFIX
|
||||
if raw is None or not str(raw).strip():
|
||||
return resolve_location(bot, raw, options=opts)
|
||||
|
||||
text = str(raw).strip()
|
||||
if opts.allow_repeater_names:
|
||||
hit = lookup_repeater_lat_lon(bot, text)
|
||||
if hit:
|
||||
lat, lon, name = hit
|
||||
return ResolvedLocation(
|
||||
lat=lat, lon=lon, location_type="repeater", query=name,
|
||||
display_name=name, address_info=None,
|
||||
)
|
||||
|
||||
default_state = opts.default_state or bot.config.get("Weather", "default_state", fallback="")
|
||||
default_country = opts.default_country or bot.config.get("Weather", "default_country", fallback="US")
|
||||
query, location_type = classify_location(
|
||||
text, use_international_cities=opts.use_international_cities
|
||||
)
|
||||
region_note = None
|
||||
if location_type == "city" and opts.use_region_capitals:
|
||||
cap = region_capital_query(query)
|
||||
if cap:
|
||||
query, region_note = cap, REGION_DEFAULT_NOTE
|
||||
|
||||
if location_type == "coordinates":
|
||||
return resolve_location(bot, text, options=opts)
|
||||
|
||||
if location_type == "zipcode" and not opts.use_structured_zip:
|
||||
lat, lon = await geocode_zipcode(
|
||||
bot, query.strip(), default_country=default_country, timeout=opts.timeout
|
||||
)
|
||||
if lat is None or lon is None:
|
||||
return ResolvedLocation(
|
||||
lat=None, lon=None, location_type="zipcode", query=query,
|
||||
display_name=None, address_info=None, error="no_location_zipcode",
|
||||
)
|
||||
return ResolvedLocation(
|
||||
lat=lat, lon=lon, location_type="zipcode", query=query.strip(),
|
||||
display_name=query.strip(), address_info=None, region_note=region_note,
|
||||
)
|
||||
|
||||
if (
|
||||
location_type == "city"
|
||||
and not opts.use_neighborhoods
|
||||
and "," not in query
|
||||
):
|
||||
lat, lon, address_info = await geocode_city(
|
||||
bot, query, default_state=default_state, default_country=default_country,
|
||||
include_address_info=opts.include_address_info, timeout=opts.timeout,
|
||||
)
|
||||
if lat is None or lon is None:
|
||||
return ResolvedLocation(
|
||||
lat=None, lon=None, location_type="city", query=query,
|
||||
display_name=None, address_info=None, error="no_location_city",
|
||||
)
|
||||
return ResolvedLocation(
|
||||
lat=lat, lon=lon, location_type="city", query=query,
|
||||
display_name=query, address_info=address_info, region_note=region_note,
|
||||
)
|
||||
|
||||
return resolve_location(bot, text, options=opts)
|
||||
return await asyncio.to_thread(resolve_location, bot, raw, options=opts)
|
||||
|
||||
|
||||
def resolve_with_message_fallbacks(
|
||||
|
||||
+132
-2
@@ -2,23 +2,28 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import configparser
|
||||
from unittest.mock import MagicMock, Mock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from modules.location import (
|
||||
GEOCODE_CACHE_CAP,
|
||||
INTERNATIONAL_CITIES,
|
||||
OPTIONS_AQI,
|
||||
OPTIONS_PREFIX,
|
||||
OPTIONS_RAIN,
|
||||
ResolveOptions,
|
||||
cache_put,
|
||||
classify_location,
|
||||
geocode_city_best_effort,
|
||||
get_neighborhood_queries,
|
||||
parse_coordinates,
|
||||
parse_coordinates_detailed,
|
||||
resolve_location,
|
||||
titlecase_location,
|
||||
join_location,
|
||||
resolve_location_async,
|
||||
zip_to_city_string,
|
||||
)
|
||||
|
||||
|
||||
@@ -96,6 +101,24 @@ class TestParseCoordinates:
|
||||
def test_invalid_format(self):
|
||||
assert parse_coordinates("seattle") is None
|
||||
|
||||
def test_detailed_invalid_latitude(self):
|
||||
coords, err, detail = parse_coordinates_detailed("91,0")
|
||||
assert coords is None
|
||||
assert err == "invalid_latitude"
|
||||
assert detail == "91.0"
|
||||
|
||||
def test_detailed_invalid_longitude(self):
|
||||
coords, err, detail = parse_coordinates_detailed("0,200")
|
||||
assert coords is None
|
||||
assert err == "invalid_longitude"
|
||||
assert detail == "200.0"
|
||||
|
||||
def test_detailed_valid(self):
|
||||
coords, err, detail = parse_coordinates_detailed("47.6,-122.3")
|
||||
assert coords == pytest.approx((47.6, -122.3))
|
||||
assert err is None
|
||||
assert detail is None
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestNeighborhoods:
|
||||
@@ -113,6 +136,50 @@ class TestKazakhstanDedup:
|
||||
assert INTERNATIONAL_CITIES["kazakhstan"] == "nur-sultan, kazakhstan"
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestCountryTokenPath:
|
||||
def test_france_uses_direct_nominatim(self, bot):
|
||||
loc = _loc(48.85, 2.35, "Paris, France", {"city": "Paris", "country": "France"})
|
||||
with patch(
|
||||
"modules.location.rate_limited_nominatim_geocode_sync", return_value=loc
|
||||
) as geo, patch(
|
||||
"modules.location.geocode_city_sync",
|
||||
) as shared, patch(
|
||||
"modules.location.rate_limited_nominatim_reverse_sync", return_value=loc
|
||||
):
|
||||
lat, lon, _ = geocode_city_best_effort(bot, "paris, france")
|
||||
geo.assert_called()
|
||||
shared.assert_not_called()
|
||||
assert lat == pytest.approx(48.85)
|
||||
|
||||
def test_texas_uses_shared_geocode(self, bot):
|
||||
with patch(
|
||||
"modules.location.geocode_city_sync",
|
||||
return_value=(33.66, -95.55, {"city": "Paris", "state": "Texas", "country": "United States"}),
|
||||
) as shared, patch(
|
||||
"modules.location.rate_limited_nominatim_geocode_sync",
|
||||
) as geo:
|
||||
lat, lon, _ = geocode_city_best_effort(bot, "paris, texas")
|
||||
shared.assert_called_once()
|
||||
for call in geo.call_args_list:
|
||||
assert call[0][1] != "paris, texas"
|
||||
assert lat == pytest.approx(33.66)
|
||||
|
||||
def test_arbitrary_second_token_not_country(self, bot):
|
||||
with patch(
|
||||
"modules.location.geocode_city_sync",
|
||||
return_value=(1.0, 2.0, {"city": "Foo"}),
|
||||
) as shared, patch(
|
||||
"modules.location.rate_limited_nominatim_geocode_sync",
|
||||
) as geo:
|
||||
geocode_city_best_effort(bot, "foo, bar")
|
||||
geocode_city_best_effort(bot, "springfield, greene")
|
||||
assert shared.call_count == 2
|
||||
for call in geo.call_args_list:
|
||||
q = call[0][1]
|
||||
assert q not in ("foo, bar", "springfield, greene")
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestResolveLocation:
|
||||
def test_coords(self, bot):
|
||||
@@ -121,6 +188,16 @@ class TestResolveLocation:
|
||||
assert r.lat == pytest.approx(47.6)
|
||||
assert r.error is None
|
||||
|
||||
def test_invalid_latitude_error(self, bot):
|
||||
r = resolve_location(bot, "91,0", options=OPTIONS_AQI)
|
||||
assert r.error == "invalid_latitude"
|
||||
assert r.error_detail == "91.0"
|
||||
|
||||
def test_invalid_longitude_error(self, bot):
|
||||
r = resolve_location(bot, "0,200", options=OPTIONS_AQI)
|
||||
assert r.error == "invalid_longitude"
|
||||
assert r.error_detail == "200.0"
|
||||
|
||||
def test_empty_no_fallback(self, bot):
|
||||
r = resolve_location(bot, None, options=OPTIONS_AQI)
|
||||
assert r.error == "no_location"
|
||||
@@ -172,6 +249,59 @@ class TestResolveLocation:
|
||||
assert r.location_type == "zipcode"
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestZipCacheCap:
|
||||
def test_cache_put_evicts_oldest(self):
|
||||
cache: dict[str, str] = {}
|
||||
for i in range(GEOCODE_CACHE_CAP + 5):
|
||||
cache_put(cache, f"{i:05d}", f"City{i}")
|
||||
assert len(cache) == GEOCODE_CACHE_CAP
|
||||
assert "00000" not in cache
|
||||
assert f"{GEOCODE_CACHE_CAP + 4:05d}" in cache
|
||||
|
||||
def test_zip_to_city_string_uses_capped_cache(self):
|
||||
cache: dict[str, str] = {}
|
||||
for i in range(GEOCODE_CACHE_CAP):
|
||||
cache[f"{i:05d}"] = f"Old{i}"
|
||||
mock_resp = Mock()
|
||||
mock_resp.ok = True
|
||||
mock_resp.json.return_value = {
|
||||
"places": [{"place name": "Seattle", "state abbreviation": "WA"}]
|
||||
}
|
||||
with patch("modules.location.requests.get", return_value=mock_resp):
|
||||
name = zip_to_city_string("99999", cache=cache)
|
||||
assert name == "Seattle, WA"
|
||||
assert len(cache) == GEOCODE_CACHE_CAP
|
||||
assert "99999" in cache
|
||||
assert "00000" not in cache
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestAsyncParity:
|
||||
def test_coords_match_sync(self, bot):
|
||||
sync = resolve_location(bot, "47.6,-122.3", options=OPTIONS_AQI)
|
||||
async_r = asyncio.run(resolve_location_async(bot, "47.6,-122.3", options=OPTIONS_AQI))
|
||||
assert async_r.lat == sync.lat
|
||||
assert async_r.lon == sync.lon
|
||||
assert async_r.error == sync.error
|
||||
assert async_r.location_type == sync.location_type
|
||||
|
||||
def test_empty_match_sync(self, bot):
|
||||
sync = resolve_location(bot, None, options=OPTIONS_PREFIX)
|
||||
async_r = asyncio.run(resolve_location_async(bot, None, options=OPTIONS_PREFIX))
|
||||
assert async_r.error == sync.error
|
||||
|
||||
def test_city_match_sync(self, bot):
|
||||
with patch(
|
||||
"modules.location.geocode_city_sync",
|
||||
return_value=(47.6, -122.3, {"city": "Seattle", "state": "Washington", "country": "United States"}),
|
||||
):
|
||||
sync = resolve_location(bot, "seattle", options=OPTIONS_AQI)
|
||||
async_r = asyncio.run(resolve_location_async(bot, "seattle", options=OPTIONS_AQI))
|
||||
assert async_r.lat == sync.lat
|
||||
assert async_r.display_name == sync.display_name
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestRainReexports:
|
||||
def test_helpers_importable_from_rain(self):
|
||||
|
||||
@@ -104,14 +104,21 @@ def aqi_cmd():
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestAqiLocationClassification:
|
||||
"""Lock AQI's front-door location typing and rewrites."""
|
||||
"""Lock AQI's front-door location typing and rewrites.
|
||||
|
||||
execute() passes the raw user location into get_aqi_for_location; typing and
|
||||
intl rewrites happen inside resolve_location / classify_location.
|
||||
"""
|
||||
|
||||
def _capture(self, cmd, content: str) -> tuple[str, str]:
|
||||
from modules.location import classify_location
|
||||
|
||||
with patch.object(cmd, "get_aqi_for_location", new_callable=AsyncMock) as m:
|
||||
m.return_value = "ok"
|
||||
_run(cmd.execute(mock_message(content=content)))
|
||||
assert m.called, f"get_aqi_for_location not called for {content!r}"
|
||||
return m.call_args[0][0], m.call_args[0][1]
|
||||
raw = m.call_args[0][0]
|
||||
return classify_location(raw, use_international_cities=True)
|
||||
|
||||
def test_coordinates_basic(self, aqi_cmd):
|
||||
loc, typ = self._capture(aqi_cmd, "aqi 47.6,-122.3")
|
||||
@@ -168,6 +175,12 @@ class TestAqiLocationClassification:
|
||||
assert typ == "city"
|
||||
assert loc == "mexico city, mexico"
|
||||
|
||||
def test_execute_passes_raw_location(self, aqi_cmd):
|
||||
with patch.object(aqi_cmd, "get_aqi_for_location", new_callable=AsyncMock) as m:
|
||||
m.return_value = "ok"
|
||||
_run(aqi_cmd.execute(mock_message(content="aqi mexico city")))
|
||||
assert m.call_args[0][0] == "mexico city"
|
||||
|
||||
def test_astronomical_early_out_skips_geocode(self, aqi_cmd):
|
||||
with patch.object(aqi_cmd, "get_aqi_for_location", new_callable=AsyncMock) as m:
|
||||
_run(aqi_cmd.execute(mock_message(content="aqi mars")))
|
||||
@@ -271,6 +284,87 @@ class TestAqiZipcodePath:
|
||||
assert result.startswith("98101:") or "ok" in result
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestAqiCoordinateErrors:
|
||||
def test_invalid_latitude_message(self, aqi_cmd):
|
||||
result = _run(aqi_cmd.get_aqi_for_location("91,0"))
|
||||
assert result == "Invalid latitude: 91.0. Must be between -90 and 90."
|
||||
|
||||
def test_invalid_longitude_message(self, aqi_cmd):
|
||||
result = _run(aqi_cmd.get_aqi_for_location("0,200"))
|
||||
assert result == "Invalid longitude: 200.0. Must be between -180 and 180."
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestAqiPrefixBudget:
|
||||
def test_under_budget_includes_display_name(self, aqi_cmd):
|
||||
from modules.location import ResolvedLocation
|
||||
|
||||
resolved = ResolvedLocation(
|
||||
lat=47.6, lon=-122.3, location_type="city", query="seattle",
|
||||
display_name="Seattle, WA",
|
||||
address_info={"city": "Seattle", "state": "Washington", "country": "United States"},
|
||||
)
|
||||
with patch("modules.commands.aqi_command.resolve_location", return_value=resolved), \
|
||||
patch.object(aqi_cmd, "get_openmeteo_aqi", return_value="🟢 20"):
|
||||
result = _run(aqi_cmd.get_aqi_for_location("seattle"))
|
||||
assert result.startswith("Seattle, WA:")
|
||||
|
||||
def test_over_budget_same_state_omits_prefix(self, aqi_cmd):
|
||||
from modules.location import ResolvedLocation
|
||||
|
||||
aqi_cmd.default_state = "WA"
|
||||
long_aqi = "X" * 120
|
||||
resolved = ResolvedLocation(
|
||||
lat=47.6, lon=-122.3, location_type="city", query="seattle",
|
||||
display_name="Seattle, WA",
|
||||
address_info={"city": "Seattle", "state": "Washington", "country": "United States"},
|
||||
)
|
||||
with patch("modules.commands.aqi_command.resolve_location", return_value=resolved), \
|
||||
patch.object(aqi_cmd, "get_openmeteo_aqi", return_value=long_aqi):
|
||||
result = _run(aqi_cmd.get_aqi_for_location("seattle"))
|
||||
assert result == long_aqi
|
||||
assert not result.startswith("Seattle")
|
||||
|
||||
def test_over_budget_different_state_keeps_short_prefix(self, aqi_cmd):
|
||||
from modules.location import ResolvedLocation
|
||||
|
||||
aqi_cmd.default_state = "WA"
|
||||
long_aqi = "X" * 120
|
||||
resolved = ResolvedLocation(
|
||||
lat=30.27, lon=-97.74, location_type="city", query="austin",
|
||||
display_name="Austin, TX",
|
||||
address_info={"city": "Austin", "state": "Texas", "country": "United States"},
|
||||
)
|
||||
with patch("modules.commands.aqi_command.resolve_location", return_value=resolved), \
|
||||
patch.object(aqi_cmd, "get_openmeteo_aqi", return_value=long_aqi):
|
||||
result = _run(aqi_cmd.get_aqi_for_location("austin"))
|
||||
assert result.endswith(long_aqi)
|
||||
assert result.startswith("Austin")
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestAqiIntlResolveWiring:
|
||||
def test_london_rewrites_via_resolve(self, aqi_cmd):
|
||||
loc = _make_geopy_location(
|
||||
51.5, -0.12, "London, UK", {"city": "London", "country": "United Kingdom"}
|
||||
)
|
||||
with patch(
|
||||
"modules.location.rate_limited_nominatim_geocode_sync", return_value=loc
|
||||
) as geo, patch(
|
||||
"modules.location.geocode_city_sync", return_value=(None, None, None)
|
||||
), patch(
|
||||
"modules.location.rate_limited_nominatim_reverse_sync", return_value=loc
|
||||
), patch.object(aqi_cmd, "get_openmeteo_aqi", return_value="ok"):
|
||||
result = _run(aqi_cmd.get_aqi_for_location("london"))
|
||||
assert "ok" in result
|
||||
# Intl rewrite should query "london, uk" on the country path
|
||||
assert any(
|
||||
isinstance(c[0][1], str) and "london" in c[0][1].lower()
|
||||
for c in geo.call_args_list
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# WX — type detection + thin geocode wrappers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user