More unit tests for Octopus and HA, New installer that doesn't depend on file list #3123
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for Octopus API and Home Assistant interface components, significantly expanding test coverage for external integrations and core infrastructure.
Key changes:
- Added 1934-line test suite for Octopus miscellaneous API methods (intelligent scheduling, saving sessions, tariffs, rates)
- Added 5 new HAInterface test modules covering state management, API calls, services, lifecycle, and websockets (total ~3000 lines)
- Added HAHistory component tests (795 lines) for history data fetching and caching
- Fixed indentation bug in ha.py where try block was incorrectly placed
- Added test helper infrastructure in test_hainterface_common.py
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/predbat/unit_test.py | Registered 13 new test functions in TEST_REGISTRY for Octopus misc and HAInterface/HAHistory components |
| apps/predbat/tests/test_octopus_misc.py | Comprehensive tests for Octopus intelligent schedule, saving sessions, tariffs, rates retrieval, and run loop |
| apps/predbat/tests/test_hainterface_websocket.py | Tests for HAInterface websocket connection, authentication, event handling, and error recovery |
| apps/predbat/tests/test_hainterface_state.py | Tests for state management (get/set/update), caching, and database mirroring |
| apps/predbat/tests/test_hainterface_service.py | Tests for service calls via websocket/loopback, set_state_external, and CONFIG_ITEMS handling |
| apps/predbat/tests/test_hainterface_lifecycle.py | Tests for initialize(), start(), is_alive(), wait_api_started(), and component lifecycle |
| apps/predbat/tests/test_hainterface_common.py | Shared mock infrastructure (MockBase, MockDatabaseManager, helper functions) for HAInterface tests |
| apps/predbat/tests/test_hainterface_api.py | Tests for API calls (GET/POST), error handling, supervisor endpoint, and get_history() |
| apps/predbat/tests/test_hahistory.py | Tests for HAHistory component (add_entity, get_history, caching, pruning, run loop) |
| apps/predbat/ha.py | Bug fix: moved HTTP request code inside try block for proper exception handling |
| .cspell/custom-dictionary-workspace.txt | Added "hahistory", "hainterface", "socketloop" to dictionary |
| # Test 2: Get from cache (should not call HAInterface again) | ||
| initial_call_count = len(mock_ha.get_history_calls) | ||
| result2 = ha_history.get_history(entity_id, days=30, tracked=True) | ||
|
|
There was a problem hiding this comment.
Variable result2 is not used.
Suggested change
| # Verify cached result matches initial result | |
| if result2 is None: | |
| print("ERROR: Cached history should not be None") | |
| failed += 1 | |
| elif len(result2) != len(result) or len(result2[0]) != len(result[0]): | |
| print("ERROR: Cached history should match initial result") | |
| failed += 1 | |
| else: | |
| print("✓ Cached history matches initial result") | |
| # Verify HAInterface was not called again (cache used) |
| print("✓ Used cache instead of fetching again") | ||
|
|
||
| # Test 3: Request more days (should fetch again) | ||
| result3 = ha_history.get_history(entity_id, days=60, tracked=True) |
There was a problem hiding this comment.
Variable result3 is not used.
Suggested change
| result3 = ha_history.get_history(entity_id, days=60, tracked=True) | |
| ha_history.get_history(entity_id, days=60, tracked=True) |
| mock_env.return_value = "supervisor_token" | ||
| mock_get.return_value = create_mock_requests_response(200, {"supervisor": "data"}) | ||
|
|
||
| result = ha_interface.api_call("/addons/self/info", core=False) |
There was a problem hiding this comment.
Variable result is not used.
| mock_get.return_value = mock_response | ||
|
|
||
| # Call with silent=True | ||
| result = ha_interface.api_call("/api/states", silent=True) |
There was a problem hiding this comment.
Variable result is not used.
| with patch("ha.requests.get") as mock_get: | ||
| mock_get.side_effect = requests.Timeout("Connection timeout") | ||
|
|
||
| result = ha_interface.api_call("/api/states") |
There was a problem hiding this comment.
Variable result is not used.
|
|
||
| api3.tariffs = {"import": {"data": tariff_data_before.copy()}} # Copy so we can check modification | ||
|
|
||
| result = api3.get_octopus_rates_direct("import", standingCharge=False) |
There was a problem hiding this comment.
This assignment to 'result' is unnecessary as it is redefined before this value is used.
Suggested change
| result = api3.get_octopus_rates_direct("import", standingCharge=False) | |
| api3.get_octopus_rates_direct("import", standingCharge=False) |
| # Mock datetime to be at 30-minute mark (e.g., 10:30) | ||
| with patch("octopus.datetime") as mock_datetime: | ||
| mock_datetime.now.return_value = datetime(2025, 1, 1, 10, 30, 0) | ||
| result = await api2.run(seconds=0, first=False) |
| # Mock datetime to be at 10-minute mark (e.g., 10:10) | ||
| with patch("octopus.datetime") as mock_datetime: | ||
| mock_datetime.now.return_value = datetime(2025, 1, 1, 10, 10, 0) | ||
| result = await api3.run(seconds=0, first=False) |
| # Mock datetime to be at 2-minute mark (e.g., 10:02) | ||
| with patch("octopus.datetime") as mock_datetime: | ||
| mock_datetime.now.return_value = datetime(2025, 1, 1, 10, 2, 0) | ||
| result = await api4.run(seconds=0, first=False) |
| # Mock datetime to be at non-10/30-minute mark (e.g., 10:05) | ||
| with patch("octopus.datetime") as mock_datetime: | ||
| mock_datetime.now.return_value = datetime(2025, 1, 1, 10, 5, 0) | ||
| result = await api5.run(seconds=0, first=False) |
springfall2008
changed the title
Comment on lines
+251
to
+255
| if len(mock_ha.get_history_calls) != initial_call_count: | ||
| print("ERROR: Should use cache, not call HAInterface again") | ||
| failed += 1 | ||
| else: | ||
| print("✓ Used cache instead of fetching again") |
There was a problem hiding this comment.
Variable result2 is not used.
Suggested change
| if len(mock_ha.get_history_calls) != initial_call_count: | |
| print("ERROR: Should use cache, not call HAInterface again") | |
| failed += 1 | |
| else: | |
| print("✓ Used cache instead of fetching again") | |
| # Verify cached result matches the original result | |
| if result2 != result: | |
| print("ERROR: Cached history does not match initial result") | |
| failed += 1 | |
| elif len(mock_ha.get_history_calls) != initial_call_count: | |
| print("ERROR: Should use cache, not call HAInterface again") | |
| failed += 1 | |
| else: | |
| print("✓ Used cache instead of fetching again and returned same data") |
| mock_get.return_value = mock_response | ||
|
|
||
| # Call with silent=True | ||
| result = ha_interface.api_call("/api/states", silent=True) |
There was a problem hiding this comment.
Variable result is not used.
Suggested change
| result = ha_interface.api_call("/api/states", silent=True) | |
| ha_interface.api_call("/api/states", silent=True) |
| with patch("ha.requests.get") as mock_get: | ||
| mock_get.side_effect = requests.Timeout("Connection timeout") | ||
|
|
||
| result = ha_interface.api_call("/api/states") |
There was a problem hiding this comment.
Variable result is not used.
Suggested change
| result = ha_interface.api_call("/api/states") | |
| ha_interface.api_call("/api/states") |
| with patch("ha.requests.get") as mock_get: | ||
| mock_get.return_value = create_mock_requests_response(200, {"result": "success"}) | ||
|
|
||
| result = ha_interface.api_call("/api/states") |
There was a problem hiding this comment.
Variable result is not used.
Suggested change
| result = ha_interface.api_call("/api/states") | |
| ha_interface.api_call("/api/states") |
|
|
||
| ha_interface.call_service_websocket_command = mock_call_service_websocket_command | ||
|
|
||
| result = ha_interface.call_service("switch/turn_on", entity_id="switch.test") |
There was a problem hiding this comment.
Variable result is not used.
| mock_session.ws_connect.return_value.__aexit__ = AsyncMock() | ||
| mock_session_class.return_value = mock_session | ||
|
|
||
| result = run_async(ha_interface.async_call_service_websocket_command("switch", "turn_on", {"entity_id": "switch.test", "return_response": True})) |
There was a problem hiding this comment.
Variable result is not used.
| mock_session.ws_connect = MagicMock(side_effect=Exception("Connection failed")) | ||
| mock_session_class.return_value = mock_session | ||
|
|
||
| result = run_async(ha_interface.async_call_service_websocket_command("switch", "turn_on", {"entity_id": "switch.test"})) |
There was a problem hiding this comment.
Variable result is not used.
Suggested change
| result = run_async(ha_interface.async_call_service_websocket_command("switch", "turn_on", {"entity_id": "switch.test"})) | |
| run_async(ha_interface.async_call_service_websocket_command("switch", "turn_on", {"entity_id": "switch.test"})) |
| mock_session.ws_connect = MagicMock(side_effect=Exception("Connection failed")) | ||
| mock_session_class.return_value = mock_session | ||
|
|
||
| result = run_async(ha_interface.async_call_service_websocket_command("switch", "turn_on", {"entity_id": "switch.test"})) |
There was a problem hiding this comment.
Variable result is not used.
Suggested change
| result = run_async(ha_interface.async_call_service_websocket_command("switch", "turn_on", {"entity_id": "switch.test"})) | |
| run_async(ha_interface.async_call_service_websocket_command("switch", "turn_on", {"entity_id": "switch.test"})) |
Comment on lines
73
to
84
| def get_history_db(self, sensor, now, days=30): | ||
| """Mock get_history_db - returns empty history""" | ||
| self.get_history_calls.append({"sensor": sensor, "now": now, "days": days}) | ||
| # Return mock history format: [[{entry1}, {entry2}, ...]] | ||
| return [[]] | ||
|
|
||
| def get_all_entities_db(self): | ||
| """Mock get_all_entities_db - returns list of entity IDs""" | ||
| return list(self.state_data.keys()) | ||
|
|
||
| def get_history_db(self, sensor, now, days=30): | ||
| """Mock get_history_db - returns empty list""" |
There was a problem hiding this comment.
This assignment to 'get_history_db' is unnecessary as it is redefined before this value is used.
Suggested change
| def get_history_db(self, sensor, now, days=30): | |
| """Mock get_history_db - returns empty history""" | |
| self.get_history_calls.append({"sensor": sensor, "now": now, "days": days}) | |
| # Return mock history format: [[{entry1}, {entry2}, ...]] | |
| return [[]] | |
| def get_all_entities_db(self): | |
| """Mock get_all_entities_db - returns list of entity IDs""" | |
| return list(self.state_data.keys()) | |
| def get_history_db(self, sensor, now, days=30): | |
| """Mock get_history_db - returns empty list""" | |
| def get_all_entities_db(self): | |
| """Mock get_all_entities_db - returns list of entity IDs""" | |
| return list(self.state_data.keys()) | |
| def get_history_db(self, sensor, now, days=30): | |
| """Mock get_history_db - returns empty list""" | |
| self.get_history_calls.append({"sensor": sensor, "now": now, "days": days}) |
| predbat_update_move(THIS_VERSION, files) | ||
| sys.exit(1) | ||
| elif modified: | ||
| print("Warn: Predbat files are installed but have modifications") |
There was a problem hiding this comment.
Print statement may execute during import.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds comprehensive unit test coverage for Octopus and HAInterface components, and introduces a new installer that dynamically fetches file lists from GitHub instead of depending on a hardcoded file list. Key improvements include: