diff --git a/EXTERNAL_SESSION.md b/EXTERNAL_SESSION.md index 7acbab5..cac8d57 100644 --- a/EXTERNAL_SESSION.md +++ b/EXTERNAL_SESSION.md @@ -1,54 +1,31 @@ -# External Session Management +# HTTP Session Management ## Overview -The `python-openevse-http` library now supports passing an external `aiohttp.ClientSession` to the `OpenEVSE` class. This allows you to manage the session lifecycle yourself and share sessions across multiple API clients. +The `python-openevse-http` library requires you to pass an external `aiohttp.ClientSession` to `OpenEVSE`. Session ownership stays with the caller, so the library no longer constructs temporary HTTP clients internally. ## Benefits -- **Session Reuse**: Share a single session across multiple OpenEVSE instances or other aiohttp-based clients -- **Custom Configuration**: Configure session settings like timeouts, connectors, and SSL verification -- **Resource Management**: Better control over connection pooling and resource cleanup -- **Integration**: Easier integration with existing applications that already manage aiohttp sessions +- **Session Reuse**: Share a single session across multiple OpenEVSE instances or other `aiohttp` clients +- **Custom Configuration**: Configure timeouts, connectors, proxies, and SSL behavior yourself +- **Resource Management**: Keep connection pooling and cleanup in one place +- **Predictable Lifecycle**: Avoid hidden session creation inside request and websocket code paths ## Usage -### With External Session +### Basic Usage ```python import aiohttp from openevsehttp import OpenEVSE async def main(): - # Create your own session with custom settings timeout = aiohttp.ClientTimeout(total=30) async with aiohttp.ClientSession(timeout=timeout) as session: - # Pass the session to OpenEVSE charger = OpenEVSE("openevse.local", session=session) - - # Use the charger normally await charger.update() print(f"Status: {charger.status}") - - # Clean up await charger.ws_disconnect() - # Session will be closed by the context manager -``` - -### Without External Session (Backward Compatible) - -```python -from openevsehttp import OpenEVSE - -async def main(): - # The library creates and manages its own sessions - charger = OpenEVSE("openevse.local") - - # Use the charger normally - await charger.update() - print(f"Status: {charger.status}") - - await charger.ws_disconnect() ``` ### Sharing a Session @@ -59,83 +36,56 @@ from openevsehttp import OpenEVSE async def main(): async with aiohttp.ClientSession() as session: - # Use the same session for multiple chargers charger1 = OpenEVSE("charger1.local", session=session) charger2 = OpenEVSE("charger2.local", session=session) - # Both chargers use the same session await charger1.update() await charger2.update() - - await charger1.ws_disconnect() - await charger2.ws_disconnect() ``` -## API Changes +### Websocket Startup -### `OpenEVSE.__init__()` +Start websocket listening from the same event loop that owns the +`aiohttp.ClientSession`: ```python -def __init__( - self, - host: str, - user: str = "", - pwd: str = "", - session: aiohttp.ClientSession | None = None, -) -> None: -``` - -**Parameters:** -- `host` (str): The hostname or IP address of the OpenEVSE charger -- `user` (str, optional): Username for authentication -- `pwd` (str, optional): Password for authentication -- `session` (aiohttp.ClientSession | None, optional): External session to use for HTTP requests. If not provided, the library will create temporary sessions as needed. - -### `OpenEVSEWebsocket.__init__()` +import aiohttp +from openevsehttp import OpenEVSE -```python -def __init__( - self, - server, - callback, - user=None, - password=None, - session: aiohttp.ClientSession | None = None, -): +async def main(): + async with aiohttp.ClientSession() as session: + charger = OpenEVSE("openevse.local", session=session) + await charger.ws_start() + await charger.ws_disconnect() ``` -**Parameters:** -- `server`: The server URL -- `callback`: Callback function for websocket events -- `user` (optional): Username for authentication -- `password` (optional): Password for authentication -- `session` (aiohttp.ClientSession | None, optional): External session to use for websocket connections. If not provided, a new session will be created. +`ws_start()` is async so websocket tasks are created on the event loop that owns +the configured `aiohttp.ClientSession`. This prevents using a session from a +private background loop it was not created on. -## Important Notes +## API Notes -1. **Session Lifecycle**: When you provide an external session, you are responsible for closing it. The library will NOT close externally provided sessions. +- `OpenEVSE(..., session=session)` uses the provided session for HTTP requests. +- `OpenEVSEWebsocket(..., session=session)` uses the provided session for websocket connections. +- If no session is configured, HTTP requests and websocket startup raise `RuntimeError`. +- Call `await charger.ws_start()` from the event loop that owns the session. +- Externally provided sessions are never closed by the library. -2. **Backward Compatibility**: This change is fully backward compatible. Existing code that doesn't provide a session will continue to work exactly as before. +## Migration -3. **Websocket Sessions**: The websocket connection will also use the provided session, ensuring consistent session management across all HTTP and WebSocket operations. +Before: -4. **Thread Safety**: If you're using the same session across multiple OpenEVSE instances, ensure you're following aiohttp's thread safety guidelines. - -## Migration Guide - -If you want to migrate existing code to use external sessions: - -**Before:** ```python charger = OpenEVSE("openevse.local") await charger.update() ``` -**After:** +After: + ```python +import aiohttp + async with aiohttp.ClientSession() as session: charger = OpenEVSE("openevse.local", session=session) await charger.update() ``` - -No other changes are required! diff --git a/README.md b/README.md index 5e5aa83..75616d0 100644 --- a/README.md +++ b/README.md @@ -29,28 +29,24 @@ pip install python_openevse_http ```python import asyncio +import aiohttp from openevsehttp import OpenEVSE async def main(): - # Initialize the charger - charger = OpenEVSE("192.168.1.30") + async with aiohttp.ClientSession() as session: + charger = OpenEVSE("192.168.1.30", session=session) + await charger.update() - # Update state - await charger.update() + print(f"Charger State: {charger.status}") + print(f"Current Charge: {charger.charge_current}A") - print(f"Charger State: {charger.status}") - print(f"Current Charge: {charger.charge_current}A") + if charger.shaper_active: + print("Shaper is active, disabling...") + else: + print("Shaper is inactive, enabling...") - # Toggle the Shaper feature - if charger.shaper_active: - print("Shaper is active, disabling...") - else: - print("Shaper is inactive, enabling...") - - await charger.toggle_shaper() - - # Clean up - await charger.close() + await charger.toggle_shaper() + await charger.ws_disconnect() if __name__ == "__main__": asyncio.run(main()) diff --git a/example_external_session.py b/example_external_session.py index 8ff0c06..8ce852d 100644 --- a/example_external_session.py +++ b/example_external_session.py @@ -31,19 +31,6 @@ async def example_with_external_session(): await charger.ws_disconnect() -async def example_without_external_session(): - """Demonstrate without external session (backward compatible).""" - # The library will create and manage its own sessions - charger = OpenEVSE("openevse.local") - - # Use the charger normally - await charger.update() - print(f"Status: {charger.status}") - print(f"Current: {charger.charging_current}A") - - await charger.ws_disconnect() - - async def example_shared_session(): """Demonstrate sharing a session between multiple clients.""" async with aiohttp.ClientSession() as session: diff --git a/openevsehttp/client.py b/openevsehttp/client.py index c703a85..becd153 100644 --- a/openevsehttp/client.py +++ b/openevsehttp/client.py @@ -17,6 +17,8 @@ from .commands import CommandsMixin from .const import ( + ERROR_SESSION_LOOP_MISMATCH, + ERROR_SESSION_REQUIRED, ERROR_TIMEOUT, UPDATE_TRIGGERS, ) @@ -68,7 +70,17 @@ def __init__( self._owns_loop = False self._loop_thread: threading.Thread | None = None self._session = session - self._session_external = session is not None + + def _get_session(self) -> aiohttp.ClientSession: + """Return the configured HTTP session or fail fast.""" + if self._session is None: + raise RuntimeError(ERROR_SESSION_REQUIRED) + try: + loop = asyncio.get_running_loop() + except RuntimeError: + return self._session + self._validate_session_loop(loop) + return self._session async def process_request( self, @@ -86,16 +98,10 @@ async def process_request( if self._user and self._pwd: auth = aiohttp.BasicAuth(self._user, self._pwd) - # Use provided session or create a temporary one - if (session := self._session) is None: - async with aiohttp.ClientSession() as session: - return await self._process_request_with_session( - session, url, method, data, rapi, auth - ) - else: - return await self._process_request_with_session( - session, url, method, data, rapi, auth - ) + session = self._get_session() + return await self._process_request_with_session( + session, url, method, data, rapi, auth + ) def _normalize_response(self, response: Any) -> dict[str, Any] | list[Any]: """Normalize response to a dict or list.""" @@ -259,36 +265,37 @@ async def test_and_get(self) -> dict[str, Any]: data = {"serial": serial, "model": model} return data - def ws_start(self) -> None: + async def ws_start(self) -> None: """Start the websocket listener.""" if self.websocket and self.websocket.state != STATE_STOPPED: raise AlreadyListening - # Detect loop mismatch - use_session = self._session - try: - asyncio.get_running_loop() - except RuntimeError: - # We are about to create a private loop in _start_listening - # If we have a session, it's likely bound to another loop - if self._session: - _LOGGER.warning( - "Caller-provided session may not work on private event loop. " - "Creating a loop-local session." - ) - use_session = None - # Clear self._session so subsequent await self.update() uses - # a loop-local session as well. - self._session = None - self._session_external = False + self._get_session() if not self.websocket or self.websocket.state == STATE_STOPPED: - self.websocket = OpenEVSEWebsocket( - self.url, self._update_status, self._user, self._pwd, use_session - ) + self._create_websocket() self._start_listening() + def _create_websocket(self) -> None: + """Create a websocket using the configured session.""" + self.websocket = OpenEVSEWebsocket( + self.url, + self._update_status, + self._user, + self._pwd, + self._session, + ) + + def _validate_session_loop(self, loop: asyncio.AbstractEventLoop) -> None: + """Ensure the configured session belongs to the active event loop.""" + session_loop = getattr(self._session, "_loop", None) + if ( + isinstance(session_loop, asyncio.AbstractEventLoop) + and session_loop is not loop + ): + raise RuntimeError(ERROR_SESSION_LOOP_MISMATCH) + def _start_listening(self) -> None: """Start the websocket listener.""" if not self._loop: diff --git a/openevsehttp/commands.py b/openevsehttp/commands.py index 6748d63..020b998 100644 --- a/openevsehttp/commands.py +++ b/openevsehttp/commands.py @@ -25,7 +25,7 @@ class CommandsMixin: url: str _status: dict[str, Any] _config: dict[str, Any] - _session: Any + _session: aiohttp.ClientSession | None # These are defined in client.py def _version_check(self, min_version: str, max_version: str = "") -> bool: @@ -46,6 +46,10 @@ def _normalize_response(self, response: Any) -> dict[str, Any] | list[Any]: """Normalize response to a dict or list.""" raise NotImplementedError + def _get_session(self) -> aiohttp.ClientSession: + """Return the configured HTTP session.""" + raise NotImplementedError + def _flag_ota_if_started(self, response: Any) -> None: """Flag OTA as active if response indicates firmware update has started.""" normalized = self._normalize_response(response) @@ -409,12 +413,8 @@ async def firmware_check(self) -> dict[str, Any] | None: return None try: - if (session := self._session) is None: - async with aiohttp.ClientSession() as session: - return await self._firmware_check_with_session(session, url, method) - else: - return await self._firmware_check_with_session(session, url, method) - + session = self._get_session() + return await self._firmware_check_with_session(session, url, method) except (TimeoutError, ServerTimeoutError): _LOGGER.error("%s: %s", "Timeout while updating", url) except ContentTypeError as err: diff --git a/openevsehttp/const.py b/openevsehttp/const.py index 4a21e70..d381640 100644 --- a/openevsehttp/const.py +++ b/openevsehttp/const.py @@ -62,3 +62,10 @@ ] SUCCESS_ANSWERS = ["OK", "done", "no change", "Created", "Updated", "Deleted"] + +ERROR_SESSION_REQUIRED = ( + "An aiohttp.ClientSession must be provided via the session argument." +) +ERROR_SESSION_LOOP_MISMATCH = ( + "The aiohttp.ClientSession is bound to a different event loop." +) diff --git a/openevsehttp/websocket.py b/openevsehttp/websocket.py index ac37843..3c14cd6 100644 --- a/openevsehttp/websocket.py +++ b/openevsehttp/websocket.py @@ -11,6 +11,11 @@ import aiohttp +from .const import ( + ERROR_SESSION_LOOP_MISMATCH, + ERROR_SESSION_REQUIRED, +) + _LOGGER = logging.getLogger(__name__) MAX_FAILED_ATTEMPTS = 5 @@ -40,7 +45,6 @@ def __init__( ) -> None: """Initialize a OpenEVSEWebsocket instance.""" self.session = session - self._session_external = session is not None self.uri = self._get_uri(server) self._user = user self._password = password @@ -224,10 +228,17 @@ async def listen(self) -> None: self._listener_loop = None async def _ensure_session(self) -> None: - """Ensure aiohttp.ClientSession exists.""" + """Ensure an external aiohttp.ClientSession exists.""" if self.session is None: - self.session = aiohttp.ClientSession() - self._session_external = False + raise RuntimeError(ERROR_SESSION_REQUIRED) + + loop = asyncio.get_running_loop() + session_loop = getattr(self.session, "_loop", None) + if ( + isinstance(session_loop, asyncio.AbstractEventLoop) + and session_loop is not loop + ): + raise RuntimeError(ERROR_SESSION_LOOP_MISMATCH) async def close(self) -> None: """Close the listening websocket.""" @@ -242,10 +253,6 @@ async def close(self) -> None: if self._client is not None: await self._client.close() self._client = None - # Only close the session if we created it - if not self._session_external and self.session is not None: - await self.session.close() - self.session = None async def keepalive(self) -> None: """Send ping requests to websocket.""" diff --git a/tests/conftest.py b/tests/conftest.py index 5623c0c..c898330 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,57 @@ TEST_TLD = "openevse.test.tld" +class MockClientSession: + """Minimal aiohttp.ClientSession-compatible adapter for tests.""" + + def __init__(self, mocker: "AiohttpClientMocker") -> None: + self._mocker = mocker + self.closed = False + + def request(self, method: str, url: Any, **kwargs: Any): + return self._request(method, url, **kwargs) + + def get(self, url: Any, **kwargs: Any): + return self._request("GET", url, **kwargs) + + def post(self, url: Any, **kwargs: Any): + return self._request("POST", url, **kwargs) + + def put(self, url: Any, **kwargs: Any): + return self._request("PUT", url, **kwargs) + + def delete(self, url: Any, **kwargs: Any): + return self._request("DELETE", url, **kwargs) + + def patch(self, url: Any, **kwargs: Any): + return self._request("PATCH", url, **kwargs) + + def head(self, url: Any, **kwargs: Any): + return self._request("HEAD", url, **kwargs) + + def options(self, url: Any, **kwargs: Any): + return self._request("OPTIONS", url, **kwargs) + + def _request(self, method: str, url: Any, **kwargs: Any): + return MockRequestContext(self._mocker._request_mock(method, url, **kwargs)) + + async def close(self) -> None: + self.closed = True + + +class MockRequestContext: + def __init__(self, request): + self._request = request + self._response = None + + async def __aenter__(self): + self._response = await self._request + return await self._response.__aenter__() + + async def __aexit__(self, exc_type, exc_val, exc_tb): + return await self._response.__aexit__(exc_type, exc_val, exc_tb) + + def _setup_charger( mock_aioclient, status_fixture="v4_json/status.json", @@ -65,7 +116,12 @@ def _setup_charger( repeat=True, ) - return main.OpenEVSE(TEST_TLD, user=user, pwd=pwd) + return main.OpenEVSE( + TEST_TLD, + user=user, + pwd=pwd, + session=MockClientSession(mock_aioclient), + ) @pytest.fixture(name="test_charger_auth") diff --git a/tests/test_client.py b/tests/test_client.py index bff5a34..28c6394 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -7,6 +7,7 @@ import aiohttp import pytest +import pytest_asyncio from aiohttp.client_exceptions import ContentTypeError, ServerTimeoutError from aiohttp.client_reqrep import ConnectionKey from awesomeversion.exceptions import AwesomeVersionCompareException @@ -32,6 +33,7 @@ OpenEVSEWebsocket, ) from tests.common import load_fixture +from tests.conftest import MockClientSession pytestmark = pytest.mark.asyncio @@ -49,6 +51,23 @@ DUMMY_PWD = "fakepassword" +@pytest_asyncio.fixture +async def charger_factory(): + """Create chargers with caller-managed sessions for tests.""" + sessions: list[aiohttp.ClientSession] = [] + + def factory(host: str = SERVER_URL, **kwargs) -> OpenEVSE: + session = aiohttp.ClientSession() + sessions.append(session) + return OpenEVSE(host, session=session, **kwargs) + + yield factory + + for session in sessions: + if not session.closed: + await session.close() + + # ── Auth / update / status ──────────────────────────────────────────── @@ -71,7 +90,7 @@ async def test_ws_state(test_charger): await test_charger.update() assert test_charger.ws_state == STATE_STOPPED with patch("openevsehttp.client.OpenEVSEWebsocket.listen", AsyncMock()): - test_charger.ws_start() + await test_charger.ws_start() value = test_charger.ws_state assert value == STATE_DISCONNECTED await test_charger.ws_disconnect() @@ -85,11 +104,11 @@ async def test_update_status(test_charger): assert test_charger._status == data -async def test_update_non_dict(mock_aioclient, caplog): +async def test_update_non_dict(mock_aioclient, caplog, charger_factory): """Test update() handles non-dict responses for status and config.""" # Use a unique host to avoid hitting mocks from conftest.py unique_host = "non-dict.test.tld" - test_charger = OpenEVSE(unique_host) + test_charger = charger_factory(unique_host) url_status = f"http://{unique_host}/status" url_config = f"http://{unique_host}/config" @@ -117,10 +136,10 @@ async def test_get_status_auth_err(test_charger_auth_err): await test_charger_auth_err.update() -async def test_update_force_status(mock_aioclient): +async def test_update_force_status(mock_aioclient, charger_factory): """Test force_status parameter when ws is listening.""" unique_host = "force-status.test.tld" - charger = OpenEVSE(unique_host) + charger = charger_factory(unique_host) url_status = f"http://{unique_host}/status" url_config = f"http://{unique_host}/config" @@ -141,10 +160,10 @@ async def test_update_force_status(mock_aioclient): assert charger._status["transient_key"] == "preserved" -async def test_update_ota_active(mock_aioclient): +async def test_update_ota_active(mock_aioclient, charger_factory): """Test automatic status polling when ota_update is active.""" unique_host = "ota-active.test.tld" - charger = OpenEVSE(unique_host) + charger = charger_factory(unique_host) url_status = f"http://{unique_host}/status" url_config = f"http://{unique_host}/config" @@ -306,14 +325,14 @@ async def test_test_and_get(test_charger, test_charger_v2, mock_aioclient, caplo assert "Older firmware detected, missing serial." in caplog.text -async def test_identify_with_buildenv(mock_aioclient): +async def test_identify_with_buildenv(mock_aioclient, charger_factory): """Test test_and_get method (identify) with buildenv in response.""" mock_aioclient.get( "http://openevse.test.tld/config", status=200, body='{"wifi_serial": "123", "buildenv": "esp32"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) data = await charger.test_and_get() assert data["model"] == "esp32" @@ -420,14 +439,14 @@ async def test_firmware_check( async def test_firmware_check_no_config(): """Test firmware_check when config is not loaded.""" - charger = OpenEVSE(SERVER_URL) + charger = OpenEVSE(SERVER_URL, session=MagicMock()) result = await charger.firmware_check() assert result is None -async def test_firmware_check_no_firmware_version(mock_aioclient): +async def test_firmware_check_no_firmware_version(mock_aioclient, charger_factory): """Test firmware_check when firmware_version is missing.""" mock_aioclient.get( TEST_URL_STATUS, @@ -440,7 +459,7 @@ async def test_firmware_check_no_firmware_version(mock_aioclient): body='{"hostname": "openevse"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) await charger.update() result = await charger.firmware_check() @@ -448,7 +467,7 @@ async def test_firmware_check_no_firmware_version(mock_aioclient): assert result is None -async def test_firmware_check_github_api_error(mock_aioclient): +async def test_firmware_check_github_api_error(mock_aioclient, charger_factory): """Test firmware_check when GitHub API fails.""" mock_aioclient.get( TEST_URL_STATUS, @@ -466,7 +485,7 @@ async def test_firmware_check_github_api_error(mock_aioclient): body='{"error": "Not found"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) await charger.update() result = await charger.firmware_check() @@ -502,7 +521,7 @@ async def test_firmware_check_external_session(mock_aioclient): assert result["latest_version"] == "v4.1.0" -async def test_firmware_check_errors(mock_aioclient): +async def test_firmware_check_errors(mock_aioclient, charger_factory): """Test firmware_check error paths.""" mock_aioclient.get( "http://openevse.test.tld/status", @@ -528,7 +547,7 @@ async def test_firmware_check_errors(mock_aioclient): # Timeout from github mock_aioclient.get(url, exception=asyncio.TimeoutError()) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger._config["version"] = "4.0.1" assert await charger.firmware_check() is None @@ -571,7 +590,7 @@ async def test_version_check(test_charger_new, mock_aioclient, caplog): async def test_version_check_exceptions(): """Test _version_check exception paths.""" - charger = OpenEVSE(SERVER_URL) + charger = OpenEVSE(SERVER_URL, session=MagicMock()) # Invalid version string should log warning and return False charger._config = {"version": "invalid"} @@ -598,7 +617,7 @@ async def test_version_check_exceptions(): async def test_version_check_master(): """Test _version_check with 'master' in version.""" - charger = OpenEVSE(SERVER_URL) + charger = OpenEVSE(SERVER_URL, session=MagicMock()) charger._config = {"version": "v4.0.1.master"} # This should set value to "dev" assert charger._version_check("2.0.0") is True @@ -606,7 +625,7 @@ async def test_version_check_master(): async def test_version_check_limit(): """Test _version_check with max_version.""" - charger = OpenEVSE(SERVER_URL) + charger = OpenEVSE(SERVER_URL, session=MagicMock()) charger._config = {"version": "2.9.1"} assert charger._version_check("2.0.0", "3.0.0") is True assert charger._version_check("3.0.0", "4.0.0") is False @@ -627,36 +646,36 @@ async def test_websocket_functions(test_charger, mock_aioclient, caplog): ) await test_charger.update() with patch("openevsehttp.client.OpenEVSEWebsocket.listen", AsyncMock()): - test_charger.ws_start() + await test_charger.ws_start() await test_charger.ws_disconnect() -async def test_ws_start_already_listening(): +async def test_ws_start_already_listening(charger_factory): """Test ws_start raises AlreadyListening if already listening.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger.websocket = MagicMock() charger.websocket.state = "connected" charger._ws_listening = True with pytest.raises(AlreadyListening): - charger.ws_start() + await charger.ws_start() -async def test_ws_start_reset_listening(): +async def test_ws_start_reset_listening(charger_factory): """Test ws_start resets _ws_listening if websocket is not connected.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger.websocket = MagicMock() charger.websocket.state = "disconnected" charger._ws_listening = True with patch.object(charger, "_start_listening"): with pytest.raises(AlreadyListening): - charger.ws_start() + await charger.ws_start() -async def test_start_listening_no_loop(): +async def test_start_listening_no_loop(charger_factory): """Test _start_listening when no running loop is found.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger.websocket = MagicMock() # Mock calls to return coroutines for create_task charger.websocket.listen.return_value = asyncio.Future() @@ -679,9 +698,9 @@ async def test_start_listening_no_loop(): mock_thread.assert_called_once() -async def test_update_status_states(): +async def test_update_status_states(charger_factory): """Test _update_status with different websocket states.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger.websocket = MagicMock() charger.websocket.uri = "ws://test" @@ -700,7 +719,7 @@ async def test_update_status_states(): assert charger._ws_listening is False -async def test_update_status_data_triggers(mock_aioclient): +async def test_update_status_data_triggers(mock_aioclient, charger_factory): """Test _update_status with data that triggers update and callback.""" mock_aioclient.get( "http://openevse.test.tld/status", @@ -713,7 +732,7 @@ async def test_update_status_data_triggers(mock_aioclient): body='{"hostname": "test"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) # Set a coroutine callback async def mock_callback_coro(): @@ -738,9 +757,9 @@ async def mock_callback_coro(): charger.callback.assert_called_once() -async def test_repeat(): +async def test_repeat(charger_factory): """Test repeat helper.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger.websocket = MagicMock() charger._ws_listening = False # Mock ws_state to stop after second iteration (one continue, one run) @@ -805,14 +824,14 @@ def sync_func(): # ── get_schedule ───────────────────────────────────────────────────── -async def test_get_schedule(mock_aioclient): +async def test_get_schedule(mock_aioclient, charger_factory): """Test get_schedule method.""" mock_aioclient.post( "http://openevse.test.tld/schedule", status=200, body='{"sc": 1}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) result = await charger.get_schedule() assert result == {"sc": 1} @@ -822,14 +841,7 @@ async def test_get_schedule(mock_aioclient): async def test_main_auth_instantiation(): """Test OpenEVSE auth instantiation.""" - charger = OpenEVSE(SERVER_URL, user="user", pwd=DUMMY_PWD) - - # Setup mock session to be an async context manager mock_session = MagicMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=None) - - # Setup mock response context to be an async context manager mock_resp = AsyncMock() mock_resp.status = 200 mock_resp.text.return_value = "{}" @@ -841,20 +853,17 @@ async def test_main_auth_instantiation(): # Ensure session.get() returns the request context mock_session.get.return_value = mock_request_ctx - with ( - patch("aiohttp.ClientSession", return_value=mock_session), - patch("aiohttp.BasicAuth") as mock_basic_auth, - ): + charger = OpenEVSE(SERVER_URL, user="user", pwd=DUMMY_PWD, session=mock_session) + + with patch("aiohttp.BasicAuth") as mock_basic_auth: await charger.update() - # Verify BasicAuth was instantiated - # Note: process_request is called multiple times in update(), so we check if called at least once mock_basic_auth.assert_called_with("user", DUMMY_PWD) -async def test_main_sync_callback(): +async def test_main_sync_callback(charger_factory): """Test synchronous callback in _update_status.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) sync_callback = MagicMock() charger.callback = sync_callback @@ -867,9 +876,9 @@ async def test_main_sync_callback(): # ── send_command fallback tests ────────────────────────────────────── -async def test_send_command_msg_fallback(): +async def test_send_command_msg_fallback(charger_factory): """Test send_command return logic fallback.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) # Mock response with 'msg' but no 'ret' with patch.object(charger, "process_request", return_value={"msg": "ErrorMsg"}): @@ -903,9 +912,9 @@ async def test_send_command_rapi_rejection(test_charger, mock_aioclient): await test_charger.toggle_override() -async def test_send_command_empty_fallback(): +async def test_send_command_empty_fallback(charger_factory): """Test send_command empty fallback.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) # Mock response with neither 'msg' nor 'ret' with patch.object(charger, "process_request", return_value={}): cmd, ret = await charger.send_command("$ST") @@ -927,27 +936,37 @@ async def test_restart_evse_rapi_failure(test_charger, mock_aioclient, caplog): assert "Problem restarting EVSE module via RAPI: $NK^21" in caplog.text -def test_ws_start_no_loop_with_session(caplog): - """Test ws_start when no loop is running but a session exists.""" +async def test_ws_start_rejects_session_from_different_loop(): + """Test ws_start rejects sessions bound to a different event loop.""" charger = OpenEVSE("test.host", session=MagicMock()) - with ( - patch("asyncio.get_running_loop", side_effect=RuntimeError), - patch.object(charger, "_start_listening"), - caplog.at_level(logging.WARNING), - ): - charger.ws_start() - assert "Caller-provided session may not work on private event loop" in caplog.text - # charger.websocket.session should be None so it can be auto-created on the new loop - assert charger.websocket.session is None + charger._session._loop = asyncio.new_event_loop() + try: + with pytest.raises(RuntimeError, match="bound to a different event loop"): + await charger.ws_start() + finally: + charger._session._loop.close() + assert charger.websocket is None -def test_ws_start_no_loop_no_session(): - """Test ws_start when no loop is running and no session exists.""" +async def test_ws_start_no_loop_no_session(): + """Test ws_start requires an explicit session.""" charger = OpenEVSE("test.host") - with patch("asyncio.get_running_loop", side_effect=RuntimeError): - with patch.object(charger, "_start_listening"): - charger.ws_start() + with pytest.raises( + RuntimeError, + match="An aiohttp.ClientSession must be provided via the session argument.", + ): + await charger.ws_start() + assert charger.websocket is None + + +async def test_ws_start_uses_running_loop(charger_factory): + """Test ws_start creates websocket tasks on the active loop.""" + charger = charger_factory("test.host") + with patch.object(charger, "_start_listening") as mock_start: + await charger.ws_start() assert charger.websocket is not None + assert charger.websocket.session is charger._session + mock_start.assert_called_once() # ── process_request error handling ─────────────────────────────────── @@ -962,7 +981,7 @@ async def test_process_request_missing_method(): await charger.process_request(TEST_URL_STATUS, method=None) -async def test_process_request_unicode_decode_error(mock_aioclient): +async def test_process_request_unicode_decode_error(mock_aioclient, charger_factory): """Test process_request handles UnicodeDecodeError.""" # Create a mock response that raises UnicodeDecodeError on text() mock_response = AsyncMock() @@ -982,13 +1001,13 @@ async def test_process_request_unicode_decode_error(mock_aioclient): with patch("aiohttp.ClientSession.get") as mock_get: mock_get.return_value.__aenter__.return_value = mock_response - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) result = await charger.process_request(TEST_URL_STATUS, method="get") assert result == {"status": "ok"} -async def test_process_request_non_json_response(mock_aioclient): +async def test_process_request_non_json_response(mock_aioclient, charger_factory): """Test process_request handles non-JSON response.""" mock_aioclient.get( TEST_URL_STATUS, @@ -996,14 +1015,14 @@ async def test_process_request_non_json_response(mock_aioclient): body="Not JSON", ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) result = await charger.process_request(TEST_URL_STATUS, method="get") # Should return the string as-is assert result == "Not JSON" -async def test_process_request_400_error_with_msg(mock_aioclient): +async def test_process_request_400_error_with_msg(mock_aioclient, charger_factory): """Test process_request handles 400 error with msg field.""" mock_aioclient.get( @@ -1012,13 +1031,15 @@ async def test_process_request_400_error_with_msg(mock_aioclient): body='{"msg": "Bad request"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) with pytest.raises(ParseJSONError): await charger.process_request(TEST_URL_STATUS, method="get") -async def test_process_request_400_error_with_error_field(mock_aioclient): +async def test_process_request_400_error_with_error_field( + mock_aioclient, charger_factory +): """Test process_request handles 400 error with error field.""" mock_aioclient.get( @@ -1027,13 +1048,13 @@ async def test_process_request_400_error_with_error_field(mock_aioclient): body='{"error": "Invalid input"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) with pytest.raises(ParseJSONError): await charger.process_request(TEST_URL_STATUS, method="get") -async def test_process_request_401_error(mock_aioclient): +async def test_process_request_401_error(mock_aioclient, charger_factory): """Test process_request handles 401 authentication error.""" mock_aioclient.get( @@ -1042,13 +1063,13 @@ async def test_process_request_401_error(mock_aioclient): body='{"error": "Unauthorized"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) with pytest.raises(AuthenticationError): await charger.process_request(TEST_URL_STATUS, method="get") -async def test_process_request_404_error(mock_aioclient): +async def test_process_request_404_error(mock_aioclient, charger_factory): """Test process_request handles 404 error.""" mock_aioclient.get( TEST_URL_STATUS, @@ -1056,13 +1077,13 @@ async def test_process_request_404_error(mock_aioclient): body='{"error": "Not found"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) # Should not raise, just log warning result = await charger.process_request(TEST_URL_STATUS, method="get") assert result == {"error": "Not found"} -async def test_process_request_405_error(mock_aioclient): +async def test_process_request_405_error(mock_aioclient, charger_factory): """Test process_request handles 405 error.""" mock_aioclient.get( TEST_URL_STATUS, @@ -1070,13 +1091,13 @@ async def test_process_request_405_error(mock_aioclient): body='{"error": "Method not allowed"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) # Should not raise, just log warning result = await charger.process_request(TEST_URL_STATUS, method="get") assert result == {"error": "Method not allowed"} -async def test_process_request_500_error(mock_aioclient): +async def test_process_request_500_error(mock_aioclient, charger_factory): """Test process_request handles 500 error.""" mock_aioclient.get( TEST_URL_STATUS, @@ -1084,35 +1105,35 @@ async def test_process_request_500_error(mock_aioclient): body='{"error": "Internal server error"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) # Should not raise, just log warning result = await charger.process_request(TEST_URL_STATUS, method="get") assert result == {"error": "Internal server error"} -async def test_process_request_timeout_error(): +async def test_process_request_timeout_error(charger_factory): """Test process_request handles TimeoutError.""" with patch("aiohttp.ClientSession.get") as mock_get: mock_get.side_effect = TimeoutError("Connection timeout") - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) with pytest.raises(TimeoutError): await charger.process_request(TEST_URL_STATUS, method="get") -async def test_process_request_server_timeout_error(): +async def test_process_request_server_timeout_error(charger_factory): """Test process_request handles ServerTimeoutError.""" with patch("aiohttp.ClientSession.get") as mock_get: mock_get.side_effect = ServerTimeoutError("Server timeout") - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) with pytest.raises(ServerTimeoutError): await charger.process_request(TEST_URL_STATUS, method="get") -async def test_process_request_content_type_error(): +async def test_process_request_content_type_error(charger_factory): """Test process_request handles ContentTypeError.""" with patch("aiohttp.ClientSession.get") as mock_get: error = ContentTypeError( @@ -1122,7 +1143,7 @@ async def test_process_request_content_type_error(): ) mock_get.side_effect = error - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) with pytest.raises(ContentTypeError): await charger.process_request(TEST_URL_STATUS, method="get") @@ -1145,7 +1166,9 @@ async def test_process_request_with_session_invalid_method(test_charger): @pytest.mark.parametrize("method", ["post", "patch", "delete"]) @pytest.mark.parametrize("trigger_key", UPDATE_TRIGGERS) -async def test_process_request_triggers_update(method, trigger_key, mock_aioclient): +async def test_process_request_triggers_update( + method, trigger_key, mock_aioclient, charger_factory +): """Test process_request calls update when response contains a trigger key.""" url = f"{TEST_URL_CONFIG}/test" # Register the mock for the specific method @@ -1165,7 +1188,7 @@ async def test_process_request_triggers_update(method, trigger_key, mock_aioclie body=load_fixture("v4_json/config.json"), ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) # Ensure _status is fresh charger._status = {} @@ -1177,7 +1200,7 @@ async def test_process_request_triggers_update(method, trigger_key, mock_aioclie assert charger._status != {} -async def test_send_command_no_ret_with_msg(mock_aioclient): +async def test_send_command_no_ret_with_msg(mock_aioclient, charger_factory): """Test send_command when response has msg but no ret.""" mock_aioclient.post( "http://openevse.test.tld/r", @@ -1185,13 +1208,13 @@ async def test_send_command_no_ret_with_msg(mock_aioclient): body='{"msg": "Command failed"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) result = await charger.send_command("test_command") assert result == (False, "Command failed") -async def test_send_command_no_ret_no_msg(mock_aioclient): +async def test_send_command_no_ret_no_msg(mock_aioclient, charger_factory): """Test send_command when response has neither ret nor msg.""" mock_aioclient.post( "http://openevse.test.tld/r", @@ -1199,7 +1222,7 @@ async def test_send_command_no_ret_no_msg(mock_aioclient): body='{"error": "Unknown"}', ) - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) result = await charger.send_command("test_command") assert result == (False, "") @@ -1219,10 +1242,10 @@ async def test_get_status_error(test_charger_timeout, caplog): assert "Config update:" not in caplog.text -async def test_update_error_payload(mock_aioclient, caplog): +async def test_update_error_payload(mock_aioclient, caplog, charger_factory): """Test update handles responses with error keys properly.""" # Temporarily set ws_listening to False to hit both /status and /config - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger._ws_listening = False mock_aioclient.get( @@ -1516,22 +1539,22 @@ async def test_websocket_listen(): """Test websocket listen calls running.""" callback = AsyncMock() - ws = OpenEVSEWebsocket(f"http://{SERVER_URL}/", callback) + async with aiohttp.ClientSession() as session: + ws = OpenEVSEWebsocket(f"http://{SERVER_URL}/", callback, session=session) - with patch.object(ws, "running", AsyncMock()) as mock_running: - # Break loop after first call - ws.state = "starting" + with patch.object(ws, "running", AsyncMock()) as mock_running: + ws.state = "starting" - async def side_effect(): - ws.state = "stopped" + async def side_effect(): + ws.state = "stopped" - mock_running.side_effect = side_effect + mock_running.side_effect = side_effect - try: - await ws.listen() - finally: - await ws.close() - mock_running.assert_called_once() + try: + await ws.listen() + finally: + await ws.close() + mock_running.assert_called_once() async def test_websocket_stop_break(): @@ -1566,9 +1589,9 @@ async def side_effect(msgtype, _data, _error): assert len(calls) == 1 -async def test_repeat_task(): +async def test_repeat_task(charger_factory): """Test the repeat background task.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger.websocket = MagicMock() charger.websocket.state = "connected" charger._ws_listening = True @@ -1595,9 +1618,9 @@ async def test_repeat_task(): assert task.done() -async def test_ws_disconnect_owned_loop(): +async def test_ws_disconnect_owned_loop(charger_factory): """Test ws_disconnect when the client owns the event loop.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger.websocket = MagicMock() charger.websocket.state = STATE_STOPPED @@ -1625,29 +1648,39 @@ async def test_ws_disconnect_owned_loop(): mock_future = ConcurrentFuture() mock_future.set_result(True) + + def run_coroutine_threadsafe(coro, _loop): + coro.close() + return mock_future + + async def fake_shutdown(): + return None + + async def fake_wait_for(_future, timeout=None): + return True + + async def fake_to_thread(func, *args): + return func(*args) + with ( patch( - "asyncio.run_coroutine_threadsafe", return_value=mock_future + "asyncio.run_coroutine_threadsafe", + side_effect=run_coroutine_threadsafe, ) as mock_run, - patch("asyncio.wait_for", AsyncMock(return_value=True)), - patch( - "asyncio.to_thread", - AsyncMock(side_effect=lambda func, *args: func(*args)), - ), - patch.object(charger, "_shutdown", AsyncMock()) as mock_shutdown, + patch("asyncio.wait_for", side_effect=fake_wait_for), + patch("asyncio.to_thread", side_effect=fake_to_thread), + patch.object(charger, "_shutdown", fake_shutdown), ): await charger.ws_disconnect() - # Check for threadsafe call assert mock_run.called - assert mock_shutdown.called assert charger._loop is None assert mock_loop.close.called -async def test_ws_disconnect_exception(): +async def test_ws_disconnect_exception(charger_factory): """Test ws_disconnect handled exceptions during shutdown.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger.websocket = MagicMock() charger.websocket.state = STATE_STOPPED @@ -1665,18 +1698,32 @@ async def test_ws_disconnect_exception(): from concurrent.futures import Future as ConcurrentFuture mock_future = ConcurrentFuture() - mock_future.set_exception(asyncio.TimeoutError) + mock_future.set_result(True) + + def run_coroutine_threadsafe(coro, _loop): + coro.close() + return mock_future + + async def fake_shutdown(): + return None + + async def fake_wait_for(_future, timeout=None): + raise asyncio.TimeoutError + + async def fake_to_thread(func, *args): + return func(*args) + with ( - patch("asyncio.run_coroutine_threadsafe", return_value=mock_future) as mock_run, - patch("asyncio.wait_for", AsyncMock(side_effect=asyncio.TimeoutError)), patch( - "asyncio.to_thread", AsyncMock(side_effect=lambda func, *args: func(*args)) - ), - patch.object(charger, "_shutdown", AsyncMock()) as mock_shutdown, + "asyncio.run_coroutine_threadsafe", + side_effect=run_coroutine_threadsafe, + ) as mock_run, + patch("asyncio.wait_for", side_effect=fake_wait_for), + patch("asyncio.to_thread", side_effect=fake_to_thread), + patch.object(charger, "_shutdown", fake_shutdown), ): await charger.ws_disconnect() assert mock_run.called - assert mock_shutdown.called # Shutdown failed, so loop should NOT be closed/cleared assert charger._loop is mock_loop @@ -1699,27 +1746,27 @@ async def test_ws_shutdown_drains_tasks(test_charger): assert test_charger.websocket is None -async def test_test_and_get_invalid_response(mock_aioclient, caplog): +async def test_test_and_get_invalid_response(mock_aioclient, caplog, charger_factory): """Test test_and_get method with invalid response.""" mock_aioclient.get(TEST_URL_CONFIG, status=200, body="not a dict") - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) with caplog.at_level(logging.DEBUG): with pytest.raises(MissingSerial): await charger.test_and_get() assert "Invalid response from config: not a dict" in caplog.text -async def test_update_status_non_mapping_data(caplog): +async def test_update_status_non_mapping_data(caplog, charger_factory): """Test _update_status with non-mapping data.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) with caplog.at_level(logging.WARNING): await charger._update_status("data", "not a dict", None) assert "Received non-Mapping websocket data: not a dict" in caplog.text -async def test_update_status_ota(): +async def test_update_status_ota(charger_factory): """Test _update_status with ota websocket events.""" - charger = OpenEVSE(SERVER_URL) + charger = charger_factory(SERVER_URL) charger._status = {"ota_update": 0} # 1. Started event @@ -1743,7 +1790,7 @@ async def test_update_status_ota(): @pytest.mark.parametrize("body", ["123", "null"]) async def test_process_request_invalid_json_primitive(mock_aioclient, body): """Test process_request with an unexpected JSON primitive (e.g., int or null).""" - charger = OpenEVSE(SERVER_URL) + charger = OpenEVSE(SERVER_URL, session=MockClientSession(mock_aioclient)) mock_aioclient.get( TEST_URL_STATUS, status=200, @@ -1755,7 +1802,7 @@ async def test_process_request_invalid_json_primitive(mock_aioclient, body): async def test_process_request_boolean_primitive(mock_aioclient): """Test process_request allows boolean JSON primitives (e.g., false).""" - charger = OpenEVSE(SERVER_URL) + charger = OpenEVSE(SERVER_URL, session=MockClientSession(mock_aioclient)) mock_aioclient.get( TEST_URL_STATUS, status=200, @@ -1763,3 +1810,10 @@ async def test_process_request_boolean_primitive(mock_aioclient): ) result = await charger.process_request(TEST_URL_STATUS, method="get") assert result is False + + +async def test_get_session_no_running_loop_mocked(charger_factory): + """Test _get_session when no event loop is running (mocked).""" + charger = charger_factory() + with patch("asyncio.get_running_loop", side_effect=RuntimeError): + assert charger._get_session() is charger._session diff --git a/tests/test_commands.py b/tests/test_commands.py index d6b8e2c..3f776eb 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -13,6 +13,7 @@ UnsupportedFeature, ) from tests.common import load_fixture +from tests.conftest import MockClientSession pytestmark = pytest.mark.asyncio @@ -228,7 +229,9 @@ async def test_custom_override_success_responses(test_charger, mock_aioclient, c async def test_toggle_override_refresh_fail(mock_aioclient, caplog): """Test toggle_override when state is missing and refresh fails.""" # Use a fresh charger to avoid fixture mock interference - charger = main.OpenEVSE("openevse.test.tld") + charger = main.OpenEVSE( + "openevse.test.tld", session=MockClientSession(mock_aioclient) + ) # Mock status for a v3 firmware (older than 4.0.1) mock_aioclient.get( "http://openevse.test.tld/status", diff --git a/tests/test_external_session.py b/tests/test_external_session.py index b28e963..b35b8c2 100644 --- a/tests/test_external_session.py +++ b/tests/test_external_session.py @@ -37,9 +37,7 @@ async def test_external_session_provided(): # Create OpenEVSE instance with external session charger = OpenEVSE(TEST_TLD, session=mock_session) - # Verify the session is stored assert charger._session is mock_session - assert charger._session_external is True # Make a request await charger.process_request(TEST_URL_STATUS, method="get") @@ -48,23 +46,17 @@ async def test_external_session_provided(): mock_session.get.assert_called_once() -async def test_no_external_session(mock_aioclient): - """Test that a temporary session is created when none is provided.""" - mock_aioclient.get( - TEST_URL_STATUS, - status=200, - body=load_fixture("v4_json/status.json"), - ) - - # Create OpenEVSE instance without external session +async def test_no_external_session(): + """Test that requests fail when no external session is provided.""" charger = OpenEVSE(TEST_TLD) - # Verify no session is stored assert charger._session is None - assert charger._session_external is False - # Make a request - should create a temporary session - await charger.process_request(TEST_URL_STATUS, method="get") + with pytest.raises( + RuntimeError, + match="An aiohttp.ClientSession must be provided via the session argument.", + ): + await charger.process_request(TEST_URL_STATUS, method="get") async def test_external_session_with_update(mock_aioclient): @@ -87,7 +79,6 @@ async def test_external_session_with_update(mock_aioclient): # Verify the session is stored assert charger._session is session - assert charger._session_external is True # Update should use the external session await charger.update() @@ -119,7 +110,7 @@ async def test_websocket_uses_external_session(mock_aioclient): await charger.update() with patch("openevsehttp.websocket.OpenEVSEWebsocket.listen"): - charger.ws_start() + await charger.ws_start() # Verify websocket was created with the session assert charger.websocket is not None @@ -154,16 +145,11 @@ async def test_firmware_check_with_external_session(mock_aioclient): body=json.dumps(github_response), ) - # Create OpenEVSE instance without external session (use mocked responses) - charger = OpenEVSE(TEST_TLD) - - # Load config first - await charger.update() - - # Check firmware - should use mocked session - result = await charger.firmware_check() + async with aiohttp.ClientSession() as session: + charger = OpenEVSE(TEST_TLD, session=session) + await charger.update() + result = await charger.firmware_check() - # Verify result assert result is not None assert result["latest_version"] == "v4.2.0" diff --git a/tests/test_main_edge_cases.py b/tests/test_main_edge_cases.py index dfdb730..5d6b0a8 100644 --- a/tests/test_main_edge_cases.py +++ b/tests/test_main_edge_cases.py @@ -3,7 +3,9 @@ import logging from unittest.mock import AsyncMock, MagicMock, patch +import aiohttp import pytest +import pytest_asyncio from openevsehttp.__main__ import OpenEVSE @@ -12,9 +14,10 @@ SERVER_URL = "openevse.test.tld" -@pytest.fixture -def charger(): - return OpenEVSE(SERVER_URL) +@pytest_asyncio.fixture +async def charger(): + async with aiohttp.ClientSession() as session: + yield OpenEVSE(SERVER_URL, session=session) async def test_process_request_decode_error(charger, caplog): diff --git a/tests/test_mixins.py b/tests/test_mixins.py index 4ee978d..0e6c349 100644 --- a/tests/test_mixins.py +++ b/tests/test_mixins.py @@ -23,6 +23,8 @@ async def test_commands_mixin_not_implemented(): await mixin.update() with pytest.raises(NotImplementedError): mixin._normalize_response({}) + with pytest.raises(NotImplementedError): + mixin._get_session() async def test_managers_mixin_not_implemented(): diff --git a/tests/test_properties.py b/tests/test_properties.py index 886fa22..687253a 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -8,6 +8,7 @@ from openevsehttp import OpenEVSE from openevsehttp.exceptions import UnsupportedFeature +from tests.conftest import MockClientSession pytestmark = pytest.mark.asyncio @@ -212,7 +213,7 @@ async def test_simple_properties(fixture, prop, expected, request): async def test_get_status_unknown(): """Test status property with unknown/invalid codes.""" - charger = OpenEVSE(SERVER_URL) + charger = OpenEVSE(SERVER_URL, session=object()) # Unknown code charger._status = {"state": 99} assert charger.status == "unknown" @@ -224,7 +225,7 @@ async def test_get_status_unknown(): async def test_get_state_unknown(): """Test state property with unknown/invalid codes.""" - charger = OpenEVSE(SERVER_URL) + charger = OpenEVSE(SERVER_URL, session=object()) # Unknown code charger._status = {"state": 99} assert charger.state == "unknown" @@ -435,7 +436,7 @@ async def test_property_getters_with_missing_data(mock_aioclient): body="{}", # Empty config ) - charger = OpenEVSE(SERVER_URL) + charger = OpenEVSE(SERVER_URL, session=MockClientSession(mock_aioclient)) await charger.update() # Test various properties that should handle missing data diff --git a/tests/test_shaper.py b/tests/test_shaper.py index 9db469e..08ac14d 100644 --- a/tests/test_shaper.py +++ b/tests/test_shaper.py @@ -5,6 +5,7 @@ import pytest from openevsehttp.exceptions import UnknownError, UnsupportedFeature +from tests.conftest import MockClientSession pytestmark = pytest.mark.asyncio @@ -133,7 +134,7 @@ async def test_toggle_shaper_failed_update(mock_aioclient, caplog): """Test toggle_shaper when state is still missing after update().""" from openevsehttp import OpenEVSE - charger = OpenEVSE("openevse.test.tld") + charger = OpenEVSE("openevse.test.tld", session=MockClientSession(mock_aioclient)) # Mock the /status call but return status without shaper mock_aioclient.get( diff --git a/tests/test_websocket.py b/tests/test_websocket.py index f4bc7a3..8ef41ef 100644 --- a/tests/test_websocket.py +++ b/tests/test_websocket.py @@ -30,11 +30,10 @@ def mock_callback(): @pytest_asyncio.fixture async def ws_client(mock_callback): """Websocket client fixture.""" - # Making this fixture async ensures an event loop is running when - # OpenEVSEWebsocket initializes aiohttp.ClientSession - client = OpenEVSEWebsocket(SERVER_URL, mock_callback) - yield client - await client.close() + async with aiohttp.ClientSession() as session: + client = OpenEVSEWebsocket(SERVER_URL, mock_callback, session=session) + yield client + await client.close() def test_get_uri(): @@ -155,9 +154,16 @@ async def test_keepalive_timeout(ws_client, mock_callback): async def ws_client_auth(): """Fixture for authenticated websocket client.""" callback = AsyncMock() - client = OpenEVSEWebsocket(SERVER_URL, callback, user="test", password="pw") - yield client - await client.close() + async with aiohttp.ClientSession() as session: + client = OpenEVSEWebsocket( + SERVER_URL, + callback, + user="test", + password="pw", + session=session, + ) + yield client + await client.close() @pytest.mark.asyncio @@ -471,24 +477,31 @@ async def mock_run_impl(): @pytest.mark.asyncio -async def test_websocket_close_owned_session(mock_callback): - """Test close() when the session is owned by the client.""" +async def test_websocket_requires_external_session(mock_callback): + """Test websocket startup fails without an external session.""" client = OpenEVSEWebsocket(SERVER_URL, mock_callback) - # Ensure session is created - await client._ensure_session() - session = client.session - assert session is not None - assert not session.closed + with pytest.raises( + RuntimeError, + match="An aiohttp.ClientSession must be provided via the session argument.", + ): + await client._ensure_session() - mock_ws = AsyncMock() - client._client = mock_ws - await client.close() - assert client.state == STATE_STOPPED - assert client._client is None - mock_ws.close.assert_called_once() - assert session.closed - assert client.session is None +@pytest.mark.asyncio +async def test_websocket_rejects_session_from_different_loop(mock_callback): + """Test websocket startup fails if session is bound to a different event loop.""" + async with aiohttp.ClientSession() as session: + client = OpenEVSEWebsocket(SERVER_URL, mock_callback, session=session) + other_loop = asyncio.new_event_loop() + try: + with patch.object(session, "_loop", other_loop): + with pytest.raises( + RuntimeError, + match="The aiohttp.ClientSession is bound to a different event loop.", + ): + await client._ensure_session() + finally: + other_loop.close() @pytest.mark.asyncio @@ -496,7 +509,6 @@ async def test_websocket_close_external_session(mock_callback): """Test close() when an external session is provided.""" async with aiohttp.ClientSession() as session: client = OpenEVSEWebsocket(SERVER_URL, mock_callback, session=session) - assert client._session_external is True mock_ws = AsyncMock() client._client = mock_ws