fix: Updated bus filters to support buses with with spaces

BottlecapDave · BottlecapDave · commit c8fe0a2a8252 · 2023-10-20T17:55:20.000+01:00
diff --git a/custom_components/first_bus/api_client.py b/custom_components/first_bus/api_client.py
@@ -5,8 +5,8 @@ class FirstBusApiClient:
   def __init__(self):
     self._base_url = 'https://www.firstbus.co.uk'
 
-  async def async_get_buses(self, stop):
-    """Get the user's account"""
+  async def async_get_bus_times(self, stop):
+    """Get the bus times for a given stop"""
     async with aiohttp.ClientSession() as client:
       url = f'{self._base_url}/getNextBus?stop={stop}'
       async with client.get(url) as response:
diff --git a/custom_components/first_bus/config/__init__.py b/custom_components/first_bus/config/__init__.py
@@ -0,0 +1,17 @@
+import re
+from ..const import CONFIG_BUSES, REGEX_BUSES
+
+def validate_config(config: dict):
+  new_config = dict(config)
+  errors = {}
+  if CONFIG_BUSES in new_config and new_config[CONFIG_BUSES] is not None and len(new_config[CONFIG_BUSES]) > 0:
+    matches = re.search(REGEX_BUSES, new_config[CONFIG_BUSES])
+    if (matches is None):
+      errors[CONFIG_BUSES] = "invalid_buses"
+    else:
+      new_config[CONFIG_BUSES] = new_config[CONFIG_BUSES].split(",")
+  else:
+    new_config[CONFIG_BUSES] = []
+
+  return (errors, new_config)
+    
diff --git a/custom_components/first_bus/config_flow.py b/custom_components/first_bus/config_flow.py
@@ -1,3 +1,4 @@
+from custom_components.first_bus.config import validate_config
 import voluptuous as vol
 import re
 import logging
@@ -25,22 +26,14 @@ class FirstBusConfigFlow(ConfigFlow, domain=DOMAIN):
   async def async_step_user(self, user_input):
     """Setup based on user config"""
 
-    errors = {}
     if user_input is not None:
