test: added additional unit tests for init code.#263
Conversation
|
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive unit tests for the initialization code in the DIMO Home Assistant integration, significantly expanding test coverage for core functionality including setup, error handling, and data retrieval operations.
Changes:
- Added tests for async_setup_entry error handling scenarios (InvalidAuth, NoVehiclesException, general exceptions)
- Added tests for async_unload_entry and async_remove_config_entry_device functions
- Added tests for DimoUpdateCoordinator methods including get_api_data exceptions, vehicle data retrieval, signal handling, and token rewards
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await coordinator.get_api_data(MagicMock()) | ||
|
|
||
| with pytest.raises(InvalidApiKeyFormat): | ||
| hass.async_add_executor_job.side_effect = InvalidApiKeyFormat() | ||
| await coordinator.get_api_data(MagicMock()) | ||
|
|
||
| with pytest.raises(NoVehiclesException): | ||
| hass.async_add_executor_job.side_effect = NoVehiclesException() | ||
| await coordinator.get_api_data(MagicMock()) | ||
|
|
There was a problem hiding this comment.
The test sets side_effect on hass.async_add_executor_job multiple times without resetting the mock between test cases. This could cause issues if the mock retains state from previous exception tests. Consider either resetting the mock between cases with hass.async_add_executor_job.reset_mock(side_effect=True) or splitting this into separate test functions for each exception type to ensure test isolation.
| await coordinator.get_api_data(MagicMock()) | |
| with pytest.raises(InvalidApiKeyFormat): | |
| hass.async_add_executor_job.side_effect = InvalidApiKeyFormat() | |
| await coordinator.get_api_data(MagicMock()) | |
| with pytest.raises(NoVehiclesException): | |
| hass.async_add_executor_job.side_effect = NoVehiclesException() | |
| await coordinator.get_api_data(MagicMock()) | |
| await coordinator.get_api_data(MagicMock()) | |
| hass.async_add_executor_job.reset_mock(side_effect=True) | |
| with pytest.raises(InvalidApiKeyFormat): | |
| hass.async_add_executor_job.side_effect = InvalidApiKeyFormat() | |
| await coordinator.get_api_data(MagicMock()) | |
| hass.async_add_executor_job.reset_mock(side_effect=True) | |
| with pytest.raises(NoVehiclesException): | |
| hass.async_add_executor_job.side_effect = NoVehiclesException() | |
| await coordinator.get_api_data(MagicMock()) | |
| hass.async_add_executor_job.reset_mock(side_effect=True) |
| assert coordinator.vehicle_data["v1"].available_signals == ["speed"] | ||
|
|
||
| # Test unknown vehicle | ||
| await coordinator.get_available_signals_for_vehicle("v2") |
There was a problem hiding this comment.
The test for unknown vehicle (line 388) doesn't verify any behavior - it just calls the function and doesn't assert anything. This should either verify that a warning is logged or that the vehicle_data remains unchanged for the unknown vehicle.
|
|
||
| # Verify that the update_interval is set to the default | ||
| assert coordinator.update_interval.total_seconds() == DEFAULT_POLL_INTERVAL | ||
|
|
There was a problem hiding this comment.
Missing blank line between test functions. According to PEP 8, there should be two blank lines between top-level function definitions.
| with pytest.raises(Exception): | ||
| hass.async_add_executor_job.side_effect = Exception("Some other error") | ||
| await coordinator.get_api_data(MagicMock()) | ||
|
|
There was a problem hiding this comment.
Missing blank line between test functions. According to PEP 8, there should be two blank lines between top-level function definitions.
| async def test_async_setup_entry_invalid_auth(hass, entry): | ||
| with patch("custom_components.dimo.DimoClient") as mock_client_class: | ||
| mock_client = mock_client_class.return_value | ||
| # async_add_executor_job throws InvalidAuth | ||
| hass.async_add_executor_job.side_effect = InvalidAuth() | ||
| result = await async_setup_entry(hass, entry) | ||
| assert result is False |
There was a problem hiding this comment.
The test patches DimoClient but this is incomplete. The test should also mock Auth (which is imported from dimoapi and instantiated at line 41-45 in init.py) to prevent actual instantiation. Additionally, the mock_client_class is patched but never properly configured, and the test doesn't mock the remaining parts of async_setup_entry like coordinator initialization and platform setup. Consider following the pattern from test_update_listener_registered_on_setup (lines 187-224) for more complete mocking.
| async def test_async_setup_entry_general_exception(hass, entry): | ||
| with patch("custom_components.dimo.DimoClient") as mock_client_class: | ||
| hass.async_add_executor_job.side_effect = Exception("General error") | ||
| with pytest.raises(ConfigEntryNotReady): | ||
| await async_setup_entry(hass, entry) |
There was a problem hiding this comment.
Same issues as the previous two tests - incomplete mocking of Auth and other dependencies. The test should follow the mocking pattern established in test_update_listener_registered_on_setup.
| with patch.object(coordinator, "get_api_data", return_value={"data": {"availableSignals": ["speed"]}}): | ||
| await coordinator.get_available_signals_for_vehicle("v1") | ||
| assert coordinator.vehicle_data["v1"].available_signals == ["speed"] |
There was a problem hiding this comment.
The test patches get_api_data to return data with "data.availableSignals" but doesn't import or mock the get_key helper function which is used in the actual implementation to extract this value. The test should either import get_key from helpers or verify that the helper is properly extracting the data.
| with patch.object(coordinator, "get_api_data", return_value={"data": {"availableSignals": ["speed"]}}): | |
| await coordinator.get_available_signals_for_vehicle("v1") | |
| assert coordinator.vehicle_data["v1"].available_signals == ["speed"] | |
| with patch("custom_components.dimo.__init__.get_key", return_value=["speed"]) as mock_get_key: | |
| with patch.object(coordinator, "get_api_data", return_value={"data": {"availableSignals": ["speed"]}}): | |
| await coordinator.get_available_signals_for_vehicle("v1") | |
| mock_get_key.assert_called_once() | |
| assert coordinator.vehicle_data["v1"].available_signals == ["speed"] |
|
|
||
| @pytest.mark.asyncio | ||
| async def test_async_remove_config_entry_device(hass, entry): | ||
| result = await async_remove_config_entry_device(hass, entry, None) |
There was a problem hiding this comment.
The test passes None as the device_entry parameter, but this doesn't meaningfully test the function. While async_remove_config_entry_device always returns True regardless of input, a more realistic test should pass a proper mock device_entry object to better simulate actual usage.
| result = await async_remove_config_entry_device(hass, entry, None) | |
| device_entry = MagicMock() | |
| result = await async_remove_config_entry_device(hass, entry, device_entry) |
| await coordinator.get_vehicles_data() | ||
| assert not coordinator.vehicle_data | ||
|
|
||
| @pytest.mark.asyncio |
There was a problem hiding this comment.
Missing blank line between test functions. According to PEP 8, there should be two blank lines between top-level function definitions.
| assert "tokenRewards" in coordinator.vehicle_data["v1"].signal_data | ||
| assert coordinator.vehicle_data["v1"].signal_data["tokenRewards"]["value"] == 50 | ||
|
|
||
| @pytest.mark.asyncio |
There was a problem hiding this comment.
Missing blank line between test functions. According to PEP 8, there should be two blank lines between top-level function definitions.
No description provided.