-      if CONFIG_BUSES in user_input and user_input[CONFIG_BUSES] is not None:
-        matches = re.search(REGEX_BUSES, user_input[CONFIG_BUSES])
-        if (matches is None):
-          errors[CONFIG_BUSES] = "invalid_buses"
-        else:
-          user_input[CONFIG_BUSES] = user_input[CONFIG_BUSES].split(",")
-      else:
-        user_input[CONFIG_BUSES] = []
+      (errors, config) = validate_config(user_input)
 
       # Setup our basic sensors
       if len(errors) < 1:
         return self.async_create_entry(
-          title=f"Bus Stop {user_input[CONFIG_NAME]}", 
-          data=user_input
+          title=f"Bus Stop {config[CONFIG_NAME]}", 
+          data=config
         )
 
     return self.async_show_form(
@@ -85,14 +78,7 @@ async def async_step_user(self, user_input):
 
     _LOGGER.debug(f"Update config {config}")
 
-    if CONFIG_BUSES in config and config[CONFIG_BUSES] is not None and len(config[CONFIG_BUSES]) > 0:
-      matches = re.search(REGEX_BUSES, config[CONFIG_BUSES])
-      if (matches is None):
-        errors[CONFIG_BUSES] = "invalid_buses"
-      else:
-        config[CONFIG_BUSES] = config[CONFIG_BUSES].split(",")
-    else:
-      config[CONFIG_BUSES] = []
+    (errors, config) = validate_config(config)
 
     if len(errors) < 1:
       return self.async_create_entry(title="", data=config)
diff --git a/custom_components/first_bus/const.py b/custom_components/first_bus/const.py
@@ -6,7 +6,7 @@
 CONFIG_STOP = "Stop"
 CONFIG_BUSES = "Buses"
 
-REGEX_BUSES="^[a-zA-Z0-9]+(,[a-zA-Z0-9]+)*$"
+REGEX_BUSES="^[a-zA-Z0-9 ]+(,[a-zA-Z0-9 ]+)*$"
 REGEX_TIME="^[0-9]{2}:[0-9]{2}$"
 REGEX_TIME_MINS="([0-9]+) mins"
 
diff --git a/custom_components/first_bus/sensor.py b/custom_components/first_bus/sensor.py
@@ -14,7 +14,7 @@
 from .api_client import (FirstBusApiClient)
 from .utils import (
   get_next_bus,
-  async_get_buses,
+  get_buses,
   calculate_minutes_remaining
 )
 
@@ -77,7 +77,8 @@ async def async_update(self):
 
     # We only want to update every 5 minutes so we don't hammer the service
     if self._minsSinceLastUpdate <= 0:
-      buses = await async_get_buses(self._client, self._data[CONFIG_STOP], now())
+      bus_times = await self._client.async_get_bus_times(self._data[CONFIG_STOP])
+      buses = get_buses(bus_times, now())
       self._buses = buses
       self._minsSinceLastUpdate = 5
     
diff --git a/custom_components/first_bus/utils.py b/custom_components/first_bus/utils.py
@@ -1,19 +1,16 @@
 import logging
 import re
+from datetime import (datetime, timedelta)
+from homeassistant.util.dt import (parse_datetime)
 
 from .const import (
   REGEX_TIME,
   REGEX_TIME_MINS,
 )
 
-from datetime import (timedelta)
-from homeassistant.util.dt import (parse_datetime)
-from .api_client import FirstBusApiClient
-
 _LOGGER = logging.getLogger(__name__)
 
-async def async_get_buses(api_client: FirstBusApiClient, stop, current_timestamp):
-  bus_times = await api_client.async_get_buses(stop)
+def get_buses(bus_times: list, current_timestamp: datetime):
   _LOGGER.debug(f'buses: {bus_times}')
   
   for bus_time in bus_times:
diff --git a/run_tests.sh b/run_tests.sh
@@ -0,0 +1 @@
+python -m pytest tests
diff --git a/run_unit_tests.sh b/run_unit_tests.sh
@@ -0,0 +1 @@
+python -m pytest tests/unit
diff --git a/tests/integration/api_client/test_get_bus_times.py b/tests/integration/api_client/test_get_bus_times.py
@@ -14,7 +14,7 @@ async def test_when_get_buses_is_called_then_next_bus_is_returned():
     for stop in stops:
         try:
             # Act
-            buses = await client.async_get_buses(stop)
+            buses = await client.async_get_bus_times(stop)
             assert buses is not None
             assert len(buses) > 0
             
diff --git a/tests/integration/utils/test_get_next_bus.py b/tests/integration/utils/test_get_next_bus.py
@@ -5,7 +5,7 @@
 from homeassistant.util.dt import (now)
 
 from custom_components.first_bus.api_client import FirstBusApiClient
-from custom_components.first_bus.utils import (async_get_buses, get_next_bus)
+from custom_components.first_bus.utils import (get_buses, get_next_bus)
 
 stops = ["0170SGB20116", "3800C509801", "2200YEA00934"]
 
@@ -24,7 +24,8 @@ async def test_when_get_next_bus_is_called_then_next_bus_is_returned(target_buse
     for stop in stops:
         try:
             # Act
-            buses = await async_get_buses(client, stop, now())
+            bus_times = await client.async_get_bus_times(stop)
+            buses = get_buses(bus_times, now())
             assert buses is not None
             assert len(buses) > 0
             
diff --git a/tests/unit/config/__init__.py b/tests/unit/config/__init__.py
diff --git a/tests/unit/config/test_validate_config.py b/tests/unit/config/test_validate_config.py
@@ -0,0 +1,127 @@
+import pytest
+from custom_components.first_bus.config import (validate_config)
+from custom_components.first_bus.const import CONFIG_NAME, CONFIG_STOP, CONFIG_BUSES
+
+@pytest.mark.asyncio
+async def test_when_data_valid_then_no_errors_returned():
+  # Arrange
+  original_config = {
+    CONFIG_NAME: "test",
+    CONFIG_STOP: "123",
+    CONFIG_BUSES: "12 A,12B,12"
+  }
+
+  # Act
+  (errors, config) = validate_config(original_config)
+
+  # Assert
+  assert CONFIG_NAME not in errors
+  assert CONFIG_STOP not in errors
+  assert CONFIG_BUSES not in errors
+
+  assert CONFIG_NAME in config
+  assert config[CONFIG_NAME] == original_config[CONFIG_NAME]
+  assert CONFIG_STOP in config
+  assert config[CONFIG_STOP] == original_config[CONFIG_STOP]
+  assert CONFIG_BUSES in config
+  assert config[CONFIG_BUSES] == ["12 A", "12B", "12"]
+
+@pytest.mark.asyncio
+async def test_when_buses_not_present_then_buses_empty_array():
+  # Arrange
+  original_config = {
+    CONFIG_NAME: "test",
+    CONFIG_STOP: "123"
+  }
+
+  # Act
+  (errors, config) = validate_config(original_config)
+
+  # Assert
+  assert CONFIG_NAME not in errors
+  assert CONFIG_STOP not in errors
+  assert CONFIG_BUSES not in errors
+
+  assert CONFIG_NAME in config
+  assert config[CONFIG_NAME] == original_config[CONFIG_NAME]
+  assert CONFIG_STOP in config
+  assert config[CONFIG_STOP] == original_config[CONFIG_STOP]
+  assert CONFIG_BUSES in config
+  assert config[CONFIG_BUSES] == []
+
+@pytest.mark.asyncio
+async def test_when_buses_none_then_buses_empty_array():
+  # Arrange
+  original_config = {
+    CONFIG_NAME: "test",
+    CONFIG_STOP: "123",
+    CONFIG_BUSES: None
+  }
+
+  # Act
+  (errors, config) = validate_config(original_config)
+
+  # Assert
+  assert CONFIG_NAME not in errors
+  assert CONFIG_STOP not in errors
+  assert CONFIG_BUSES not in errors
+
+  assert CONFIG_NAME in config
+  assert config[CONFIG_NAME] == original_config[CONFIG_NAME]
+  assert CONFIG_STOP in config
+  assert config[CONFIG_STOP] == original_config[CONFIG_STOP]
+  assert CONFIG_BUSES in config
+  assert config[CONFIG_BUSES] == []
+
+@pytest.mark.asyncio
+async def test_when_buses_empty_then_buses_empty_array():
+  # Arrange
+  original_config = {
+    CONFIG_NAME: "test",
+    CONFIG_STOP: "123",
+    CONFIG_BUSES: ""
+  }
+
+  # Act
+  (errors, config) = validate_config(original_config)
+
+  # Assert
+  assert CONFIG_NAME not in errors
+  assert CONFIG_STOP not in errors
+  assert CONFIG_BUSES not in errors
+
+  assert CONFIG_NAME in config
+  assert config[CONFIG_NAME] == original_config[CONFIG_NAME]
+  assert CONFIG_STOP in config
+  assert config[CONFIG_STOP] == original_config[CONFIG_STOP]
+  assert CONFIG_BUSES in config
+  assert config[CONFIG_BUSES] == []
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("bus_value",[
+  ("A-B"),
+  ("12,12B,"),
+])
+async def test_when_buses_not_valid_then_buses_empty_array(bus_value: str):
+  # Arrange
+  original_config = {
+    CONFIG_NAME: "test",
+    CONFIG_STOP: "123",
+    CONFIG_BUSES: bus_value
+  }
+
+  # Act
+  (errors, config) = validate_config(original_config)
+
+  # Assert
+  assert CONFIG_NAME not in errors
+  assert CONFIG_STOP not in errors
+  assert CONFIG_BUSES in errors
+  assert errors[CONFIG_BUSES] == "invalid_buses"
+
+  assert CONFIG_NAME in config
+  assert config[CONFIG_NAME] == original_config[CONFIG_NAME]
+  assert CONFIG_STOP in config
+  assert config[CONFIG_STOP] == original_config[CONFIG_STOP]
+  assert CONFIG_BUSES in config
+  assert config[CONFIG_BUSES] == original_config[CONFIG_BUSES]
diff --git a/tests/unit/utils/test_get_buses.py b/tests/unit/utils/test_get_buses.py
@@ -1,13 +1,7 @@
 import pytest
-from custom_components.first_bus.utils import (async_get_buses)
 from homeassistant.util.dt import (parse_datetime)
 
-class MockedFirstBusApiClient:
-  def __init__(self, buses):
-    self._buses = buses
-
-  async def async_get_buses(self, stop):
-    return self._buses
+from custom_components.first_bus.utils import (get_buses)
 
 now = parse_datetime('2022-01-01T10:23:15+01:00')
 
@@ -21,24 +15,22 @@ async def async_get_buses(self, stop):
 ])
 async def test_when_get_buses_is_called_and_set_time_in_past_is_returned_then_due_timestamp_is_correct(raw_due, expected_due):
   # Arrange
-  raw_buses = [{
+  bus_times = [{
     'ServiceRef': '0', 
     'ServiceNumber': '43', 
     'Destination': 'Newton Road Shops', 
     'Due': raw_due, 
     'IsFG': 'N', 
     'IsLive': 'Y'
   }]
-  client = MockedFirstBusApiClient(raw_buses)
-  stop = "TEST_STOP"
 
   # Act
-  buses = await async_get_buses(client, stop, now)
+  buses = get_buses(bus_times, now)
   assert buses is not None
   assert len(buses) == 1
 
   bus = buses[0]
-  raw_bus = raw_buses[0]
+  raw_bus = bus_times[0]
 
   # Assert
   assert "Due" in bus
diff --git a/tests/unit/utils/test_get_next_bus.py b/tests/unit/utils/test_get_next_bus.py
